On 05/08/2020 13:22, Chris Wilson wrote:
Directly seralise the atomic pinning with evicting the vma from unbind
with a pair of coupled cmpxchg to avoid fighting over vm->mutex.
Assumption being bind/unbind should never contend and create a
busy-spinny section? And motivation being.. ?
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_vma.c | 45 ++++++++++-----------------------
1 file changed, 14 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index dbe11b349175..17ce0bce318e 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -742,12 +742,10 @@ i915_vma_detach(struct i915_vma *vma)
bool i915_vma_pin_inplace(struct i915_vma *vma, unsigned int flags)
{
- unsigned int bound;
- bool pinned = true;
+ unsigned int bound = atomic_read(&vma->flags);
GEM_BUG_ON(flags & ~I915_VMA_BIND_MASK);
- bound = atomic_read(&vma->flags);
do {
if (unlikely(flags & ~bound))
return false;
@@ -755,34 +753,10 @@ bool i915_vma_pin_inplace(struct i915_vma *vma, unsigned int flags)
if (unlikely(bound & (I915_VMA_OVERFLOW | I915_VMA_ERROR)))
return false;
- if (!(bound & I915_VMA_PIN_MASK))
- goto unpinned;
-
GEM_BUG_ON(((bound + 1) & I915_VMA_PIN_MASK) == 0);
} while (!atomic_try_cmpxchg(&vma->flags, &bound, bound + 1));
return true;
-
-unpinned:
- /*
- * If pin_count==0, but we are bound, check under the lock to avoid
- * racing with a concurrent i915_vma_unbind().
- */
- mutex_lock(&vma->vm->mutex);
- do {
- if (unlikely(bound & (I915_VMA_OVERFLOW | I915_VMA_ERROR))) {
- pinned = false;
- break;
- }
-
- if (unlikely(flags & ~bound)) {
- pinned = false;
- break;
- }
- } while (!atomic_try_cmpxchg(&vma->flags, &bound, bound + 1));
- mutex_unlock(&vma->vm->mutex);
-
- return pinned;
}
static int vma_get_pages(struct i915_vma *vma)
@@ -1292,6 +1266,7 @@ void __i915_vma_evict(struct i915_vma *vma)
int __i915_vma_unbind(struct i915_vma *vma)
{
+ unsigned int bound;
int ret;
lockdep_assert_held(&vma->vm->mutex);
@@ -1299,10 +1274,18 @@ int __i915_vma_unbind(struct i915_vma *vma)
if (!drm_mm_node_allocated(&vma->node))
return 0;
- if (i915_vma_is_pinned(vma)) {
- vma_print_allocator(vma, "is pinned");
- return -EAGAIN;
- }
+ /* Serialise with i915_vma_pin_inplace() */
+ bound = atomic_read(&vma->flags);
+ do {
+ if (unlikely(bound & I915_VMA_PIN_MASK)) {
+ vma_print_allocator(vma, "is pinned");
+ return -EAGAIN;
+ }
+
+ if (unlikely(bound & I915_VMA_ERROR))
+ break;
+ } while (!atomic_try_cmpxchg(&vma->flags,
+ &bound, bound | I915_VMA_ERROR));
Using the error flag is somehow critical for this scheme to work? Can
you please explain in the comment and/or commit message?
/*
* After confirming that no one else is pinning this vma, wait for
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
