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 <hdego...@redhat.com> > Cc: Ben Skeggs <bske...@redhat.com> > Cc: Jiri Slaby <jirisl...@gmail.com> > Cc: Peter Zijlstra <pet...@infradead.org> > Cc: Ingo Molnar <mi...@redhat.com> > Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
But fwiw, the patch isn't wrong and fixes a false positive, so Reviewed-by: Chris Wilson <ch...@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); _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel