linux-kernel - Re: [PATCH 2/3] gpiolib: use kref in gpio

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 5 Mar 2020 17:49:28 +0100
From:   Bartosz Golaszewski <brgl@...ev.pl>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Khouloud Touil <ktouil@...libre.com>
Subject: Re: [PATCH 2/3] gpiolib: use kref in gpio_desc

pon., 24 lut 2020 o 10:42 Bartosz Golaszewski <brgl@...ev.pl> napisał(a):
>
> From: Bartosz Golaszewski <bgolaszewski@...libre.com>
>
> GPIO descriptors are freed by consumers using gpiod_put(). The name of
> this function suggests some reference counting is going on but it's not
> true.
>
> Use kref to actually introduce reference counting for gpio_desc objects.
> Add a corresponding gpiod_get() helper for increasing the reference count.
>
> This doesn't change anything for already existing (correct) drivers but
> allows us to keep track of GPIO descs used by multiple users.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@...libre.com>

Linus,

This is in response to your suggestion under the previous version of this patch.

I refreshed my memory on device links and reference counting. I think
that device links are not the right tool for the problem I'm trying to
solve. You're right on the other hand about the need for reference
counting of gpiochip devices. Right now if we remove the chip with
GPIOs still requested the only thing that happens is a big splat:
"REMOVING GPIOCHIP WITH GPIOS STILL REQUESTED".

We should probably have a kref on the gpiochip structure which would
be set to 1 when registering the chip, increased and decreased on
every operation such as requesting and releasing a GPIO respectively
and decreased by gpiochip_remove() too. That way if we call
gpiochip_remove() while some users are still holding GPIO descriptors
then the only thing that happens is: the reference count for this
gpiochip is decreased. Once the final consumer calls the appropriate
release routine and the reference count goes to 0, we'd call the
actual gpiochip release code. This is similar to what the clock
framework does IIRC.

This patch however addresses a different issue: I'd like to add
reference counting to descriptors associated with GPIOs requested by
consumers. The kref release function would not trigger a destruction
of the gpiochip - just releasing of the requested GPIO. In this
particular use-case: we can pass an already requested GPIO descriptor
to nvmem. I'd like the nvmem framework to be able to reference it and
then drop the reference once it's done with the line, so that the life
of this resource is not controlled only by the entity that initially
requested it.

In other words: we could use two kref objects: one for the gpiochip
and one for GPIO descriptors.

I hope that makes it more clear.

Best regards,
Bartosz