[Intel-gfx] [PATCH 04/23] drm/i915/gem: Only revoke mmap handlers if active

[Chris Wilson]

Quoting Tvrtko Ursulin (2020-07-02 13:35:41)
> 
> On 02/07/2020 09:32, Chris Wilson wrote:
> > Avoid waking up the device and taking stale locks if we know that the
> > object is not currently mmapped. This is particularly useful as not many
> > object are actually mmapped and so we can destroy them without waking
> > the device up, and gives us a little more freedom of workqueue ordering
> > during shutdown.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/gem/i915_gem_mman.c | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > index fe27c5b344e3..522ca4f51b53 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> > @@ -516,8 +516,11 @@ void i915_gem_object_release_mmap_offset(struct drm_i915_gem_object *obj)
> >    */
> >   void i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
> >   {
> > -     i915_gem_object_release_mmap_gtt(obj);
> > -     i915_gem_object_release_mmap_offset(obj);
> > +     if (obj->userfault_count)
> > +             i915_gem_object_release_mmap_gtt(obj);
> > +
> > +     if (!RB_EMPTY_ROOT(&obj->mmo.offsets))
> > +             i915_gem_object_release_mmap_offset(obj);
> >   }
> >   
> >   static struct i915_mmap_offset *
> > 
> 
> Both conditions will need explaining why they are not racy.

It's an identical race even if you do take the mutex.

Thread A		Thread B
release_mmap		create_mmap_offset
  mutex_lock/unlock	...
  			mutex_lock/unlock

Thread A will only operate on a snapshot of the current state with or
without the mutex; if Thread B is concurrently adding new mmaps, that
may occur before after Thread A makes decision the object is clean.
Thread A can only assess the state at that moment in time, and only
cares enough to ensure that from its pov, it has cleared the old
mmaps.

During free, we know there can be no concurrency (refcnt==0) and so the
snapshot is true.
-Chris