From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: [Intel-gfx] [PATCH 07/20] drm/i915: Use per object locking in execbuf on top of struct_mutex, v4.
Date: Tue, 25 Feb 2020 15:38:11 +0100 [thread overview]
Message-ID: <20200225143824.1858038-7-maarten.lankhorst@linux.intel.com> (raw)
In-Reply-To: <20200225143824.1858038-1-maarten.lankhorst@linux.intel.com>
Now that we changed execbuf submission slightly to allow us to do all
pinning in one place, we can now simply add ww versions on top of
struct_mutex. All we have to do is a separate path for -EDEADLK
handling, which needs to unpin all gem bo's before dropping the lock,
then starting over.
This finally allows us to do parallel submission, but because vma
pinning does not use the ww ctx yet, we cannot drop struct_mutex yet.
Changes since v1:
- Keep struct_mutex for now. :(
Changes since v2:
- Make sure we always lock the ww context in slowpath.
Changes since v3:
- Don't call __eb_unreserve_vma in eb_move_to_gpu now; this can be
done on normal unlock path.
- Unconditionally release vmas and context.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
.../gpu/drm/i915/gem/i915_gem_execbuffer.c | 169 ++++++++----------
1 file changed, 78 insertions(+), 91 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index bcbcd0ea8da6..8f39ffc277e7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -251,6 +251,8 @@ struct i915_execbuffer {
/** list of vma that have execobj.relocation_count */
struct list_head relocs;
+ struct i915_gem_ww_ctx ww;
+
/**
* Track the most recently used object for relocations, as we
* frequently have to perform multiple relocations within the same
@@ -406,24 +408,18 @@ eb_pin_vma(struct i915_execbuffer *eb,
return !eb_vma_misplaced(entry, vma, ev->flags);
}
-static inline void __eb_unreserve_vma(struct i915_vma *vma, unsigned int flags)
-{
- GEM_BUG_ON(!(flags & __EXEC_OBJECT_HAS_PIN));
-
- if (unlikely(flags & __EXEC_OBJECT_HAS_FENCE))
- __i915_vma_unpin_fence(vma);
-
- __i915_vma_unpin(vma);
-}
-
static inline void
eb_unreserve_vma(struct eb_vma *ev)
{
if (!(ev->flags & __EXEC_OBJECT_HAS_PIN))
return;
- __eb_unreserve_vma(ev->vma, ev->flags);
ev->flags &= ~__EXEC_OBJECT_RESERVED;
+
+ if (unlikely(ev->flags & __EXEC_OBJECT_HAS_FENCE))
+ __i915_vma_unpin_fence(ev->vma);
+
+ __i915_vma_unpin(ev->vma);
}
static int
@@ -813,6 +809,10 @@ static int eb_validate_vmas(struct i915_execbuffer *eb)
struct eb_vma *ev = &eb->vma[i];
struct i915_vma *vma = ev->vma;
+ err = i915_gem_object_lock(vma->obj, &eb->ww);
+ if (err)
+ return err;
+
if (eb_pin_vma(eb, entry, ev)) {
if (entry->offset != vma->node.start) {
entry->offset = vma->node.start | UPDATE;
@@ -959,7 +959,6 @@ static void reloc_cache_reset(struct reloc_cache *cache)
kunmap_atomic(vaddr);
i915_gem_object_finish_access(obj);
- i915_gem_object_unlock(obj);
} else {
struct i915_ggtt *ggtt = cache_to_ggtt(cache);
@@ -994,15 +993,9 @@ static void *reloc_kmap(struct drm_i915_gem_object *obj,
unsigned int flushes;
int err;
- err = i915_gem_object_lock_interruptible(obj, NULL);
- if (err)
- return ERR_PTR(err);
-
err = i915_gem_object_prepare_write(obj, &flushes);
- if (err) {
- i915_gem_object_unlock(obj);
+ if (err)
return ERR_PTR(err);
- }
BUILD_BUG_ON(KMAP & CLFLUSH_FLAGS);
BUILD_BUG_ON((KMAP | CLFLUSH_FLAGS) & PAGE_MASK);
@@ -1041,9 +1034,7 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
if (use_cpu_reloc(cache, obj))
return NULL;
- i915_gem_object_lock(obj, NULL);
err = i915_gem_object_set_to_gtt_domain(obj, true);
- i915_gem_object_unlock(obj);
if (err)
return ERR_PTR(err);
@@ -1132,7 +1123,7 @@ static int reloc_move_to_gpu(struct i915_request *rq, struct i915_vma *vma)
struct drm_i915_gem_object *obj = vma->obj;
int err;
- i915_vma_lock(vma);
+ assert_vma_held(vma);
if (obj->cache_dirty & ~obj->cache_coherent)
i915_gem_clflush_object(obj, 0);
@@ -1142,8 +1133,6 @@ static int reloc_move_to_gpu(struct i915_request *rq, struct i915_vma *vma)
if (err == 0)
err = i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
- i915_vma_unlock(vma);
-
return err;
}
@@ -1162,6 +1151,10 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
if (IS_ERR(pool))
return PTR_ERR(pool);
+ err = i915_gem_object_lock(pool->obj, &eb->ww);
+ if (err)
+ goto out_pool;
+
cmd = i915_gem_object_pin_map(pool->obj,
cache->has_llc ?
I915_MAP_FORCE_WB :
@@ -1201,11 +1194,10 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
if (err)
goto skip_request;
- i915_vma_lock(batch);
+ assert_vma_held(batch);
err = i915_request_await_object(rq, batch->obj, false);
if (err == 0)
err = i915_vma_move_to_active(batch, rq, 0);
- i915_vma_unlock(batch);
if (err)
goto skip_request;
@@ -1286,7 +1278,9 @@ relocate_entry(struct i915_vma *vma,
len = 3;
batch = reloc_gpu(eb, vma, len);
- if (IS_ERR(batch))
+ if (batch == ERR_PTR(-EDEADLK))
+ return (s64)-EDEADLK;
+ else if (IS_ERR(batch))
goto repeat;
addr = gen8_canonical_addr(vma->node.start + offset);
@@ -1695,6 +1689,7 @@ static noinline int eb_relocate_parse_slow(struct i915_execbuffer *eb)
/* We may process another execbuffer during the unlock... */
eb_release_vmas(eb);
+ i915_gem_ww_ctx_fini(&eb->ww);
mutex_unlock(&dev->struct_mutex);
/*
@@ -1719,21 +1714,22 @@ static noinline int eb_relocate_parse_slow(struct i915_execbuffer *eb)
cond_resched();
err = 0;
}
- if (err) {
- mutex_lock(&dev->struct_mutex);
- goto out;
- }
- /* A frequent cause for EAGAIN are currently unavailable client pages */
- flush_workqueue(eb->i915->mm.userptr_wq);
+ if (!err) {
+ /* A frequent cause for EAGAIN are currently unavailable client pages */
+ flush_workqueue(eb->i915->mm.userptr_wq);
- err = mutex_lock_interruptible(&dev->struct_mutex);
+ err = mutex_lock_interruptible(&dev->struct_mutex);
+ }
if (err) {
mutex_lock(&dev->struct_mutex);
+ i915_gem_ww_ctx_init(&eb->ww, true);
goto out;
}
+ i915_gem_ww_ctx_init(&eb->ww, true);
/* reacquire the objects */
+repeat_validate:
err = eb_validate_vmas(eb);
if (err)
goto err;
@@ -1745,7 +1741,9 @@ static noinline int eb_relocate_parse_slow(struct i915_execbuffer *eb)
pagefault_disable();
err = eb_relocate_vma(eb, ev);
pagefault_enable();
- if (err)
+ if (err == -EDEADLK)
+ goto err;
+ else if (err)
goto repeat;
} else {
err = eb_relocate_vma_slow(eb, ev);
@@ -1767,6 +1765,13 @@ static noinline int eb_relocate_parse_slow(struct i915_execbuffer *eb)
*/
err:
+ if (err == -EDEADLK) {
+ eb_release_vmas(eb);
+ err = i915_gem_ww_ctx_backoff(&eb->ww);
+ if (!err)
+ goto repeat_validate;
+ }
+
if (err == -EAGAIN)
goto repeat;
@@ -1799,64 +1804,58 @@ static int eb_relocate_parse(struct i915_execbuffer *eb)
if (err)
return err;
+retry:
err = eb_validate_vmas(eb);
if (err)
- goto slow;
+ goto err;
/* The objects are in their final locations, apply the relocations. */
if (eb->args->flags & __EXEC_HAS_RELOC) {
struct eb_vma *ev;
list_for_each_entry(ev, &eb->relocs, reloc_link) {
- if (eb_relocate_vma(eb, ev))
- goto slow;
+ err = eb_relocate_vma(eb, ev);
+ if (err)
+ goto err;
}
}
err = eb_parse(eb);
- if (err)
+err:
+ if (err == -EDEADLK) {
+ eb_release_vmas(eb);
+ err = i915_gem_ww_ctx_backoff(&eb->ww);
+ if (err)
+ return err;
+
+ goto retry;
+ }
+ else if (err)
goto slow;
return 0;
slow:
- return eb_relocate_parse_slow(eb);
+ err = eb_relocate_parse_slow(eb);
+ if (err)
+ /*
+ * If the user expects the execobject.offset and
+ * reloc.presumed_offset to be an exact match,
+ * as for using NO_RELOC, then we cannot update
+ * the execobject.offset until we have completed
+ * relocation.
+ */
+ eb->args->flags &= ~__EXEC_HAS_RELOC;
+
+ return err;
}
static int eb_move_to_gpu(struct i915_execbuffer *eb)
{
const unsigned int count = eb->buffer_count;
- struct ww_acquire_ctx acquire;
- unsigned int i;
+ unsigned int i = count;
int err = 0;
- ww_acquire_init(&acquire, &reservation_ww_class);
-
- for (i = 0; i < count; i++) {
- struct eb_vma *ev = &eb->vma[i];
- struct i915_vma *vma = ev->vma;
-
- err = ww_mutex_lock_interruptible(&vma->resv->lock, &acquire);
- if (err == -EDEADLK) {
- GEM_BUG_ON(i == 0);
- do {
- int j = i - 1;
-
- ww_mutex_unlock(&eb->vma[j].vma->resv->lock);
-
- swap(eb->vma[i], eb->vma[j]);
- } while (--i);
-
- err = ww_mutex_lock_slow_interruptible(&vma->resv->lock,
- &acquire);
- }
- if (err == -EALREADY)
- err = 0;
- if (err)
- break;
- }
- ww_acquire_done(&acquire);
-
while (i--) {
struct eb_vma *ev = &eb->vma[i];
struct i915_vma *vma = ev->vma;
@@ -1900,22 +1899,11 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
if (err == 0)
err = i915_vma_move_to_active(vma, eb->request, flags);
-
- i915_vma_unlock(vma);
-
- __eb_unreserve_vma(vma, flags);
- if (unlikely(flags & __EXEC_OBJECT_HAS_REF))
- i915_vma_put(vma);
-
- ev->vma = NULL;
}
- ww_acquire_fini(&acquire);
if (unlikely(err))
goto err_skip;
- eb->exec = NULL;
-
/* Unconditionally flush any chipset caches (for streaming writes). */
intel_gt_chipset_flush(eb->engine->gt);
return 0;
@@ -2065,10 +2053,6 @@ static int eb_parse_pipeline(struct i915_execbuffer *eb,
pw->shadow = shadow;
pw->trampoline = trampoline;
- err = dma_resv_lock_interruptible(pw->batch->resv, NULL);
- if (err)
- goto err_trampoline;
-
err = dma_resv_reserve_shared(pw->batch->resv, 1);
if (err)
goto err_batch_unlock;
@@ -2083,19 +2067,14 @@ static int eb_parse_pipeline(struct i915_execbuffer *eb,
/* Keep the batch alive and unwritten as we parse */
dma_resv_add_shared_fence(pw->batch->resv, &pw->base.dma);
- dma_resv_unlock(pw->batch->resv);
-
/* Force execution to wait for completion of the parser */
- dma_resv_lock(shadow->resv, NULL);
dma_resv_add_excl_fence(shadow->resv, &pw->base.dma);
- dma_resv_unlock(shadow->resv);
dma_fence_work_commit(&pw->base);
return 0;
err_batch_unlock:
dma_resv_unlock(pw->batch->resv);
-err_trampoline:
if (trampoline)
i915_active_release(&trampoline->active);
err_shadow:
@@ -2137,6 +2116,10 @@ static int eb_parse(struct i915_execbuffer *eb)
if (IS_ERR(pool))
return PTR_ERR(pool);
+ err = i915_gem_object_lock(pool->obj, &eb->ww);
+ if (err)
+ goto err;
+
shadow = shadow_batch_pin(pool->obj, eb->context->vm, PIN_USER);
if (IS_ERR(shadow)) {
err = PTR_ERR(shadow);
@@ -2691,6 +2674,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
err = mutex_lock_interruptible(&dev->struct_mutex);
if (err)
goto err_engine;
+ i915_gem_ww_ctx_init(&eb.ww, true);
err = eb_relocate_parse(&eb);
if (err) {
@@ -2705,6 +2689,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
goto err_vma;
}
+ ww_acquire_done(&eb.ww.ctx);
+
/*
* snb/ivb/vlv conflate the "batch in ppgtt" bit with the "non-secure
* batch" bit. Hence we need to pin secure batches into the global gtt.
@@ -2809,10 +2795,11 @@ i915_gem_do_execbuffer(struct drm_device *dev,
if (batch->private)
intel_engine_pool_put(batch->private);
err_vma:
- if (eb.exec)
- eb_release_vmas(&eb);
+ eb_release_vmas(&eb);
if (eb.trampoline)
i915_vma_unpin(eb.trampoline);
+ WARN_ON(err == -EDEADLK);
+ i915_gem_ww_ctx_fini(&eb.ww);
mutex_unlock(&dev->struct_mutex);
err_engine:
eb_unpin_engine(&eb);
--
2.25.0.24.g3f081b084b0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-02-25 14:38 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-25 14:38 [Intel-gfx] [PATCH 01/20] drm/i915: Drop inspection of execbuf flags during evict Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 02/20] drm/i915/gem: Extract transient execbuf flags from i915_vma Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 03/20] drm/i915: Separate lookup and pinning in execbuf Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 04/20] drm/i915: Add an implementation for i915_gem_ww_ctx locking, v2 Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 05/20] drm/i915: Remove locking from i915_gem_object_prepare_read/write Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 06/20] drm/i915: Parse command buffer earlier in eb_relocate(slow) Maarten Lankhorst
2020-02-25 14:38 ` Maarten Lankhorst [this message]
2020-02-25 14:38 ` [Intel-gfx] [PATCH 08/20] drm/i915: Use ww locking in intel_renderstate Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 09/20] drm/i915: Add ww context handling to context_barrier_task Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 10/20] drm/i915: Nuke arguments to eb_pin_engine Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 11/20] drm/i915: Pin engine before pinning all objects, v3 Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 12/20] drm/i915: Rework intel_context pinning to do everything outside of pin_mutex Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 13/20] drm/i915: Make sure execbuffer always passes ww state to i915_vma_pin Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 14/20] drm/i915: Convert i915_gem_object/client_blt.c to use ww locking as well, v2 Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 15/20] drm/i915: Kill last user of intel_context_create_request outside of selftests Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 16/20] drm/i915: Convert i915_perf to ww locking as well Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 17/20] drm/i915: Dirty hack to fix selftests locking inversion Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 18/20] drm/i915/selftests: Fix locking inversion in lrc selftest Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 19/20] drm/i915: Use ww pinning for intel_context_create_request() Maarten Lankhorst
2020-02-25 14:38 ` [Intel-gfx] [PATCH 20/20] drm/i915: Move i915_vma_lock in the live selftest to avoid lock inversion Maarten Lankhorst
2020-02-26 17:02 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with [01/20] drm/i915: Drop inspection of execbuf flags during evict Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200225143824.1858038-7-maarten.lankhorst@linux.intel.com \
--to=maarten.lankhorst@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.