From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Adrian Ambrozewicz <aambrozewicz@nvidia.com>
Cc: Bartosz Golaszewski <brgl@kernel.org>,
Linus Walleij <linusw@kernel.org>,
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>,
Kent Gibson <warthog618@gmail.com>,
"Paul E. McKenney" <paulmck@kernel.org>,
Herve Codina <herve.codina@bootlin.com>,
Jia Jia <physicalmtea@gmail.com>,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
Subject: Re: [PATCH 3/6] gpiolib: cdev: do not free line IRQs again after the chip is removed
Date: Tue, 13 Oct 2026 18:44:33 +0300 [thread overview]
Message-ID: <as5R4WDqXfMXkD2L@ashevche-desk.local> (raw)
In-Reply-To: <20261012-gpiolib-chip-removal-v1-3-aambrozewicz@nvidia.com>
On Mon, Oct 12, 2026 at 04:49:57PM +0300, Adrian Ambrozewicz wrote:
> 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+0x193/0x2d0, CPU#2: gpio-hold/111
> Call Trace:
> <TASK>
> linereq_free+0xd2/0x120
> linereq_release+0xd/0x20
> __fput+0xed/0x2a0
> __x64_sys_close+0x38/0x80
> do_syscall_64+0x109/0x5d0
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> </TASK>
>
> 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.
...
> +/*
> + * 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)
> +{
> + if (irq_has_action(irq))
> + free_irq_label(free_irq(irq, dev_id));
In all cases it also checks for irq to be valid, so embed it here and drop
those checks below.
if (irq && irq_has_action(irq))
> +}
...
> static void edge_detector_stop(struct line *line)
> {
> if (line->irq) {
> - free_irq_label(free_irq(line->irq, line));
> + free_line_irq(line->irq, line);
> line->irq = 0;
> }
This, for instance, will become
free_line_irq(line->irq, line);
line->irq = 0;
...
> +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);
Slightly better to have it
struct linereq *lr = container_of(nb, struct linereq, device_unregistered_nb);
or
struct linereq *lr =
container_of(nb, struct linereq, device_unregistered_nb);
> + unsigned int i;
Not used outside of the loop.
> + /*
> + * 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 (i = 0; i < lr->num_lines; i++)
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;
> +}
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2026-10-13 15:44 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-12 13:49 [PATCH 0/6] gpiolib: fix chip removal with lines still requested from user space Adrian Ambrozewicz
2026-10-12 13:49 ` [PATCH 1/6] gpiolib: wait for pending label frees before cleaning up desc_srcu Adrian Ambrozewicz
2026-10-12 15:46 ` Paul E. McKenney
2026-10-12 20:42 ` Andy Shevchenko
2026-10-12 13:49 ` [PATCH 2/6] gpiolib: free the label of lines released after their chip was removed Adrian Ambrozewicz
2026-10-13 15:38 ` Andy Shevchenko
2026-10-12 13:49 ` [PATCH 3/6] gpiolib: cdev: do not free line IRQs again after the chip is removed Adrian Ambrozewicz
2026-10-13 15:44 ` Andy Shevchenko [this message]
2026-10-12 13:49 ` [PATCH 4/6] gpiolib: cdev: don't sleep in the SRCU read section in read() Adrian Ambrozewicz
2026-10-13 15:48 ` Andy Shevchenko
2026-10-12 13:49 ` [PATCH 5/6] gpiolib: cdev: don't leak the IRQ name when the chip is removed Adrian Ambrozewicz
2026-10-13 15:59 ` Andy Shevchenko
2026-10-12 13:50 ` [PATCH 6/6] gpiolib: let the character device free its IRQs first on chip removal Adrian Ambrozewicz
2026-10-13 16:05 ` Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=as5R4WDqXfMXkD2L@ashevche-desk.local \
--to=andriy.shevchenko@linux.intel.com \
--cc=aambrozewicz@nvidia.com \
--cc=bartosz.golaszewski@oss.qualcomm.com \
--cc=brgl@kernel.org \
--cc=herve.codina@bootlin.com \
--cc=linusw@kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=paulmck@kernel.org \
--cc=physicalmtea@gmail.com \
--cc=stable@vger.kernel.org \
--cc=warthog618@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®