[Intel-gfx] [PATCH] drm/i915/userptr: Wrap mmu_notifier inside its own rw_semaphore

Chris Wilson chris at chris-wilson.co.uk
Mon Mar 26 16:28:58 UTC 2018


Quoting Tvrtko Ursulin (2018-03-26 16:59:20)
> 
> On 26/03/2018 15:59, Chris Wilson wrote:
> > We've always been blatantly ignoring mmu_notifier.h:
> > 
> >   * Invalidation of multiple concurrent ranges may be
> >   * optionally permitted by the driver. Either way the
> >   * establishment of sptes is forbidden in the range passed to
> >   * invalidate_range_begin/end for the whole duration of the
> >   * invalidate_range_begin/end critical section.
> > 
> > by not preventing concurrent calls to gup while an invalidate_range is
> > being processed. Wrap gup and invalidate_range in a paired rw_semaphore
> > to allow concurrent lookups, that are then interrupted and disabled
> > across the invalidate_range. Further refinement can be applied by
> > tracking the invalidate_range versus individual gup, which should be
> > done using a common set of helpers for all mmu_notifier subsystems share
> > the same need. I hear HMM is one such toolbox...
> > 
> > For the time being, assume concurrent invalidate and lookup are rare,
> > but not rare enough to completely ignore.
> 
> I think I suggested a few times we should just "ban" the object on first 
> invalidate and never ever for its lifetime allow it to obtain backing 
> store again. I just don't remember why we decided not to go with that 
> approach. :( Thinking about it now I still don't see that this 
> restriction would be a problem and would simplify things.

You still have the problem where it is being banned as we are trying to
instantiate it the first time. Imo, we are re-implementing mmap_sem
crudely. (Even more so when every mmu notifier must implement the same
code, and more than one will be called everytime the mm is touched.)

And we can get perfectly innocent invalidates, e.g. mprotect.
 
> With more locks I am quite fearful what lockdep will say, but lets see...

Same here.
-Chris


More information about the Intel-gfx mailing list