Re: [Intel-gfx] [PATCH 2/2] drm/i915: Change i915_vma_unbind() to report -EAGAIN on activity

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	 Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>,
	Matthew Auld <matthew.auld@intel.com>
Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915: Change i915_vma_unbind() to report -EAGAIN on activity
Date: Fri, 21 Feb 2020 15:54:39 +0100	[thread overview]
Message-ID: <CAKMK7uGE0Fzw9CcmAZDN00tt4GCCX2Dk6O=JT7iODyvCXN3aag@mail.gmail.com> (raw)
In-Reply-To: <20191208161252.3015727-2-chris@chris-wilson.co.uk>

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx