Re: cdev/devm_* issues (was Re: [TECH TOPIC] Rust for Linux)

Linus Walleij
2021-07-08T23:06:35Z

Thanks Laurent, Greg, this is important.

Replying here to get this in the right thread.

> On Wed, Jul 07, 2021 at 08:17:08PM +0300, Laurent Pinchart wrote:

> > Here's what
> > I think should be done for drivers that expose character devices.
> >
> > - Drivers must stop allocating their internal data structure (the one
> > they set as device data with dev_set_drvdata()) with devm_kzalloc().
> > The data structure must instead be allocated with a plain kzalloc()
> > and reference-counted.
> >
> > Most drivers will register a single character device using a
> > subsystem-specific API (e.g. video_register_device() in V4L2). The
> > subsystem needs to provide a .release() callback, called when the
> > last reference to the character device is released. Drivers must
> > implement this, and can simply free their internal data structure at
> > this point.

What we did in GPIO to get around the whole issue is to split
our device in two abstractions. The device that spawns a GPIO
driver can be a GPIO on any bus (platform, PCI, SPI, I2C, ...)
then that populates and registers one (or more) gpio_chips.
That is a struct that deal with handling the hardware but does
not contain any struct device.

When registering gpio_chip we create a struct gpio_device which
contains a struct device and a struct cdev (exposed to userspace)
and all in-kernel consumer handles (called struct gpio_desc). This
is reference counted on the struct device and uses
->release() to eventually clean itself up.

The crucial part is what happens when a device with GPIOs
disappears, if e.g. the USB device is unplugged or the driver
is rmmod:ed by force from the command line. We then unregister
the struct gpio_chip and drop all devm_* resources taken by the
driver (referencing the struct dev in the USB device or so) so these
go away, but the struct gpio_device stays around
until the last reference from userspace is dropped.

In order to not crash calls from the character device the device is
"numbed", so any calls will just return "OK" but nothing happens.
We then hope userspace will be so nice to terminate once it realizes
that it is no longer needed, closing the chardev and releasing the
resources held.

This works for us but I admit it is a bit kludgy. I guess it is not a
generally useful practice, we just had to come up with something.

Yours,
Linus Walleij