| Message ID | 20230817184958.25349-5-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: cdev: bail out of poll() if the device goes down | expand |
On Thu, Aug 17, 2023 at 08:49:56PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Add a notifier block to the gpio_chardev_data structure and register it > with the gpio_device's device notifier. Upon reception of an event, wake > up the wait queue so that the user-space be forced out of poll() and need > to go into a new system call which will then fail due to the chip being > gone. ... > + struct gpio_chardev_data *cdev = container_of(nb, > + struct gpio_chardev_data, > + device_unregistered_nb); struct gpio_chardev_data *cdev = container_of(nb, struct gpio_chardev_data, device_unregistered_nb); ?
On Fri, Aug 18, 2023 at 12:31 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Aug 17, 2023 at 08:49:56PM +0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > Add a notifier block to the gpio_chardev_data structure and register it > > with the gpio_device's device notifier. Upon reception of an event, wake > > up the wait queue so that the user-space be forced out of poll() and need > > to go into a new system call which will then fail due to the chip being > > gone. > > ... > > > + struct gpio_chardev_data *cdev = container_of(nb, > > + struct gpio_chardev_data, > > + device_unregistered_nb); > > struct gpio_chardev_data *cdev = > container_of(nb, struct gpio_chardev_data, device_unregistered_nb); > > ? I could live with the other version but sure, why not. I will send a v3 with a helper wrapper around blocking_notifier_call_chain() for more brevity. Bart
On Mon, Aug 21, 2023 at 2:43 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > On Fri, Aug 18, 2023 at 12:31 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Thu, Aug 17, 2023 at 08:49:56PM +0200, Bartosz Golaszewski wrote: > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > > > Add a notifier block to the gpio_chardev_data structure and register it > > > with the gpio_device's device notifier. Upon reception of an event, wake > > > up the wait queue so that the user-space be forced out of poll() and need > > > to go into a new system call which will then fail due to the chip being > > > gone. > > > > ... > > > > > + struct gpio_chardev_data *cdev = container_of(nb, > > > + struct gpio_chardev_data, > > > + device_unregistered_nb); > > > > struct gpio_chardev_data *cdev = > > container_of(nb, struct gpio_chardev_data, device_unregistered_nb); > > > > ? > > I could live with the other version but sure, why not. > > I will send a v3 with a helper wrapper around > blocking_notifier_call_chain() for more brevity. > Scratch that, I need coffee. This was supposed to go under the line state notifications patch. Bart
On Fri, Aug 18, 2023 at 12:31 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Aug 17, 2023 at 08:49:56PM +0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > Add a notifier block to the gpio_chardev_data structure and register it > > with the gpio_device's device notifier. Upon reception of an event, wake > > up the wait queue so that the user-space be forced out of poll() and need > > to go into a new system call which will then fail due to the chip being > > gone. > > ... > > > + struct gpio_chardev_data *cdev = container_of(nb, > > + struct gpio_chardev_data, > > + device_unregistered_nb); > > struct gpio_chardev_data *cdev = > container_of(nb, struct gpio_chardev_data, device_unregistered_nb); > Ah, this goes over 80 chars. I will leave it as it is, I will fix the other one when applying. Bart > ? > > -- > With Best Regards, > Andy Shevchenko > >
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c index f95e1b110311..8b081c9b8d27 100644 --- a/drivers/gpio/gpiolib-cdev.c +++ b/drivers/gpio/gpiolib-cdev.c @@ -2320,6 +2320,7 @@ struct gpio_chardev_data { wait_queue_head_t wait; DECLARE_KFIFO(events, struct gpio_v2_line_info_changed, 32); struct notifier_block lineinfo_changed_nb; + struct notifier_block device_unregistered_nb; unsigned long *watched_lines; #ifdef CONFIG_GPIO_CDEV_V1 atomic_t watch_abi_version; @@ -2518,6 +2519,18 @@ static int lineinfo_changed_notify(struct notifier_block *nb, return NOTIFY_OK; } +static int gpio_device_unregistered_notify(struct notifier_block *nb, + unsigned long action, void *data) +{ + struct gpio_chardev_data *cdev = container_of(nb, + struct gpio_chardev_data, + device_unregistered_nb); + + wake_up_poll(&cdev->wait, EPOLLIN | EPOLLERR); + + return NOTIFY_OK; +} + static __poll_t lineinfo_watch_poll_unlocked(struct file *file, struct poll_table_struct *pollt) { @@ -2672,17 +2685,27 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file) if (ret) goto out_free_bitmap; + cdev->device_unregistered_nb.notifier_call = + gpio_device_unregistered_notify; + ret = blocking_notifier_chain_register(&gdev->device_notifier, + &cdev->device_unregistered_nb); + if (ret) + goto out_unregister_line_notifier; + file->private_data = cdev; ret = nonseekable_open(inode, file); if (ret) - goto out_unregister_notifier; + goto out_unregister_device_notifier; up_read(&gdev->sem); return ret; -out_unregister_notifier: +out_unregister_device_notifier: + blocking_notifier_chain_unregister(&gdev->device_notifier, + &cdev->device_unregistered_nb); +out_unregister_line_notifier: blocking_notifier_chain_unregister(&gdev->line_state_notifier, &cdev->lineinfo_changed_nb); out_free_bitmap: @@ -2707,6 +2730,8 @@ static int gpio_chrdev_release(struct inode *inode, struct file *file) struct gpio_device *gdev = cdev->gdev; bitmap_free(cdev->watched_lines); + blocking_notifier_chain_unregister(&gdev->device_notifier, + &cdev->device_unregistered_nb); blocking_notifier_chain_unregister(&gdev->line_state_notifier, &cdev->lineinfo_changed_nb); gpio_device_put(gdev);