On Mon, Aug 07, 2017 at 01:22:23PM +0300, Laurent Pinchart wrote: > Hi Daniel, > > On Monday 07 Aug 2017 11:25:07 Daniel Vetter wrote: > > On Sat, Aug 05, 2017 at 12:59:07PM +0200, Noralf Trønnes wrote: > > > Den 05.08.2017 00.19, skrev Ilia Mirkin: > > >> On Fri, Aug 4, 2017 at 4:43 PM, Eric Anholt <eric@xxxxxxxxxx> wrote: > > >>> Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> writes: > > >>>> On Tuesday 18 Jul 2017 14:05:06 Eric Anholt wrote: > > >>>>> This will let drivers reduce the error cleanup they need, in > > >>>>> particular the "is_panel_bridge" flag. > > >>>>> > > >>>>> v2: Slight cleanup of remove function by Andrzej > > >>>> > > >>>> I just want to point out that, in the context of Daniel's work on > > >>>> hot-unplug, 90% of the devm_* allocations are wrong and will get in > > >>>> the way. All DRM core objects that are accessible one way or > > >>>> another from userspace will need to be properly reference-counted > > >>>> and freed only when the last reference disappears, which could be > > >>>> well after the corresponding device is removed. I believe this > > >>>> could be one such objects :-/ > > >>> > > >>> Sure, if you're hotplugging, your life is pain. For non-hotpluggable > > >>> devices, like our SOC platform devices (current panel-bridge > > >>> consumers), this still seems like an excellent simplification of > > >>> memory management. > > >> > > >> At that point you may as well make your module non-unloadable, and > > >> return failure when trying to remove a device from management by the > > >> driver (whatever the opposite of "probe" is, I forget). Hotplugging > > >> doesn't only happen when physically removing, it can happen for all > > >> kinds of reasons... and userspace may still hold references in some of > > >> those cases. > > > > > > If drm_open() gets a ref on dev->dev and puts it in drm_release(), > > > won't that delay devm_* cleanup until userspace is done? > > > > No. drm_device is the thing that is refcounted for userspace references > > like open FD (we're not perfect about it, e.g. sysfs and dma-buf/fence > > don't). > > > > devm_ otoh is tied to the lifetime of the underlying device, and that one > > can get outlived by drm_device. Or at least afaiui, devm_ stuff is nuked > > on unplug, and not when the final sw reference of the struct device > > disappears. > > > > Not sure tough, it's complicated. > > It's complicated when you have to understand the behaviour by reading the > code, but the behaviour isn't that complex. devm resources are released both > > 1. right after the driver's .remove() operation returns > 2. when the device is deleted (last reference released) Right, I had vague memories when reading the code ... But iirc there's also some way to delay cleanup until it's deleted, maybe we could exploit that (and wrap it up into a drm_devm_add wrapper)? Plan B, but that is a lot more ugly, would be to create a fake struct device that we never bind (hence remove can't be called) and use to track the lifetime of drm_device stuff. Would avoid re-inventing all the devm_ functions, but would also result in a dummy device in sysfs. Why is this stuff tied to struct device and not struct kobject anyway. Or at least something we could embed in random cool place (like drm_device). Greg, any ideas how we could simplify management of stuff that's created at driver load, but its lifetime tied to something which isn't directly a struct device? > It's the first one that makes devm_* allocation unsuitable for any structure > that is accessible from userspace (such as in file operation handlers). Yeah, if we could release at 2. it would be perfect I think ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
