[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <47ea20d8-444b-4c72-97d3-b04a6918b121@baylibre.com>
Date: Wed, 20 Nov 2024 12:33:36 -0600
From: David Lechner <dlechner@...libre.com>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>, linux-input@...r.kernel.org
Cc: Erick Archer <erick.archer@...look.com>,
Christophe JAILLET <christophe.jaillet@...adoo.fr>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] Input: db9 - use guard notation when acquiring mutex
On 9/3/24 11:30 PM, Dmitry Torokhov wrote:
> Using guard notation makes the code more compact and error handling
> more robust by ensuring that mutexes are released in all code paths
> when control leaves critical section.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
> ---
> drivers/input/joystick/db9.c | 30 ++++++++++++++----------------
> 1 file changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/input/joystick/db9.c b/drivers/input/joystick/db9.c
> index 682a29c27832..7ac0cfc3e786 100644
> --- a/drivers/input/joystick/db9.c
> +++ b/drivers/input/joystick/db9.c
> @@ -505,24 +505,22 @@ static int db9_open(struct input_dev *dev)
> {
> struct db9 *db9 = input_get_drvdata(dev);
> struct parport *port = db9->pd->port;
> - int err;
>
> - err = mutex_lock_interruptible(&db9->mutex);
> - if (err)
> - return err;
> -
> - if (!db9->used++) {
> - parport_claim(db9->pd);
> - parport_write_data(port, 0xff);
> - if (db9_modes[db9->mode].reverse) {
> - parport_data_reverse(port);
> - parport_write_control(port, DB9_NORMAL);
> + scoped_guard(mutex_intr, &db9->mutex) {
> + if (!db9->used++) {
> + parport_claim(db9->pd);
> + parport_write_data(port, 0xff);
> + if (db9_modes[db9->mode].reverse) {
> + parport_data_reverse(port);
> + parport_write_control(port, DB9_NORMAL);
> + }
> + mod_timer(&db9->timer, jiffies + DB9_REFRESH_TIME);
> }
> - mod_timer(&db9->timer, jiffies + DB9_REFRESH_TIME);
> +
> + return 0;
> }
>
> - mutex_unlock(&db9->mutex);
> - return 0;
> + return -EINTR;
This patch and any others like it are potentially introducing a bug.
>From inspecting the source code, it looks like
mutex_lock_interruptible() can return -EINTR, -EALREADY, or -EDEADLK.
Before this patch, the return value of mutex_lock_interruptible() was
passed to the caller. Now, the return value is reduced to pass/fail
and only -EINTR is returned on failure when the reason could have
been something else.
Powered by blists - more mailing lists