[Intel-gfx] [PATCH] drm: Don't complain too much about struct_mutex.

[Chris Wilson]

Quoting Daniel Vetter (2017-07-15 10:53:28)
> For modern drivers the DRM core doesn't use struct_mutex at all, which
> means it's defacto a driver-private lock. But since we still need it
> for legacy drivers we can't initialize it in drivers, which means all
> the different instances share one lockdep key. Despite that they might
> be placed in totally different places in the locking hierarchy.
> 
> This results in a lot of bogus lockdep splats when running stuff on
> systems with multiple gpus. Partially remedy the situation by only
> doing might_lock checks on drivers that do use struct_mutex still for
> gem locking.
> 
> A more complete solution would be to do the mutex_init in the drm core
> only for legacy drivers, plus add it to each modern driver that still
> needs it, which would also give each its own lockdep key. Trying to do
> that dynamically doesn't work, because lockdep requires it's keys to
> be statically allocated.
> 
> Cc: Hans de Goede <hdegoede at redhat.com>
> Cc: Ben Skeggs <bskeggs at redhat.com>
> Cc: Jiri Slaby <jirislaby at gmail.com>
> Cc: Peter Zijlstra <peterz at infradead.org>
> Cc: Ingo Molnar <mingo at redhat.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>

But fwiw, the patch isn't wrong and fixes a false positive, so
Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

>  drivers/gpu/drm/drm_gem.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> 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)

coding-style nit, if one branch needs {} they all need {}.
(The real reason why I didn't want to move might_lock around in this
function ;)

>                 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);