linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bartosz Golaszewski <brgl@bgdev.pl>
To: Linus Walleij <linus.walleij@linaro.org>,
	 Bartosz Golaszewski <brgl@bgdev.pl>,
	Kent Gibson <warthog618@gmail.com>
Cc: linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Subject: [PATCH v2 4/6] gpio: cdev: put emitting the line state events on a workqueue
Date: Thu, 10 Oct 2024 11:10:25 +0200	[thread overview]
Message-ID: <20241010-gpio-notify-in-kernel-events-v2-4-b560411f7c59@linaro.org> (raw)
In-Reply-To: <20241010-gpio-notify-in-kernel-events-v2-0-b560411f7c59@linaro.org>

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

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.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpiolib-cdev.c | 72 +++++++++++++++++++++++++++++++++++++--------
 1 file changed, 60 insertions(+), 12 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index dec6c9a6005f..2677134b52cd 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -2451,6 +2451,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)
@@ -2621,29 +2622,75 @@ 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;
+
+	scoped_guard(srcu, &ctx->gdev->srcu) {
+		gc = srcu_dereference(ctx->gdev->chip, &ctx->gdev->srcu);
+		if (!gc)
+			return;
+
+		/*
+		 * 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.
+		 */
+		if (!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);
-	supinfo_to_lineinfo(desc, &chg.info);
+	ctx = kzalloc(sizeof(*ctx), GFP_ATOMIC);
+	if (!ctx) {
+		pr_err("Failed to allocate memory for line info notification\n");
+		return NOTIFY_DONE;
+	}
 
-	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");
+	ctx->chg.event_type = action;
+	ctx->chg.timestamp_ns = ktime_get_ns();
+	gpio_desc_to_lineinfo(desc, &ctx->chg.info, true);
+	supinfo_to_lineinfo(desc, &ctx->chg.info);
+	/* 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);
+
+	INIT_WORK(&ctx->work, lineinfo_changed_func);
+	queue_work(ctx->gdev->line_state_wq, &ctx->work);
 
 	return NOTIFY_OK;
 }
@@ -2803,6 +2850,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)

-- 
2.43.0


  parent reply	other threads:[~2024-10-10  9:10 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-10  9:10 [PATCH v2 0/6] gpio: notify user-space about config changes in the kernel Bartosz Golaszewski
2024-10-10  9:10 ` [PATCH v2 1/6] gpiolib: notify user-space when a driver requests its own desc Bartosz Golaszewski
2024-10-10  9:10 ` [PATCH v2 2/6] gpio: cdev: prepare gpio_desc_to_lineinfo() for being called from atomic Bartosz Golaszewski
2024-10-14  1:58   ` Kent Gibson
2024-10-14  7:45     ` Bartosz Golaszewski
2024-10-14  8:32       ` Kent Gibson
2024-10-14  8:41         ` Bartosz Golaszewski
2024-10-10  9:10 ` [PATCH v2 3/6] gpiolib: add a per-gpio_device line state notification workqueue Bartosz Golaszewski
2024-10-10  9:10 ` Bartosz Golaszewski [this message]
2024-10-14  2:09   ` [PATCH v2 4/6] gpio: cdev: put emitting the line state events on a workqueue Kent Gibson
2024-10-14  7:47     ` Bartosz Golaszewski
2024-10-10  9:10 ` [PATCH v2 5/6] gpiolib: switch the line state notifier to atomic Bartosz Golaszewski
2024-10-14  2:11   ` Kent Gibson
2024-10-14  7:48     ` Bartosz Golaszewski
2024-10-14  9:24       ` Kent Gibson
2024-10-14  9:27         ` Bartosz Golaszewski
2024-10-14  9:29           ` Kent Gibson
2024-10-14  9:32             ` Bartosz Golaszewski
2024-10-14  9:55               ` Kent Gibson
2024-10-14  9:57                 ` Bartosz Golaszewski
2024-10-10  9:10 ` [PATCH v2 6/6] gpiolib: notify user-space about in-kernel line state changes Bartosz Golaszewski
2024-10-14  2:24   ` Kent Gibson
2024-10-14  8:13     ` Bartosz Golaszewski
2024-10-14  8:42       ` Kent Gibson

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=20241010-gpio-notify-in-kernel-events-v2-4-b560411f7c59@linaro.org \
    --to=brgl@bgdev.pl \
    --cc=bartosz.golaszewski@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@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;
as well as URLs for NNTP newsgroup(s).