Re: [PATCH] drm: Don't reference objects in the flink name idr

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Dec 03, 2013 at 08:33:46AM -0800, Kristian Høgsberg wrote:
> On Tue, Dec 3, 2013 at 7:26 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Mon, Dec 02, 2013 at 05:36:17PM -0800, Kristian Høgsberg wrote:
> >> There's no reason to keep a reference to objects in the name idr.  Each
> >> handle to an object has a reference to the object and just before we
> >> destroy the last handle we take the object out of the name idr.  Thus,
> >> if an object is in the name idr, there's at least one reference to the
> >> object.
> >>
> >> Or to put it another way, the name idr reference will never keep the
> >> object alive.  It just looks like it, which is confusing.
> >>
> >> Signed-off-by: Kristian Høgsberg <krh@bitplanet.net>
> >
> > I expect this to blow up when you race gem_close ioctl calls with flink
> > open. i-g-t/gem_flink_close tests actually have been written specifically
> > to exercise these races.
> > -Daniel
> 
> Can you be more specific about what race you see?  The one thing that
> could go wrong is that the last handle is delete after we enter
> drm_gem_flink_ioctl() and look up the object but before taking the
> object_name_lock, but that's handled by checking obj->handle_count
> under the lock.  Deleting handles and removing the name is always done
> under the object_name_lock from
> drm_gem_object_handle_unreference_unlocked().

Too many nightmares around lifetime rules, I've imagined some monster that
isn't ther and stand corrected.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch