]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
gpio: cdev: put emitting the line state events on a workqueue
authorBartosz Golaszewski <bartosz.golaszewski@linaro.org>
Fri, 18 Oct 2024 09:10:14 +0000 (11:10 +0200)
committerBartosz Golaszewski <bartosz.golaszewski@linaro.org>
Tue, 22 Oct 2024 06:59:09 +0000 (08:59 +0200)
In order to allow line state notifications to be emitted from atomic
context (for instance: from gpiod_direction_input/output()), we must
stop calling any sleeping functions in lineinfo_changed_notify(). To
that end let's use the new workqueue.

Let's atomically allocate small structures containing the required data
and fill it with information immediately upon being notified about the
change except for the pinctrl state which will be retrieved later from
process context. We can pretty reliably do this as pin functions are
typically set once per boot.

Let's make sure to bump the reference count of GPIO device and the GPIO
character device file descriptor to keep both alive until the event was
queued.

Reviewed-by: Kent Gibson <warthog618@gmail.com>
Link: https://lore.kernel.org/r/20241018-gpio-notify-in-kernel-events-v5-6-c79135e58a1c@linaro.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
drivers/gpio/gpiolib-cdev.c

index b242fdb1ad288f5f94f11fd839519a5121390baf..7759dca92f8bb381b701660bcd403cb3e7415575 100644 (file)
@@ -2333,6 +2333,7 @@ struct gpio_chardev_data {
 #ifdef CONFIG_GPIO_CDEV_V1
        atomic_t watch_abi_version;
 #endif
+       struct file *fp;
 };
 
 static int chipinfo_get(struct gpio_chardev_data *cdev, void __user *ip)
@@ -2502,28 +2503,86 @@ static long gpio_ioctl_compat(struct file *file, unsigned int cmd,
 }
 #endif
 
+struct lineinfo_changed_ctx {
+       struct work_struct work;
+       struct gpio_v2_line_info_changed chg;
+       struct gpio_device *gdev;
+       struct gpio_chardev_data *cdev;
+};
+
+static void lineinfo_changed_func(struct work_struct *work)
+{
+       struct lineinfo_changed_ctx *ctx =
+                       container_of(work, struct lineinfo_changed_ctx, work);
+       struct gpio_chip *gc;
+       int ret;
+
+       if (!(ctx->chg.info.flags & GPIO_V2_LINE_FLAG_USED)) {
+               /*
+                * If nobody set the USED flag earlier, let's see with pinctrl
+                * now. We're doing this late because it's a sleeping function.
+                * Pin functions are in general much more static and while it's
+                * not 100% bullet-proof, it's good enough for most cases.
+                */
+               scoped_guard(srcu, &ctx->gdev->srcu) {
+                       gc = srcu_dereference(ctx->gdev->chip, &ctx->gdev->srcu);
+                       if (gc &&
+                           !pinctrl_gpio_can_use_line(gc, ctx->chg.info.offset))
+                               ctx->chg.info.flags |= GPIO_V2_LINE_FLAG_USED;
+               }
+       }
+
+       ret = kfifo_in_spinlocked(&ctx->cdev->events, &ctx->chg, 1,
+                                 &ctx->cdev->wait.lock);
+       if (ret)
+               wake_up_poll(&ctx->cdev->wait, EPOLLIN);
+       else
+               pr_debug_ratelimited("lineinfo event FIFO is full - event dropped\n");
+
+       gpio_device_put(ctx->gdev);
+       fput(ctx->cdev->fp);
+       kfree(ctx);
+}
+
 static int lineinfo_changed_notify(struct notifier_block *nb,
                                   unsigned long action, void *data)
 {
        struct gpio_chardev_data *cdev =
                container_of(nb, struct gpio_chardev_data, lineinfo_changed_nb);
-       struct gpio_v2_line_info_changed chg;
+       struct lineinfo_changed_ctx *ctx;
        struct gpio_desc *desc = data;
-       int ret;
 
        if (!test_bit(gpio_chip_hwgpio(desc), cdev->watched_lines))
                return NOTIFY_DONE;
 
-       memset(&chg, 0, sizeof(chg));
-       chg.event_type = action;
-       chg.timestamp_ns = ktime_get_ns();
-       gpio_desc_to_lineinfo(desc, &chg.info, false);
+       /*
+        * If this is called from atomic context (for instance: with a spinlock
+        * taken by the atomic notifier chain), any sleeping calls must be done
+        * outside of this function in process context of the dedicated
+        * workqueue.
+        *
+        * Let's gather as much info as possible from the descriptor and
+        * postpone just the call to pinctrl_gpio_can_use_line() until the work
+        * is executed.
+        */
+
+       ctx = kzalloc(sizeof(*ctx), GFP_ATOMIC);
+       if (!ctx) {
+               pr_err("Failed to allocate memory for line info notification\n");
+               return NOTIFY_DONE;
+       }
+
+       ctx->chg.event_type = action;
+       ctx->chg.timestamp_ns = ktime_get_ns();
+       gpio_desc_to_lineinfo(desc, &ctx->chg.info, true);
+       /* Keep the GPIO device alive until we emit the event. */
+       ctx->gdev = gpio_device_get(desc->gdev);
+       ctx->cdev = cdev;
+       /* Keep the file descriptor alive too. */
+       get_file(ctx->cdev->fp);
 
-       ret = kfifo_in_spinlocked(&cdev->events, &chg, 1, &cdev->wait.lock);
-       if (ret)
-               wake_up_poll(&cdev->wait, EPOLLIN);
-       else
-               pr_debug_ratelimited("lineinfo event FIFO is full - event dropped\n");
+       INIT_WORK(&ctx->work, lineinfo_changed_func);
+       queue_work(ctx->gdev->line_state_wq, &ctx->work);
 
        return NOTIFY_OK;
 }
@@ -2683,6 +2742,7 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
                goto out_unregister_line_notifier;
 
        file->private_data = cdev;
+       cdev->fp = file;
 
        ret = nonseekable_open(inode, file);
        if (ret)