From: sashiko-bot@kernel.org
To: "Nguyen Ngoc Thang" <ngocthang2710.1999@gmail.com>
Cc: linux-input@vger.kernel.org, dmitry.torokhov@gmail.com
Subject: Re: [PATCH v2] Input: serio - don't sleep on serio_mutex from drvctl_store()
Date: Thu, 24 Sep 2026 04:17:35 +0000 [thread overview]
Message-ID: <20260924041735.D5FE81F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260924040430.711439-1-ngocthang2710.1999@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] sysfs: write returning -EAGAIN breaks userspace ABI
--
commit 587df4e4a0dc7a2a601093d20c02612cf36f0cbd
Author: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
Input: serio - don't sleep on serio_mutex from drvctl_store()
Writing to the drvctl attribute takes serio_mutex while holding the
attribute's kernfs active reference. serio_unregister_port() does the
opposite: it holds serio_mutex and device_del() then waits for active
references to drain in kernfs_drain(). If a drvctl write is in flight
when the port is unregistered, the writer waits for serio_mutex and the
unregistering task waits for the writer, and neither makes progress.
> diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c
> index 54dd26249b027..5b650421e0663 100644
> --- a/drivers/input/serio/serio.c
> +++ b/drivers/input/serio/serio.c
> @@ -357,7 +357,8 @@ static ssize_t drvctl_store(struct device *dev, struct device_attribute *attr, c
> struct device_driver *drv;
> int error;
>
> - scoped_cond_guard(mutex_intr, return -EINTR, &serio_mutex) {
> + /* Port removal holds serio_mutex and drains us: don't wait for it. */
> + scoped_cond_guard(mutex_try, return -EAGAIN, &serio_mutex) {
[Severity: High]
Does returning -EAGAIN directly to the VFS here break the userspace ABI?
Standard userspace tools (like echo) performing blocking writes do not
automatically retry on -EAGAIN. This causes them to fail outright with
'Resource temporarily unavailable' when the trylock fails in drvctl_store()
during concurrent port removal.
Could this be resolved by using sysfs_break_active_protection() or returning
restart_syscall() (which sets TIF_SIGPENDING and returns -ERESTARTNOINTR)
instead, to allow transparent restarting?
> if (!strncmp(buf, "none", count)) {
> serio_disconnect_port(serio);
> } else if (!strncmp(buf, "reconnect", count)) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260924040430.711439-1-ngocthang2710.1999@gmail.com?part=1
prev parent reply other threads:[~2026-09-24 4:17 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-24 4:04 [PATCH v2] Input: serio - don't sleep on serio_mutex from drvctl_store() Nguyen Ngoc Thang
2026-09-24 4:17 ` sashiko-bot [this message]
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=20260924041735.D5FE81F000FF@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=dmitry.torokhov@gmail.com \
--cc=linux-input@vger.kernel.org \
--cc=ngocthang2710.1999@gmail.com \
--cc=sashiko-reviews@lists.linux.dev \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.