| Message ID | 20191208161252.3015727-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/gem: Avoid rcu_barrier() from shrinker paths | expand |
On Sun, 8 Dec 2019 at 16:13, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > If someone else acquires the i915_vma before we complete our wait and > unbind it, we currently error out with -EBUSY. Use -EAGAIN instead so > that if necessary the caller is prepared to try again. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
On Sun, Dec 8, 2019 at 5:13 PM Chris Wilson <chris@chris-wilson.co.uk> wrote: > > If someone else acquires the i915_vma before we complete our wait and > unbind it, we currently error out with -EBUSY. Use -EAGAIN instead so > that if necessary the caller is prepared to try again. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Matthew Auld <matthew.auld@intel.com> > --- > drivers/gpu/drm/i915/i915_vma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c > index 9ca6664c190c..6794c742fbbf 100644 > --- a/drivers/gpu/drm/i915/i915_vma.c > +++ b/drivers/gpu/drm/i915/i915_vma.c > @@ -1181,7 +1181,7 @@ int __i915_vma_unbind(struct i915_vma *vma) > GEM_BUG_ON(i915_vma_is_active(vma)); > if (i915_vma_is_pinned(vma)) { > vma_print_allocator(vma, "is pinned"); > - return -EBUSY; > + return -EAGAIN; Maarten is apparently hitting this somehow in his dma_resv work, and no idea yet why. But just general comment: We can't be leaking temporary pins outside of holding the right locks, because then other threads can spot these pins and fail, because parts of whatever they need more of (vma or object doesn't really matter) can't be evicted properly. And sprinkling more EAGAIN all over the place really isn't the solution to get us out of these problems in the long term. So if we do have random spurious pins that leak out from under their locks, then we need to tug them back under those locks (struct_mutex is the worst case for a bunch of these right now). That's the fundamental shift in locking design with dma_resv vs the previous design that had the explicit goal of lots of temporary and not-so-temporary pins to untangle the locking. Now fully clear that we have a lot of that still lying around, but we really can't spread it further. -Daniel > } > > GEM_BUG_ON(i915_vma_is_active(vma)); > -- > 2.24.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index 9ca6664c190c..6794c742fbbf 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -1181,7 +1181,7 @@ int __i915_vma_unbind(struct i915_vma *vma) GEM_BUG_ON(i915_vma_is_active(vma)); if (i915_vma_is_pinned(vma)) { vma_print_allocator(vma, "is pinned"); - return -EBUSY; + return -EAGAIN; } GEM_BUG_ON(i915_vma_is_active(vma));
If someone else acquires the i915_vma before we complete our wait and unbind it, we currently error out with -EBUSY. Use -EAGAIN instead so that if necessary the caller is prepared to try again. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Matthew Auld <matthew.auld@intel.com> --- drivers/gpu/drm/i915/i915_vma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)