Re: 4.9-rc1 lockdep warning suggesting a deadlock between nouveau and i915 with prime video outputs active

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jul 13, 2017 at 03:04:58PM +0100, Chris Wilson wrote:
> Quoting Jiri Slaby (2017-07-13 14:57:31)
> > Stealing this thread as an opensuse user hit that too:
> > https://bugzilla.suse.com/show_bug.cgi?id=1045105
> 
> It's a false positive. I did once upon a time send some patches to move
> the lockdep warning to kref so that didn't need to call it from drm
> before an unlocked path. Basically you want
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 8dc11064253d..3118aed844f1 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -826,7 +826,6 @@ drm_gem_object_put_unlocked(struct drm_gem_object *obj)
>                 return;
>  
>         dev = obj->dev;
> -       might_lock(&dev->struct_mutex);
>  
>         if (dev->driver->gem_free_object_unlocked)
>                 kref_put(&obj->refcount, drm_gem_object_free);
> diff --git a/include/linux/kref.h b/include/linux/kref.h
> index 29220724bf1c..4b1133cd5d20 100644
> --- a/include/linux/kref.h
> +++ b/include/linux/kref.h
> @@ -77,6 +77,8 @@ static inline int kref_put_mutex(struct kref *kref,
>                                  void (*release)(struct kref *kref),
>                                  struct mutex *lock)
>  {
> +       might_lock(lock);
> +
>         if (refcount_dec_and_mutex_lock(&kref->refcount, lock)) {
>                 release(kref);
>                 return 1;
> 
> 
> Though now we probably want to move that might_lock() into refcount.

I think we want this here:

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 8dc11064253d..9663a79dd363 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -826,13 +826,15 @@ drm_gem_object_put_unlocked(struct drm_gem_object *obj)
 		return;
 
 	dev = obj->dev;
-	might_lock(&dev->struct_mutex);
 
 	if (dev->driver->gem_free_object_unlocked)
 		kref_put(&obj->refcount, drm_gem_object_free);
-	else if (kref_put_mutex(&obj->refcount, drm_gem_object_free,
+	else {
+		might_lock(&dev->struct_mutex);
+		if (kref_put_mutex(&obj->refcount, drm_gem_object_free,
 				&dev->struct_mutex))
-		mutex_unlock(&dev->struct_mutex);
+			mutex_unlock(&dev->struct_mutex);
+	}
 }
 EXPORT_SYMBOL(drm_gem_object_put_unlocked);
 
If it works I'll wrap it into a patch. Note you need rather new-ish kernel
to make sure we're sufficiently struct_mutex free in i915 (otherwise
there's still the locking loop). v4.10 should be new enough afaics.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel