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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Mon, Oct 12, 2026 at 04:49:58PM +0300, Adrian Ambrozewicz wrote:
> 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.

...

> -	guard(srcu)(&lr->gdev->srcu);
> -
> -	if (!rcu_access_pointer(lr->gdev->chip))
> -		return -ENODEV;
> +	/*
> +	 * Don't stay in the SRCU read section while sleeping: that would stall
> +	 * gpiochip_remove() until an event arrives.
> +	 */
> +	scoped_guard(srcu, &lr->gdev->srcu) {
> +		if (!rcu_access_pointer(lr->gdev->chip))
> +			return -ENODEV;
> +	}

It's duplicated a few times, perhaps you can make a static helper?

static int ...()
{
	guard(srcu)(&gdev->srcu);

	if (!rcu_access_pointer(gdev->chip))
		return -ENODEV;

	return 0;
}

Caveat: not an SRCU expert, so it might be actually wrong from run-time
behaviour perspective.

-- 
With Best Regards,
Andy Shevchenko