| Message ID | 20230817184958.25349-1-brgl@bgdev.pl |
|---|---|
| Headers | show |
| Series | gpio: cdev: bail out of poll() if the device goes down | expand |
On Thu, Aug 17, 2023 at 8:50 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Wake up all three wake queues (the one associated with the character > device file, the one for V1 line events and the V2 line request one) > when the underlying GPIO device is unregistered. This way we won't get > stuck in poll() after the chip is gone as user-space will be forced to > go back into a new system call and will see that gdev->chip is NULL. > > v1 -> v2: > - not much is left from v1, this time we don't repurpose the existing > gpio_device notifier but add a new one so that cdev structures don't > get unwanted events This is nice, thanks for quick respin! Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Thu, Aug 17, 2023 at 08:49:52PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Wake up all three wake queues (the one associated with the character > device file, the one for V1 line events and the V2 line request one) > when the underlying GPIO device is unregistered. This way we won't get > stuck in poll() after the chip is gone as user-space will be forced to > go back into a new system call and will see that gdev->chip is NULL. Why can't you use the global device unbind notifications and filter out what you are interested in?
On Fri, Aug 18, 2023 at 02:06:21PM +0200, Bartosz Golaszewski wrote: > On Fri, Aug 18, 2023 at 12:34 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Thu, Aug 17, 2023 at 08:49:52PM +0200, Bartosz Golaszewski wrote: > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > > > Wake up all three wake queues (the one associated with the character > > > device file, the one for V1 line events and the V2 line request one) > > > when the underlying GPIO device is unregistered. This way we won't get > > > stuck in poll() after the chip is gone as user-space will be forced to > > > go back into a new system call and will see that gdev->chip is NULL. > > > > Why can't you use the global device unbind notifications and filter out > > what you are interested in? > > There's no truly global device unbind notification - only per-bus. > GPIO devices can reside on any bus, there are no limitations and so > we'd have to subscribe to all of them. We have, but it requires a bit of code patching. Look at device_platform_notify()/device_platform_notify_remove(). I noticed, btw, that platform_notify() and Co is a dead code :-) Maybe it can be converted to a list and a manager of that list, so specific cases can utilize it.
On Fri, Aug 18, 2023 at 3:36 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > On Fri, Aug 18, 2023 at 3:29 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Fri, Aug 18, 2023 at 02:06:21PM +0200, Bartosz Golaszewski wrote: > > > On Fri, Aug 18, 2023 at 12:34 PM Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > > > On Thu, Aug 17, 2023 at 08:49:52PM +0200, Bartosz Golaszewski wrote: > > > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > > > > > > > Wake up all three wake queues (the one associated with the character > > > > > device file, the one for V1 line events and the V2 line request one) > > > > > when the underlying GPIO device is unregistered. This way we won't get > > > > > stuck in poll() after the chip is gone as user-space will be forced to > > > > > go back into a new system call and will see that gdev->chip is NULL. > > > > > > > > Why can't you use the global device unbind notifications and filter out > > > > what you are interested in? > > > > > > There's no truly global device unbind notification - only per-bus. > > > GPIO devices can reside on any bus, there are no limitations and so > > > we'd have to subscribe to all of them. > > > > We have, but it requires a bit of code patching. > > Look at device_platform_notify()/device_platform_notify_remove(). > > > > I noticed, btw, that platform_notify() and Co is a dead code :-) > > Maybe it can be converted to a list and a manager of that list, > > so specific cases can utilize it. > > > > That's not going to happen anytime soon and I doubt Greg would like > the idea without more users interested in receiving these events. :( > > This GPIO notifier is an implementation detail - once we do have > global notifiers, we can switch to using them instead. > > Bart Seems to me the whole platform_notify() thing was meant for a different purpose. There's no trace for it ever being used for notifying users about driver bind and unbind events. It appears to me too that adding a global notifier would be as simple as extending the functionality of bus_notify() with a second, global notifier chain. But I won't be the one to propose this to Greg KH. :) Bart
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Wake up all three wake queues (the one associated with the character device file, the one for V1 line events and the V2 line request one) when the underlying GPIO device is unregistered. This way we won't get stuck in poll() after the chip is gone as user-space will be forced to go back into a new system call and will see that gdev->chip is NULL. v1 -> v2: - not much is left from v1, this time we don't repurpose the existing gpio_device notifier but add a new one so that cdev structures don't get unwanted events Bartosz Golaszewski (6): gpiolib: rename the gpio_device notifier gpio: cdev: open-code to_gpio_chardev_data() gpiolib: add a second blocking notifier to struct gpio_device gpio: cdev: wake up chardev poll() on device unbind gpio: cdev: wake up linereq poll() on device unbind gpio: cdev: wake up lineevent poll() on device unbind drivers/gpio/gpiolib-cdev.c | 101 ++++++++++++++++++++++++++++++------ drivers/gpio/gpiolib.c | 7 +-- drivers/gpio/gpiolib.h | 9 ++-- 3 files changed, 94 insertions(+), 23 deletions(-)