Re: [PATCH v2 1/3] misc: pci_endpoint_test: Avoid issue of interrupts remaining after request_irq error

[Manivannan Sadhasivam]

On Wed, Jan 29, 2025 at 04:54:46PM +0900, Kunihiko Hayashi wrote:
> Hi Manivannan,
> 
> On 2025/01/28 23:12, Manivannan Sadhasivam wrote:
> > On Wed, Jan 22, 2025 at 11:24:44AM +0900, Kunihiko Hayashi wrote:
> > > After devm_request_irq() fails with error,
> > > pci_endpoint_test_free_irq_vectors() is called to free allocated vectors
> > > with pci_free_irq_vectors().
> > > 
> > 
> > You should mention the function name which you are referring to. Here it is,
> > pci_endpoint_test_request_irq().
> 
> I see. I'll make the commit message more clear.
> 
> > > However some requested IRQs are still allocated, so there are still
> > 
> > This is confusing. Are you saying that the previously requested IRQs are not
> > freed when an error happens during the for loop in
> > pci_endpoint_test_request_irq()?
> 
> Yes, after jumping to "fail:" label, it just prints an error message and
> returns the function.
> 
> The pci_endpoint_test_request_irq() is called from the following functions:
> - pci_endpoint_test_probe()
> - pci_endpoint_test_set_irq()
> 
> Both call pci_endpoint_test_free_irq_vectors() after the error, though,
> requested IRQs are not freed anywhere.
> 

You should not use the word 'allocated' since that has a different meaning
and the source of confusion.

> > > /proc/irq/* entries remaining and we encounters WARN() with the following
> > > message:
> > > 
> > >      remove_proc_entry: removing non-empty directory 'irq/30', leaking at
> > >      least 'pci-endpoint-test.0'
> > >      WARNING: CPU: 0 PID: 80 at fs/proc/generic.c:717 remove_proc_entry
> > >      +0x190/0x19c
> > 
> > When did you encounter this WARN?
> 
> Usually request_irq() can successfully get an interrupt.
> If request_irq() returned an error, pci_endpoint_test_free_irq_vectors() was
> called and the following call-trace was obtained:
> 
> [   18.772522] Call trace:
> [   18.773743]  remove_proc_entry+0x190/0x19c
> [   18.775789]  unregister_irq_proc+0xd0/0x104
> [   18.777881]  free_desc+0x4c/0xcc
> [   18.779495]  irq_free_descs+0x68/0x8c
> [   18.781325]  irq_domain_free_irqs+0x15c/0x1bc
> [   18.783502]  msi_domain_free_locked.part.0+0x184/0x1d4
> [   18.786069]  msi_domain_free_irqs_all_locked+0x64/0x8c
> [   18.788637]  pci_msi_teardown_msi_irqs+0x48/0x54
> [   18.790947]  pci_free_msi_irqs+0x18/0x38
> [   18.792907]  pci_free_irq_vectors+0x64/0x8c
> [   18.794997]  pci_endpoint_test_ioctl+0x7e8/0xf40
> [   18.797304]  __arm64_sys_ioctl+0xa4/0xe8
> [   18.799265]  invoke_syscall+0x48/0x110
> [   18.801139]  el0_svc_common.constprop.0+0x40/0xe8
> [   18.803489]  do_el0_svc+0x20/0x2c
> [   18.805145]  el0_svc+0x30/0xd0
> [   18.806673]  el0t_64_sync_handler+0x13c/0x158
> [   18.808850]  el0t_64_sync+0x190/0x194
> [   18.810680] ---[ end trace 0000000000000000 ]---
> 

Please add this info to the patch description.

- Mani

-- 
மணிவண்ணன் சதாசிவம்