[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.

I placed it in drm_driver which is static to get around that, did you
see the patch adding drm_driver_class? I didn't choose that path since
this might_lock clearly belongs to kref_put_mutex...

Similarly might_lock can also include might_sleep

+#define lock_is_mutex(lock) \
+       __builtin_types_compatible_p(typeof(*lock), struct mutex)
+
 # define might_lock(lock)                                              \
 do {                                                                   \
+       might_sleep_if(lock_is_mutex(lock));    


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