[PATCH 09/25] drm/gem: Check locking in drm_gem_object_unreference

[Daniel Vetter]

On Thu, Oct 15, 2015 at 10:36:08AM +0200, David Herrmann wrote:
> Hi
> 
> On Thu, Oct 15, 2015 at 9:36 AM, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> > Pretty soon only some drivers will need dev->struct_mutex in their
> > gem_free_object callbacks. Hence it's really important to make sure
> > everything still keeps getting this right.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > ---
> >  include/drm/drm_gem.h | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> > index 7a592d7e398b..5b3754864fb0 100644
> > --- a/include/drm/drm_gem.h
> > +++ b/include/drm/drm_gem.h
> > @@ -142,6 +142,8 @@ drm_gem_object_reference(struct drm_gem_object *obj)
> >  static inline void
> >  drm_gem_object_unreference(struct drm_gem_object *obj)
> >  {
> > +       WARN_ON(!mutex_is_locked(&obj->dev->struct_mutex));
> > +
> 
> This doesn't work. mutex_is_locked() is not context-aware, so it does
> not check whether the holder is the current thread or someone else.
> You *have* to use lockdep if you want negative lock checks.
> 
> In other words, if some other thread holds the mutex in parallel to
> this being called, you will trigger the WARN_ON.

It's the other way round: If another thread holds the lock, but the caller
doesn't, then we won't WARN: It's a false negative, not a false positive.

And since lockdep is a serious hog and no one runs it I prefer this way
round than the "perfect" lockdep_assert_hold.

Not that in the unlocked variant we can't afford this mistake, so there we
do rely upon the perfect lockdep locking tracking and use might_lock.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch