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

Manivannan Sadhasivam
2025-02-07T18:02:34Z

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

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