[Intel-gfx] [PATCH] drm: Don't complain too much about struct_mutex.
Chris Wilson
chris at chris-wilson.co.uk
Sat Jul 15 09:59:19 UTC 2017
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);
More information about the Intel-gfx
mailing list