Chris Wilson <[email protected]> writes: > We cannot nest i915_reset_trylock() as the inner may wait for the > I915_RESET_BACKOFF which in turn is waiting upon sync_srcu who is > waiting for our outermost lock. As we take the reset srcu around the > fence update, we have to defer taking it in i915_gem_fault() until after > we acquire the pin on the fence to avoid nesting. This is a little ugly, > but still works. If a reset occurs between i915_vma_pin_fence() and the > second reset lock, the reset will restore the fence register back to the > pinned value before the reset lock allows us to proceed (our mmap won't > be revoked as we haven't yet marked it as being a userfault as that > requires us to hold the reset lock), so the pagefault is still > serialised with the revocation in reset. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109605 > Fixes: 2caffbf11762 ("drm/i915: Revoke mmaps and prevent access to fence > registers across reset") > Signed-off-by: Chris Wilson <[email protected]> > Cc: Mika Kuoppala <[email protected]> > --- > drivers/gpu/drm/i915/i915_gem.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index c8c355bec091..ae1467a74a08 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -1923,16 +1923,16 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf) > if (ret) > goto err_unpin; > > + ret = i915_vma_pin_fence(vma); > + if (ret) > + goto err_unpin; > +
As this is obviusness slipped past us, would it be worthwhile, in retrospect, to build a debug in i915_reset_trylock to be vocal about it failing to make progress? Reviewed-by: Mika Kuoppala <[email protected]> > srcu = i915_reset_trylock(dev_priv); > if (srcu < 0) { > ret = srcu; > - goto err_unpin; > + goto err_fence; > } > > - ret = i915_vma_pin_fence(vma); > - if (ret) > - goto err_reset; > - > /* Finally, remap it using the new GTT offset */ > ret = remap_io_mapping(area, > area->vm_start + (vma->ggtt_view.partial.offset > << PAGE_SHIFT), > @@ -1940,7 +1940,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf) > min_t(u64, vma->size, area->vm_end - > area->vm_start), > &ggtt->iomap); > if (ret) > - goto err_fence; > + goto err_reset; > > /* Mark as being mmapped into userspace for later revocation */ > assert_rpm_wakelock_held(dev_priv); > @@ -1950,10 +1950,10 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf) > > i915_vma_set_ggtt_write(vma); > > -err_fence: > - i915_vma_unpin_fence(vma); > err_reset: > i915_reset_unlock(dev_priv, srcu); > +err_fence: > + i915_vma_unpin_fence(vma); > err_unpin: > __i915_vma_unpin(vma); > err_unlock: > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
