Re: [TECH TOPIC] Rust for Linux

[Mauro Carvalho Chehab <mchehab@kernel.org>]

Em Wed, 7 Jul 2021 20:17:08 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> On Wed, Jul 07, 2021 at 02:45:04PM +0200, Greg KH wrote:
> > On Wed, Jul 07, 2021 at 01:38:44PM +0100, James Bottomley wrote:  
> > > On Wed, 2021-07-07 at 14:20 +0200, Greg KH wrote:  
> > > > On Wed, Jul 07, 2021 at 12:34:31PM +0100, James Bottomley wrote:  
> > > > > On Wed, 2021-07-07 at 11:50 +0100, Mark Brown wrote:  
> > > > > > On Tue, Jul 06, 2021 at 10:36:14PM +0200, Linus Walleij wrote:  
> > > > > > > On Tue, Jul 6, 2021 at 10:00 PM Roland Dreier wrote:  
> > > > > > > > "devres" / devm_xxx was an attempt to deal with this in C,
> > > > > > > > but it only solves some cases of this and has not received a
> > > > > > > > lot of adoption (we can argue about the reasons).  
> > > > > > >  
> > > > > > > Really? From my point of view that is adopted all over the map.
> > > > > > > I add new users all the time and use it as much as I can when
> > > > > > > writing new drivers.  
> > > > > > 
> > > > > > Yes, it's *super* widely used in most of the kernel.  Perhaps
> > > > > > there's some subsystems that reject it for some reason.
> > > > > >   
> > > > > > > I think it's a formidable success, people just need to learn to
> > > > > > > do it more.  
> > > > > > 
> > > > > > There *are* issues with people adopting it too enthusiastically -
> > > > > > as well as the memory lifetime issues that Laurent mentioned it's
> > > > > > easy for it to cause problems with interrupt handlers that are
> > > > > > left live longer than they should be and try to use things that
> > > > > > were already deallocated.  
> 
> (CC'ing Daniel Vetter as the author of the DRM devres-based resource
> management)
> 
> I've given this lots of thoughts lately, in the context of V4L2, but it
> should be roughly the same for character devices in general. 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.
> 
>   For drivers that register multiple character devices, or in general
>   expose multiple interfaces to userspace or other parts of the kernel,
>   the internal data structure must be properly reference-counted, with a
>   reference released in each .release() callback. There may be ways to
>   simplify this.

Good point. Yeah, indeed some work seems to be required on that area.

Yet, V4L2 is somewhat different here, as most (if not all) devices 
expose multiple cdevs. 

Also, in the case of V4L2 USB and PCI devices, their "dev->parent" is 
usually a PCI bridge, or an USB device with multiple functions on it,
as those hardware contain both audio and video and sometimes input
(either buttons or remote controllers), typically using different 
drivers. So, when the hardware is hot-unplugged or unbind, several 
drivers and multiple cdevs will be released. Ensuring that those will
happen at the right time can be a challenge, specially if there are
pending syscalls and/or threads by the time the device is unbound.

The DRM subsystem likely fits on the same case, as drivers also 
usually create multiple cdevs, and there are DMABUF objects shared
between different struct devices.

So, yeah, using devm_* for V4L2 and DRM can indeed bring troubles.
I can't see a solution there currently but to avoid using devm*,
handling it using an approach similar to the one you described.

-

I'm not so sure if using devm_* is a problem on several cases, though.
I mean, when the hardware is not hot-pluggable, the data lifetime
is a lot simpler.

So, for instance, a regulator driver probably can use devm_* without
any issues, as it doesn't seem to make much sense to unbind a regulator
once the device was probed. On drivers like that, not using devm_*
would just make the probing part of the driver more complex, without
bringing any real benefit.

Thanks,
Mauro