Hi,
When a gpio chip device is removed while some related gpio are used by
the user-space (gpiomon for instance), the following warning can appear:
remove_proc_entry: removing non-empty directory 'irq/233', leaking at least 'gpiomon'
WARNING: CPU: 2 PID: 72 at fs/proc/generic.c:717 remove_proc_entry+0x190/0x19c
...
Call trace:
remove_proc_entry+0x190/0x19c
unregister_irq_proc+0xd0/0x104
free_desc+0x4c/0xc4
irq_free_descs+0x6c/0x90
irq_dispose_mapping+0x104/0x14c
gpiochip_irqchip_remove+0xcc/0x1a4
gpiochip_remove+0x48/0x100
...
Indeed, even if the gpio removal is notified to the gpio-cdev, the
IRQ used is not released when it should be.
This series calls the gpio removal notifier sooner in the removal
process in order to give a chance to a notifier function to release
the IRQ before releasing the IRQ mapping and adds the needed
operations to release the IRQ in the gpio cdev notifier function.
Best regards,
Hervé Codina
Herve Codina (2):
gpiolib: call gcdev_unregister() sooner in the removal operations
gpiolib: cdev: release IRQs when the gpio chip device is removed
drivers/gpio/gpiolib-cdev.c | 33 ++++++++++++++++++++++-----------
drivers/gpio/gpiolib.c | 8 +++++++-
2 files changed, 29 insertions(+), 12 deletions(-)
--
2.43.0
When gpio chip device is removed while some related gpio are used by the
user-space, the following warning can appear:
remove_proc_entry: removing non-empty directory 'irq/233', leaking at least 'gpiomon'
WARNING: CPU: 2 PID: 72 at fs/proc/generic.c:717 remove_proc_entry+0x190/0x19c
...
Call trace:
remove_proc_entry+0x190/0x19c
unregister_irq_proc+0xd0/0x104
free_desc+0x4c/0xc4
irq_free_descs+0x6c/0x90
irq_dispose_mapping+0x104/0x14c
gpiochip_irqchip_remove+0xcc/0x1a4
gpiochip_remove+0x48/0x100
...
Indeed, the gpio cdev uses an IRQ but this IRQ is not released
(irq_free() call) before the call to gpiochip_irqchip_remove().
In order to give a chance to the gpio dev driver to release this
irq before removing the IRQ mapping, notify the cdev driver about
the gpio device removal before the gpiochip_irqchip_remove() call.
Signed-off-by: Herve Codina <[email protected]>
---
drivers/gpio/gpiolib.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 8b3a0f45b574..079181b9daa8 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1051,6 +1051,13 @@ void gpiochip_remove(struct gpio_chip *gc)
/* FIXME: should the legacy sysfs handling be moved to gpio_device? */
gpiochip_sysfs_unregister(gdev);
+
+ /*
+ * Tell gcdev that the device is removing. If any gpio resources are in
+ * use (irqs for instance), it's time for gcdev to release them.
+ */
+ gcdev_unregister(gdev);
+
gpiochip_free_hogs(gc);
/* Numb the device, cancelling all outstanding operations */
gdev->chip = NULL;
@@ -1085,7 +1092,6 @@ void gpiochip_remove(struct gpio_chip *gc)
* be removed, else it will be dangling until the last user is
* gone.
*/
- gcdev_unregister(gdev);
up_write(&gdev->sem);
gpio_device_put(gdev);
}
--
2.43.0
On Tue, Feb 20, 2024 at 12:10 PM Herve Codina <herve.codina@bootlincom> wrote:
>
> Hi,
>
> When a gpio chip device is removed while some related gpio are used by
> the user-space (gpiomon for instance), the following warning can appear:
> remove_proc_entry: removing non-empty directory 'irq/233', leaking at least 'gpiomon'
> WARNING: CPU: 2 PID: 72 at fs/proc/generic.c:717 remove_proc_entry+0x190/0x19c
> ...
> Call trace:
> remove_proc_entry+0x190/0x19c
> unregister_irq_proc+0xd0/0x104
> free_desc+0x4c/0xc4
> irq_free_descs+0x6c/0x90
> irq_dispose_mapping+0x104/0x14c
> gpiochip_irqchip_remove+0xcc/0x1a4
> gpiochip_remove+0x48/0x100
> ...
>
> Indeed, even if the gpio removal is notified to the gpio-cdev, the
> IRQ used is not released when it should be.
>
> This series calls the gpio removal notifier sooner in the removal
> process in order to give a chance to a notifier function to release
> the IRQ before releasing the IRQ mapping and adds the needed
> operations to release the IRQ in the gpio cdev notifier function.
>
> Best regards,
> Hervé Codina
>
> Herve Codina (2):
> gpiolib: call gcdev_unregister() sooner in the removal operations
> gpiolib: cdev: release IRQs when the gpio chip device is removed
>
> drivers/gpio/gpiolib-cdev.c | 33 ++++++++++++++++++++++-----------
> drivers/gpio/gpiolib.c | 8 +++++++-
> 2 files changed, 29 insertions(+), 12 deletions(-)
>
> --
> 2.43.0
>
Thanks for taking a stab at it. I saw this issue some time ago, tried
to fix it directly in interrupt procfs code[1], got yelled at by
Thomas Gleixner for 20 or so emails and eventually forgot about it.
Nice to see someone tackle it again.
Bart
[1] https://lore.kernel.org/lkml/[email protected]/
On Tue, Feb 20, 2024 at 12:10 PM Herve Codina <herve.codina@bootlincom> wrote:
>
> When gpio chip device is removed while some related gpio are used by the
> user-space, the following warning can appear:
> remove_proc_entry: removing non-empty directory 'irq/233', leaking at least 'gpiomon'
> WARNING: CPU: 2 PID: 72 at fs/proc/generic.c:717 remove_proc_entry+0x190/0x19c
> ...
> Call trace:
> remove_proc_entry+0x190/0x19c
> unregister_irq_proc+0xd0/0x104
> free_desc+0x4c/0xc4
> irq_free_descs+0x6c/0x90
> irq_dispose_mapping+0x104/0x14c
> gpiochip_irqchip_remove+0xcc/0x1a4
> gpiochip_remove+0x48/0x100
> ...
>
> Indeed, the gpio cdev uses an IRQ but this IRQ is not released
> (irq_free() call) before the call to gpiochip_irqchip_remove().
>
> In order to give a chance to the gpio dev driver to release this
> irq before removing the IRQ mapping, notify the cdev driver about
> the gpio device removal before the gpiochip_irqchip_remove() call.
>
> Signed-off-by: Herve Codina <[email protected]>
> ---
> drivers/gpio/gpiolib.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 8b3a0f45b574..079181b9daa8 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1051,6 +1051,13 @@ void gpiochip_remove(struct gpio_chip *gc)
>
> /* FIXME: should the legacy sysfs handling be moved to gpio_device? */
> gpiochip_sysfs_unregister(gdev);
> +
> + /*
> + * Tell gcdev that the device is removing. If any gpio resources are in
> + * use (irqs for instance), it's time for gcdev to release them.
> + */
> + gcdev_unregister(gdev);
> +
> gpiochip_free_hogs(gc);
> /* Numb the device, cancelling all outstanding operations */
> gdev->chip = NULL;
> @@ -1085,7 +1092,6 @@ void gpiochip_remove(struct gpio_chip *gc)
> * be removed, else it will be dangling until the last user is
> * gone.
> */
> - gcdev_unregister(gdev);
> up_write(&gdev->sem);
Please rebase it on top of the for-next branch of the GPIO tree. We've
had some significant rework recently, we no longer even have this
semaphore.
Bart
> gpio_device_put(gdev);
> }
> --
> 2.43.0
>