i915_gem_evict_something() is charged with finding a slot within the GTT that we may reuse. Since our goal is not to stall, we first look for a slot that only overlaps idle vma. To this end, on the first pass we move any active vma to the end of the search list. However, we only stopped moving active vma after we see the first active vma twice. If during the search, that first active vma completed, we would not notice and keep on extending the search list.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1746 Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Tvrtko Ursulin tvrtko.ursulin@intel.com Cc: stable@vger.kernel.org # v5.5+ --- drivers/gpu/drm/i915/i915_gem_evict.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c index 0ba7b1e881c0..6501939929d5 100644 --- a/drivers/gpu/drm/i915/i915_gem_evict.c +++ b/drivers/gpu/drm/i915/i915_gem_evict.c @@ -128,6 +128,13 @@ i915_gem_evict_something(struct i915_address_space *vm, active = NULL; INIT_LIST_HEAD(&eviction_list); list_for_each_entry_safe(vma, next, &vm->bound_list, vm_link) { + if (vma == active) { /* now seen this vma twice */ + if (flags & PIN_NONBLOCK) + break; + + active = ERR_PTR(-EAGAIN); + } + /* * We keep this list in a rough least-recently scanned order * of active elements (inactive elements are cheap to reap). @@ -143,21 +150,12 @@ i915_gem_evict_something(struct i915_address_space *vm, * To notice when we complete one full cycle, we record the * first active element seen, before moving it to the tail. */ - if (i915_vma_is_active(vma)) { - if (vma == active) { - if (flags & PIN_NONBLOCK) - break; - - active = ERR_PTR(-EAGAIN); - } - - if (active != ERR_PTR(-EAGAIN)) { - if (!active) - active = vma; + if (active != ERR_PTR(-EAGAIN) && i915_vma_is_active(vma)) { + if (!active) + active = vma;
- list_move_tail(&vma->vm_link, &vm->bound_list); - continue; - } + list_move_tail(&vma->vm_link, &vm->bound_list); + continue; }
if (mark_free(&scan, vma, flags, &eviction_list))
Quoting Chris Wilson (2020-05-09 12:52:17)
i915_gem_evict_something() is charged with finding a slot within the GTT that we may reuse. Since our goal is not to stall, we first look for a slot that only overlaps idle vma. To this end, on the first pass we move any active vma to the end of the search list. However, we only stopped moving active vma after we see the first active vma twice. If during the search, that first active vma completed, we would not notice and keep on extending the search list.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1746
Fixes: 2850748ef876 ("drm/i915: Pull i915_vma_pin under the vm->mutex") Fixes: b1e3177bd1d8 ("drm/i915: Coordinate i915_active with its own mutex")
Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Tvrtko Ursulin tvrtko.ursulin@intel.com Cc: stable@vger.kernel.org # v5.5+
Chris Wilson chris@chris-wilson.co.uk writes:
i915_gem_evict_something() is charged with finding a slot within the GTT that we may reuse. Since our goal is not to stall, we first look for a slot that only overlaps idle vma. To this end, on the first pass we move any active vma to the end of the search list. However, we only stopped moving active vma after we see the first active vma twice. If during the search, that first active vma completed, we would not notice and keep on extending the search list.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1746 Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Tvrtko Ursulin tvrtko.ursulin@intel.com Cc: stable@vger.kernel.org # v5.5+
Only thing I would change is tune up the subject line. It fixes a possible busy loop in eviction so I feel 'watch out' is not strong enough for my liking.
Reviewed-by: Mika Kuoppala mika.kuoppala@linux.intel.com
drivers/gpu/drm/i915/i915_gem_evict.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c index 0ba7b1e881c0..6501939929d5 100644 --- a/drivers/gpu/drm/i915/i915_gem_evict.c +++ b/drivers/gpu/drm/i915/i915_gem_evict.c @@ -128,6 +128,13 @@ i915_gem_evict_something(struct i915_address_space *vm, active = NULL; INIT_LIST_HEAD(&eviction_list); list_for_each_entry_safe(vma, next, &vm->bound_list, vm_link) {
if (vma == active) { /* now seen this vma twice */if (flags & PIN_NONBLOCK)break;active = ERR_PTR(-EAGAIN);}- /*
- We keep this list in a rough least-recently scanned order
- of active elements (inactive elements are cheap to reap).
@@ -143,21 +150,12 @@ i915_gem_evict_something(struct i915_address_space *vm, * To notice when we complete one full cycle, we record the * first active element seen, before moving it to the tail. */
if (i915_vma_is_active(vma)) {if (vma == active) {if (flags & PIN_NONBLOCK)break;active = ERR_PTR(-EAGAIN);}if (active != ERR_PTR(-EAGAIN)) {if (!active)active = vma;
if (active != ERR_PTR(-EAGAIN) && i915_vma_is_active(vma)) {if (!active)active = vma;
list_move_tail(&vma->vm_link, &vm->bound_list);continue;}
list_move_tail(&vma->vm_link, &vm->bound_list);continue;if (mark_free(&scan, vma, flags, &eviction_list)) -- 2.20.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Mika Kuoppala (2020-05-12 11:39:31)
Chris Wilson chris@chris-wilson.co.uk writes:
i915_gem_evict_something() is charged with finding a slot within the GTT that we may reuse. Since our goal is not to stall, we first look for a slot that only overlaps idle vma. To this end, on the first pass we move any active vma to the end of the search list. However, we only stopped moving active vma after we see the first active vma twice. If during the search, that first active vma completed, we would not notice and keep on extending the search list.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/1746 Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Tvrtko Ursulin tvrtko.ursulin@intel.com Cc: stable@vger.kernel.org # v5.5+
Only thing I would change is tune up the subject line. It fixes a possible busy loop in eviction so I feel 'watch out' is not strong enough for my liking.
Duck and cover for the idling is a-coming! -Chris
linux-stable-mirror@lists.linaro.org
-
Chris Wilson -
Mika Kuoppala