[Intel-gfx] [PATCH 01/12] drm/i915/guc: Avoid reclaim locks during reset

[Chris Wilson]

Quoting Michal Wajdeczko (2019-07-01 13:36:28)
> On Mon, 01 Jul 2019 12:04:51 +0200, Chris Wilson  
> <chris at chris-wilson.co.uk> wrote:
> 
> > During reset, we must be very selective in which locks we take as most
> > are tainted by being held across a wait or reclaim (kmalloc) which
> > implicitly waits. Inside the guc reset path, we reset the ADS to sane
> > defaults, but must keep it pinned from initialisation to avoid having to
> > pin it during reset.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> 
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> 
> but I'm little worried about leaving stale guc->ads_blob below:
> 
> > @@ -183,7 +183,7 @@ int intel_guc_ads_create(struct intel_guc *guc)
> > void intel_guc_ads_destroy(struct intel_guc *guc)
> >  {
> > -     i915_vma_unpin_and_release(&guc->ads_vma, 0);
> > +     i915_vma_unpin_and_release(&guc->ads_vma, I915_VMA_RELEASE_MAP);
> >  }
> 
> maybe there is a way to get ptr right from the pinned/mapped vma
> without introducing extra separate field that might go out of sync ?

You mean the vaddr? I look at it as your token of ownership: this is the
address I pinned. While you own that pin, it is not allowed to change.

I expect, If we ever start wanting separate concurrent views of the
object, the return from pin_map will be its own little refcount -- or
simply not be cached. So to remind myself, the cache is because vmap is
slow and we use it frequently for cmdparsing.

So we could just transfer ownership of the map entirely to the caller
and leave it to utilities like the buffer cache to retain the map. I
don't think we actually have concurrent users of the maps, but I
wouldn't bet on it.
-Chris