linux-kernel - Re: [PATCH v1 1/3] gpiolib: sysfs: Move sysfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMRc=McRYRCa6A_Gynm-u-Ph9Z4L-cqT9_0e81M1_6nQ+yLDXw@mail.gmail.com>
Date:   Wed, 16 Feb 2022 16:03:38 +0100
From:   Bartosz Golaszewski <brgl@...ev.pl>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linus Walleij <linus.walleij@...aro.org>
Subject: Re: [PATCH v1 1/3] gpiolib: sysfs: Move sysfs_emit() calls outside of
 the mutex lock

On Wed, Feb 9, 2022 at 12:31 PM Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> wrote:
>
> In a few places we perform sysfs_emit() operations under mutex that
> do not require any locking. Move them outside of the mutex locks.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> ---
>  drivers/gpio/gpiolib-sysfs.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
> index 78ca7dee8b64..57d8ecab53b7 100644
> --- a/drivers/gpio/gpiolib-sysfs.c
> +++ b/drivers/gpio/gpiolib-sysfs.c
> @@ -61,17 +61,16 @@ static ssize_t direction_show(struct device *dev,
>  {
>         struct gpiod_data *data = dev_get_drvdata(dev);
>         struct gpio_desc *desc = data->desc;
> -       ssize_t                 status;
> +       int value;
>
>         mutex_lock(&data->mutex);
>
>         gpiod_get_direction(desc);
> -       status = sysfs_emit(buf, "%s\n",
> -                           test_bit(FLAG_IS_OUT, &desc->flags) ? "out" : "in");
> +       value = !!test_bit(FLAG_IS_OUT, &desc->flags);
>
>         mutex_unlock(&data->mutex);
>
> -       return status;
> +       return sysfs_emit(buf, "%s\n", value ? "out" : "in");
>  }
>
>  static ssize_t direction_store(struct device *dev,
> @@ -108,12 +107,13 @@ static ssize_t value_show(struct device *dev,
>         mutex_lock(&data->mutex);
>
>         status = gpiod_get_value_cansleep(desc);
> -       if (status >= 0)
> -               status = sysfs_emit(buf, "%zd\n", status);
>
>         mutex_unlock(&data->mutex);
>
> -       return status;
> +       if (status < 0)
> +               return status;
> +
> +       return sysfs_emit(buf, "%zd\n", status);
>  }
>
>  static ssize_t value_store(struct device *dev,
> @@ -238,7 +238,6 @@ static ssize_t edge_show(struct device *dev,
>                 struct device_attribute *attr, char *buf)
>  {
>         struct gpiod_data *data = dev_get_drvdata(dev);
> -       ssize_t status = 0;
>         int i;
>
>         mutex_lock(&data->mutex);
> @@ -247,12 +246,13 @@ static ssize_t edge_show(struct device *dev,
>                 if (data->irq_flags == trigger_types[i].flags)
>                         break;
>         }
> -       if (i < ARRAY_SIZE(trigger_types))
> -               status = sysfs_emit(buf, "%s\n", trigger_types[i].name);
>
>         mutex_unlock(&data->mutex);
>
> -       return status;
> +       if (i >= ARRAY_SIZE(trigger_types))
> +               return 0;
> +
> +       return sysfs_emit(buf, "%s\n", trigger_types[i].name);
>  }
>
>  static ssize_t edge_store(struct device *dev,
> @@ -324,16 +324,15 @@ static ssize_t active_low_show(struct device *dev,
>  {
>         struct gpiod_data *data = dev_get_drvdata(dev);
>         struct gpio_desc *desc = data->desc;
> -       ssize_t                 status;
> +       int value;
>
>         mutex_lock(&data->mutex);
>
> -       status = sysfs_emit(buf, "%d\n",
> -                           !!test_bit(FLAG_ACTIVE_LOW, &desc->flags));
> +       value = !!test_bit(FLAG_ACTIVE_LOW, &desc->flags);
>
>         mutex_unlock(&data->mutex);
>
> -       return status;
> +       return sysfs_emit(buf, "%d\n", value);
>  }
>
>  static ssize_t active_low_store(struct device *dev,
> --
> 2.34.1
>

Applied all three, thanks Andy!

Bart