-----Original Message----- From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter Sent: Wednesday, April 27, 2016 3:09 AM To: Alex Deucher Cc: Chris Wilson; Daniel Vetter; DRI Development; Deucher, Alexander; Intel Graphics Development; Daniel Vetter Subject: Re: [Intel-gfx] [PATCH 08/35] drm: Protect dev->filelist with its own mutex
On Wed, Apr 27, 2016 at 09:06:09AM +0200, Daniel Vetter wrote:
On Tue, Apr 26, 2016 at 05:45:44PM -0400, Alex Deucher wrote:
On Tue, Apr 26, 2016 at 4:52 PM, Chris Wilson chris@chris-wilson.co.uk
wrote:
On Tue, Apr 26, 2016 at 07:29:41PM +0200, Daniel Vetter wrote:
amdgpu gained dev->struct_mutex usage, and that's because it's
walking
the dev->filelist list. Protect that list with it's own lock to take one more step towards getting rid of struct_mutex usage in drivers once and for all.
While doing the conversion I noticed that 2 debugfs files in i915 completely lacked appropriate locking. Fix that up too.
v2: don't forget to switch to
drm_gem_object_unreference_unlocked.
Cc: Alex Deucher alexander.deucher@amd.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
Just wondering if this worth converting over. Opening/closing isn't going to be high contention, I hope, though we can certainly write stress cases for it! The goal for drivers to stop using the struct_mutex as their BKL, which doesn't preclude keeping the struct_mutex around
for
where it makes sense to have a single mutex rather than a multitude.
I have some misgivings over this, but only because I think its overkill. Reviewed-by: Chris Wilson chris@chris-wilson.co.uk
I agree with Chris' sentiments.
It's not to have more speed or less contention, but just to have fewer things to worry about when reviewing locking. Hence orthogonal locks for independent parts.
My goal is that in the end dev->struct_mutex is only used by some existing drivers for their internals, plus all the legacy core stuff. And never even used by modern drivers. New locks are pretty cheap, and not
dragging
in the entire legacy horror show has benefits.
When/once I tackle the one thing left (master locking) I might move the master handling under this lock too (since it's closely related to open files). Not sure yet.
Oh and the main reason I did it here: Much easier to review using git grep struct_mutex than auditing codepaths. With this drivers have no reason to ever grab struct_mutex (not even for debugfs or similar), since the last remaining bit (master control) really shouldn't be a concern for drivers ever.
That also makes reviewing new drivers simpler: Contains struct_mutex? Reject it!
I'm convinced! Thanks for cleaning this up.
Alex
Cheers, Daniel
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch