Re: [RESEND PATCH v6 5/7] gpiolib: provide a dedicated function for setting lineinfo

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Bartosz,

On Tue, Feb 11, 2020 at 10:21 AM Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
> From: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
> We'll soon be filling out the gpioline_info structure in multiple
> places. Add a separate function that given a gpio_desc sets all relevant
> fields.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

This is now commit d2ac25798208fb85 ("gpiolib: provide a dedicated
function for setting lineinfo") in gpio/for-next.

> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1147,6 +1147,60 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
>         return ret;
>  }
>
> +static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
> +                                 struct gpioline_info *info)
> +{
> +       struct gpio_chip *chip = desc->gdev->chip;
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&gpio_lock, flags);

spinlock taken

> +
> +       if (desc->name) {
> +               strncpy(info->name, desc->name, sizeof(info->name));
> +               info->name[sizeof(info->name) - 1] = '\0';
> +       } else {
> +               info->name[0] = '\0';
> +       }
> +
> +       if (desc->label) {
> +               strncpy(info->consumer, desc->label, sizeof(info->consumer));
> +               info->consumer[sizeof(info->consumer) - 1] = '\0';
> +       } else {
> +               info->consumer[0] = '\0';
> +       }
> +
> +       /*
> +        * Userspace only need to know that the kernel is using this GPIO so
> +        * it can't use it.
> +        */
> +       info->flags = 0;
> +       if (test_bit(FLAG_REQUESTED, &desc->flags) ||
> +           test_bit(FLAG_IS_HOGGED, &desc->flags) ||
> +           test_bit(FLAG_USED_AS_IRQ, &desc->flags) ||
> +           test_bit(FLAG_EXPORT, &desc->flags) ||
> +           test_bit(FLAG_SYSFS, &desc->flags) ||
> +           !pinctrl_gpio_can_use_line(chip->base + info->line_offset))

pinctrl_gpio_can_use_line(), and pinctrl_get_device_gpio_range() called
from it, call mutex_lock():

    BUG: sleeping function called from invalid context at
kernel/locking/mutex.c:281
    in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid: 652, name: lsgpio
    CPU: 1 PID: 652 Comm: lsgpio Not tainted
5.6.0-rc1-koelsch-00008-gd2ac25798208fb85 #755
    Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
    [<c020e3f0>] (unwind_backtrace) from [<c020a5b8>] (show_stack+0x10/0x14)
    [<c020a5b8>] (show_stack) from [<c07d31b4>] (dump_stack+0x88/0xa8)
    [<c07d31b4>] (dump_stack) from [<c0241318>] (___might_sleep+0xf8/0x168)
    [<c0241318>] (___might_sleep) from [<c07ec13c>] (mutex_lock+0x24/0x7c)
    [<c07ec13c>] (mutex_lock) from [<c046f47c>]
(pinctrl_get_device_gpio_range+0x1c/0xb4)
    [<c046f47c>] (pinctrl_get_device_gpio_range) from [<c046f5e8>]
(pinctrl_gpio_can_use_line+0x24/0x88)
    [<c046f5e8>] (pinctrl_gpio_can_use_line) from [<c0478bd0>]
(gpio_ioctl+0x270/0x584)
    [<c0478bd0>] (gpio_ioctl) from [<c03194c0>] (vfs_ioctl+0x20/0x38)

Reproducer is "lsgpio" with CONFIG_DEBUG_ATOMIC_SLEEP=y.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds