lkml.org 
[lkml]   [2022]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] drm/i915: Fix random -ENOSPC eviction errors due to locked vma objects
Date
The i915_gem_object_trylock we had in the grab_vma() makes it return false
when the vma->obj is already locked. In this case we'll skip this vma
during eviction, and eventually might be forced to return -ENOSPC even
though we could've evicted this vma if we waited for the lock a bit.

To fix this, replace the i915_gem_object_trylock with i915_gem_object_lock.
And because we have to worry about the potential deadlock now, bubble-up
the error code, so it will be correctly handled by the WW mechanism.

This fixes the issue https://gitlab.freedesktop.org/drm/intel/-/issues/6564

Fixes: 7e00897be8bf ("drm/i915: Add object locking to i915_gem_evict_for_node and i915_gem_evict_something, v2.")
Signed-off-by: Sviatoslav Peleshko <sviatoslav.peleshko@globallogic.com>
---
drivers/gpu/drm/i915/i915_gem_evict.c | 69 ++++++++++++++++++---------
1 file changed, 46 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index f025ee4fa526..9d43f213f68f 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -55,49 +55,58 @@ static int ggtt_flush(struct intel_gt *gt)
return intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
}

-static bool grab_vma(struct i915_vma *vma, struct i915_gem_ww_ctx *ww)
+static int grab_vma(struct i915_vma *vma, struct i915_gem_ww_ctx *ww)
{
+ int ret = 0;
+
/*
* We add the extra refcount so the object doesn't drop to zero until
* after ungrab_vma(), this way trylock is always paired with unlock.
*/
if (i915_gem_object_get_rcu(vma->obj)) {
- if (!i915_gem_object_trylock(vma->obj, ww)) {
+ ret = i915_gem_object_lock(vma->obj, ww);
+ if (ret)
i915_gem_object_put(vma->obj);
- return false;
- }
} else {
/* Dead objects don't need pins */
atomic_and(~I915_VMA_PIN_MASK, &vma->flags);
}

- return true;
+ return ret;
}

-static void ungrab_vma(struct i915_vma *vma)
+static void ungrab_vma(struct i915_vma *vma, struct i915_gem_ww_ctx *ww)
{
if (dying_vma(vma))
return;

- i915_gem_object_unlock(vma->obj);
+ if (!ww)
+ i915_gem_object_unlock(vma->obj);
+
i915_gem_object_put(vma->obj);
}

-static bool
+static int
mark_free(struct drm_mm_scan *scan,
struct i915_gem_ww_ctx *ww,
struct i915_vma *vma,
unsigned int flags,
struct list_head *unwind)
{
+ int err;
+
if (i915_vma_is_pinned(vma))
- return false;
+ return -ENOSPC;

- if (!grab_vma(vma, ww))
- return false;
+ err = grab_vma(vma, ww);
+ if (err)
+ return err;

list_add(&vma->evict_link, unwind);
- return drm_mm_scan_add_block(scan, &vma->node);
+ if (!drm_mm_scan_add_block(scan, &vma->node))
+ return -ENOSPC;
+
+ return 0;
}

static bool defer_evict(struct i915_vma *vma)
@@ -150,6 +159,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
enum drm_mm_insert_mode mode;
struct i915_vma *active;
int ret;
+ int err = 0;

lockdep_assert_held(&vm->mutex);
trace_i915_gem_evict(vm, min_size, alignment, flags);
@@ -210,17 +220,23 @@ i915_gem_evict_something(struct i915_address_space *vm,
continue;
}

- if (mark_free(&scan, ww, vma, flags, &eviction_list))
+ err = mark_free(&scan, ww, vma, flags, &eviction_list);
+ if (!err)
goto found;
+ if (err == -EDEADLK)
+ break;
}

/* Nothing found, clean up and bail out! */
list_for_each_entry_safe(vma, next, &eviction_list, evict_link) {
ret = drm_mm_scan_remove_block(&scan, &vma->node);
BUG_ON(ret);
- ungrab_vma(vma);
+ ungrab_vma(vma, ww);
}

+ if (err == -EDEADLK)
+ return err;
+
/*
* Can we unpin some objects such as idle hw contents,
* or pending flips? But since only the GGTT has global entries
@@ -267,7 +283,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
__i915_vma_pin(vma);
} else {
list_del(&vma->evict_link);
- ungrab_vma(vma);
+ ungrab_vma(vma, ww);
}
}

@@ -277,17 +293,21 @@ i915_gem_evict_something(struct i915_address_space *vm,
__i915_vma_unpin(vma);
if (ret == 0)
ret = __i915_vma_unbind(vma);
- ungrab_vma(vma);
+ ungrab_vma(vma, ww);
}

while (ret == 0 && (node = drm_mm_scan_color_evict(&scan))) {
vma = container_of(node, struct i915_vma, node);

/* If we find any non-objects (!vma), we cannot evict them */
- if (vma->node.color != I915_COLOR_UNEVICTABLE &&
- grab_vma(vma, ww)) {
- ret = __i915_vma_unbind(vma);
- ungrab_vma(vma);
+ if (vma->node.color != I915_COLOR_UNEVICTABLE) {
+ ret = grab_vma(vma, ww);
+ if (!ret) {
+ ret = __i915_vma_unbind(vma);
+ ungrab_vma(vma, ww);
+ } else if (ret != -EDEADLK) {
+ ret = -ENOSPC;
+ }
} else {
ret = -ENOSPC;
}
@@ -382,8 +402,11 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
break;
}

- if (!grab_vma(vma, ww)) {
- ret = -ENOSPC;
+ ret = grab_vma(vma, ww);
+ if (ret) {
+ if (ret != -EDEADLK)
+ ret = -ENOSPC;
+
break;
}

@@ -405,7 +428,7 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
if (ret == 0)
ret = __i915_vma_unbind(vma);

- ungrab_vma(vma);
+ ungrab_vma(vma, ww);
}

return ret;
--
2.37.1
\
 
 \ /
  Last update: 2022-08-17 09:04    [W:0.057 / U:1.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site