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@xxxxxxxxxx>
> Cc: Ben Skeggs <bskeggs@xxxxxxxxxx>
> Cc: Jiri Slaby <jirislaby@xxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
But fwiw, the patch isn't wrong and fixes a false positive, so
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> 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);
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
