Date:	Wed, 15 Jul 2015 18:34:13 +0200
From:	Takashi Iwai <tiwai@...e.de>
To:	Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: Is devm_* broken ?

On Wed, 15 Jul 2015 18:27:42 +0200,
Laurent Pinchart wrote:
> 
> On Wednesday 15 July 2015 18:20:02 Takashi Iwai wrote:
> > On Wed, 15 Jul 2015 18:08:34 +0200, Laurent Pinchart wrote:
> > > On Wednesday 15 July 2015 17:51:28 Takashi Iwai wrote:
> > > > On Wed, 15 Jul 2015 00:34:53 +0200, Laurent Pinchart wrote:
> > > > > Hello,
> > > > > 
> > > > > I came to realize not too long ago that the following sequence of
> > > > > events will lead to a crash with any platform driver that uses devm_*
> > > > > and creates device nodes.
> > > > > 
> > > > > 1. Get a platform device bound it its driver
> > > > > 2. Open the corresponding device node in userspace and keep it open
> > > > > 3. Unbind the platform device from its driver through sysfs
> > > > > 
> > > > > echo <device-name> > /sys/bus/platform/drivers/<driver-name>/unbind
> > > > > 
> > > > > (or for hotpluggable devices just unplug the device)
> > > > > 
> > > > > 4. Close the device node
> > > > > 5. Enjoy the fireworks
> > > > > 
> > > > > While having a device node open prevents modules from being unloaded,
> > > > > it doesn't prevent devices from being unbound from drivers. If the
> > > > > driver uses devm_* helpers to allocate memory the memory will be freed
> > > > > when the device is unbound from the driver, but that memory will still
> > > > > be used by any operation touching an open device node.
> > > > > 
> > > > > Is devm_* inherently broken ? It's so widely used, tell me I'm missing
> > > > > something obvious.
> > > > 
> > > > I don't think this is specific to devm_*() but it's about the resource
> > > > management in general.  After bus or driver's remove callback, all
> > > > device resources that have been assigned by the driver are supposed to
> > > > be freed, or ready to be freed.
> > > 
> > > The remove callback notifies drivers that the device has been removed and
> > > that it's time to clean up. However, drivers have no control over
> > > userspace, so they can't force applications to close all open file
> > > handles, unmap memory and otherwise free all device-related resources
> > > immediately and synchronously. The best a driver can do is prevent any
> > > new reference to a resource from being taken by userspace (returning an
> > > error from open() for instance) and wait until all existing references
> > > get released before finally freeing resources. This is where devm_* hurts
> > > as a driver can't delay freeing resources until after all references held
> > > by userspace are released.
> > 
> > Right, and this is what ALSA drivers does in general.
> 
> Does that mean that an ALSA driver that uses devm_* will crash if the device 
> is unbound from the driver (possibly because it gets disconnected) while 
> userspace uses the ALSA device ? Isn't that considered as an issue ?

No, usually the driver calls snd_card_remove() and it blocks until all
resources are closed/freed, thus devm_*() (that will be released after
that) won't crash.  For the disconnection, there is an asynchronous
version, and the rest is managed at the last close.

> > > If I were to switch the uvcvideo driver from kzalloc to devm_kzalloc it
> > > would crash if the webcam gets disconnected while userspace has the V4L2
> > > device node open.
> > 
> > The disconnection is a bit different story, but I see your concern.
> 
> From a resources release point of view disconnection and unbind are similar.

Similar but slightly different, IIRC.  The disconnect (in USB) is a
part of unbind, but not vice versa.


Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists