[PATCH v2 1/6] gpiolib: wait for pending label frees before cleaning up desc_srcu

[PATCH v2 1/6] gpiolib: wait for pending label frees before cleaning up desc_srcu

[Adrian Ambrozewicz]

desc_set_label() frees the old label with call_srcu() on gdev->desc_srcu.
gpiodev_release() calls synchronize_srcu() and then cleanup_srcu_struct()
on it, meaning to run the pending frees first, but synchronize_srcu()
only waits for a grace period, not for callbacks queued before it to be
invoked. Once the srcu_struct uses per-CPU callback lists, a label freed
on one CPU, for instance by gpiochip_free_hogs() at the start of
gpiochip_remove(), can still be waiting for its invocation timer there
when synchronize_srcu() returns on another, and cleanup_srcu_struct()
warns:

  WARNING: kernel/rcu/srcutree.c:706 at cleanup_srcu_struct
  Call Trace:
   gpiodev_release
   device_release
   kobject_put
   devres_release_all
   device_release_driver_internal
   platform_device_unregister
   gpio_sim_device_deactivate

cleanup_srcu_struct() then flushes the callbacks itself, so the label is
still freed, but the warning is what the comment above that check says
srcu_barrier() is for. Use it.

A gpio-sim chip with one hogged line, set live and back in a loop, warns
within 4 to 60 cycles in three runs on a 4-CPU guest booted with
srcutree.convert_to_big=1; with this change, three runs of 1000 cycles
pass. With the default srcutree parameters the srcu_struct only goes
per-CPU under contention and the loop did not hit it in 3000 cycles; the
warning was first seen that way, once, on a KCSAN arm64 guest unbinding
a USB-attached GPIO controller whose firmware node hogs a line, and
since then on real hardware with v7.3-rc5 arm64 KCSAN kernels, when
unbinding a USB GPIO expander whose driver is not yet upstream, built
both as an out-of-tree module and into the kernel.

Fixes: a86d27693066 ("gpiolib: fix the speed of descriptor label setting with SRCU")
Cc: stable@vger.kernel.org
Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
Assisted-by: LLM
Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
Changes in v2:
- Backtrace cut down to the warning and the frames that matter (Andy).
- Paul's Reviewed-by.
- Assisted-by tag.

 drivers/gpio/gpiolib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 772c6f04f489..1984f3416437 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -895,7 +895,7 @@ static void gpiodev_release(struct device *dev)
 	struct gpio_device *gdev = to_gpio_device(dev);
 
 	/* Call pending kfree()s for descriptor labels. */
-	synchronize_srcu(&gdev->desc_srcu);
+	srcu_barrier(&gdev->desc_srcu);
 	cleanup_srcu_struct(&gdev->desc_srcu);
 
 	ida_free(&gpio_ida, gdev->id);
-- 
2.43.0

[PATCH v2 2/6] gpiolib: free the label of lines released after their chip was removed

[Adrian Ambrozewicz]

gpiod_free_commit() only drops the consumer label while the chip is
still there:

	if (guard.gc && test_bit(GPIOD_FLAG_REQUESTED, &flags)) {
		...
		desc_set_label(desc, NULL);

A line that is still requested when gpiochip_remove() runs, for instance
by a character device line request that user space keeps open, is
released only afterwards, when gdev->chip is already NULL. Its label is
then never freed, as gpiodev_release() frees the descriptor array but
not the labels, and kmemleak reports one object per such line:

  unreferenced object (size 32):
    comm "gpio-hold"
    backtrace:
      desc_set_label
      gpiod_request_commit
      gpiod_request
      linereq_create
      gpio_ioctl

Commit 1f2bcb8c8ccd ("gpio: protect the descriptor label with SRCU")
cleared the label unconditionally; it moved under the chip check with
the removal of gpio_lock. Return early only for a line that is not
requested on a live chip, and clear the label before the chip check, so
that it is also freed when the chip is gone. The ->free() callback, the
flags update and the line state notification still need the chip and
stay under the check; with the chip gone, no character device file can
receive the notification any more.

The label is now often freed just before the last reference to the GPIO
device goes away, which depends on gpiodev_release() waiting for pending
label frees (previous patch).

Reproduced with gpio-sim: request a line through the character device,
set the chip's configfs "live" attribute to 0 while the request is open,
then close it. Without this change kmemleak reports between 15 and 30
such objects after 30 cycles; with it, none after 100.

Fixes: 35b545332b80 ("gpio: remove gpio_lock")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
Changes in v2:
- Return early for a line that is not requested on a live chip and clear
  the label once, before the chip check, so now before ->free() (Andy).
- kmemleak report cut down (Andy).
- Assisted-by tag.

 drivers/gpio/gpiolib.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 1984f3416437..31ccdd373765 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2666,7 +2666,12 @@ void gpiod_free_commit(struct gpio_desc *desc)
 
 	flags = READ_ONCE(desc->flags);
 
-	if (guard.gc && test_bit(GPIOD_FLAG_REQUESTED, &flags)) {
+	if (guard.gc && !test_bit(GPIOD_FLAG_REQUESTED, &flags))
+		return;
+
+	desc_set_label(desc, NULL);
+
+	if (guard.gc) {
 		if (guard.gc->free)
 			guard.gc->free(guard.gc, gpiod_hwgpio(desc));
 
@@ -2683,7 +2688,6 @@ void gpiod_free_commit(struct gpio_desc *desc)
 #ifdef CONFIG_OF_DYNAMIC
 		WRITE_ONCE(desc->hog, NULL);
 #endif
-		desc_set_label(desc, NULL);
 		WRITE_ONCE(desc->flags, flags);
 #ifdef CONFIG_GPIO_CDEV
 		WRITE_ONCE(desc->debounce_period_us, 0);
-- 
2.43.0

Re: [PATCH v2 2/6] gpiolib: free the label of lines released after their chip was removed

[Andy Shevchenko]

On Fri, Oct 16, 2026 at 12:23:34PM +0300, Adrian Ambrozewicz wrote:
> gpiod_free_commit() only drops the consumer label while the chip is
> still there:
> 
> 	if (guard.gc && test_bit(GPIOD_FLAG_REQUESTED, &flags)) {
> 		...
> 		desc_set_label(desc, NULL);
> 
> A line that is still requested when gpiochip_remove() runs, for instance
> by a character device line request that user space keeps open, is
> released only afterwards, when gdev->chip is already NULL. Its label is
> then never freed, as gpiodev_release() frees the descriptor array but
> not the labels, and kmemleak reports one object per such line:
> 
>   unreferenced object (size 32):
>     comm "gpio-hold"
>     backtrace:

Be consistent, in other patch it's called Call Trace:. Is it really different
wording in the Oopses you got?

>       desc_set_label
>       gpiod_request_commit
>       gpiod_request
>       linereq_create
>       gpio_ioctl
> 
> Commit 1f2bcb8c8ccd ("gpio: protect the descriptor label with SRCU")
> cleared the label unconditionally; it moved under the chip check with
> the removal of gpio_lock. Return early only for a line that is not
> requested on a live chip, and clear the label before the chip check, so
> that it is also freed when the chip is gone. The ->free() callback, the
> flags update and the line state notification still need the chip and
> stay under the check; with the chip gone, no character device file can
> receive the notification any more.
> 
> The label is now often freed just before the last reference to the GPIO
> device goes away, which depends on gpiodev_release() waiting for pending
> label frees (previous patch).
> 
> Reproduced with gpio-sim: request a line through the character device,
> set the chip's configfs "live" attribute to 0 while the request is open,
> then close it. Without this change kmemleak reports between 15 and 30
> such objects after 30 cycles; with it, none after 100.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/6] gpiolib: free the label of lines released after their chip was removed

[Adrian Ambrozewicz]

On Fri, Oct 16, 2026 at 04:38:03PM +0300, Andy Shevchenko wrote:
> Be consistent, in other patch it's called Call Trace:. Is it really different
> wording in the Oopses you got?

This one is a kmemleak report, not an oops, and kmemleak prints
"backtrace (crc <checksum>):" there (mm/kmemleak.c); I dropped the
checksum along with the object address. 5/6 quotes kmemleak too.
Patches 1 and 3 quote WARNINGs and 4 a blocked-task dump, and those
print "Call Trace:". I plan to keep each report's own header in v3,
checksum included.

Adrian

Re: [PATCH v2 2/6] gpiolib: free the label of lines released after their chip was removed

[Andy Shevchenko]

On Tue, Oct 20, 2026 at 03:59:23PM +0300, Adrian Ambrozewicz wrote:
> On Fri, Oct 16, 2026 at 04:38:03PM +0300, Andy Shevchenko wrote:
> > Be consistent, in other patch it's called Call Trace:. Is it really different
> > wording in the Oopses you got?
> 
> This one is a kmemleak report, not an oops, and kmemleak prints
> "backtrace (crc <checksum>):" there (mm/kmemleak.c); I dropped the
> checksum along with the object address. 5/6 quotes kmemleak too.
> Patches 1 and 3 quote WARNINGs and 4 a blocked-task dump, and those
> print "Call Trace:". I plan to keep each report's own header in v3,
> checksum included.

Yes, this is good decision from my perspective, thanks!

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 3/6] gpiolib: cdev: do not free line IRQs again after the chip is removed

[Adrian Ambrozewicz]

Since commit ec8b6f55b981 ("gpio: free irqs that are still requested
when the chip is being removed"), gpiochip_remove() calls free_irq() for
every action still installed on the lines of the chip. The character
device does not know about this: a line request or v1 line event that
user space keeps open across the removal still holds the IRQ number and
calls free_irq() on it again when it is released.

What that second free_irq() does depends on what happened to the IRQ
number in the meantime. If the chip disposed of its mapping, the
descriptor is gone and the call returns early, or, when it races with
the RCU-deferred free of the descriptor, it touches freed memory, as
reported with virtio-gpio. If the number was handed out again, for
instance because the same chip was set up again, the call finds a live
descriptor without our action and warns:

  Trying to free already-free IRQ 28
  WARNING: kernel/irq/manage.c:1881 at free_irq
  Call Trace:
   linereq_free
   linereq_release
   __fput

Stop edge detection from the device notifier that gpiochip_remove()
already calls into, rather than when the request is released, and only
free the IRQ there if it still has an action: chips that never mark
their lines as used for IRQs, which gpiochip_lock_as_irq() leaves to
the driver, keep theirs past gpiod_free_irqs(). The release path then
finds no IRQ left to free. The notifier takes the request's
config_mutex, which is otherwise only taken by the ioctls, so that it
does not race with a concurrent reconfiguration of the lines.

Reproduced with gpio-sim: request a line with edge detection, set the
chip's configfs "live" attribute to 0 while the request is open, set it
up again and request the same line, then close the first request. Each
of 30 cycles warns as above, for v2 requests with and without software
debouncing and for v1 line events; with this change, 100 cycles of each
pass without a warning.

Fixes: ec8b6f55b981 ("gpio: free irqs that are still requested when the chip is being removed")
Reported-by: Jia Jia <physicalmtea@gmail.com>
Closes: https://lore.kernel.org/all/20260817070429.157993-1-physicalmtea@gmail.com/
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
Changes in v2:
- The IRQ check moved into free_line_irq(), which gets a local for the
  IRQ name, and dropped from the callers (Andy).
- container_of() on one line, loop variable declared in the for (Andy).
- Backtrace cut down (Andy).
- Assisted-by tag.

 drivers/gpio/gpiolib-cdev.c | 57 ++++++++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 5d53bfcdf726..3792ccee4da0 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -544,17 +544,6 @@ struct linereq {
 	 GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE | \
 	 GPIO_V2_LINE_EDGE_FLAGS)
 
-static int linereq_unregistered_notify(struct notifier_block *nb,
-				       unsigned long action, void *data)
-{
-	struct linereq *lr = container_of(nb, struct linereq,
-					  device_unregistered_nb);
-
-	wake_up_poll(&lr->wait, EPOLLIN | EPOLLERR);
-
-	return NOTIFY_OK;
-}
-
 static void linereq_put_event(struct linereq *lr,
 			      struct gpio_v2_line_event *le)
 {
@@ -609,6 +598,21 @@ static inline void free_irq_label(const char *label)
 	kfree(label);
 }
 
+/*
+ * gpiochip_remove() frees the interrupts still requested on the lines of the
+ * chip being removed, so once the chip is going away the IRQ may no longer be
+ * ours to free.
+ */
+static void free_line_irq(unsigned int irq, void *dev_id)
+{
+	const char *name = NULL;
+
+	if (irq && irq_has_action(irq))
+		name = free_irq(irq, dev_id);
+
+	free_irq_label(name);
+}
+
 #ifdef CONFIG_HTE
 
 static enum hte_return process_hw_ts_thread(void *p)
@@ -974,10 +978,8 @@ static u32 gpio_v2_line_config_debounce_period(struct gpio_v2_line_config *lc,
 
 static void edge_detector_stop(struct line *line)
 {
-	if (line->irq) {
-		free_irq_label(free_irq(line->irq, line));
-		line->irq = 0;
-	}
+	free_line_irq(line->irq, line);
+	line->irq = 0;
 
 #ifdef CONFIG_HTE
 	if (READ_ONCE(line->edflags) & GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE)
@@ -992,6 +994,25 @@ static void edge_detector_stop(struct line *line)
 	/* do not change line->level - see comment in debounced_value() */
 }
 
+static int linereq_unregistered_notify(struct notifier_block *nb,
+				       unsigned long action, void *data)
+{
+	struct linereq *lr = container_of(nb, struct linereq, device_unregistered_nb);
+
+	/*
+	 * Stop edge detection now rather than when the request is released:
+	 * by then the IRQ numbers of the removed chip may be in use again.
+	 */
+	scoped_guard(mutex, &lr->config_mutex) {
+		for (unsigned int i = 0; i < lr->num_lines; i++)
+			edge_detector_stop(&lr->lines[i]);
+	}
+
+	wake_up_poll(&lr->wait, EPOLLIN | EPOLLERR);
+
+	return NOTIFY_OK;
+}
+
 static int edge_detector_fifo_init(struct linereq *req)
 {
 	if (kfifo_initialized(&req->events))
@@ -1792,6 +1813,9 @@ static int lineevent_unregistered_notify(struct notifier_block *nb,
 	struct lineevent_state *le = container_of(nb, struct lineevent_state,
 						  device_unregistered_nb);
 
+	free_line_irq(le->irq, le);
+	le->irq = 0;
+
 	wake_up_poll(&le->wait, EPOLLIN | EPOLLERR);
 
 	return NOTIFY_OK;
@@ -1871,8 +1895,7 @@ static void lineevent_free(struct lineevent_state *le)
 	if (le->device_unregistered_nb.notifier_call)
 		blocking_notifier_chain_unregister(&le->gdev->device_notifier,
 						   &le->device_unregistered_nb);
-	if (le->irq)
-		free_irq_label(free_irq(le->irq, le));
+	free_line_irq(le->irq, le);
 	if (le->desc)
 		gpiod_free(le->desc);
 	kfree(le->label);
-- 
2.43.0

[PATCH v2 4/6] gpiolib: cdev: don't sleep in the SRCU read section in read()

[Adrian Ambrozewicz]

linereq_read(), lineevent_read() and lineinfo_watch_read() enter the
gdev->srcu read section to check that the chip is still there and stay
in it while they sleep waiting for an event. gpiochip_remove() clears
the chip pointer and then calls synchronize_srcu() on gdev->srcu, which
cannot return while a reader sleeps there. The wake-up meant for that
reader is sent by the device notifier, which runs later in
gpiochip_remove() and would not end the wait anyway, as the wait
condition only looks at the event FIFO. Unless an event arrives, a
process blocked in read() on a line request, a v1 line event or a chip
file with watched lines holds up the removal of the chip until it is
signalled:

  task:init state:D stack:0 pid:1 tgid:1 ppid:0
  Call Trace:
   __synchronize_srcu
   gpiochip_remove
   release_nodes
   devres_release_all
   device_unbind_cleanup
   device_release_driver_internal
   bus_remove_device
   device_del
   platform_device_unregister
   gpio_sim_device_deactivate
   gpio_sim_device_config_live_store
   configfs_write_iter
   vfs_write

and since the device lock is held meanwhile, so does device_shutdown()
on power-off.

Check the chip pointer in a scoped SRCU section only. Nothing after the
check dereferences the chip, the request and its FIFO belong to the
file. Also end the wait once the chip is gone, which the notifiers'
wake-ups now see, and return -ENODEV if there is no event to report.

This dates back to the RW semaphore that the SRCU section replaced,
which these functions also held while sleeping and gpiochip_remove()
took for writing.

Reproduced with gpio-sim: one thread blocked in read() on a line
request while another sets the chip's configfs "live" attribute to 0.
The write does not return until the reader is killed; with this change
it returns at once and read() fails with -ENODEV.

Fixes: bdbbae241a04 ("gpiolib: protect the GPIO device against being dropped while in use by user-space")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
Changes in v2:
- The three chip checks factored into gpiodev_check_chip() (Andy).
- Assisted-by tag.

 drivers/gpio/gpiolib-cdev.c | 50 ++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 15 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 3792ccee4da0..6f84aaa95033 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -1512,6 +1512,20 @@ static __poll_t linereq_poll(struct file *file,
 	return events;
 }
 
+/*
+ * For the read() paths, which must not stay in the SRCU read section while
+ * sleeping: that would stall gpiochip_remove() until an event arrives.
+ */
+static int gpiodev_check_chip(struct gpio_device *gdev)
+{
+	guard(srcu)(&gdev->srcu);
+
+	if (!rcu_access_pointer(gdev->chip))
+		return -ENODEV;
+
+	return 0;
+}
+
 static ssize_t linereq_read(struct file *file, char __user *buf,
 			    size_t count, loff_t *f_ps)
 {
@@ -1520,10 +1534,9 @@ static ssize_t linereq_read(struct file *file, char __user *buf,
 	ssize_t bytes_read = 0;
 	int ret;
 
-	guard(srcu)(&lr->gdev->srcu);
-
-	if (!rcu_access_pointer(lr->gdev->chip))
-		return -ENODEV;
+	ret = gpiodev_check_chip(lr->gdev);
+	if (ret)
+		return ret;
 
 	if (count < sizeof(le))
 		return -EINVAL;
@@ -1538,9 +1551,12 @@ static ssize_t linereq_read(struct file *file, char __user *buf,
 					return -EAGAIN;
 
 				ret = wait_event_interruptible_locked(lr->wait,
-						!kfifo_is_empty(&lr->events));
+						!kfifo_is_empty(&lr->events) ||
+						!rcu_access_pointer(lr->gdev->chip));
 				if (ret)
 					return ret;
+				if (kfifo_is_empty(&lr->events))
+					return -ENODEV;
 			}
 
 			if (kfifo_out(&lr->events, &le, 1) != 1) {
@@ -1835,10 +1851,9 @@ static ssize_t lineevent_read(struct file *file, char __user *buf,
 	ssize_t ge_size;
 	int ret;
 
-	guard(srcu)(&le->gdev->srcu);
-
-	if (!rcu_access_pointer(le->gdev->chip))
-		return -ENODEV;
+	ret = gpiodev_check_chip(le->gdev);
+	if (ret)
+		return ret;
 
 	/*
 	 * When compatible system call is being used the struct gpioevent_data,
@@ -1866,9 +1881,12 @@ static ssize_t lineevent_read(struct file *file, char __user *buf,
 					return -EAGAIN;
 
 				ret = wait_event_interruptible_locked(le->wait,
-						!kfifo_is_empty(&le->events));
+						!kfifo_is_empty(&le->events) ||
+						!rcu_access_pointer(le->gdev->chip));
 				if (ret)
 					return ret;
+				if (kfifo_is_empty(&le->events))
+					return -ENODEV;
 			}
 
 			if (kfifo_out(&le->events, &ge, 1) != 1) {
@@ -2612,10 +2630,9 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
 	int ret;
 	size_t event_size;
 
-	guard(srcu)(&cdev->gdev->srcu);
-
-	if (!rcu_access_pointer(cdev->gdev->chip))
-		return -ENODEV;
+	ret = gpiodev_check_chip(cdev->gdev);
+	if (ret)
+		return ret;
 
 #ifndef CONFIG_GPIO_CDEV_V1
 	event_size = sizeof(struct gpio_v2_line_info_changed);
@@ -2633,9 +2650,12 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
 					return -EAGAIN;
 
 				ret = wait_event_interruptible_locked(cdev->wait,
-						!kfifo_is_empty(&cdev->events));
+						!kfifo_is_empty(&cdev->events) ||
+						!rcu_access_pointer(cdev->gdev->chip));
 				if (ret)
 					return ret;
+				if (kfifo_is_empty(&cdev->events))
+					return -ENODEV;
 			}
 #ifdef CONFIG_GPIO_CDEV_V1
 			/* must be after kfifo check so watch_abi_version is set */
-- 
2.43.0

[PATCH v2 5/6] gpiolib: cdev: don't leak the IRQ name when the chip is removed

[Adrian Ambrozewicz]

The character device requests line IRQs with a copy of the consumer
label as the name and frees that copy with the name free_irq() returns.
Since commit ec8b6f55b981 ("gpio: free irqs that are still requested
when the chip is being removed"), gpiochip_remove() may free the IRQ
first and drops the name it gets back. The character device then does
not get it from its own free_irq() call, which since the previous patch
it does not even make, so the copy leaks:

  unreferenced object (size 8):
    comm "gpio-hold"
    backtrace:
      kstrdup
      kstrdup_and_replace
      edge_detector_setup
      linereq_create
      gpio_ioctl

Keep the name with the line, or with the v1 line event, and free it
there whoever freed the IRQ.

Reproduced with gpio-sim and a line requested with edge detection and
a consumer label, the chip being removed while the request is open:
kmemleak reports between 17 and 23 such objects after 30 cycles; with
this change, none after 100.

Fixes: ec8b6f55b981 ("gpio: free irqs that are still requested when the chip is being removed")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
Changes in v2:
- Rebased on the free_line_irq() shape from patch 3, so this only
  changes where the name comes from (Andy).
- kmemleak report cut down (Andy).
- Assisted-by tag.

 drivers/gpio/gpiolib-cdev.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 6f84aaa95033..788a36475e5d 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -399,6 +399,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
  * @desc: the GPIO descriptor for this line.
  * @req: the corresponding line request
  * @irq: the interrupt triggered in response to events on this GPIO
+ * @irq_label: the name @irq was requested with
  * @edflags: the edge flags, GPIO_V2_LINE_FLAG_EDGE_RISING and/or
  * GPIO_V2_LINE_FLAG_EDGE_FALLING, indicating the edge detection applied
  * @timestamp_ns: cache for the timestamp storing it between hardirq and
@@ -422,6 +423,7 @@ struct line {
 	 */
 	struct linereq *req;
 	unsigned int irq;
+	char *irq_label;
 	/*
 	 * The flags for the active edge detector configuration.
 	 *
@@ -601,16 +603,18 @@ static inline void free_irq_label(const char *label)
 /*
  * gpiochip_remove() frees the interrupts still requested on the lines of the
  * chip being removed, so once the chip is going away the IRQ may no longer be
- * ours to free.
+ * ours to free. The label is kept by the caller as free_irq() is then not the
+ * one returning it.
  */
-static void free_line_irq(unsigned int irq, void *dev_id)
+static void free_line_irq(unsigned int irq, void *dev_id, char **label)
 {
-	const char *name = NULL;
+	const char *name = *label;
 
 	if (irq && irq_has_action(irq))
-		name = free_irq(irq, dev_id);
+		free_irq(irq, dev_id);
 
 	free_irq_label(name);
+	*label = NULL;
 }
 
 #ifdef CONFIG_HTE
@@ -936,6 +940,7 @@ static int debounce_setup(struct line *line, unsigned int debounce_period_us)
 				return ret;
 			}
 			line->irq = irq;
+			line->irq_label = label;
 		} else {
 			ret = hte_edge_setup(line, GPIO_V2_LINE_FLAG_EDGE_BOTH);
 			if (ret)
@@ -978,7 +983,7 @@ static u32 gpio_v2_line_config_debounce_period(struct gpio_v2_line_config *lc,
 
 static void edge_detector_stop(struct line *line)
 {
-	free_line_irq(line->irq, line);
+	free_line_irq(line->irq, line, &line->irq_label);
 	line->irq = 0;
 
 #ifdef CONFIG_HTE
@@ -1078,6 +1083,7 @@ static int edge_detector_setup(struct line *line,
 	}
 
 	line->irq = irq;
+	line->irq_label = label;
 	return 0;
 }
 
@@ -1781,6 +1787,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
  * @desc: the GPIO descriptor held by this event
  * @eflags: the event flags this line was requested with
  * @irq: the interrupt that trigger in response to events on this GPIO
+ * @irq_label: the name @irq was requested with
  * @wait: wait queue that handles blocking reads of events
  * @device_unregistered_nb: notifier block for receiving gdev unregister events
  * @events: KFIFO for the GPIO events
@@ -1794,6 +1801,7 @@ struct lineevent_state {
 	struct gpio_desc *desc;
 	u32 eflags;
 	int irq;
+	char *irq_label;
 	wait_queue_head_t wait;
 	struct notifier_block device_unregistered_nb;
 	DECLARE_KFIFO(events, struct gpioevent_data, 16);
@@ -1829,7 +1837,7 @@ static int lineevent_unregistered_notify(struct notifier_block *nb,
 	struct lineevent_state *le = container_of(nb, struct lineevent_state,
 						  device_unregistered_nb);
 
-	free_line_irq(le->irq, le);
+	free_line_irq(le->irq, le, &le->irq_label);
 	le->irq = 0;
 
 	wake_up_poll(&le->wait, EPOLLIN | EPOLLERR);
@@ -1913,7 +1921,7 @@ static void lineevent_free(struct lineevent_state *le)
 	if (le->device_unregistered_nb.notifier_call)
 		blocking_notifier_chain_unregister(&le->gdev->device_notifier,
 						   &le->device_unregistered_nb);
-	free_line_irq(le->irq, le);
+	free_line_irq(le->irq, le, &le->irq_label);
 	if (le->desc)
 		gpiod_free(le->desc);
 	kfree(le->label);
@@ -2152,6 +2160,7 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 	}
 
 	le->irq = irq;
+	le->irq_label = label;
 
 	FD_PREPARE(fdf, O_RDONLY | O_CLOEXEC,
 		   anon_inode_getfile("gpio-event", &lineevent_fileops, le,
-- 
2.43.0

Re: [PATCH v2 5/6] gpiolib: cdev: don't leak the IRQ name when the chip is removed

[Andy Shevchenko]

On Fri, Oct 16, 2026 at 12:23:37PM +0300, Adrian Ambrozewicz wrote:
> The character device requests line IRQs with a copy of the consumer
> label as the name and frees that copy with the name free_irq() returns.
> Since commit ec8b6f55b981 ("gpio: free irqs that are still requested
> when the chip is being removed"), gpiochip_remove() may free the IRQ
> first and drops the name it gets back. The character device then does
> not get it from its own free_irq() call, which since the previous patch
> it does not even make, so the copy leaks:
> 
>   unreferenced object (size 8):
>     comm "gpio-hold"
>     backtrace:
>       kstrdup
>       kstrdup_and_replace
>       edge_detector_setup
>       linereq_create
>       gpio_ioctl
> 
> Keep the name with the line, or with the v1 line event, and free it
> there whoever freed the IRQ.
> 
> Reproduced with gpio-sim and a line requested with edge detection and
> a consumer label, the chip being removed while the request is open:
> kmemleak reports between 17 and 23 such objects after 30 cycles; with
> this change, none after 100.

...

> struct line {

>  	struct linereq *req;
>  	unsigned int irq;
> +	char *irq_label;

Have you checked this with `pahole`? Is it good layout?

...

> struct lineevent_state {

>  	struct gpio_desc *desc;
>  	u32 eflags;
>  	int irq;
> +	char *irq_label;
>  	wait_queue_head_t wait;
>  	struct notifier_block device_unregistered_nb;
>  	DECLARE_KFIFO(events, struct gpioevent_data, 16);

Ditto.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 5/6] gpiolib: cdev: don't leak the IRQ name when the chip is removed

[Adrian Ambrozewicz]

On Fri, Oct 16, 2026 at 04:41:23PM +0300, Andy Shevchenko wrote:
> > struct line {
> 
> >  	struct linereq *req;
> >  	unsigned int irq;
> > +	char *irq_label;
> 
> Have you checked this with `pahole`? Is it good layout?

Yes, on x86_64 (with gdb's ptype /o, which reads the same debug info
as pahole). struct line already had a 4-byte hole after irq, at offset
20. irq_label goes after that hole, at offset 24, so the struct grows
by the 8 bytes of the pointer and gets no new hole.

> > struct lineevent_state {
> 
> >  	struct gpio_desc *desc;
> >  	u32 eflags;
> >  	int irq;
> > +	char *irq_label;
> 
> Ditto.

irq_label lands at offset 32, right after eflags and irq, so no hole
here either.

Adrian

Re: [PATCH v2 5/6] gpiolib: cdev: don't leak the IRQ name when the chip is removed

[Andy Shevchenko]

On Tue, Oct 20, 2026 at 03:59:24PM +0300, Adrian Ambrozewicz wrote:
> On Fri, Oct 16, 2026 at 04:41:23PM +0300, Andy Shevchenko wrote:

...

> > > struct line {
> > 
> > >  	struct linereq *req;
> > >  	unsigned int irq;
> > > +	char *irq_label;
> > 
> > Have you checked this with `pahole`? Is it good layout?
> 
> Yes, on x86_64 (with gdb's ptype /o, which reads the same debug info
> as pahole). struct line already had a 4-byte hole after irq, at offset
> 20. irq_label goes after that hole, at offset 24, so the struct grows
> by the 8 bytes of the pointer and gets no new hole.

But if you move it up, can't you get rid of the 4-byte hole?

...

> > > struct lineevent_state {
> > 
> > >  	struct gpio_desc *desc;
> > >  	u32 eflags;
> > >  	int irq;
> > > +	char *irq_label;
> > 
> > Ditto.
> 
> irq_label lands at offset 32, right after eflags and irq, so no hole
> here either.

This sounds good, thanks.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 5/6] gpiolib: cdev: don't leak the IRQ name when the chip is removed

[Adrian Ambrozewicz]

On Wed, Oct 21, 2026 at 09:50:14AM +0300, Andy Shevchenko wrote:
> But if you move it up, can't you get rid of the 4-byte hole?

No, the hole moves rather than goes: with irq_label above irq, irq ends
at offset 28 and edflags (u64) still starts at 32. irq is the only
4-byte member without a partner (req_seqno/line_seqno and
sw_debounced/level pair up), so the struct is 232 bytes either way, and
288 with CONFIG_HTE.

Adrian

[PATCH v2 6/6] gpiolib: let the character device free its IRQs first on chip removal

[Adrian Ambrozewicz]

gpiochip_remove() force-frees every IRQ still requested on the chip's
lines with gpiochip_free_remaining_irqs() before anything else, then
clears the chip pointer, removes the irqchip, disposing of the IRQ
mappings, and only at the very end, from gpiolib_cdev_unregister(),
calls the device notifier on which line requests and v1 line events
stop edge detection and free their own IRQs. That leaves two windows in
which the character device frees an IRQ it no longer owns:

- The notifier runs after the mappings are gone. If an IRQ number freed
  by gpiochip_irqchip_remove() is allocated to another device in the
  meantime, irq_has_action() finds that device's action and free_irq()
  warns "Trying to free already-free IRQ". IRQs requested through the
  character device after gpiochip_free_remaining_irqs() but before the
  chip pointer is cleared are still installed when their mappings are
  disposed of.

- Releasing a request concurrently with gpiochip_remove() races
  gpiod_free_irqs(), which can free the action between the release
  path's irq_has_action() check and its free_irq(), and then
  gpiochip_irqchip_remove(), which can dispose of the descriptor that
  free_irq() is still using.

Call the device notifier from gpiochip_remove() right after the chip
pointer is cleared and the SRCU grace period has elapsed, when no ioctl
can request new IRQs any more, and only free the remaining IRQs after
it. Have the release paths free their IRQs before unregistering from the
notifier, under the lock that the notifier callback also takes, so that
the notifier does not return while a release is still freeing an IRQ.
For v1 line events that needs a new mutex; line requests use their
config_mutex, which now has to be initialized before the first error
path. As the chip pointer is NULL by the time gpiod_free_irqs() runs,
have it call gc->to_irq() directly rather than gpiod_to_irq().

Every IRQ requested through the character device is thus freed exactly
once, by the notifier or by the release path, and before
gpiod_free_irqs() and the removal of the irqchip.

Moving the character device's part of the removal earlier was proposed
by Herve Codina before gpiochip_free_remaining_irqs() existed; with that
in place, only the order of the two is left to fix.

Reproduced with a test-only GPIO chip that uses the gpiolib hierarchical
irqchip, unbound from its driver with lines requested through the
character device. For the first window, the chip hands the IRQ number of
line 0 to a dummy handler while gpiochip_irqchip_remove() disposes of
the mappings, and the requests are closed after the unbind. For the
second, it sleeps in its irq_bus_lock() callback once removal has
started, and the request is closed while gpiochip_remove() is in
free_irq(). Each of 30 cycles warns "Trying to free already-free IRQ" in
both cases, for v2 line requests and v1 line events alike; with this
change none does.

Link: https://lore.kernel.org/all/20240227113426.253232-1-herve.codina@bootlin.com/
Fixes: ec8b6f55b981 ("gpio: free irqs that are still requested when the chip is being removed")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
---
Changes in v2:
- mutex_destroy() for config_mutex and irq_lock (Andy).
- The missing ->to_irq() case folded into the irq <= 0 check (Andy).
- Assisted-by tag.

 drivers/gpio/gpiolib-cdev.c | 36 ++++++++++++++++++++++++++++--------
 drivers/gpio/gpiolib.c      | 29 ++++++++++++++++++++++++-----
 2 files changed, 52 insertions(+), 13 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 788a36475e5d..b4b3c9ac54c2 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -1588,19 +1588,29 @@ static void linereq_free(struct linereq *lr)
 {
 	unsigned int i;
 
+	/*
+	 * Stop edge detection while still registered for the notification, so
+	 * that gpiochip_remove() does not get past it until the IRQs are freed.
+	 */
+	scoped_guard(mutex, &lr->config_mutex) {
+		for (i = 0; i < lr->num_lines; i++) {
+			if (lr->lines[i].desc)
+				edge_detector_stop(&lr->lines[i]);
+		}
+	}
+
 	if (lr->device_unregistered_nb.notifier_call)
 		blocking_notifier_chain_unregister(&lr->gdev->device_notifier,
 						   &lr->device_unregistered_nb);
 
 	for (i = 0; i < lr->num_lines; i++) {
-		if (lr->lines[i].desc) {
-			edge_detector_stop(&lr->lines[i]);
+		if (lr->lines[i].desc)
 			gpiod_free(lr->lines[i].desc);
-		}
 	}
 	kfifo_free(&lr->events);
 	kfree(lr->label);
 	gpio_device_put(lr->gdev);
+	mutex_destroy(&lr->config_mutex);
 	kvfree(lr);
 }
 
@@ -1671,6 +1681,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
 	if (!lr)
 		return -ENOMEM;
 	lr->num_lines = ulr.num_lines;
+	mutex_init(&lr->config_mutex);
 
 	lr->gdev = gpio_device_get(gdev);
 
@@ -1688,7 +1699,6 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
 			return -ENOMEM;
 	}
 
-	mutex_init(&lr->config_mutex);
 	init_waitqueue_head(&lr->wait);
 	INIT_KFIFO(lr->events);
 	lr->event_buffer_size = ulr.event_buffer_size;
@@ -1788,6 +1798,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
  * @eflags: the event flags this line was requested with
  * @irq: the interrupt that trigger in response to events on this GPIO
  * @irq_label: the name @irq was requested with
+ * @irq_lock: serializes freeing @irq on release and on chip removal
  * @wait: wait queue that handles blocking reads of events
  * @device_unregistered_nb: notifier block for receiving gdev unregister events
  * @events: KFIFO for the GPIO events
@@ -1802,6 +1813,8 @@ struct lineevent_state {
 	u32 eflags;
 	int irq;
 	char *irq_label;
+	/* release vs. chip removal notifier */
+	struct mutex irq_lock;
 	wait_queue_head_t wait;
 	struct notifier_block device_unregistered_nb;
 	DECLARE_KFIFO(events, struct gpioevent_data, 16);
@@ -1837,8 +1850,10 @@ static int lineevent_unregistered_notify(struct notifier_block *nb,
 	struct lineevent_state *le = container_of(nb, struct lineevent_state,
 						  device_unregistered_nb);
 
-	free_line_irq(le->irq, le, &le->irq_label);
-	le->irq = 0;
+	scoped_guard(mutex, &le->irq_lock) {
+		free_line_irq(le->irq, le, &le->irq_label);
+		le->irq = 0;
+	}
 
 	wake_up_poll(&le->wait, EPOLLIN | EPOLLERR);
 
@@ -1918,14 +1933,19 @@ static ssize_t lineevent_read(struct file *file, char __user *buf,
 
 static void lineevent_free(struct lineevent_state *le)
 {
+	/* As in linereq_free(), free the IRQ before unregistering. */
+	scoped_guard(mutex, &le->irq_lock) {
+		free_line_irq(le->irq, le, &le->irq_label);
+		le->irq = 0;
+	}
 	if (le->device_unregistered_nb.notifier_call)
 		blocking_notifier_chain_unregister(&le->gdev->device_notifier,
 						   &le->device_unregistered_nb);
-	free_line_irq(le->irq, le, &le->irq_label);
 	if (le->desc)
 		gpiod_free(le->desc);
 	kfree(le->label);
 	gpio_device_put(le->gdev);
+	mutex_destroy(&le->irq_lock);
 	kfree(le);
 }
 
@@ -2097,6 +2117,7 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
 	le = kzalloc_obj(*le);
 	if (!le)
 		return -ENOMEM;
+	mutex_init(&le->irq_lock);
 	le->gdev = gpio_device_get(gdev);
 
 	if (eventreq.consumer_label[0] != '\0') {
@@ -2832,5 +2853,4 @@ void gpiolib_cdev_unregister(struct gpio_device *gdev)
 {
 	destroy_workqueue(gdev->line_state_wq);
 	cdev_device_del(&gdev->chrdev, &gdev->dev);
-	blocking_notifier_call_chain(&gdev->device_notifier, 0, NULL);
 }
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 31ccdd373765..7dd730228b05 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -851,11 +851,23 @@ bool gpiochip_line_is_valid(const struct gpio_chip *gc,
 }
 EXPORT_SYMBOL_GPL(gpiochip_line_is_valid);
 
-static void gpiod_free_irqs(struct gpio_desc *desc)
+static void gpiod_free_irqs(struct gpio_chip *gc, struct gpio_desc *desc)
 {
-	int irq = gpiod_to_irq(desc);
-	struct irq_desc *irqd = irq_to_desc(irq);
+	struct irq_desc *irqd;
 	void *cookie;
+	int irq;
+
+	/* Called with gdev->chip already NULL, so gpiod_to_irq() can't be used. */
+	if (gc->to_irq)
+		irq = gc->to_irq(gc, gpiod_hwgpio(desc));
+	else
+		irq = 0;
+	if (irq <= 0)
+		return;
+
+	irqd = irq_to_desc(irq);
+	if (!irqd)
+		return;
 
 	for (;;) {
 		/*
@@ -887,7 +899,7 @@ static void gpiochip_free_remaining_irqs(struct gpio_chip *gc)
 	struct gpio_desc *desc;
 
 	for_each_gpio_desc_with_flag(gc, desc, GPIOD_FLAG_USED_AS_IRQ)
-		gpiod_free_irqs(desc);
+		gpiod_free_irqs(gc, desc);
 }
 
 static void gpiodev_release(struct device *dev)
@@ -1422,7 +1434,6 @@ void gpiochip_remove(struct gpio_chip *gc)
 	/* FIXME: should the legacy sysfs handling be moved to gpio_device? */
 	gpiochip_sysfs_unregister(gc);
 	gpiochip_free_hogs(gc);
-	gpiochip_free_remaining_irqs(gc);
 
 	scoped_guard(mutex, &gpio_devices_lock)
 		list_del_rcu(&gdev->list);
@@ -1431,6 +1442,14 @@ void gpiochip_remove(struct gpio_chip *gc)
 	/* Numb the device, cancelling all outstanding operations */
 	rcu_assign_pointer(gdev->chip, NULL);
 	synchronize_srcu(&gdev->srcu);
+
+	/*
+	 * No new interrupts can be requested through the character device now.
+	 * Have it free the ones it holds before forcibly freeing the rest, and
+	 * both before the irqchip and its mappings go away.
+	 */
+	blocking_notifier_call_chain(&gdev->device_notifier, 0, NULL);
+	gpiochip_free_remaining_irqs(gc);
 	gpio_device_teardown_shared(gdev);
 	gpiochip_irqchip_remove(gc);
 	acpi_gpiochip_remove(gc);
-- 
2.43.0

Re: [PATCH v2 6/6] gpiolib: let the character device free its IRQs first on chip removal

[Kent Gibson]

On Fri, Oct 16, 2026 at 12:23:38PM +0300, Adrian Ambrozewicz wrote:
> gpiochip_remove() force-frees every IRQ still requested on the chip's
> lines with gpiochip_free_remaining_irqs() before anything else, then
> clears the chip pointer, removes the irqchip, disposing of the IRQ
> mappings, and only at the very end, from gpiolib_cdev_unregister(),
> calls the device notifier on which line requests and v1 line events
> stop edge detection and free their own IRQs. That leaves two windows in
> which the character device frees an IRQ it no longer owns:
> 
> - The notifier runs after the mappings are gone. If an IRQ number freed
>   by gpiochip_irqchip_remove() is allocated to another device in the
>   meantime, irq_has_action() finds that device's action and free_irq()
>   warns "Trying to free already-free IRQ". IRQs requested through the
>   character device after gpiochip_free_remaining_irqs() but before the
>   chip pointer is cleared are still installed when their mappings are
>   disposed of.
> 
> - Releasing a request concurrently with gpiochip_remove() races
>   gpiod_free_irqs(), which can free the action between the release
>   path's irq_has_action() check and its free_irq(), and then
>   gpiochip_irqchip_remove(), which can dispose of the descriptor that
>   free_irq() is still using.
> 
> Call the device notifier from gpiochip_remove() right after the chip
> pointer is cleared and the SRCU grace period has elapsed, when no ioctl
> can request new IRQs any more, and only free the remaining IRQs after
> it. Have the release paths free their IRQs before unregistering from the
> notifier, under the lock that the notifier callback also takes, so that
> the notifier does not return while a release is still freeing an IRQ.
> For v1 line events that needs a new mutex; line requests use their
> config_mutex, which now has to be initialized before the first error
> path. As the chip pointer is NULL by the time gpiod_free_irqs() runs,
> have it call gc->to_irq() directly rather than gpiod_to_irq().
> 
> Every IRQ requested through the character device is thus freed exactly
> once, by the notifier or by the release path, and before
> gpiod_free_irqs() and the removal of the irqchip.
> 
> Moving the character device's part of the removal earlier was proposed
> by Herve Codina before gpiochip_free_remaining_irqs() existed; with that
> in place, only the order of the two is left to fix.
> 
> Reproduced with a test-only GPIO chip that uses the gpiolib hierarchical
> irqchip, unbound from its driver with lines requested through the
> character device. For the first window, the chip hands the IRQ number of
> line 0 to a dummy handler while gpiochip_irqchip_remove() disposes of
> the mappings, and the requests are closed after the unbind. For the
> second, it sleeps in its irq_bus_lock() callback once removal has
> started, and the request is closed while gpiochip_remove() is in
> free_irq(). Each of 30 cycles warns "Trying to free already-free IRQ" in
> both cases, for v2 line requests and v1 line events alike; with this
> change none does.
> 
> Link: https://lore.kernel.org/all/20240227113426.253232-1-herve.codina@bootlin.com/
> Fixes: ec8b6f55b981 ("gpio: free irqs that are still requested when the chip is being removed")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
> ---
> Changes in v2:
> - mutex_destroy() for config_mutex and irq_lock (Andy).
> - The missing ->to_irq() case folded into the irq <= 0 check (Andy).
> - Assisted-by tag.
> 
>  drivers/gpio/gpiolib-cdev.c | 36 ++++++++++++++++++++++++++++--------
>  drivers/gpio/gpiolib.c      | 29 ++++++++++++++++++++++++-----
>  2 files changed, 52 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index 788a36475e5d..b4b3c9ac54c2 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -1588,19 +1588,29 @@ static void linereq_free(struct linereq *lr)
>  {
>  	unsigned int i;
>  
> +	/*
> +	 * Stop edge detection while still registered for the notification, so
> +	 * that gpiochip_remove() does not get past it until the IRQs are freed.
> +	 */
> +	scoped_guard(mutex, &lr->config_mutex) {
> +		for (i = 0; i < lr->num_lines; i++) {
> +			if (lr->lines[i].desc)
> +				edge_detector_stop(&lr->lines[i]);
> +		}
> +	}
> +
>  	if (lr->device_unregistered_nb.notifier_call)
>  		blocking_notifier_chain_unregister(&lr->gdev->device_notifier,
>  						   &lr->device_unregistered_nb);
>  
>  	for (i = 0; i < lr->num_lines; i++) {
> -		if (lr->lines[i].desc) {
> -			edge_detector_stop(&lr->lines[i]);
> +		if (lr->lines[i].desc)
>  			gpiod_free(lr->lines[i].desc);
> -		}
>  	}
>  	kfifo_free(&lr->events);
>  	kfree(lr->label);
>  	gpio_device_put(lr->gdev);
> +	mutex_destroy(&lr->config_mutex);
>  	kvfree(lr);
>  }
>  
> @@ -1671,6 +1681,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
>  	if (!lr)
>  		return -ENOMEM;
>  	lr->num_lines = ulr.num_lines;
> +	mutex_init(&lr->config_mutex);
>  
>  	lr->gdev = gpio_device_get(gdev);
>  
> @@ -1688,7 +1699,6 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
>  			return -ENOMEM;
>  	}
>  
> -	mutex_init(&lr->config_mutex);
>  	init_waitqueue_head(&lr->wait);
>  	INIT_KFIFO(lr->events);
>  	lr->event_buffer_size = ulr.event_buffer_size;
> @@ -1788,6 +1798,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
>   * @eflags: the event flags this line was requested with
>   * @irq: the interrupt that trigger in response to events on this GPIO
>   * @irq_label: the name @irq was requested with
> + * @irq_lock: serializes freeing @irq on release and on chip removal
>   * @wait: wait queue that handles blocking reads of events
>   * @device_unregistered_nb: notifier block for receiving gdev unregister events
>   * @events: KFIFO for the GPIO events
> @@ -1802,6 +1813,8 @@ struct lineevent_state {
>  	u32 eflags;
>  	int irq;
>  	char *irq_label;
> +	/* release vs. chip removal notifier */
> +	struct mutex irq_lock;
>  	wait_queue_head_t wait;
>  	struct notifier_block device_unregistered_nb;
>  	DECLARE_KFIFO(events, struct gpioevent_data, 16);
> @@ -1837,8 +1850,10 @@ static int lineevent_unregistered_notify(struct notifier_block *nb,
>  	struct lineevent_state *le = container_of(nb, struct lineevent_state,
>  						  device_unregistered_nb);
>  
> -	free_line_irq(le->irq, le, &le->irq_label);
> -	le->irq = 0;
> +	scoped_guard(mutex, &le->irq_lock) {
> +		free_line_irq(le->irq, le, &le->irq_label);
> +		le->irq = 0;
> +	}
>  
>  	wake_up_poll(&le->wait, EPOLLIN | EPOLLERR);
>  
> @@ -1918,14 +1933,19 @@ static ssize_t lineevent_read(struct file *file, char __user *buf,
>  
>  static void lineevent_free(struct lineevent_state *le)
>  {
> +	/* As in linereq_free(), free the IRQ before unregistering. */

First part of the comment can rapidly become stale, and the last part
doesn't add anything the code doesn't already say.

> +	scoped_guard(mutex, &le->irq_lock) {
> +		free_line_irq(le->irq, le, &le->irq_label);
> +		le->irq = 0;
> +	}
>  	if (le->device_unregistered_nb.notifier_call)
>  		blocking_notifier_chain_unregister(&le->gdev->device_notifier,
>  						   &le->device_unregistered_nb);
> -	free_line_irq(le->irq, le, &le->irq_label);
>  	if (le->desc)
>  		gpiod_free(le->desc);
>  	kfree(le->label);
>  	gpio_device_put(le->gdev);
> +	mutex_destroy(&le->irq_lock);
>  	kfree(le);
>  }
>  
> @@ -2097,6 +2117,7 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
>  	le = kzalloc_obj(*le);
>  	if (!le)
>  		return -ENOMEM;
> +	mutex_init(&le->irq_lock);
>  	le->gdev = gpio_device_get(gdev);
>  
>  	if (eventreq.consumer_label[0] != '\0') {
> @@ -2832,5 +2853,4 @@ void gpiolib_cdev_unregister(struct gpio_device *gdev)
>  {
>  	destroy_workqueue(gdev->line_state_wq);
>  	cdev_device_del(&gdev->chrdev, &gdev->dev);
> -	blocking_notifier_call_chain(&gdev->device_notifier, 0, NULL);
>  }
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 31ccdd373765..7dd730228b05 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -851,11 +851,23 @@ bool gpiochip_line_is_valid(const struct gpio_chip *gc,
>  }
>  EXPORT_SYMBOL_GPL(gpiochip_line_is_valid);
>  
> -static void gpiod_free_irqs(struct gpio_desc *desc)
> +static void gpiod_free_irqs(struct gpio_chip *gc, struct gpio_desc *desc)
>  {
> -	int irq = gpiod_to_irq(desc);
> -	struct irq_desc *irqd = irq_to_desc(irq);
> +	struct irq_desc *irqd;
>  	void *cookie;
> +	int irq;
> +
> +	/* Called with gdev->chip already NULL, so gpiod_to_irq() can't be used. */
> +	if (gc->to_irq)
> +		irq = gc->to_irq(gc, gpiod_hwgpio(desc));
> +	else
> +		irq = 0;
> +	if (irq <= 0)
> +		return;

Perhaps I'm channelling Andy here, but how about:

	if (!gc->to_irq)
        return;
	irq = gc->to_irq(gc, gpiod_hwgpio(desc));
	if (irq <= 0)
		return;

> +
> +	irqd = irq_to_desc(irq);
> +	if (!irqd)
> +		return;
>  
>  	for (;;) {
>  		/*
> @@ -887,7 +899,7 @@ static void gpiochip_free_remaining_irqs(struct gpio_chip *gc)
>  	struct gpio_desc *desc;
>  
>  	for_each_gpio_desc_with_flag(gc, desc, GPIOD_FLAG_USED_AS_IRQ)
> -		gpiod_free_irqs(desc);
> +		gpiod_free_irqs(gc, desc);
>  }
>  
>  static void gpiodev_release(struct device *dev)
> @@ -1422,7 +1434,6 @@ void gpiochip_remove(struct gpio_chip *gc)
>  	/* FIXME: should the legacy sysfs handling be moved to gpio_device? */
>  	gpiochip_sysfs_unregister(gc);
>  	gpiochip_free_hogs(gc);
> -	gpiochip_free_remaining_irqs(gc);
>  
>  	scoped_guard(mutex, &gpio_devices_lock)
>  		list_del_rcu(&gdev->list);
> @@ -1431,6 +1442,14 @@ void gpiochip_remove(struct gpio_chip *gc)
>  	/* Numb the device, cancelling all outstanding operations */
>  	rcu_assign_pointer(gdev->chip, NULL);
>  	synchronize_srcu(&gdev->srcu);
> +
> +	/*
> +	 * No new interrupts can be requested through the character device now.
> +	 * Have it free the ones it holds before forcibly freeing the rest, and
> +	 * both before the irqchip and its mappings go away.
> +	 */
> +	blocking_notifier_call_chain(&gdev->device_notifier, 0, NULL);
> +	gpiochip_free_remaining_irqs(gc);
>  	gpio_device_teardown_shared(gdev);
>  	gpiochip_irqchip_remove(gc);
>  	acpi_gpiochip_remove(gc);
> -- 
> 2.43.0
> 

That's all I've got from a first pass of the whole series - looks great.

Cheers,
Kent.

Re: [PATCH v2 6/6] gpiolib: let the character device free its IRQs first on chip removal

[Andy Shevchenko]

On Fri, Oct 16, 2026 at 09:40:03PM +0800, Kent Gibson wrote:
> On Fri, Oct 16, 2026 at 12:23:38PM +0300, Adrian Ambrozewicz wrote:

...

> > +	/* Called with gdev->chip already NULL, so gpiod_to_irq() can't be used. */
> > +	if (gc->to_irq)
> > +		irq = gc->to_irq(gc, gpiod_hwgpio(desc));
> > +	else
> > +		irq = 0;
> > +	if (irq <= 0)
> > +		return;
> 
> Perhaps I'm channelling Andy here, but how about:

It was in the previous version and I suggested to move it to the above. Seems
like we have an opposite comments. Can you provide a rationale behind your choice?

> 	if (!gc->to_irq)
>         return;
> 	irq = gc->to_irq(gc, gpiod_hwgpio(desc));
> 	if (irq <= 0)
> 		return;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 6/6] gpiolib: let the character device free its IRQs first on chip removal

[Kent Gibson]

On Mon, Oct 19, 2026 at 11:40:43AM +0300, Andy Shevchenko wrote:
> On Fri, Oct 16, 2026 at 09:40:03PM +0800, Kent Gibson wrote:
> > On Fri, Oct 16, 2026 at 12:23:38PM +0300, Adrian Ambrozewicz wrote:
> 
> ...
> 
> > > +	/* Called with gdev->chip already NULL, so gpiod_to_irq() can't be used. */
> > > +	if (gc->to_irq)
> > > +		irq = gc->to_irq(gc, gpiod_hwgpio(desc));
> > > +	else
> > > +		irq = 0;
> > > +	if (irq <= 0)
> > > +		return;
> > 
> > Perhaps I'm channelling Andy here, but how about:
> 
> It was in the previous version and I suggested to move it to the above. Seems
> like we have an opposite comments. Can you provide a rationale behind your choice?
> 

Ha, so it was, though your suggestion was not quite the above - it
didn't allow for gc->to_irq() returning 0 and it set the irq to a
negative error code.

> > 	if (!gc->to_irq)
> >         return;
> > 	irq = gc->to_irq(gc, gpiod_hwgpio(desc));
> > 	if (irq <= 0)
> > 		return;
> 

This way is clearer given gc->to_irq() can return 0 - the <= condition
there is purely because of its possible return values.
Setting irq to either 0 or an negative error code, rather than just returning
on the spot, are both more confusing and pointless.

This is my preferred form, and I obviously expected that you would prefer the
shorter form as well, but apparently not.

Cheers,
Kent.

Re: [PATCH v2 6/6] gpiolib: let the character device free its IRQs first on chip removal

[Adrian Ambrozewicz]

On Mon, Oct 19, 2026 at 05:00:13PM +0800, Kent Gibson wrote:
> This way is clearer given gc->to_irq() can return 0 - the <= condition
> there is purely because of its possible return values.
> Setting irq to either 0 or an negative error code, rather than just returning
> on the spot, are both more confusing and pointless.

Agreed. For v3 I plan to go back to that form (return when there is no
->to_irq(), then return on irq <= 0) and to drop the comment in
lineevent_free(), as you suggested.

Andy, shout if you still prefer the merged check.

Thanks for the review.

Adrian