Re: [PATCH 00/18] dev->struct_mutex crusade

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2361 bytes --]

On Mon, Aug 10, 2015 at 12:53:23PM +0100, Chris Wilson wrote:
> On Mon, Aug 10, 2015 at 01:35:58PM +0200, Thierry Reding wrote:
> > On Thu, Jul 09, 2015 at 11:32:32PM +0200, Daniel Vetter wrote:
> > > Hi all,
> > > 
> > > I wanted to take another look at struct_mutex usage in modern (gem) drivers and
> > > noticed that for a fair lot we're very to be completely struct_mutex free.
> > > 
> > > This pile here is the the simple part, which mostly just removes code and
> > > mutex_lock/unlock calls. All the patches here are independent and can be merged
> > > in any order whatsoever. My plan is to send out a pull request for all those not
> > > picked up by driver maintainers in 2-3 weeks or so, assuming no one complains.
> > > 
> > > Of course review & comments still very much welcome.
> > > 
> > > The more tricky 2nd part of this (and that one's not yet done) is to rework the
> > > gem mmap handler to use the same kref_get_unless_zero trick as ttm. With that
> > > there's no core requirement to hold struct_mutex over the final unref, which
> > > means we can make that one lockless. I plan to add a gem_object_free_unlocked
> > > for all the drivers which don't have any need for this lock.
> > > 
> > > Also there's a few more drivers which can be made struct_mutex free easily, I'll
> > > propably stitch together poc patches for those.
> > 
> > There's a concurrency bug in Tegra DRM currently because we don't lock
> > accesses to drm_mm (I guess this demonstrates how badly we need better
> > testing...) and it seems like this is typically protected by the very
> > same struct_mutex that you're on a crusade to remove. If your goal is
> > to get rid of it for good, should we simply add a separate lock just
> > for the drm_mm? We don't have another one that would fit.
> 
> Actually that is one of the first targets for more fine-grained locking.
> I would not add a new lock to drm_mm as at least for i915, we want to use
> a similar per-vm lock (of which the drm_mm is just one part).

Sorry if I was being unclear. I wasn't suggesting adding the lock to
struct drm_mm, but rather add a driver-specific one specifically to
serialize accesses to the drm_mm. I agree that it's better to do this
in a driver-specific way because other structures may need to be
protected by the same lock.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel