[PATCH v2 1/3] misc: pci_endpoint_test: Avoid issue of interrupts remaining after request_irq error
After devm_request_irq() fails with error, pci_endpoint_test_free_irq_vectors() is called to free allocated vectors with pci_free_irq_vectors().
However some requested IRQs are still allocated, so there are still /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
To solve this issue, set the number of remaining IRQs and release the IRQs in advance by calling pci_endpoint_test_release_irq().
Cc: stable@vger.kernel.org Fixes: e03327122e2c ("pci_endpoint_test: Add 2 ioctl commands") Signed-off-by: Kunihiko Hayashi hayashi.kunihiko@socionext.com --- drivers/misc/pci_endpoint_test.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c index 3702dcc89ab7..302955c20979 100644 --- a/drivers/misc/pci_endpoint_test.c +++ b/drivers/misc/pci_endpoint_test.c @@ -252,6 +252,9 @@ static bool pci_endpoint_test_request_irq(struct pci_endpoint_test *test) break; }
+ test->num_irqs = i; + pci_endpoint_test_release_irq(test); + return false; }
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().
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()?
/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?
To solve this issue, set the number of remaining IRQs and release the IRQs in advance by calling pci_endpoint_test_release_irq().
First of all, using devm_request_irq() is wrong here as pci_endpoint_test_request_irq() might be called multiple times by the userspace. So we cannot use managed version of request_irq(). It is infact pointless since pci_endpoint_test_clear_irq() would free them anyway. Instead we should just use request_irq() and call pci_endpoint_test_clear_irq() in the error path as like this patch does.
But this should be a separate patch.
- Mani
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.
/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/0x19cWhen 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 ]---
To solve this issue, set the number of remaining IRQs and release the IRQs in advance by calling pci_endpoint_test_release_irq().
First of all, using devm_request_irq() is wrong here as pci_endpoint_test_request_irq() might be called multiple times by the userspace. So we cannot use managed version of request_irq(). It is infact pointless since pci_endpoint_test_clear_irq() would free them anyway. Instead we should just use request_irq() and call pci_endpoint_test_clear_irq() in the error path as like this patch does.
The pci_endpoint_test_clear_irq() will free all the number of IRQs. However, in this error case, since only some IRQs are allocated, need to change the number of IRQs that need to be freed.
And pci_endpoint_test_free_irq_vectors() is called just before return from the function, call only pci_endpoint_test_release_irq() in the error case of pci_endpoint_test_request_irq().
For example: pci_endpoint_test_set_irq() pci_endpoint_test_request_irq() request_irq() fail: >> test->num_irqs = i // add in this patch >> pci_endpoint_test_release_irq() // add in this patch err: pci_endpoint_test_free_irq_vectors()
But this should be a separate patch.
Agreed. I'll add another patch to use non-managed version.
Thank you,
--- Best Regards Kunihiko Hayashi
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/0x19cWhen 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
Hi Manivannan,
On 2025/02/08 3:02, Manivannan Sadhasivam wrote:
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.
Surely, 'allocated' means the vectors allocated with pci_irq_vectors(), not the interrupts requested with request_irq().
I'll rewrite with a right description.
/proc/irq/* entries remaining and we encounters WARN() with the following message:
remove_proc_entry: removing non-empty directory 'irq/30', leakingat 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.
Okay, I'll add the call trace into the patch.
Thank you,
--- Best Regards Kunihiko Hayashi
linux-stable-mirror@lists.linaro.org
-
Kunihiko Hayashi -
Manivannan Sadhasivam