intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker
@ 2021-08-13 20:30 Daniel Vetter
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 02/11] drm/i915: Release ctx->syncobj on final put, not on ctx close Daniel Vetter
                   ` (21 more replies)
  0 siblings, 22 replies; 33+ messages in thread
From: Daniel Vetter @ 2021-08-13 20:30 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Jon Bloomfield, Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

The only reason for this really is the i915_gem_engines->fence
callback engines_notify(), which exists purely as a fairly funky
reference counting scheme for that. Otherwise all other callers are
from process context, and generally fairly benign locking context.

Unfortunately untangling that requires some major surgery, and we have
a few i915_gem_context reference counting bugs that need fixing, and
they blow in the current hardirq calling context, so we need a
stop-gap measure.

Put a FIXME comment in when this should be removable again.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jason Ekstrand <jason@jlekstrand.net>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c       | 13 +++++++++++--
 drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 12 ++++++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index fd169cf2f75a..051bc357ff65 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -986,9 +986,10 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx,
 	return err;
 }
 
-void i915_gem_context_release(struct kref *ref)
+static void i915_gem_context_release_work(struct work_struct *work)
 {
-	struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
+	struct i915_gem_context *ctx = container_of(work, typeof(*ctx),
+						    release_work);
 
 	trace_i915_context_free(ctx);
 	GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
@@ -1002,6 +1003,13 @@ void i915_gem_context_release(struct kref *ref)
 	kfree_rcu(ctx, rcu);
 }
 
+void i915_gem_context_release(struct kref *ref)
+{
+	struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
+
+	queue_work(ctx->i915->wq, &ctx->release_work);
+}
+
 static inline struct i915_gem_engines *
 __context_engines_static(const struct i915_gem_context *ctx)
 {
@@ -1303,6 +1311,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
 	ctx->sched = pc->sched;
 	mutex_init(&ctx->mutex);
 	INIT_LIST_HEAD(&ctx->link);
+	INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
 
 	spin_lock_init(&ctx->stale.lock);
 	INIT_LIST_HEAD(&ctx->stale.engines);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index 94c03a97cb77..0c38789bd4a8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -288,6 +288,18 @@ struct i915_gem_context {
 	 */
 	struct kref ref;
 
+	/**
+	 * @release_work:
+	 *
+	 * Work item for deferred cleanup, since i915_gem_context_put() tends to
+	 * be called from hardirq context.
+	 *
+	 * FIXME: The only real reason for this is &i915_gem_engines.fence, all
+	 * other callers are from process context and need at most some mild
+	 * shuffling to pull the i915_gem_context_put() call out of a spinlock.
+	 */
+	struct work_struct release_work;
+
 	/**
 	 * @rcu: rcu_head for deferred freeing.
 	 */
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Intel-gfx] [PATCH 02/11] drm/i915: Release ctx->syncobj on final put, not on ctx close
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
@ 2021-08-13 20:30 ` Daniel Vetter
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 03/11] drm/i915: Keep gem ctx->vm alive until the final put Daniel Vetter
                   ` (20 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2021-08-13 20:30 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Jason Ekstrand, Chris Wilson, Tvrtko Ursulin, Joonas Lahtinen,
	Matthew Brost, Matthew Auld, Maarten Lankhorst,
	Thomas Hellström, Lionel Landwerlin, Dave Airlie

gem context refcounting is another exercise in least locking design it
seems, where most things get destroyed upon context closure (which can
race with anything really). Only the actual memory allocation and the
locks survive while holding a reference.

This tripped up Jason when reimplementing the single timeline feature
in

commit 00dae4d3d35d4f526929633b76e00b0ab4d3970d
Author: Jason Ekstrand <jason@jlekstrand.net>
Date:   Thu Jul 8 10:48:12 2021 -0500

    drm/i915: Implement SINGLE_TIMELINE with a syncobj (v4)

We could fix the bug by holding ctx->mutex in execbuf and clear the
pointer (again while holding the mutex) context_close, but it's
cleaner to just make the context object actually invariant over its
_entire_ lifetime. This way any other ioctl that's potentially racing,
but holding a full reference, can still rely on ctx->syncobj being
an immutable pointer. Which without this change, is not the case.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Fixes: 00dae4d3d35d ("drm/i915: Implement SINGLE_TIMELINE with a syncobj (v4)")
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: "Thomas Hellström" <thomas.hellstrom@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 051bc357ff65..5a053cf14948 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -994,6 +994,9 @@ static void i915_gem_context_release_work(struct work_struct *work)
 	trace_i915_context_free(ctx);
 	GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
 
+	if (ctx->syncobj)
+		drm_syncobj_put(ctx->syncobj);
+
 	mutex_destroy(&ctx->engines_mutex);
 	mutex_destroy(&ctx->lut_mutex);
 
@@ -1220,9 +1223,6 @@ static void context_close(struct i915_gem_context *ctx)
 	if (vm)
 		i915_vm_close(vm);
 
-	if (ctx->syncobj)
-		drm_syncobj_put(ctx->syncobj);
-
 	ctx->file_priv = ERR_PTR(-EBADF);
 
 	/*
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Intel-gfx] [PATCH 03/11] drm/i915: Keep gem ctx->vm alive until the final put
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 02/11] drm/i915: Release ctx->syncobj on final put, not on ctx close Daniel Vetter
@ 2021-08-13 20:30 ` Daniel Vetter
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 04/11] drm/i915: Drop code to handle set-vm races from execbuf Daniel Vetter
                   ` (19 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2021-08-13 20:30 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Jason Ekstrand,
	Chris Wilson, Tvrtko Ursulin, Joonas Lahtinen, Matthew Brost,
	Matthew Auld, Maarten Lankhorst, Thomas Hellström,
	Lionel Landwerlin, Dave Airlie, Daniel Vetter

The comment added in

    commit b81dde719439c8f09bb61e742ed95bfc4b33946b
    Author: Chris Wilson <chris@chris-wilson.co.uk>
    Date:   Tue May 21 22:11:29 2019 +0100

        drm/i915: Allow userspace to clone contexts on creation

and moved in

    commit 27dbae8f36c1c25008b7885fc07c57054b7dfba3
    Author: Chris Wilson <chris@chris-wilson.co.uk>
    Date:   Wed Nov 6 09:13:12 2019 +0000

        drm/i915/gem: Safely acquire the ctx->vm when copying

suggested that i915_address_space were at least intended to be managed
through SLAB_TYPESAFE_BY_RCU:

                * This ppgtt may have be reallocated between
                * the read and the kref, and reassigned to a third
                * context. In order to avoid inadvertent sharing
                * of this ppgtt with that third context (and not
                * src), we have to confirm that we have the same
                * ppgtt after passing through the strong memory
                * barrier implied by a successful
                * kref_get_unless_zero().

But extensive git history search has not brough any such reuse to
light.

What has come to light though is that ever since

commit 2850748ef8763ab46958e43a4d1c445f29eeb37d
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Oct 4 14:39:58 2019 +0100

    drm/i915: Pull i915_vma_pin under the vm->mutex

(yes this commit is earlier) the final i915_vma_put call has been
moved from i915_gem_context_free (now called _release) to
context_close, which means it's not actually safe anymore to access
the ctx->vm pointer without lock helds, because it might disappear at
any moment. Note that superficially things all still work, because the
i915_address_space is RCU protected since

    commit b32fa811156328aea5a3c2ff05cc096490382456
    Author: Chris Wilson <chris@chris-wilson.co.uk>
    Date:   Thu Jun 20 19:37:05 2019 +0100

        drm/i915/gtt: Defer address space cleanup to an RCU worker

except the very clever macro above (which is designed to protected
against object reuse due to SLAB_TYPESAFE_BY_RCU or similar tricks)
results in an endless loop if the refcount of the ctx->vm ever
permanently drops to 0. Which it totally now can.

Fix that by moving the final i915_vm_put to where it should be.

Note that i915_gem_context is rcu protected, but _only_ the final
kfree. This means anyone who chases a pointer to a gem ctx solely
under the protection can pretty only call kref_get_unless_zero(). This
seems to be pretty much the case, aside from a bunch of cases that
consult the scheduling information without any further protection.

Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: "Thomas Hellström" <thomas.hellstrom@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Fixes: 2850748ef876 ("drm/i915: Pull i915_vma_pin under the vm->mutex")
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 5a053cf14948..12e2de1db1a2 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -990,6 +990,7 @@ static void i915_gem_context_release_work(struct work_struct *work)
 {
 	struct i915_gem_context *ctx = container_of(work, typeof(*ctx),
 						    release_work);
+	struct i915_address_space *vm;
 
 	trace_i915_context_free(ctx);
 	GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
@@ -997,6 +998,10 @@ static void i915_gem_context_release_work(struct work_struct *work)
 	if (ctx->syncobj)
 		drm_syncobj_put(ctx->syncobj);
 
+	vm = i915_gem_context_vm(ctx);
+	if (vm)
+		i915_vm_put(vm);
+
 	mutex_destroy(&ctx->engines_mutex);
 	mutex_destroy(&ctx->lut_mutex);
 
@@ -1220,8 +1225,15 @@ static void context_close(struct i915_gem_context *ctx)
 	set_closed_name(ctx);
 
 	vm = i915_gem_context_vm(ctx);
-	if (vm)
+	if (vm) {
+		/* i915_vm_close drops the final reference, which is a bit too
+		 * early and could result in surprises with concurrent
+		 * operations racing with thist ctx close. Keep a full reference
+		 * until the end.
+		 */
+		i915_vm_get(vm);
 		i915_vm_close(vm);
+	}
 
 	ctx->file_priv = ERR_PTR(-EBADF);
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Intel-gfx] [PATCH 04/11] drm/i915: Drop code to handle set-vm races from execbuf
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 02/11] drm/i915: Release ctx->syncobj on final put, not on ctx close Daniel Vetter
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 03/11] drm/i915: Keep gem ctx->vm alive until the final put Daniel Vetter
@ 2021-08-13 20:30 ` Daniel Vetter
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 05/11] drm/i915: Rename i915_gem_context_get_vm_rcu to i915_gem_context_get_eb_vm Daniel Vetter
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2021-08-13 20:30 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Jon Bloomfield, Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

Changing the vm from a finalized gem ctx is no longer possible, which
means we don't have to check for that anymore.

I was pondering whether to keep the check as a WARN_ON, but things go
boom real bad real fast if the vm of a vma is wrong. Plus we'd need to
also get the ggtt vm for !full-ppgtt platforms. Ditching it all seemed
like a better idea.

References: ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running contexts (v4)")
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jason Ekstrand <jason@jlekstrand.net>
---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index e809aca00f72..905b1cbd22d5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -775,11 +775,7 @@ static int __eb_add_lut(struct i915_execbuffer *eb,
 	/* Check that the context hasn't been closed in the meantime */
 	err = -EINTR;
 	if (!mutex_lock_interruptible(&ctx->lut_mutex)) {
-		struct i915_address_space *vm = rcu_access_pointer(ctx->vm);
-
-		if (unlikely(vm && vma->vm != vm))
-			err = -EAGAIN; /* user racing with ctx set-vm */
-		else if (likely(!i915_gem_context_is_closed(ctx)))
+		if (likely(!i915_gem_context_is_closed(ctx)))
 			err = radix_tree_insert(&ctx->handles_vma, handle, vma);
 		else
 			err = -ENOENT;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Intel-gfx] [PATCH 05/11] drm/i915: Rename i915_gem_context_get_vm_rcu to i915_gem_context_get_eb_vm
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (2 preceding siblings ...)
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 04/11] drm/i915: Drop code to handle set-vm races from execbuf Daniel Vetter
@ 2021-08-13 20:30 ` Daniel Vetter
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 06/11] drm/i915: Use i915_gem_context_get_eb_vm in ctx_getparam Daniel Vetter
                   ` (17 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2021-08-13 20:30 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Jon Bloomfield, Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

The important part isn't so much that this does an rcu lookup - that's
more an implementation detail, which will also be removed.

The thing that makes this different from other functions is that it's
gettting you the vm that batchbuffers will run in for that gem
context, which is either a full ppgtt stored in gem->ctx, or the ggtt.

We'll make more use of this function later on.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jason Ekstrand <jason@jlekstrand.net>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.h           | 2 +-
 drivers/gpu/drm/i915/gem/selftests/huge_pages.c       | 4 ++--
 drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 4 ++--
 drivers/gpu/drm/i915/gt/selftest_execlists.c          | 2 +-
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c          | 2 +-
 drivers/gpu/drm/i915/selftests/i915_gem_gtt.c         | 4 ++--
 drivers/gpu/drm/i915/selftests/i915_vma.c             | 2 +-
 7 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h b/drivers/gpu/drm/i915/gem/i915_gem_context.h
index 18060536b0c2..da6e8b506d96 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h
@@ -155,7 +155,7 @@ i915_gem_context_vm(struct i915_gem_context *ctx)
 }
 
 static inline struct i915_address_space *
-i915_gem_context_get_vm_rcu(struct i915_gem_context *ctx)
+i915_gem_context_get_eb_vm(struct i915_gem_context *ctx)
 {
 	struct i915_address_space *vm;
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index a094f3ce1a90..6c68fe26bb32 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1456,7 +1456,7 @@ static int igt_tmpfs_fallback(void *arg)
 	struct i915_gem_context *ctx = arg;
 	struct drm_i915_private *i915 = ctx->i915;
 	struct vfsmount *gemfs = i915->mm.gemfs;
-	struct i915_address_space *vm = i915_gem_context_get_vm_rcu(ctx);
+	struct i915_address_space *vm = i915_gem_context_get_eb_vm(ctx);
 	struct drm_i915_gem_object *obj;
 	struct i915_vma *vma;
 	u32 *vaddr;
@@ -1512,7 +1512,7 @@ static int igt_shrink_thp(void *arg)
 {
 	struct i915_gem_context *ctx = arg;
 	struct drm_i915_private *i915 = ctx->i915;
-	struct i915_address_space *vm = i915_gem_context_get_vm_rcu(ctx);
+	struct i915_address_space *vm = i915_gem_context_get_eb_vm(ctx);
 	struct drm_i915_gem_object *obj;
 	struct i915_gem_engines_iter it;
 	struct intel_context *ce;
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index 8eb5050f8cb3..d436ce7fa25c 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -1528,7 +1528,7 @@ static int write_to_scratch(struct i915_gem_context *ctx,
 
 	intel_gt_chipset_flush(engine->gt);
 
-	vm = i915_gem_context_get_vm_rcu(ctx);
+	vm = i915_gem_context_get_eb_vm(ctx);
 	vma = i915_vma_instance(obj, vm, NULL);
 	if (IS_ERR(vma)) {
 		err = PTR_ERR(vma);
@@ -1607,7 +1607,7 @@ static int read_from_scratch(struct i915_gem_context *ctx,
 	if (GRAPHICS_VER(i915) >= 8) {
 		const u32 GPR0 = engine->mmio_base + 0x600;
 
-		vm = i915_gem_context_get_vm_rcu(ctx);
+		vm = i915_gem_context_get_eb_vm(ctx);
 		vma = i915_vma_instance(obj, vm, NULL);
 		if (IS_ERR(vma)) {
 			err = PTR_ERR(vma);
diff --git a/drivers/gpu/drm/i915/gt/selftest_execlists.c b/drivers/gpu/drm/i915/gt/selftest_execlists.c
index f12ffe797639..b3863abc51f5 100644
--- a/drivers/gpu/drm/i915/gt/selftest_execlists.c
+++ b/drivers/gpu/drm/i915/gt/selftest_execlists.c
@@ -3493,7 +3493,7 @@ static int smoke_submit(struct preempt_smoke *smoke,
 	if (batch) {
 		struct i915_address_space *vm;
 
-		vm = i915_gem_context_get_vm_rcu(ctx);
+		vm = i915_gem_context_get_eb_vm(ctx);
 		vma = i915_vma_instance(batch, vm, NULL);
 		i915_vm_put(vm);
 		if (IS_ERR(vma))
diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index 08f011f893b2..6023c418ee8a 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -117,7 +117,7 @@ static struct i915_request *
 hang_create_request(struct hang *h, struct intel_engine_cs *engine)
 {
 	struct intel_gt *gt = h->gt;
-	struct i915_address_space *vm = i915_gem_context_get_vm_rcu(h->ctx);
+	struct i915_address_space *vm = i915_gem_context_get_eb_vm(h->ctx);
 	struct drm_i915_gem_object *obj;
 	struct i915_request *rq = NULL;
 	struct i915_vma *hws, *vma;
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
index f843a5040706..2d60a5a5b065 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
@@ -1300,7 +1300,7 @@ static int exercise_mock(struct drm_i915_private *i915,
 	if (!ctx)
 		return -ENOMEM;
 
-	vm = i915_gem_context_get_vm_rcu(ctx);
+	vm = i915_gem_context_get_eb_vm(ctx);
 	err = func(vm, 0, min(vm->total, limit), end_time);
 	i915_vm_put(vm);
 
@@ -1848,7 +1848,7 @@ static int igt_cs_tlb(void *arg)
 		goto out_unlock;
 	}
 
-	vm = i915_gem_context_get_vm_rcu(ctx);
+	vm = i915_gem_context_get_eb_vm(ctx);
 	if (i915_is_ggtt(vm))
 		goto out_vm;
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c
index dd0607254a95..79ba72da0813 100644
--- a/drivers/gpu/drm/i915/selftests/i915_vma.c
+++ b/drivers/gpu/drm/i915/selftests/i915_vma.c
@@ -118,7 +118,7 @@ static int create_vmas(struct drm_i915_private *i915,
 				struct i915_vma *vma;
 				int err;
 
-				vm = i915_gem_context_get_vm_rcu(ctx);
+				vm = i915_gem_context_get_eb_vm(ctx);
 				vma = checked_vma_instance(obj, vm, NULL);
 				i915_vm_put(vm);
 				if (IS_ERR(vma))
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Intel-gfx] [PATCH 06/11] drm/i915: Use i915_gem_context_get_eb_vm in ctx_getparam
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (3 preceding siblings ...)
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 05/11] drm/i915: Rename i915_gem_context_get_vm_rcu to i915_gem_context_get_eb_vm Daniel Vetter
@ 2021-08-13 20:30 ` Daniel Vetter
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 07/11] drm/i915: Add i915_gem_context_is_full_ppgtt Daniel Vetter
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2021-08-13 20:30 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Jon Bloomfield, Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

Consolidates the "which is the vm my execbuf runs in" code a bit. We
do some get/put which isn't really required, but all the other users
want the refcounting, and I figured doing a function just for this
getparam to avoid 2 atomis is a bit much.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jason Ekstrand <jason@jlekstrand.net>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 12e2de1db1a2..7a566fb7cca4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -2108,6 +2108,7 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
 	struct drm_i915_file_private *file_priv = file->driver_priv;
 	struct drm_i915_gem_context_param *args = data;
 	struct i915_gem_context *ctx;
+	struct i915_address_space *vm;
 	int ret = 0;
 
 	ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
@@ -2117,12 +2118,10 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
 	switch (args->param) {
 	case I915_CONTEXT_PARAM_GTT_SIZE:
 		args->size = 0;
-		rcu_read_lock();
-		if (rcu_access_pointer(ctx->vm))
-			args->value = rcu_dereference(ctx->vm)->total;
-		else
-			args->value = to_i915(dev)->ggtt.vm.total;
-		rcu_read_unlock();
+		vm = i915_gem_context_get_eb_vm(ctx);
+		args->value = vm->total;
+		i915_vm_put(vm);
+
 		break;
 
 	case I915_CONTEXT_PARAM_NO_ERROR_CAPTURE:
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Intel-gfx] [PATCH 07/11] drm/i915: Add i915_gem_context_is_full_ppgtt
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (4 preceding siblings ...)
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 06/11] drm/i915: Use i915_gem_context_get_eb_vm in ctx_getparam Daniel Vetter
@ 2021-08-13 20:30 ` Daniel Vetter
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 08/11] drm/i915: Use i915_gem_context_get_eb_vm in intel_context_set_gem Daniel Vetter
                   ` (15 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2021-08-13 20:30 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Jon Bloomfield, Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

And use it anywhere we have open-coded checks for ctx->vm that really
only check for full ppgtt.

Plus for paranoia add a GEM_BUG_ON that checks it's really only set
when we have full ppgtt, just in case. gem_context->vm is different
since it's NULL in ggtt mode, unlike intel_context->vm or gt->vm,
which is always set.

v2: 0day found a testcase that I missed.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jason Ekstrand <jason@jlekstrand.net>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c           | 2 +-
 drivers/gpu/drm/i915/gem/i915_gem_context.h           | 7 +++++++
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c        | 2 +-
 drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c | 6 +++---
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 7a566fb7cca4..1eec85944c1f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1566,7 +1566,7 @@ static int get_ppgtt(struct drm_i915_file_private *file_priv,
 	int err;
 	u32 id;
 
-	if (!rcu_access_pointer(ctx->vm))
+	if (!i915_gem_context_is_full_ppgtt(ctx))
 		return -ENODEV;
 
 	rcu_read_lock();
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h b/drivers/gpu/drm/i915/gem/i915_gem_context.h
index da6e8b506d96..37536a260e6e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h
@@ -154,6 +154,13 @@ i915_gem_context_vm(struct i915_gem_context *ctx)
 	return rcu_dereference_protected(ctx->vm, lockdep_is_held(&ctx->mutex));
 }
 
+static inline bool i915_gem_context_is_full_ppgtt(struct i915_gem_context *ctx)
+{
+	GEM_BUG_ON(!!rcu_access_pointer(ctx->vm) != HAS_FULL_PPGTT(ctx->i915));
+
+	return !!rcu_access_pointer(ctx->vm);
+}
+
 static inline struct i915_address_space *
 i915_gem_context_get_eb_vm(struct i915_gem_context *ctx)
 {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 905b1cbd22d5..40f08948f0b2 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -749,7 +749,7 @@ static int eb_select_context(struct i915_execbuffer *eb)
 		return PTR_ERR(ctx);
 
 	eb->gem_context = ctx;
-	if (rcu_access_pointer(ctx->vm))
+	if (i915_gem_context_is_full_ppgtt(ctx))
 		eb->invalid_flags |= EXEC_OBJECT_NEEDS_GTT;
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index d436ce7fa25c..0708b9cdeb9f 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -704,7 +704,7 @@ static int igt_ctx_exec(void *arg)
 				pr_err("Failed to fill dword %lu [%lu/%lu] with gpu (%s) [full-ppgtt? %s], err=%d\n",
 				       ndwords, dw, max_dwords(obj),
 				       engine->name,
-				       yesno(!!rcu_access_pointer(ctx->vm)),
+				       yesno(i915_gem_context_is_full_ppgtt(ctx)),
 				       err);
 				intel_context_put(ce);
 				kernel_context_close(ctx);
@@ -838,7 +838,7 @@ static int igt_shared_ctx_exec(void *arg)
 				pr_err("Failed to fill dword %lu [%lu/%lu] with gpu (%s) [full-ppgtt? %s], err=%d\n",
 				       ndwords, dw, max_dwords(obj),
 				       engine->name,
-				       yesno(!!rcu_access_pointer(ctx->vm)),
+				       yesno(i915_gem_context_is_full_ppgtt(ctx)),
 				       err);
 				intel_context_put(ce);
 				kernel_context_close(ctx);
@@ -1417,7 +1417,7 @@ static int igt_ctx_readonly(void *arg)
 				pr_err("Failed to fill dword %lu [%lu/%lu] with gpu (%s) [full-ppgtt? %s], err=%d\n",
 				       ndwords, dw, max_dwords(obj),
 				       ce->engine->name,
-				       yesno(!!ctx_vm(ctx)),
+				       yesno(i915_gem_context_is_full_ppgtt(ctx)),
 				       err);
 				i915_gem_context_unlock_engines(ctx);
 				goto out_file;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Intel-gfx] [PATCH 08/11] drm/i915: Use i915_gem_context_get_eb_vm in intel_context_set_gem
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (5 preceding siblings ...)
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 07/11] drm/i915: Add i915_gem_context_is_full_ppgtt Daniel Vetter
@ 2021-08-13 20:30 ` Daniel Vetter
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 09/11] drm/i915: Drop __rcu from gem_context->vm Daniel Vetter
                   ` (14 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2021-08-13 20:30 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Jon Bloomfield, Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

Since

commit ccbc1b97948ab671335e950271e39766729736c3
Author: Jason Ekstrand <jason@jlekstrand.net>
Date:   Thu Jul 8 10:48:30 2021 -0500

    drm/i915/gem: Don't allow changing the VM on running contexts (v4)

the gem_ctx->vm can't change anymore. Plus we always set the
intel_context->vm, so might as well use the helper we have for that.

This makes it very clear that we always overwrite intel_context->vm
for userspace contexts, since the default is gt->vm, which is
explicitly reserved for kernel context use. It would be good to split
things up a bit further and avoid any possibility for an accident
where we run kernel stuff in userspace vm or the other way round.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jason Ekstrand <jason@jlekstrand.net>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 1eec85944c1f..18e23d9220ae 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -791,16 +791,8 @@ static int intel_context_set_gem(struct intel_context *ce,
 
 	ce->ring_size = SZ_16K;
 
-	if (rcu_access_pointer(ctx->vm)) {
-		struct i915_address_space *vm;
-
-		rcu_read_lock();
-		vm = context_get_vm_rcu(ctx); /* hmm */
-		rcu_read_unlock();
-
-		i915_vm_put(ce->vm);
-		ce->vm = vm;
-	}
+	i915_vm_put(ce->vm);
+	ce->vm = i915_gem_context_get_eb_vm(ctx);
 
 	if (ctx->sched.priority >= I915_PRIORITY_NORMAL &&
 	    intel_engine_has_timeslices(ce->engine) &&
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Intel-gfx] [PATCH 09/11] drm/i915: Drop __rcu from gem_context->vm
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (6 preceding siblings ...)
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 08/11] drm/i915: Use i915_gem_context_get_eb_vm in intel_context_set_gem Daniel Vetter
@ 2021-08-13 20:30 ` Daniel Vetter
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 10/11] drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups Daniel Vetter
                   ` (13 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2021-08-13 20:30 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Jon Bloomfield, Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

It's been invariant since

    commit ccbc1b97948ab671335e950271e39766729736c3
    Author: Jason Ekstrand <jason@jlekstrand.net>
    Date:   Thu Jul 8 10:48:30 2021 -0500

        drm/i915/gem: Don't allow changing the VM on running contexts (v4)

this just completes the deed. I've tried to split out prep work for
more careful review as much as possible, this is what's left:

- get_ppgtt gets simplified since we don't need to grab a temporary
  reference - we can rely on the temporary reference for the gem_ctx
  while we inspect the vm. The new vm_id still needs a full
  i915_vm_open ofc. This also removes the final caller of context_get_vm_rcu

- A pile of selftests can now just look at ctx->vm instead of
  rcu_dereference_protected( , true) or similar things.

- All callers of i915_gem_context_vm also disappear.

- I've changed the hugepage selftest to set scrub_64K without any
  locking, because when we inspect that setting we're also not taking
  any locks either. It works because it's a selftests that's careful
  (single threaded gives you nice ordering) and not a live driver
  where races can happen from anywhere.

These can only be split up further if we have some intermediate state
with a bunch more rcu_dereference_protected(ctx->vm, true), just to
shut up lockdep and sparse.

The conversion to __rcu happened in

commit a4e7ccdac38ec8335d9e4e2656c1a041c77feae1
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Oct 4 14:40:09 2019 +0100

    drm/i915: Move context management under GEM

Note that we're not breaking the actual bugfix in there: The real
bugfix is pushing the i915_vm_relase onto a separate worker, to avoid
locking inversion issues. The rcu conversion was just thrown in for
entertainment value on top (no vm lookup isn't even close to anything
that's a hotpath where removing the single spinlock can be measured).

v2: Rebase over the change to move the i915_vm_put() into
i915_gem_context_release().

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jason Ekstrand <jason@jlekstrand.net>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c   | 55 ++-----------------
 drivers/gpu/drm/i915/gem/i915_gem_context.h   | 14 +----
 .../gpu/drm/i915/gem/i915_gem_context_types.h |  2 +-
 .../gpu/drm/i915/gem/selftests/huge_pages.c   |  4 +-
 .../drm/i915/gem/selftests/i915_gem_context.c | 24 +++-----
 drivers/gpu/drm/i915/i915_trace.h             |  2 +-
 drivers/gpu/drm/i915/selftests/i915_vma.c     |  2 +-
 7 files changed, 22 insertions(+), 81 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 18e23d9220ae..fe8cd5456438 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -742,44 +742,6 @@ static int set_proto_ctx_param(struct drm_i915_file_private *fpriv,
 	return ret;
 }
 
-static struct i915_address_space *
-context_get_vm_rcu(struct i915_gem_context *ctx)
-{
-	GEM_BUG_ON(!rcu_access_pointer(ctx->vm));
-
-	do {
-		struct i915_address_space *vm;
-
-		/*
-		 * We do not allow downgrading from full-ppgtt [to a shared
-		 * global gtt], so ctx->vm cannot become NULL.
-		 */
-		vm = rcu_dereference(ctx->vm);
-		if (!kref_get_unless_zero(&vm->ref))
-			continue;
-
-		/*
-		 * This ppgtt may have be reallocated between
-		 * the read and the kref, and reassigned to a third
-		 * context. In order to avoid inadvertent sharing
-		 * of this ppgtt with that third context (and not
-		 * src), we have to confirm that we have the same
-		 * ppgtt after passing through the strong memory
-		 * barrier implied by a successful
-		 * kref_get_unless_zero().
-		 *
-		 * Once we have acquired the current ppgtt of ctx,
-		 * we no longer care if it is released from ctx, as
-		 * it cannot be reallocated elsewhere.
-		 */
-
-		if (vm == rcu_access_pointer(ctx->vm))
-			return rcu_pointer_handoff(vm);
-
-		i915_vm_put(vm);
-	} while (1);
-}
-
 static int intel_context_set_gem(struct intel_context *ce,
 				 struct i915_gem_context *ctx,
 				 struct intel_sseu sseu)
@@ -990,7 +952,7 @@ static void i915_gem_context_release_work(struct work_struct *work)
 	if (ctx->syncobj)
 		drm_syncobj_put(ctx->syncobj);
 
-	vm = i915_gem_context_vm(ctx);
+	vm = ctx->vm;
 	if (vm)
 		i915_vm_put(vm);
 
@@ -1216,7 +1178,7 @@ static void context_close(struct i915_gem_context *ctx)
 
 	set_closed_name(ctx);
 
-	vm = i915_gem_context_vm(ctx);
+	vm = ctx->vm;
 	if (vm) {
 		/* i915_vm_close drops the final reference, which is a bit too
 		 * early and could result in surprises with concurrent
@@ -1335,7 +1297,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
 		vm = &ppgtt->vm;
 	}
 	if (vm) {
-		RCU_INIT_POINTER(ctx->vm, i915_vm_open(vm));
+		ctx->vm = i915_vm_open(vm);
 
 		/* i915_vm_open() takes a reference */
 		i915_vm_put(vm);
@@ -1561,15 +1523,12 @@ static int get_ppgtt(struct drm_i915_file_private *file_priv,
 	if (!i915_gem_context_is_full_ppgtt(ctx))
 		return -ENODEV;
 
-	rcu_read_lock();
-	vm = context_get_vm_rcu(ctx);
-	rcu_read_unlock();
-	if (!vm)
-		return -ENODEV;
+	vm = ctx->vm;
+	GEM_BUG_ON(!vm);
 
 	err = xa_alloc(&file_priv->vm_xa, &id, vm, xa_limit_32b, GFP_KERNEL);
 	if (err)
-		goto err_put;
+		return err;
 
 	i915_vm_open(vm);
 
@@ -1577,8 +1536,6 @@ static int get_ppgtt(struct drm_i915_file_private *file_priv,
 	args->value = id;
 	args->size = 0;
 
-err_put:
-	i915_vm_put(vm);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h b/drivers/gpu/drm/i915/gem/i915_gem_context.h
index 37536a260e6e..7696bc91647d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h
@@ -148,17 +148,11 @@ static inline void i915_gem_context_put(struct i915_gem_context *ctx)
 	kref_put(&ctx->ref, i915_gem_context_release);
 }
 
-static inline struct i915_address_space *
-i915_gem_context_vm(struct i915_gem_context *ctx)
-{
-	return rcu_dereference_protected(ctx->vm, lockdep_is_held(&ctx->mutex));
-}
-
 static inline bool i915_gem_context_is_full_ppgtt(struct i915_gem_context *ctx)
 {
-	GEM_BUG_ON(!!rcu_access_pointer(ctx->vm) != HAS_FULL_PPGTT(ctx->i915));
+	GEM_BUG_ON(!!ctx->vm != HAS_FULL_PPGTT(ctx->i915));
 
-	return !!rcu_access_pointer(ctx->vm);
+	return !!ctx->vm;
 }
 
 static inline struct i915_address_space *
@@ -166,12 +160,10 @@ i915_gem_context_get_eb_vm(struct i915_gem_context *ctx)
 {
 	struct i915_address_space *vm;
 
-	rcu_read_lock();
-	vm = rcu_dereference(ctx->vm);
+	vm = ctx->vm;
 	if (!vm)
 		vm = &ctx->i915->ggtt.vm;
 	vm = i915_vm_get(vm);
-	rcu_read_unlock();
 
 	return vm;
 }
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index 0c38789bd4a8..c4617e4d9fa9 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -262,7 +262,7 @@ struct i915_gem_context {
 	 * In other modes, this is a NULL pointer with the expectation that
 	 * the caller uses the shared global GTT.
 	 */
-	struct i915_address_space __rcu *vm;
+	struct i915_address_space *vm;
 
 	/**
 	 * @pid: process id of creator
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index 6c68fe26bb32..5d71626a1ee5 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1688,11 +1688,9 @@ int i915_gem_huge_page_live_selftests(struct drm_i915_private *i915)
 		goto out_file;
 	}
 
-	mutex_lock(&ctx->mutex);
-	vm = i915_gem_context_vm(ctx);
+	vm = ctx->vm;
 	if (vm)
 		WRITE_ONCE(vm->scrub_64K, true);
-	mutex_unlock(&ctx->mutex);
 
 	err = i915_subtests(tests, ctx);
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
index 0708b9cdeb9f..4370a90d8a50 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
@@ -27,12 +27,6 @@
 
 #define DW_PER_PAGE (PAGE_SIZE / sizeof(u32))
 
-static inline struct i915_address_space *ctx_vm(struct i915_gem_context *ctx)
-{
-	/* single threaded, private ctx */
-	return rcu_dereference_protected(ctx->vm, true);
-}
-
 static int live_nop_switch(void *arg)
 {
 	const unsigned int nctx = 1024;
@@ -813,7 +807,7 @@ static int igt_shared_ctx_exec(void *arg)
 			struct i915_gem_context *ctx;
 			struct intel_context *ce;
 
-			ctx = kernel_context(i915, ctx_vm(parent));
+			ctx = kernel_context(i915, parent->vm);
 			if (IS_ERR(ctx)) {
 				err = PTR_ERR(ctx);
 				goto out_test;
@@ -823,7 +817,7 @@ static int igt_shared_ctx_exec(void *arg)
 			GEM_BUG_ON(IS_ERR(ce));
 
 			if (!obj) {
-				obj = create_test_object(ctx_vm(parent),
+				obj = create_test_object(parent->vm,
 							 file, &objects);
 				if (IS_ERR(obj)) {
 					err = PTR_ERR(obj);
@@ -1380,7 +1374,7 @@ static int igt_ctx_readonly(void *arg)
 		goto out_file;
 	}
 
-	vm = ctx_vm(ctx) ?: &i915->ggtt.alias->vm;
+	vm = ctx->vm ?: &i915->ggtt.alias->vm;
 	if (!vm || !vm->has_read_only) {
 		err = 0;
 		goto out_file;
@@ -1499,7 +1493,7 @@ static int write_to_scratch(struct i915_gem_context *ctx,
 
 	GEM_BUG_ON(offset < I915_GTT_PAGE_SIZE);
 
-	err = check_scratch(ctx_vm(ctx), offset);
+	err = check_scratch(ctx->vm, offset);
 	if (err)
 		return err;
 
@@ -1596,7 +1590,7 @@ static int read_from_scratch(struct i915_gem_context *ctx,
 
 	GEM_BUG_ON(offset < I915_GTT_PAGE_SIZE);
 
-	err = check_scratch(ctx_vm(ctx), offset);
+	err = check_scratch(ctx->vm, offset);
 	if (err)
 		return err;
 
@@ -1739,7 +1733,7 @@ static int check_scratch_page(struct i915_gem_context *ctx, u32 *out)
 	u32 *vaddr;
 	int err = 0;
 
-	vm = ctx_vm(ctx);
+	vm = ctx->vm;
 	if (!vm)
 		return -ENODEV;
 
@@ -1801,7 +1795,7 @@ static int igt_vm_isolation(void *arg)
 	}
 
 	/* We can only test vm isolation, if the vm are distinct */
-	if (ctx_vm(ctx_a) == ctx_vm(ctx_b))
+	if (ctx_a->vm == ctx_b->vm)
 		goto out_file;
 
 	/* Read the initial state of the scratch page */
@@ -1813,8 +1807,8 @@ static int igt_vm_isolation(void *arg)
 	if (err)
 		goto out_file;
 
-	vm_total = ctx_vm(ctx_a)->total;
-	GEM_BUG_ON(ctx_vm(ctx_b)->total != vm_total);
+	vm_total = ctx_a->vm->total;
+	GEM_BUG_ON(ctx_b->vm->total != vm_total);
 
 	count = 0;
 	num_engines = 0;
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index 806ad688274b..237e5061381b 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -1246,7 +1246,7 @@ DECLARE_EVENT_CLASS(i915_context,
 	TP_fast_assign(
 			__entry->dev = ctx->i915->drm.primary->index;
 			__entry->ctx = ctx;
-			__entry->vm = rcu_access_pointer(ctx->vm);
+			__entry->vm = ctx->vm;
 	),
 
 	TP_printk("dev=%u, ctx=%p, ctx_vm=%p",
diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c
index 79ba72da0813..1f10fe36619b 100644
--- a/drivers/gpu/drm/i915/selftests/i915_vma.c
+++ b/drivers/gpu/drm/i915/selftests/i915_vma.c
@@ -39,7 +39,7 @@ static bool assert_vma(struct i915_vma *vma,
 {
 	bool ok = true;
 
-	if (vma->vm != rcu_access_pointer(ctx->vm)) {
+	if (vma->vm != ctx->vm) {
 		pr_err("VMA created with wrong VM\n");
 		ok = false;
 	}
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Intel-gfx] [PATCH 10/11] drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (7 preceding siblings ...)
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 09/11] drm/i915: Drop __rcu from gem_context->vm Daniel Vetter
@ 2021-08-13 20:30 ` Daniel Vetter
  2021-08-31  9:29   ` Maarten Lankhorst
  2021-08-31 12:14   ` [Intel-gfx] [PATCH] " Daniel Vetter
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 11/11] drm/i915: Stop rcu support for i915_address_space Daniel Vetter
                   ` (12 subsequent siblings)
  21 siblings, 2 replies; 33+ messages in thread
From: Daniel Vetter @ 2021-08-13 20:30 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Jon Bloomfield, Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

We don't need the absolute speed of rcu for this. And
i915_address_space in general dont need rcu protection anywhere else,
after we've made gem contexts and engines a lot more immutable.

Note that this semantically reverts

commit aabbe344dc3ca5f7d8263a02608ba6179e8a4499
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Aug 30 19:03:25 2019 +0100

    drm/i915: Use RCU for unlocked vm_idr lookup

except we have the conversion from idr to xarray in between.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jason Ekstrand <jason@jlekstrand.net>
---
 drivers/gpu/drm/i915/i915_drv.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 005b1cec7007..e37fac8fac0c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1881,11 +1881,11 @@ i915_gem_vm_lookup(struct drm_i915_file_private *file_priv, u32 id)
 {
 	struct i915_address_space *vm;
 
-	rcu_read_lock();
+	xa_lock(&file_priv->vm_xa);
 	vm = xa_load(&file_priv->vm_xa, id);
 	if (vm && !kref_get_unless_zero(&vm->ref))
 		vm = NULL;
-	rcu_read_unlock();
+	xa_unlock(&file_priv->vm_xa);
 
 	return vm;
 }
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Intel-gfx] [PATCH 11/11] drm/i915: Stop rcu support for i915_address_space
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (8 preceding siblings ...)
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 10/11] drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups Daniel Vetter
@ 2021-08-13 20:30 ` Daniel Vetter
  2021-08-13 21:48 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/11] drm/i915: Release i915_gem_context from a worker Patchwork
                   ` (11 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2021-08-13 20:30 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Jon Bloomfield, Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

The full audit is quite a bit of work:

- i915_dpt has very simple lifetime (somehow we create a display pagetable vm
  per object, so its _very_ simple, there's only ever a single vma in there),
  and uses i915_vm_close(), which internally does a i915_vm_put(). No rcu.

  Aside: wtf is i915_dpt doing in the intel_display.c garbage collector as a new
  feature, instead of added as a separate file with some clean-ish interface.

  Also, i915_dpt unfortunately re-introduces some coding patterns from
  pre-dma_resv_lock conversion times.

- i915_gem_proto_ctx is fully refcounted and no rcu, all protected by
  fpriv->proto_context_lock.

- i915_gem_context is itself rcu protected, and that might leak to anything it
  points at. Before

	commit cf977e18610e66e48c31619e7e0cfa871be9eada
	Author: Chris Wilson <chris@chris-wilson.co.uk>
	Date:   Wed Dec 2 11:21:40 2020 +0000

	    drm/i915/gem: Spring clean debugfs

  and

	commit db80a1294c231b6ac725085f046bb2931e00c9db
	Author: Chris Wilson <chris@chris-wilson.co.uk>
	Date:   Mon Jan 18 11:08:54 2021 +0000

	    drm/i915/gem: Remove per-client stats from debugfs/i915_gem_objects

  we had a bunch of debugfs files that relied on rcu protecting everything, but
  those are gone now. The main one was removed even earlier with

  There doesn't seem to be anything left that's actually protecting
  stuff now that the ctx->vm itself is invariant. See

	commit ccbc1b97948ab671335e950271e39766729736c3
	Author: Jason Ekstrand <jason@jlekstrand.net>
	Date:   Thu Jul 8 10:48:30 2021 -0500

	    drm/i915/gem: Don't allow changing the VM on running contexts (v4)

  Note that we drop the vm refcount before the final release of the gem context
  refcount, so this is all very dangerous even without rcu. Note that aside from
  later on creating new engines (a defunct feature) and debug output we're never
  looked at gem_ctx->vm for anything functional, hence why this is ok.
  Fingers crossed.

  Preceeding patches removed all vestiges of rcu use from gem_ctx->vm
  derferencing to make it clear it's really not used.

  The gem_ctx->rcu protection was introduced in

	commit a4e7ccdac38ec8335d9e4e2656c1a041c77feae1
	Author: Chris Wilson <chris@chris-wilson.co.uk>
	Date:   Fri Oct 4 14:40:09 2019 +0100

	    drm/i915: Move context management under GEM

  The commit message is somewhat entertaining because it fails to
  mention this fact completely, and compensates that by an in-commit
  changelog entry that claims that ctx->vm is protected by ctx->mutex.
  Which was the case _before_ this commit, but no longer after it.

- intel_context holds a full reference. Unfortunately intel_context is also rcu
  protected and the reference to the ->vm is dropped before the
  rcu barrier - only the kfree is delayed. So again we need to check
  whether that leaks anywhere on the intel_context->vm. RCU is only
  used to protect intel_context sitting on the breadcrumb lists, which
  don't look at the vm anywhere, so we are fine.

  Nothing else relies on rcu protection of intel_context and hence is
  fully protected by the kref refcount alone, which protects
  intel_context->vm in turn.

  The breadcrumbs rcu usage was added in

	commit c744d50363b714783bbc88d986cc16def13710f7
	Author: Chris Wilson <chris@chris-wilson.co.uk>
	Date:   Thu Nov 26 14:04:06 2020 +0000

	    drm/i915/gt: Split the breadcrumb spinlock between global and contexts

  its parent commit added the intel_context rcu protection:

	commit 14d1eaf08845c534963c83f754afe0cb14cb2512
	Author: Chris Wilson <chris@chris-wilson.co.uk>
	Date:   Thu Nov 26 14:04:05 2020 +0000

	    drm/i915/gt: Protect context lifetime with RCU

  given some credence to my claim that I've actually caught them all.

- drm_i915_gem_object's shares_resv_from pointer has a full refcount to the
  dma_resv, which is a sub-refcount that's released after the final
  i915_vm_put() has been called. Safe.

  Aside: Maybe we should have a struct dma_resv_shared which is just dma_resv +
  kref as a stand-alone thing. It's a pretty useful pattern which other drivers
  might want to copy.

  For a bit more context see

	commit 4d8151ae5329cf50781a02fd2298a909589a5bab
	Author: Thomas Hellström <thomas.hellstrom@linux.intel.com>
	Date:   Tue Jun 1 09:46:41 2021 +0200

	    drm/i915: Don't free shared locks while shared

- the fpriv->vm_xa was relying on rcu_read_lock for lookup, but that
  was updated in a prep patch too to just be a spinlock-protected
  lookup.

- intel_gt->vm is set at driver load in intel_gt_init() and released
  in intel_gt_driver_release(). There seems to be some issue that
  in some error paths this is called twice, but otherwise no rcu to be
  found anywhere. This was added in the below commit, which
  unfortunately doesn't explain why this complication exists.

	commit e6ba76480299a0d77c51d846f7467b1673aad25b
	Author: Chris Wilson <chris@chris-wilson.co.uk>
	Date:   Sat Dec 21 16:03:24 2019 +0000

	    drm/i915: Remove i915->kernel_context

  The proper fix most likely for this is to start using drmm_ at large
  scale, but that's also huge amounts of work.

- i915_vma->vm is some real pain, because rcu is rcu protected, at
  least in the vma lookup in the context lookup cache in
  eb_lookup_vma(). This was added in

	commit 4ff4b44cbb70c269259958cbcc48d7b8a2cb9ec8
	Author: Chris Wilson <chris@chris-wilson.co.uk>
	Date:   Fri Jun 16 15:05:16 2017 +0100

	    drm/i915: Store a direct lookup from object handle to vma

  This was changed to a radix tree from the hashtable in, but with the
  locking unchanged, in

	commit d1b48c1e7184d9bc4ae6d7f9fe2eed9efed11ffc
	Author: Chris Wilson <chris@chris-wilson.co.uk>
	Date:   Wed Aug 16 09:52:08 2017 +0100

	    drm/i915: Replace execbuf vma ht with an idr

  In

	commit 93159e12353c2a47e5576d642845a91fa00530bf
	Author: Chris Wilson <chris@chris-wilson.co.uk>
	Date:   Mon Mar 23 09:28:41 2020 +0000

	    drm/i915/gem: Avoid gem_context->mutex for simple vma lookup

  the locking was changed from dev->struct_mutex to rcu, which added
  the requirement to rcu protect i915_vma. Somehow this was missed in
  review (or I'm completely blind).

  Irrespective of all that the vma lookup cache rcu_read_lock grabs a
  full reference of the vma and the rcu doesn't leak further. So no
  impact on i915_address_space from that.

  I have not found any other rcu use for i915_vma, but given that it
  seems broken I also didn't bother to do a careful in-depth audit.

Alltogether there's nothing left in-tree anymore which requires that a
pointer deref to an i915_address_space is safe undre rcu_read_lock
only.

rcu protection of i915_address_space was introduced in

commit b32fa811156328aea5a3c2ff05cc096490382456
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Thu Jun 20 19:37:05 2019 +0100

    drm/i915/gtt: Defer address space cleanup to an RCU worker

by mixing up a bugfixing (i915_address_space needs to be released from
a worker) with enabling rcu support. The commit message also seems
somewhat confused, because it talks about cleanup of WC pages
requiring sleep, while the code and linked bugzilla are about a
requirement to take dev->struct_mutex (which yes sleeps but it's a
much more specific problem). Since final kref_put can be called from
pretty much anywhere (including hardirq context through the
scheduler's i915_active cleanup) we need a worker here. Hence that
part must be kept.

Ideally all these reclaim workers should have some kind of integration
with our shrinkers, but for some of these it's rather tricky. Anyway,
that's a preexisting condition in the codeebase that we wont fix in
this patch here.

We also remove the rcu_barrier in ggtt_cleanup_hw added in

commit 60a4233a4952729089e4df152e730f8f4d0e82ce
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Mon Jul 29 14:24:12 2019 +0100

    drm/i915: Flush the i915_vm_release before ggtt shutdown

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jason Ekstrand <jason@jlekstrand.net>
---
 drivers/gpu/drm/i915/gt/intel_ggtt.c | 1 -
 drivers/gpu/drm/i915/gt/intel_gtt.c  | 6 +++---
 drivers/gpu/drm/i915/gt/intel_gtt.h  | 2 +-
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index de3ac58fceec..8d71f67926f1 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -727,7 +727,6 @@ static void ggtt_cleanup_hw(struct i915_ggtt *ggtt)
 
 	atomic_set(&ggtt->vm.open, 0);
 
-	rcu_barrier(); /* flush the RCU'ed__i915_vm_release */
 	flush_workqueue(ggtt->vm.i915->wq);
 
 	mutex_lock(&ggtt->vm.mutex);
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
index e137dd32b5b8..a0c2b952aa57 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -155,7 +155,7 @@ void i915_vm_resv_release(struct kref *kref)
 static void __i915_vm_release(struct work_struct *work)
 {
 	struct i915_address_space *vm =
-		container_of(work, struct i915_address_space, rcu.work);
+		container_of(work, struct i915_address_space, release_work);
 
 	vm->cleanup(vm);
 	i915_address_space_fini(vm);
@@ -171,7 +171,7 @@ void i915_vm_release(struct kref *kref)
 	GEM_BUG_ON(i915_is_ggtt(vm));
 	trace_i915_ppgtt_release(vm);
 
-	queue_rcu_work(vm->i915->wq, &vm->rcu);
+	queue_work(vm->i915->wq, &vm->release_work);
 }
 
 void i915_address_space_init(struct i915_address_space *vm, int subclass)
@@ -185,7 +185,7 @@ void i915_address_space_init(struct i915_address_space *vm, int subclass)
 	if (!kref_read(&vm->resv_ref))
 		kref_init(&vm->resv_ref);
 
-	INIT_RCU_WORK(&vm->rcu, __i915_vm_release);
+	INIT_WORK(&vm->release_work, __i915_vm_release);
 	atomic_set(&vm->open, 1);
 
 	/*
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
index bc7153018ebd..5b539bd7645d 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -213,7 +213,7 @@ struct i915_vma_ops {
 
 struct i915_address_space {
 	struct kref ref;
-	struct rcu_work rcu;
+	struct work_struct release_work;
 
 	struct drm_mm mm;
 	struct intel_gt *gt;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/11] drm/i915: Release i915_gem_context from a worker
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (9 preceding siblings ...)
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 11/11] drm/i915: Stop rcu support for i915_address_space Daniel Vetter
@ 2021-08-13 21:48 ` Patchwork
  2021-08-13 21:49 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Patchwork @ 2021-08-13 21:48 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/11] drm/i915: Release i915_gem_context from a worker
URL   : https://patchwork.freedesktop.org/series/93693/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
3d5b2e6041c2 drm/i915: Release i915_gem_context from a worker
-:94: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 50 lines checked
3dbd3c669f81 drm/i915: Release ctx->syncobj on final put, not on ctx close
-:17: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 00dae4d3d35d ("drm/i915: Implement SINGLE_TIMELINE with a syncobj (v4)")'
#17: 
commit 00dae4d3d35d4f526929633b76e00b0ab4d3970d

-:66: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 1 errors, 1 warnings, 0 checks, 18 lines checked
d7d44ea0347d drm/i915: Keep gem ctx->vm alive until the final put
-:11: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit b81dde719439 ("drm/i915: Allow userspace to clone contexts on creation")'
#11: 
    commit b81dde719439c8f09bb61e742ed95bfc4b33946b

-:19: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 27dbae8f36c1 ("drm/i915/gem: Safely acquire the ctx->vm when copying")'
#19: 
    commit 27dbae8f36c1c25008b7885fc07c57054b7dfba3

-:42: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 2850748ef876 ("drm/i915: Pull i915_vma_pin under the vm->mutex")'
#42: 
commit 2850748ef8763ab46958e43a4d1c445f29eeb37d

-:55: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit b32fa8111563 ("drm/i915/gtt: Defer address space cleanup to an RCU worker")'
#55: 
    commit b32fa811156328aea5a3c2ff05cc096490382456

-:126: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 4 errors, 1 warnings, 0 checks, 33 lines checked
94b97e41d26f drm/i915: Drop code to handle set-vm races from execbuf
-:17: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#17: 
References: ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running contexts (v4)")

-:17: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running contexts (v4)")'
#17: 
References: ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running contexts (v4)")

-:46: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 1 errors, 2 warnings, 0 checks, 12 lines checked
93bdf24d3f4e drm/i915: Rename i915_gem_context_get_vm_rcu to i915_gem_context_get_eb_vm
-:148: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 80 lines checked
66c32a8632a6 drm/i915: Use i915_gem_context_get_eb_vm in ctx_getparam
-:54: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 23 lines checked
a17030520ca3 drm/i915: Add i915_gem_context_is_full_ppgtt
-:105: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 53 lines checked
bce9c7854c53 drm/i915: Use i915_gem_context_get_eb_vm in intel_context_set_gem
-:12: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running contexts (v4)")'
#12: 
commit ccbc1b97948ab671335e950271e39766729736c3

-:61: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 1 errors, 1 warnings, 0 checks, 18 lines checked
014eb08b69c3 drm/i915: Drop __rcu from gem_context->vm
-:11: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running contexts (v4)")'
#11: 
    commit ccbc1b97948ab671335e950271e39766729736c3

-:23: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#23: 
  i915_vm_open ofc. This also removes the final caller of context_get_vm_rcu

-:42: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit a4e7ccdac38e ("drm/i915: Move context management under GEM")'
#42: 
commit a4e7ccdac38ec8335d9e4e2656c1a041c77feae1

-:357: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 2 errors, 2 warnings, 0 checks, 240 lines checked
ca6069dbc0fe drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups
-:15: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit aabbe344dc3c ("drm/i915: Use RCU for unlocked vm_idr lookup")'
#15: 
commit aabbe344dc3ca5f7d8263a02608ba6179e8a4499

-:52: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 1 errors, 1 warnings, 0 checks, 13 lines checked
a20d82053e1a drm/i915: Stop rcu support for i915_address_space
-:11: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#11: 
- i915_dpt has very simple lifetime (somehow we create a display pagetable vm

-:27: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit cf977e18610e ("drm/i915/gem: Spring clean debugfs")'
#27: 
	commit cf977e18610e66e48c31619e7e0cfa871be9eada

-:35: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit db80a1294c23 ("drm/i915/gem: Remove per-client stats from debugfs/i915_gem_objects")'
#35: 
	commit db80a1294c231b6ac725085f046bb2931e00c9db

-:47: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running contexts (v4)")'
#47: 
	commit ccbc1b97948ab671335e950271e39766729736c3

-:59: WARNING:TYPO_SPELLING: 'Preceeding' may be misspelled - perhaps 'Preceding'?
#59: 
  Preceeding patches removed all vestiges of rcu use from gem_ctx->vm
  ^^^^^^^^^^

-:64: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit a4e7ccdac38e ("drm/i915: Move context management under GEM")'
#64: 
	commit a4e7ccdac38ec8335d9e4e2656c1a041c77feae1

-:88: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit c744d50363b7 ("drm/i915/gt: Split the breadcrumb spinlock between global and contexts")'
#88: 
	commit c744d50363b714783bbc88d986cc16def13710f7

-:94: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit added5fce61e ("ARM: mxs_defconfig: add CONFIG_USB_PHY")'
#94: 
  its parent commit added the intel_context rcu protection:

-:96: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 14d1eaf08845 ("drm/i915/gt: Protect context lifetime with RCU")'
#96: 
	commit 14d1eaf08845c534963c83f754afe0cb14cb2512

-:114: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 4d8151ae5329 ("drm/i915: Don't free shared locks while shared")'
#114: 
	commit 4d8151ae5329cf50781a02fd2298a909589a5bab

-:130: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit e6ba76480299 ("drm/i915: Remove i915->kernel_context")'
#130: 
	commit e6ba76480299a0d77c51d846f7467b1673aad25b

-:143: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 4ff4b44cbb70 ("drm/i915: Store a direct lookup from object handle to vma")'
#143: 
	commit 4ff4b44cbb70c269259958cbcc48d7b8a2cb9ec8

-:152: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit d1b48c1e7184 ("drm/i915: Replace execbuf vma ht with an idr")'
#152: 
	commit d1b48c1e7184d9bc4ae6d7f9fe2eed9efed11ffc

-:160: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 93159e12353c ("drm/i915/gem: Avoid gem_context->mutex for simple vma lookup")'
#160: 
	commit 93159e12353c2a47e5576d642845a91fa00530bf

-:183: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit b32fa8111563 ("drm/i915/gtt: Defer address space cleanup to an RCU worker")'
#183: 
commit b32fa811156328aea5a3c2ff05cc096490382456

-:201: WARNING:TYPO_SPELLING: 'wont' may be misspelled - perhaps 'won't'?
#201: 
that's a preexisting condition in the codeebase that we wont fix in
                                                        ^^^^

-:206: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 60a4233a4952 ("drm/i915: Flush the i915_vm_release before ggtt shutdown")'
#206: 
commit 60a4233a4952729089e4df152e730f8f4d0e82ce

-:279: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 14 errors, 4 warnings, 0 checks, 39 lines checked



^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [01/11] drm/i915: Release i915_gem_context from a worker
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (10 preceding siblings ...)
  2021-08-13 21:48 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/11] drm/i915: Release i915_gem_context from a worker Patchwork
@ 2021-08-13 21:49 ` Patchwork
  2021-08-13 22:18 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Patchwork @ 2021-08-13 21:49 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/11] drm/i915: Release i915_gem_context from a worker
URL   : https://patchwork.freedesktop.org/series/93693/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-drivers/gpu/drm/i915/gem/i915_gem_context.c:1349:34:    expected struct i915_address_space *vm
-drivers/gpu/drm/i915/gem/i915_gem_context.c:1349:34:    got struct i915_address_space [noderef] __rcu *vm
-drivers/gpu/drm/i915/gem/i915_gem_context.c:1349:34: warning: incorrect type in argument 1 (different address spaces)
-drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25:    expected struct i915_address_space [noderef] __rcu *vm
-drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25:    got struct i915_address_space *
-drivers/gpu/drm/i915/gem/selftests/mock_context.c:43:25: warning: incorrect type in assignment (different address spaces)
-drivers/gpu/drm/i915/gem/selftests/mock_context.c:60:34:    expected struct i915_address_space *vm
-drivers/gpu/drm/i915/gem/selftests/mock_context.c:60:34:    got struct i915_address_space [noderef] __rcu *vm
-drivers/gpu/drm/i915/gem/selftests/mock_context.c:60:34: warning: incorrect type in argument 1 (different address spaces)



^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [01/11] drm/i915: Release i915_gem_context from a worker
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (11 preceding siblings ...)
  2021-08-13 21:49 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2021-08-13 22:18 ` Patchwork
  2021-08-14  1:26 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Patchwork @ 2021-08-13 22:18 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 3361 bytes --]

== Series Details ==

Series: series starting with [01/11] drm/i915: Release i915_gem_context from a worker
URL   : https://patchwork.freedesktop.org/series/93693/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10484 -> Patchwork_20821
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/index.html

Known issues
------------

  Here are the changes found in Patchwork_20821 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-gfx:
    - fi-rkl-guc:         NOTRUN -> [SKIP][1] ([fdo#109315]) +17 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/fi-rkl-guc/igt@amdgpu/amd_basic@cs-gfx.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload:
    - {fi-tgl-dsi}:       [DMESG-WARN][2] ([i915#1982] / [k.org#205379]) -> [PASS][3]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/fi-tgl-dsi/igt@i915_module_load@reload.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/fi-tgl-dsi/igt@i915_module_load@reload.html

  * igt@i915_selftest@live@workarounds:
    - fi-rkl-guc:         [DMESG-FAIL][4] ([i915#3928]) -> [PASS][5]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/fi-rkl-guc/igt@i915_selftest@live@workarounds.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/fi-rkl-guc/igt@i915_selftest@live@workarounds.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#3928]: https://gitlab.freedesktop.org/drm/intel/issues/3928
  [k.org#205379]: https://bugzilla.kernel.org/show_bug.cgi?id=205379


Participating hosts (37 -> 34)
------------------------------

  Missing    (3): fi-bdw-samus fi-bsw-cyan bat-jsl-1 


Build changes
-------------

  * Linux: CI_DRM_10484 -> Patchwork_20821

  CI-20190529: 20190529
  CI_DRM_10484: 7de02d5cb1f35bd3f068237444063844dea47ddc @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6175: c91f99c74b966f635d7e2eb898bf0f78383d281b @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_20821: a20d82053e1a2412667fdb30c99ad74830434546 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a20d82053e1a drm/i915: Stop rcu support for i915_address_space
ca6069dbc0fe drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups
014eb08b69c3 drm/i915: Drop __rcu from gem_context->vm
bce9c7854c53 drm/i915: Use i915_gem_context_get_eb_vm in intel_context_set_gem
a17030520ca3 drm/i915: Add i915_gem_context_is_full_ppgtt
66c32a8632a6 drm/i915: Use i915_gem_context_get_eb_vm in ctx_getparam
93bdf24d3f4e drm/i915: Rename i915_gem_context_get_vm_rcu to i915_gem_context_get_eb_vm
94b97e41d26f drm/i915: Drop code to handle set-vm races from execbuf
d7d44ea0347d drm/i915: Keep gem ctx->vm alive until the final put
3dbd3c669f81 drm/i915: Release ctx->syncobj on final put, not on ctx close
3d5b2e6041c2 drm/i915: Release i915_gem_context from a worker

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/index.html

[-- Attachment #2: Type: text/html, Size: 4091 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [01/11] drm/i915: Release i915_gem_context from a worker
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (12 preceding siblings ...)
  2021-08-13 22:18 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2021-08-14  1:26 ` Patchwork
  2021-08-14 10:43 ` [Intel-gfx] [PATCH] " Daniel Vetter
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Patchwork @ 2021-08-14  1:26 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 30300 bytes --]

== Series Details ==

Series: series starting with [01/11] drm/i915: Release i915_gem_context from a worker
URL   : https://patchwork.freedesktop.org/series/93693/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_10484_full -> Patchwork_20821_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_20821_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_20821_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_20821_full:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@mock@requests:
    - shard-kbl:          [PASS][1] -> [DMESG-FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-kbl1/igt@i915_selftest@mock@requests.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl1/igt@i915_selftest@mock@requests.html
    - shard-tglb:         [PASS][3] -> [DMESG-FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-tglb5/igt@i915_selftest@mock@requests.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb5/igt@i915_selftest@mock@requests.html
    - shard-glk:          [PASS][5] -> [DMESG-FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-glk4/igt@i915_selftest@mock@requests.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-glk8/igt@i915_selftest@mock@requests.html
    - shard-skl:          [PASS][7] -> [DMESG-FAIL][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl7/igt@i915_selftest@mock@requests.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-skl4/igt@i915_selftest@mock@requests.html
    - shard-iclb:         [PASS][9] -> [DMESG-FAIL][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-iclb6/igt@i915_selftest@mock@requests.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb4/igt@i915_selftest@mock@requests.html

  
#### Warnings ####

  * igt@runner@aborted:
    - shard-iclb:         ([FAIL][11], [FAIL][12]) ([i915#3002]) -> ([FAIL][13], [FAIL][14], [FAIL][15], [FAIL][16]) ([i915#1814] / [i915#3002])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-iclb8/igt@runner@aborted.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-iclb4/igt@runner@aborted.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@runner@aborted.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb6/igt@runner@aborted.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@runner@aborted.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb4/igt@runner@aborted.html

  
Known issues
------------

  Here are the changes found in Patchwork_20821_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_create@create-massive:
    - shard-snb:          NOTRUN -> [DMESG-WARN][17] ([i915#3002]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-snb7/igt@gem_create@create-massive.html

  * igt@gem_ctx_persistence@engines-mixed:
    - shard-snb:          NOTRUN -> [SKIP][18] ([fdo#109271] / [i915#1099]) +5 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-snb6/igt@gem_ctx_persistence@engines-mixed.html

  * igt@gem_eio@in-flight-contexts-1us:
    - shard-tglb:         [PASS][19] -> [TIMEOUT][20] ([i915#3063])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-tglb6/igt@gem_eio@in-flight-contexts-1us.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb3/igt@gem_eio@in-flight-contexts-1us.html

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - shard-iclb:         [PASS][21] -> [FAIL][22] ([i915#2842])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-iclb7/igt@gem_exec_fair@basic-none-share@rcs0.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb8/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_exec_fair@basic-none-solo@rcs0:
    - shard-glk:          [PASS][23] -> [FAIL][24] ([i915#2842]) +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-glk7/igt@gem_exec_fair@basic-none-solo@rcs0.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-glk1/igt@gem_exec_fair@basic-none-solo@rcs0.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-tglb:         [PASS][25] -> [FAIL][26] ([i915#2842])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-tglb1/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb2/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_pread@exhaustion:
    - shard-kbl:          NOTRUN -> [WARN][27] ([i915#2658])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl2/igt@gem_pread@exhaustion.html

  * igt@gem_render_copy@x-tiled-to-vebox-yf-tiled:
    - shard-kbl:          NOTRUN -> [SKIP][28] ([fdo#109271]) +115 similar issues
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl4/igt@gem_render_copy@x-tiled-to-vebox-yf-tiled.html

  * igt@gem_render_copy@y-tiled-mc-ccs-to-yf-tiled-ccs:
    - shard-iclb:         NOTRUN -> [SKIP][29] ([i915#768])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@gem_render_copy@y-tiled-mc-ccs-to-yf-tiled-ccs.html

  * igt@gem_userptr_blits@input-checking:
    - shard-kbl:          NOTRUN -> [DMESG-WARN][30] ([i915#3002])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl4/igt@gem_userptr_blits@input-checking.html

  * igt@gen9_exec_parse@batch-invalid-length:
    - shard-snb:          NOTRUN -> [SKIP][31] ([fdo#109271]) +411 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-snb6/igt@gen9_exec_parse@batch-invalid-length.html

  * igt@gen9_exec_parse@cmd-crossing-page:
    - shard-tglb:         NOTRUN -> [SKIP][32] ([i915#2856]) +1 similar issue
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb3/igt@gen9_exec_parse@cmd-crossing-page.html

  * igt@i915_pm_dc@dc6-dpms:
    - shard-tglb:         NOTRUN -> [FAIL][33] ([i915#454])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb3/igt@i915_pm_dc@dc6-dpms.html

  * igt@i915_pm_rpm@modeset-lpsp-stress:
    - shard-apl:          NOTRUN -> [SKIP][34] ([fdo#109271]) +273 similar issues
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-apl7/igt@i915_pm_rpm@modeset-lpsp-stress.html

  * igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait:
    - shard-tglb:         NOTRUN -> [SKIP][35] ([fdo#111644] / [i915#1397] / [i915#2411])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb3/igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait.html

  * igt@i915_query@query-topology-known-pci-ids:
    - shard-tglb:         NOTRUN -> [SKIP][36] ([fdo#109303])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb8/igt@i915_query@query-topology-known-pci-ids.html
    - shard-iclb:         NOTRUN -> [SKIP][37] ([fdo#109303])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@i915_query@query-topology-known-pci-ids.html

  * igt@kms_big_fb@linear-8bpp-rotate-90:
    - shard-tglb:         NOTRUN -> [SKIP][38] ([fdo#111614])
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb3/igt@kms_big_fb@linear-8bpp-rotate-90.html

  * igt@kms_big_fb@x-tiled-64bpp-rotate-180:
    - shard-iclb:         [PASS][39] -> [DMESG-WARN][40] ([i915#3621])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-iclb2/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@kms_big_fb@x-tiled-64bpp-rotate-180.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip:
    - shard-kbl:          NOTRUN -> [SKIP][41] ([fdo#109271] / [i915#3777]) +1 similar issue
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl2/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-hflip:
    - shard-apl:          NOTRUN -> [SKIP][42] ([fdo#109271] / [i915#3777])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-apl7/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-hflip.html

  * igt@kms_big_fb@yf-tiled-addfb-size-overflow:
    - shard-tglb:         NOTRUN -> [SKIP][43] ([fdo#111615]) +4 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb8/igt@kms_big_fb@yf-tiled-addfb-size-overflow.html

  * igt@kms_ccs@pipe-a-ccs-on-another-bo-y_tiled_gen12_mc_ccs:
    - shard-apl:          NOTRUN -> [SKIP][44] ([fdo#109271] / [i915#3886]) +12 similar issues
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-apl1/igt@kms_ccs@pipe-a-ccs-on-another-bo-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-c-ccs-on-another-bo-y_tiled_gen12_mc_ccs:
    - shard-kbl:          NOTRUN -> [SKIP][45] ([fdo#109271] / [i915#3886]) +3 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl2/igt@kms_ccs@pipe-c-ccs-on-another-bo-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-c-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc:
    - shard-iclb:         NOTRUN -> [SKIP][46] ([fdo#109278] / [i915#3886]) +2 similar issues
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@kms_ccs@pipe-c-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-c-crc-sprite-planes-basic-y_tiled_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][47] ([i915#3689]) +2 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb3/igt@kms_ccs@pipe-c-crc-sprite-planes-basic-y_tiled_ccs.html

  * igt@kms_ccs@pipe-c-missing-ccs-buffer-y_tiled_gen12_mc_ccs:
    - shard-skl:          NOTRUN -> [SKIP][48] ([fdo#109271] / [i915#3886])
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-skl3/igt@kms_ccs@pipe-c-missing-ccs-buffer-y_tiled_gen12_mc_ccs.html
    - shard-tglb:         NOTRUN -> [SKIP][49] ([i915#3689] / [i915#3886])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb8/igt@kms_ccs@pipe-c-missing-ccs-buffer-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-d-random-ccs-data-y_tiled_ccs:
    - shard-iclb:         NOTRUN -> [SKIP][50] ([fdo#109278]) +2 similar issues
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@kms_ccs@pipe-d-random-ccs-data-y_tiled_ccs.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - shard-iclb:         NOTRUN -> [SKIP][51] ([fdo#109284] / [fdo#111827])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@dp-mode-timings:
    - shard-tglb:         NOTRUN -> [SKIP][52] ([fdo#109284] / [fdo#111827]) +2 similar issues
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb3/igt@kms_chamelium@dp-mode-timings.html

  * igt@kms_chamelium@hdmi-hpd-with-enabled-mode:
    - shard-kbl:          NOTRUN -> [SKIP][53] ([fdo#109271] / [fdo#111827]) +11 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl7/igt@kms_chamelium@hdmi-hpd-with-enabled-mode.html

  * igt@kms_chamelium@vga-edid-read:
    - shard-apl:          NOTRUN -> [SKIP][54] ([fdo#109271] / [fdo#111827]) +25 similar issues
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-apl2/igt@kms_chamelium@vga-edid-read.html

  * igt@kms_color@pipe-a-ctm-0-5:
    - shard-skl:          [PASS][55] -> [DMESG-WARN][56] ([i915#1982])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl3/igt@kms_color@pipe-a-ctm-0-5.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-skl6/igt@kms_color@pipe-a-ctm-0-5.html

  * igt@kms_color_chamelium@pipe-c-ctm-red-to-blue:
    - shard-snb:          NOTRUN -> [SKIP][57] ([fdo#109271] / [fdo#111827]) +18 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-snb7/igt@kms_color_chamelium@pipe-c-ctm-red-to-blue.html

  * igt@kms_content_protection@legacy:
    - shard-kbl:          NOTRUN -> [TIMEOUT][58] ([i915#1319])
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl7/igt@kms_content_protection@legacy.html

  * igt@kms_content_protection@lic:
    - shard-apl:          NOTRUN -> [TIMEOUT][59] ([i915#1319]) +1 similar issue
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-apl1/igt@kms_content_protection@lic.html

  * igt@kms_content_protection@uevent:
    - shard-tglb:         NOTRUN -> [SKIP][60] ([fdo#111828])
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb8/igt@kms_content_protection@uevent.html
    - shard-iclb:         NOTRUN -> [SKIP][61] ([fdo#109300] / [fdo#111066])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@kms_content_protection@uevent.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          NOTRUN -> [DMESG-WARN][62] ([i915#180])
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-b-cursor-max-size-onscreen:
    - shard-tglb:         NOTRUN -> [SKIP][63] ([i915#3359]) +2 similar issues
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb8/igt@kms_cursor_crc@pipe-b-cursor-max-size-onscreen.html

  * igt@kms_cursor_crc@pipe-d-cursor-512x512-onscreen:
    - shard-tglb:         NOTRUN -> [SKIP][64] ([fdo#109279] / [i915#3359]) +1 similar issue
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb3/igt@kms_cursor_crc@pipe-d-cursor-512x512-onscreen.html

  * igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic:
    - shard-iclb:         NOTRUN -> [SKIP][65] ([fdo#109274] / [fdo#109278])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic:
    - shard-skl:          [PASS][66] -> [FAIL][67] ([i915#2346])
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl5/igt@kms_cursor_legacy@flip-vs-cursor-atomic.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-skl2/igt@kms_cursor_legacy@flip-vs-cursor-atomic.html

  * igt@kms_cursor_legacy@pipe-a-single-move:
    - shard-glk:          [PASS][68] -> [DMESG-WARN][69] ([i915#118] / [i915#95])
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-glk5/igt@kms_cursor_legacy@pipe-a-single-move.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-glk6/igt@kms_cursor_legacy@pipe-a-single-move.html

  * igt@kms_dp_tiled_display@basic-test-pattern:
    - shard-tglb:         NOTRUN -> [SKIP][70] ([i915#426])
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb3/igt@kms_dp_tiled_display@basic-test-pattern.html

  * igt@kms_flip@2x-absolute-wf_vblank-interruptible:
    - shard-iclb:         NOTRUN -> [SKIP][71] ([fdo#109274]) +1 similar issue
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@kms_flip@2x-absolute-wf_vblank-interruptible.html

  * igt@kms_flip@flip-vs-blocking-wf-vblank@a-edp1:
    - shard-skl:          [PASS][72] -> [FAIL][73] ([i915#2122]) +2 similar issues
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl4/igt@kms_flip@flip-vs-blocking-wf-vblank@a-edp1.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-skl9/igt@kms_flip@flip-vs-blocking-wf-vblank@a-edp1.html

  * igt@kms_flip@flip-vs-suspend@c-dp1:
    - shard-kbl:          [PASS][74] -> [DMESG-WARN][75] ([i915#180]) +6 similar issues
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-kbl6/igt@kms_flip@flip-vs-suspend@c-dp1.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl1/igt@kms_flip@flip-vs-suspend@c-dp1.html

  * igt@kms_flip@plain-flip-ts-check@a-hdmi-a1:
    - shard-glk:          [PASS][76] -> [FAIL][77] ([i915#2122]) +2 similar issues
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-glk4/igt@kms_flip@plain-flip-ts-check@a-hdmi-a1.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-glk8/igt@kms_flip@plain-flip-ts-check@a-hdmi-a1.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-shrfb-plflip-blt:
    - shard-glk:          [PASS][78] -> [FAIL][79] ([i915#1888] / [i915#2546])
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-glk9/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-shrfb-plflip-blt.html
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-glk1/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-shrfb-plflip-blt.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-blt:
    - shard-skl:          NOTRUN -> [SKIP][80] ([fdo#109271]) +17 similar issues
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-skl4/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-pwrite:
    - shard-tglb:         NOTRUN -> [SKIP][81] ([fdo#111825]) +13 similar issues
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb3/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-indfb-fliptrack-mmap-gtt:
    - shard-iclb:         NOTRUN -> [SKIP][82] ([fdo#109280]) +3 similar issues
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-2p-indfb-fliptrack-mmap-gtt.html

  * igt@kms_hdr@static-toggle-suspend:
    - shard-tglb:         NOTRUN -> [SKIP][83] ([i915#1187])
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb3/igt@kms_hdr@static-toggle-suspend.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - shard-apl:          NOTRUN -> [SKIP][84] ([fdo#109271] / [i915#533]) +4 similar issues
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-apl1/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-d:
    - shard-kbl:          NOTRUN -> [SKIP][85] ([fdo#109271] / [i915#533])
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl2/igt@kms_pipe_crc_basic@read-crc-pipe-d.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-apl:          NOTRUN -> [FAIL][86] ([fdo#108145] / [i915#265]) +1 similar issue
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-apl2/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html
    - shard-skl:          NOTRUN -> [FAIL][87] ([fdo#108145] / [i915#265])
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-transparent-fb:
    - shard-apl:          NOTRUN -> [FAIL][88] ([i915#265])
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-apl2/igt@kms_plane_alpha_blend@pipe-a-alpha-transparent-fb.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][89] -> [FAIL][90] ([fdo#108145] / [i915#265]) +2 similar issues
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-glk:          [PASS][91] -> [DMESG-FAIL][92] ([i915#118] / [i915#1888] / [i915#95])
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-glk9/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-glk1/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-2:
    - shard-apl:          NOTRUN -> [SKIP][93] ([fdo#109271] / [i915#658]) +6 similar issues
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-apl1/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-2.html

  * igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-1:
    - shard-kbl:          NOTRUN -> [SKIP][94] ([fdo#109271] / [i915#658]) +1 similar issue
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl2/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-1.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [PASS][95] -> [SKIP][96] ([fdo#109642] / [fdo#111068] / [i915#658])
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_cursor_mmap_gtt:
    - shard-tglb:         NOTRUN -> [FAIL][97] ([i915#132] / [i915#3467]) +1 similar issue
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb3/igt@kms_psr@psr2_cursor_mmap_gtt.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         NOTRUN -> [SKIP][98] ([fdo#109441])
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@kms_psr@psr2_primary_page_flip.html

  * igt@kms_psr@psr2_suspend:
    - shard-iclb:         [PASS][99] -> [SKIP][100] ([fdo#109441])
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-iclb2/igt@kms_psr@psr2_suspend.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb4/igt@kms_psr@psr2_suspend.html

  * igt@kms_writeback@writeback-check-output:
    - shard-iclb:         NOTRUN -> [SKIP][101] ([i915#2437])
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@kms_writeback@writeback-check-output.html
    - shard-skl:          NOTRUN -> [SKIP][102] ([fdo#109271] / [i915#2437])
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-skl3/igt@kms_writeback@writeback-check-output.html
    - shard-tglb:         NOTRUN -> [SKIP][103] ([i915#2437])
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb8/igt@kms_writeback@writeback-check-output.html

  * igt@kms_writeback@writeback-fb-id:
    - shard-kbl:          NOTRUN -> [SKIP][104] ([fdo#109271] / [i915#2437])
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl2/igt@kms_writeback@writeback-fb-id.html

  * igt@kms_writeback@writeback-invalid-parameters:
    - shard-apl:          NOTRUN -> [SKIP][105] ([fdo#109271] / [i915#2437])
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-apl1/igt@kms_writeback@writeback-invalid-parameters.html

  * igt@perf@mi-rpc:
    - shard-tglb:         NOTRUN -> [SKIP][106] ([fdo#109289])
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb8/igt@perf@mi-rpc.html

  * igt@prime_nv_test@nv_write_i915_gtt_mmap_read:
    - shard-tglb:         NOTRUN -> [SKIP][107] ([fdo#109291])
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb8/igt@prime_nv_test@nv_write_i915_gtt_mmap_read.html
    - shard-iclb:         NOTRUN -> [SKIP][108] ([fdo#109291])
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@prime_nv_test@nv_write_i915_gtt_mmap_read.html

  * igt@runner@aborted:
    - shard-snb:          NOTRUN -> ([FAIL][109], [FAIL][110]) ([i915#3002])
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-snb7/igt@runner@aborted.html
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-snb7/igt@runner@aborted.html

  * igt@sysfs_clients@fair-7:
    - shard-iclb:         NOTRUN -> [SKIP][111] ([i915#2994])
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb1/igt@sysfs_clients@fair-7.html
    - shard-tglb:         NOTRUN -> [SKIP][112] ([i915#2994]) +1 similar issue
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb8/igt@sysfs_clients@fair-7.html

  * igt@sysfs_clients@recycle-many:
    - shard-apl:          NOTRUN -> [SKIP][113] ([fdo#109271] / [i915#2994]) +1 similar issue
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-apl1/igt@sysfs_clients@recycle-many.html

  * igt@sysfs_clients@split-50:
    - shard-kbl:          NOTRUN -> [SKIP][114] ([fdo#109271] / [i915#2994]) +2 similar issues
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl7/igt@sysfs_clients@split-50.html

  
#### Possible fixes ####

  * igt@gem_exec_fair@basic-none@rcs0:
    - shard-glk:          [FAIL][115] ([i915#2842]) -> [PASS][116]
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-glk8/igt@gem_exec_fair@basic-none@rcs0.html
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-glk5/igt@gem_exec_fair@basic-none@rcs0.html

  * igt@gem_exec_fair@basic-none@vcs0:
    - shard-kbl:          [FAIL][117] ([i915#2842]) -> [PASS][118] +1 similar issue
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-kbl7/igt@gem_exec_fair@basic-none@vcs0.html
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl3/igt@gem_exec_fair@basic-none@vcs0.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
    - shard-iclb:         [FAIL][119] ([i915#2849]) -> [PASS][120]
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-iclb4/igt@gem_exec_fair@basic-throttle@rcs0.html
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-iclb5/igt@gem_exec_fair@basic-throttle@rcs0.html

  * igt@gem_exec_flush@basic-uc-prw-default:
    - shard-tglb:         [INCOMPLETE][121] -> [PASS][122]
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-tglb6/igt@gem_exec_flush@basic-uc-prw-default.html
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-tglb3/igt@gem_exec_flush@basic-uc-prw-default.html

  * igt@gem_pipe_control_store_loop@fresh-buffer:
    - shard-glk:          [DMESG-WARN][123] ([i915#118] / [i915#95]) -> [PASS][124]
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-glk8/igt@gem_pipe_control_store_loop@fresh-buffer.html
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-glk9/igt@gem_pipe_control_store_loop@fresh-buffer.html

  * igt@i915_pm_rpm@system-suspend:
    - shard-kbl:          [INCOMPLETE][125] ([i915#151] / [i915#155]) -> [PASS][126]
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-kbl4/igt@i915_pm_rpm@system-suspend.html
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl7/igt@i915_pm_rpm@system-suspend.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [DMESG-WARN][127] ([i915#180]) -> [PASS][128] +4 similar issues
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-apl2/igt@i915_suspend@fence-restore-tiled2untiled.html
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-apl1/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@i915_suspend@forcewake:
    - shard-skl:          [INCOMPLETE][129] ([i915#636]) -> [PASS][130]
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl1/igt@i915_suspend@forcewake.html
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-skl4/igt@i915_suspend@forcewake.html

  * igt@kms_flip@flip-vs-expired-vblank@a-edp1:
    - shard-skl:          [FAIL][131] ([i915#79]) -> [PASS][132]
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl4/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-skl9/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@c-dp1:
    - shard-kbl:          [INCOMPLETE][133] ([i915#636]) -> [PASS][134]
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-kbl2/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl2/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render:
    - shard-skl:          [DMESG-WARN][135] ([i915#1982]) -> [PASS][136] +1 similar issue
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl10/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render.html
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-skl10/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [FAIL][137] ([i915#1188]) -> [PASS][138] +1 similar issue
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl2/igt@kms_hdr@bpc-switch-dpms.html
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-skl8/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes:
    - shard-kbl:          [DMESG-WARN][139] ([i915#180]) -> [PASS][140] +3 similar issues
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-kbl1/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes.html
   [140]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/shard-kbl4/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes.html

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         [SKIP][141] ([fdo#1094

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20821/index.html

[-- Attachment #2: Type: text/html, Size: 33577 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Intel-gfx] [PATCH] drm/i915: Release i915_gem_context from a worker
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (13 preceding siblings ...)
  2021-08-14  1:26 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
@ 2021-08-14 10:43 ` Daniel Vetter
  2021-08-31  9:38   ` Maarten Lankhorst
  2021-08-14 10:55 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with drm/i915: Release i915_gem_context from a worker (rev2) Patchwork
                   ` (6 subsequent siblings)
  21 siblings, 1 reply; 33+ messages in thread
From: Daniel Vetter @ 2021-08-14 10:43 UTC (permalink / raw)
  To: Intel Graphics Development
  Cc: DRI Development, Daniel Vetter, Daniel Vetter, Jon Bloomfield,
	Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

The only reason for this really is the i915_gem_engines->fence
callback engines_notify(), which exists purely as a fairly funky
reference counting scheme for that. Otherwise all other callers are
from process context, and generally fairly benign locking context.

Unfortunately untangling that requires some major surgery, and we have
a few i915_gem_context reference counting bugs that need fixing, and
they blow in the current hardirq calling context, so we need a
stop-gap measure.

Put a FIXME comment in when this should be removable again.

v2: Fix mock_context(), noticed by intel-gfx-ci.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jason Ekstrand <jason@jlekstrand.net>
---
 drivers/gpu/drm/i915/gem/i915_gem_context.c       | 13 +++++++++++--
 drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 12 ++++++++++++
 drivers/gpu/drm/i915/gem/selftests/mock_context.c |  1 +
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index fd169cf2f75a..051bc357ff65 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -986,9 +986,10 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx,
 	return err;
 }
 
-void i915_gem_context_release(struct kref *ref)
+static void i915_gem_context_release_work(struct work_struct *work)
 {
-	struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
+	struct i915_gem_context *ctx = container_of(work, typeof(*ctx),
+						    release_work);
 
 	trace_i915_context_free(ctx);
 	GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
@@ -1002,6 +1003,13 @@ void i915_gem_context_release(struct kref *ref)
 	kfree_rcu(ctx, rcu);
 }
 
+void i915_gem_context_release(struct kref *ref)
+{
+	struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
+
+	queue_work(ctx->i915->wq, &ctx->release_work);
+}
+
 static inline struct i915_gem_engines *
 __context_engines_static(const struct i915_gem_context *ctx)
 {
@@ -1303,6 +1311,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
 	ctx->sched = pc->sched;
 	mutex_init(&ctx->mutex);
 	INIT_LIST_HEAD(&ctx->link);
+	INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
 
 	spin_lock_init(&ctx->stale.lock);
 	INIT_LIST_HEAD(&ctx->stale.engines);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
index 94c03a97cb77..0c38789bd4a8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
@@ -288,6 +288,18 @@ struct i915_gem_context {
 	 */
 	struct kref ref;
 
+	/**
+	 * @release_work:
+	 *
+	 * Work item for deferred cleanup, since i915_gem_context_put() tends to
+	 * be called from hardirq context.
+	 *
+	 * FIXME: The only real reason for this is &i915_gem_engines.fence, all
+	 * other callers are from process context and need at most some mild
+	 * shuffling to pull the i915_gem_context_put() call out of a spinlock.
+	 */
+	struct work_struct release_work;
+
 	/**
 	 * @rcu: rcu_head for deferred freeing.
 	 */
diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
index fee070df1c97..067d68a6fe4c 100644
--- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
+++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
@@ -23,6 +23,7 @@ mock_context(struct drm_i915_private *i915,
 	kref_init(&ctx->ref);
 	INIT_LIST_HEAD(&ctx->link);
 	ctx->i915 = i915;
+	INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
 
 	mutex_init(&ctx->mutex);
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with drm/i915: Release i915_gem_context from a worker (rev2)
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (14 preceding siblings ...)
  2021-08-14 10:43 ` [Intel-gfx] [PATCH] " Daniel Vetter
@ 2021-08-14 10:55 ` Patchwork
  2021-08-14 10:56 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
                   ` (5 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Patchwork @ 2021-08-14 10:55 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: series starting with drm/i915: Release i915_gem_context from a worker (rev2)
URL   : https://patchwork.freedesktop.org/series/93693/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
9d0fdbfd766c drm/i915: Release i915_gem_context from a worker
-:108: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 57 lines checked
ad55490a42e9 drm/i915: Release ctx->syncobj on final put, not on ctx close
-:17: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 00dae4d3d35d ("drm/i915: Implement SINGLE_TIMELINE with a syncobj (v4)")'
#17: 
commit 00dae4d3d35d4f526929633b76e00b0ab4d3970d

-:66: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 1 errors, 1 warnings, 0 checks, 18 lines checked
f2eb67b88f1a drm/i915: Keep gem ctx->vm alive until the final put
-:11: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit b81dde719439 ("drm/i915: Allow userspace to clone contexts on creation")'
#11: 
    commit b81dde719439c8f09bb61e742ed95bfc4b33946b

-:19: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 27dbae8f36c1 ("drm/i915/gem: Safely acquire the ctx->vm when copying")'
#19: 
    commit 27dbae8f36c1c25008b7885fc07c57054b7dfba3

-:42: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 2850748ef876 ("drm/i915: Pull i915_vma_pin under the vm->mutex")'
#42: 
commit 2850748ef8763ab46958e43a4d1c445f29eeb37d

-:55: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit b32fa8111563 ("drm/i915/gtt: Defer address space cleanup to an RCU worker")'
#55: 
    commit b32fa811156328aea5a3c2ff05cc096490382456

-:126: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 4 errors, 1 warnings, 0 checks, 33 lines checked
7726f2246bfb drm/i915: Drop code to handle set-vm races from execbuf
-:17: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#17: 
References: ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running contexts (v4)")

-:17: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running contexts (v4)")'
#17: 
References: ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running contexts (v4)")

-:46: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 1 errors, 2 warnings, 0 checks, 12 lines checked
4aa6a88a50ab drm/i915: Rename i915_gem_context_get_vm_rcu to i915_gem_context_get_eb_vm
-:148: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 80 lines checked
9b1dc2d805d6 drm/i915: Use i915_gem_context_get_eb_vm in ctx_getparam
-:54: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 23 lines checked
e40cf3c567e6 drm/i915: Add i915_gem_context_is_full_ppgtt
-:105: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 0 errors, 1 warnings, 0 checks, 53 lines checked
0286c52c3f12 drm/i915: Use i915_gem_context_get_eb_vm in intel_context_set_gem
-:12: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running contexts (v4)")'
#12: 
commit ccbc1b97948ab671335e950271e39766729736c3

-:61: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 1 errors, 1 warnings, 0 checks, 18 lines checked
d5d3e981bd04 drm/i915: Drop __rcu from gem_context->vm
-:11: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running contexts (v4)")'
#11: 
    commit ccbc1b97948ab671335e950271e39766729736c3

-:23: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#23: 
  i915_vm_open ofc. This also removes the final caller of context_get_vm_rcu

-:42: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit a4e7ccdac38e ("drm/i915: Move context management under GEM")'
#42: 
commit a4e7ccdac38ec8335d9e4e2656c1a041c77feae1

-:357: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 2 errors, 2 warnings, 0 checks, 240 lines checked
dcd0c89767ee drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups
-:15: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit aabbe344dc3c ("drm/i915: Use RCU for unlocked vm_idr lookup")'
#15: 
commit aabbe344dc3ca5f7d8263a02608ba6179e8a4499

-:52: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 1 errors, 1 warnings, 0 checks, 13 lines checked
ce80c060cbb0 drm/i915: Stop rcu support for i915_address_space
-:11: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#11: 
- i915_dpt has very simple lifetime (somehow we create a display pagetable vm

-:27: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit cf977e18610e ("drm/i915/gem: Spring clean debugfs")'
#27: 
	commit cf977e18610e66e48c31619e7e0cfa871be9eada

-:35: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit db80a1294c23 ("drm/i915/gem: Remove per-client stats from debugfs/i915_gem_objects")'
#35: 
	commit db80a1294c231b6ac725085f046bb2931e00c9db

-:47: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit ccbc1b97948a ("drm/i915/gem: Don't allow changing the VM on running contexts (v4)")'
#47: 
	commit ccbc1b97948ab671335e950271e39766729736c3

-:59: WARNING:TYPO_SPELLING: 'Preceeding' may be misspelled - perhaps 'Preceding'?
#59: 
  Preceeding patches removed all vestiges of rcu use from gem_ctx->vm
  ^^^^^^^^^^

-:64: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit a4e7ccdac38e ("drm/i915: Move context management under GEM")'
#64: 
	commit a4e7ccdac38ec8335d9e4e2656c1a041c77feae1

-:88: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit c744d50363b7 ("drm/i915/gt: Split the breadcrumb spinlock between global and contexts")'
#88: 
	commit c744d50363b714783bbc88d986cc16def13710f7

-:94: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit added5fce61e ("ARM: mxs_defconfig: add CONFIG_USB_PHY")'
#94: 
  its parent commit added the intel_context rcu protection:

-:96: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 14d1eaf08845 ("drm/i915/gt: Protect context lifetime with RCU")'
#96: 
	commit 14d1eaf08845c534963c83f754afe0cb14cb2512

-:114: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 4d8151ae5329 ("drm/i915: Don't free shared locks while shared")'
#114: 
	commit 4d8151ae5329cf50781a02fd2298a909589a5bab

-:130: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit e6ba76480299 ("drm/i915: Remove i915->kernel_context")'
#130: 
	commit e6ba76480299a0d77c51d846f7467b1673aad25b

-:143: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 4ff4b44cbb70 ("drm/i915: Store a direct lookup from object handle to vma")'
#143: 
	commit 4ff4b44cbb70c269259958cbcc48d7b8a2cb9ec8

-:152: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit d1b48c1e7184 ("drm/i915: Replace execbuf vma ht with an idr")'
#152: 
	commit d1b48c1e7184d9bc4ae6d7f9fe2eed9efed11ffc

-:160: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 93159e12353c ("drm/i915/gem: Avoid gem_context->mutex for simple vma lookup")'
#160: 
	commit 93159e12353c2a47e5576d642845a91fa00530bf

-:183: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit b32fa8111563 ("drm/i915/gtt: Defer address space cleanup to an RCU worker")'
#183: 
commit b32fa811156328aea5a3c2ff05cc096490382456

-:201: WARNING:TYPO_SPELLING: 'wont' may be misspelled - perhaps 'won't'?
#201: 
that's a preexisting condition in the codeebase that we wont fix in
                                                        ^^^^

-:206: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 60a4233a4952 ("drm/i915: Flush the i915_vm_release before ggtt shutdown")'
#206: 
commit 60a4233a4952729089e4df152e730f8f4d0e82ce

-:279: WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Daniel Vetter <daniel.vetter@ffwll.ch>' != 'Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>'

total: 14 errors, 4 warnings, 0 checks, 39 lines checked



^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with drm/i915: Release i915_gem_context from a worker (rev2)
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (15 preceding siblings ...)
  2021-08-14 10:55 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with drm/i915: Release i915_gem_context from a worker (rev2) Patchwork
@ 2021-08-14 10:56 ` Patchwork
  2021-08-14 11:19 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Patchwork @ 2021-08-14 10:56 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: series starting with drm/i915: Release i915_gem_context from a worker (rev2)
URL   : https://patchwork.freedesktop.org/series/93693/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-drivers/gpu/drm/i915/gem/i915_gem_context.c:1349:34:    expected struct i915_address_space *vm
-drivers/gpu/drm/i915/gem/i915_gem_context.c:1349:34:    got struct i915_address_space [noderef] __rcu *vm
-drivers/gpu/drm/i915/gem/i915_gem_context.c:1349:34: warning: incorrect type in argument 1 (different address spaces)
-drivers/gpu/drm/i915/gem/selftests/mock_context.c:44:25:    expected struct i915_address_space [noderef] __rcu *vm
-drivers/gpu/drm/i915/gem/selftests/mock_context.c:44:25:    got struct i915_address_space *
-drivers/gpu/drm/i915/gem/selftests/mock_context.c:44:25: warning: incorrect type in assignment (different address spaces)
-drivers/gpu/drm/i915/gem/selftests/mock_context.c:61:34:    expected struct i915_address_space *vm
-drivers/gpu/drm/i915/gem/selftests/mock_context.c:61:34:    got struct i915_address_space [noderef] __rcu *vm
-drivers/gpu/drm/i915/gem/selftests/mock_context.c:61:34: warning: incorrect type in argument 1 (different address spaces)



^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with drm/i915: Release i915_gem_context from a worker (rev2)
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (16 preceding siblings ...)
  2021-08-14 10:56 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2021-08-14 11:19 ` Patchwork
  2021-08-14 12:38 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Patchwork @ 2021-08-14 11:19 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 3655 bytes --]

== Series Details ==

Series: series starting with drm/i915: Release i915_gem_context from a worker (rev2)
URL   : https://patchwork.freedesktop.org/series/93693/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10484 -> Patchwork_20822
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/index.html

Known issues
------------

  Here are the changes found in Patchwork_20822 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-gfx:
    - fi-rkl-guc:         NOTRUN -> [SKIP][1] ([fdo#109315]) +17 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/fi-rkl-guc/igt@amdgpu/amd_basic@cs-gfx.html

  * igt@i915_selftest@live@gt_lrc:
    - fi-rkl-guc:         NOTRUN -> [DMESG-WARN][2] ([i915#3958])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/fi-rkl-guc/igt@i915_selftest@live@gt_lrc.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload:
    - {fi-tgl-dsi}:       [DMESG-WARN][3] ([i915#1982] / [k.org#205379]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/fi-tgl-dsi/igt@i915_module_load@reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/fi-tgl-dsi/igt@i915_module_load@reload.html

  * igt@i915_selftest@live@workarounds:
    - fi-rkl-guc:         [DMESG-FAIL][5] ([i915#3928]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/fi-rkl-guc/igt@i915_selftest@live@workarounds.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/fi-rkl-guc/igt@i915_selftest@live@workarounds.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#3928]: https://gitlab.freedesktop.org/drm/intel/issues/3928
  [i915#3958]: https://gitlab.freedesktop.org/drm/intel/issues/3958
  [k.org#205379]: https://bugzilla.kernel.org/show_bug.cgi?id=205379


Participating hosts (37 -> 33)
------------------------------

  Missing    (4): fi-kbl-soraka fi-bdw-samus fi-bsw-cyan bat-jsl-1 


Build changes
-------------

  * Linux: CI_DRM_10484 -> Patchwork_20822

  CI-20190529: 20190529
  CI_DRM_10484: 7de02d5cb1f35bd3f068237444063844dea47ddc @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6175: c91f99c74b966f635d7e2eb898bf0f78383d281b @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_20822: ce80c060cbb0fdbaf11f5c9e54e70048600fdee6 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

ce80c060cbb0 drm/i915: Stop rcu support for i915_address_space
dcd0c89767ee drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups
d5d3e981bd04 drm/i915: Drop __rcu from gem_context->vm
0286c52c3f12 drm/i915: Use i915_gem_context_get_eb_vm in intel_context_set_gem
e40cf3c567e6 drm/i915: Add i915_gem_context_is_full_ppgtt
9b1dc2d805d6 drm/i915: Use i915_gem_context_get_eb_vm in ctx_getparam
4aa6a88a50ab drm/i915: Rename i915_gem_context_get_vm_rcu to i915_gem_context_get_eb_vm
7726f2246bfb drm/i915: Drop code to handle set-vm races from execbuf
f2eb67b88f1a drm/i915: Keep gem ctx->vm alive until the final put
ad55490a42e9 drm/i915: Release ctx->syncobj on final put, not on ctx close
9d0fdbfd766c drm/i915: Release i915_gem_context from a worker

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/index.html

[-- Attachment #2: Type: text/html, Size: 4422 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Intel-gfx] ✓ Fi.CI.IGT: success for series starting with drm/i915: Release i915_gem_context from a worker (rev2)
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (17 preceding siblings ...)
  2021-08-14 11:19 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2021-08-14 12:38 ` Patchwork
  2021-08-31 10:16 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with drm/i915: Release i915_gem_context from a worker (rev3) Patchwork
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 33+ messages in thread
From: Patchwork @ 2021-08-14 12:38 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 30299 bytes --]

== Series Details ==

Series: series starting with drm/i915: Release i915_gem_context from a worker (rev2)
URL   : https://patchwork.freedesktop.org/series/93693/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10484_full -> Patchwork_20822_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_20822_full:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@kms_busy@basic-hang:
    - {shard-rkl}:        NOTRUN -> [SKIP][1] +5 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-rkl-1/igt@kms_busy@basic-hang.html

  
Known issues
------------

  Here are the changes found in Patchwork_20822_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_create@create-massive:
    - shard-snb:          NOTRUN -> [DMESG-WARN][2] ([i915#3002]) +1 similar issue
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-snb6/igt@gem_create@create-massive.html

  * igt@gem_ctx_persistence@engines-mixed:
    - shard-snb:          NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#1099]) +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-snb6/igt@gem_ctx_persistence@engines-mixed.html

  * igt@gem_ctx_persistence@many-contexts:
    - shard-tglb:         [PASS][4] -> [FAIL][5] ([i915#2410])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-tglb1/igt@gem_ctx_persistence@many-contexts.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb7/igt@gem_ctx_persistence@many-contexts.html

  * igt@gem_eio@in-flight-suspend:
    - shard-skl:          [PASS][6] -> [INCOMPLETE][7] ([i915#198]) +1 similar issue
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl4/igt@gem_eio@in-flight-suspend.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl9/igt@gem_eio@in-flight-suspend.html

  * igt@gem_exec_fair@basic-flow@rcs0:
    - shard-tglb:         [PASS][8] -> [FAIL][9] ([i915#2842]) +2 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-tglb3/igt@gem_exec_fair@basic-flow@rcs0.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb3/igt@gem_exec_fair@basic-flow@rcs0.html

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - shard-iclb:         [PASS][10] -> [FAIL][11] ([i915#2842])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-iclb7/igt@gem_exec_fair@basic-none-share@rcs0.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb6/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_exec_fair@basic-none-solo@rcs0:
    - shard-glk:          [PASS][12] -> [FAIL][13] ([i915#2842]) +1 similar issue
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-glk7/igt@gem_exec_fair@basic-none-solo@rcs0.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-glk1/igt@gem_exec_fair@basic-none-solo@rcs0.html

  * igt@gem_exec_fair@basic-pace@rcs0:
    - shard-kbl:          [PASS][14] -> [SKIP][15] ([fdo#109271])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-kbl1/igt@gem_exec_fair@basic-pace@rcs0.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl4/igt@gem_exec_fair@basic-pace@rcs0.html

  * igt@gem_exec_fair@basic-pace@vcs1:
    - shard-iclb:         NOTRUN -> [FAIL][16] ([i915#2842]) +1 similar issue
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb2/igt@gem_exec_fair@basic-pace@vcs1.html

  * igt@gem_exec_fair@basic-pace@vecs0:
    - shard-kbl:          [PASS][17] -> [FAIL][18] ([i915#2842]) +4 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-kbl1/igt@gem_exec_fair@basic-pace@vecs0.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl4/igt@gem_exec_fair@basic-pace@vecs0.html

  * igt@gem_huc_copy@huc-copy:
    - shard-apl:          NOTRUN -> [SKIP][19] ([fdo#109271] / [i915#2190])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-apl2/igt@gem_huc_copy@huc-copy.html

  * igt@gem_mmap_gtt@basic-small-bo:
    - shard-glk:          [PASS][20] -> [FAIL][21] ([i915#1888])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-glk6/igt@gem_mmap_gtt@basic-small-bo.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-glk3/igt@gem_mmap_gtt@basic-small-bo.html

  * igt@gem_mmap_gtt@cpuset-big-copy-xy:
    - shard-iclb:         [PASS][22] -> [FAIL][23] ([i915#307]) +1 similar issue
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-iclb2/igt@gem_mmap_gtt@cpuset-big-copy-xy.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb7/igt@gem_mmap_gtt@cpuset-big-copy-xy.html

  * igt@gem_pread@exhaustion:
    - shard-kbl:          NOTRUN -> [WARN][24] ([i915#2658])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl6/igt@gem_pread@exhaustion.html

  * igt@gem_pwrite@basic-exhaustion:
    - shard-apl:          NOTRUN -> [WARN][25] ([i915#2658])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-apl3/igt@gem_pwrite@basic-exhaustion.html

  * igt@gem_render_copy@x-tiled-to-vebox-yf-tiled:
    - shard-kbl:          NOTRUN -> [SKIP][26] ([fdo#109271]) +132 similar issues
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl3/igt@gem_render_copy@x-tiled-to-vebox-yf-tiled.html

  * igt@gem_render_copy@y-tiled-mc-ccs-to-yf-tiled-ccs:
    - shard-iclb:         NOTRUN -> [SKIP][27] ([i915#768])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb2/igt@gem_render_copy@y-tiled-mc-ccs-to-yf-tiled-ccs.html

  * igt@gem_userptr_blits@input-checking:
    - shard-kbl:          NOTRUN -> [DMESG-WARN][28] ([i915#3002])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl3/igt@gem_userptr_blits@input-checking.html

  * igt@gen9_exec_parse@batch-invalid-length:
    - shard-snb:          NOTRUN -> [SKIP][29] ([fdo#109271]) +264 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-snb6/igt@gen9_exec_parse@batch-invalid-length.html

  * igt@gen9_exec_parse@cmd-crossing-page:
    - shard-tglb:         NOTRUN -> [SKIP][30] ([i915#2856]) +1 similar issue
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb7/igt@gen9_exec_parse@cmd-crossing-page.html

  * igt@i915_pm_dc@dc6-dpms:
    - shard-tglb:         NOTRUN -> [FAIL][31] ([i915#454])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb7/igt@i915_pm_dc@dc6-dpms.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-iclb:         [PASS][32] -> [FAIL][33] ([i915#454])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-iclb2/igt@i915_pm_dc@dc6-psr.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb2/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait:
    - shard-tglb:         NOTRUN -> [SKIP][34] ([fdo#111644] / [i915#1397] / [i915#2411])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb7/igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait.html

  * igt@i915_query@query-topology-known-pci-ids:
    - shard-tglb:         NOTRUN -> [SKIP][35] ([fdo#109303])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb1/igt@i915_query@query-topology-known-pci-ids.html
    - shard-iclb:         NOTRUN -> [SKIP][36] ([fdo#109303])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb2/igt@i915_query@query-topology-known-pci-ids.html

  * igt@kms_big_fb@linear-8bpp-rotate-90:
    - shard-tglb:         NOTRUN -> [SKIP][37] ([fdo#111614])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb7/igt@kms_big_fb@linear-8bpp-rotate-90.html

  * igt@kms_big_fb@x-tiled-32bpp-rotate-180:
    - shard-glk:          [PASS][38] -> [DMESG-WARN][39] ([i915#118] / [i915#95])
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-glk3/igt@kms_big_fb@x-tiled-32bpp-rotate-180.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-glk9/igt@kms_big_fb@x-tiled-32bpp-rotate-180.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip:
    - shard-kbl:          NOTRUN -> [SKIP][40] ([fdo#109271] / [i915#3777]) +1 similar issue
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl3/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip.html

  * igt@kms_big_fb@yf-tiled-addfb-size-overflow:
    - shard-tglb:         NOTRUN -> [SKIP][41] ([fdo#111615]) +4 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb1/igt@kms_big_fb@yf-tiled-addfb-size-overflow.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0:
    - shard-apl:          NOTRUN -> [SKIP][42] ([fdo#109271]) +239 similar issues
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-apl3/igt@kms_big_fb@yf-tiled-max-hw-stride-64bpp-rotate-0.html

  * igt@kms_ccs@pipe-a-ccs-on-another-bo-y_tiled_gen12_mc_ccs:
    - shard-apl:          NOTRUN -> [SKIP][43] ([fdo#109271] / [i915#3886]) +11 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-apl1/igt@kms_ccs@pipe-a-ccs-on-another-bo-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-c-ccs-on-another-bo-y_tiled_gen12_mc_ccs:
    - shard-kbl:          NOTRUN -> [SKIP][44] ([fdo#109271] / [i915#3886]) +4 similar issues
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl3/igt@kms_ccs@pipe-c-ccs-on-another-bo-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-c-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc:
    - shard-iclb:         NOTRUN -> [SKIP][45] ([fdo#109278] / [i915#3886]) +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb2/igt@kms_ccs@pipe-c-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-c-crc-sprite-planes-basic-y_tiled_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][46] ([i915#3689]) +2 similar issues
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb7/igt@kms_ccs@pipe-c-crc-sprite-planes-basic-y_tiled_ccs.html

  * igt@kms_ccs@pipe-c-missing-ccs-buffer-y_tiled_gen12_mc_ccs:
    - shard-skl:          NOTRUN -> [SKIP][47] ([fdo#109271] / [i915#3886])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl8/igt@kms_ccs@pipe-c-missing-ccs-buffer-y_tiled_gen12_mc_ccs.html
    - shard-tglb:         NOTRUN -> [SKIP][48] ([i915#3689] / [i915#3886])
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb1/igt@kms_ccs@pipe-c-missing-ccs-buffer-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-d-random-ccs-data-y_tiled_ccs:
    - shard-iclb:         NOTRUN -> [SKIP][49] ([fdo#109278]) +2 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb2/igt@kms_ccs@pipe-d-random-ccs-data-y_tiled_ccs.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - shard-iclb:         NOTRUN -> [SKIP][50] ([fdo#109284] / [fdo#111827])
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb2/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@dp-mode-timings:
    - shard-apl:          NOTRUN -> [SKIP][51] ([fdo#109271] / [fdo#111827]) +20 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-apl2/igt@kms_chamelium@dp-mode-timings.html
    - shard-tglb:         NOTRUN -> [SKIP][52] ([fdo#109284] / [fdo#111827]) +2 similar issues
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb7/igt@kms_chamelium@dp-mode-timings.html

  * igt@kms_chamelium@hdmi-hpd-with-enabled-mode:
    - shard-kbl:          NOTRUN -> [SKIP][53] ([fdo#109271] / [fdo#111827]) +12 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl7/igt@kms_chamelium@hdmi-hpd-with-enabled-mode.html

  * igt@kms_color_chamelium@pipe-c-ctm-red-to-blue:
    - shard-snb:          NOTRUN -> [SKIP][54] ([fdo#109271] / [fdo#111827]) +10 similar issues
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-snb6/igt@kms_color_chamelium@pipe-c-ctm-red-to-blue.html

  * igt@kms_content_protection@legacy:
    - shard-kbl:          NOTRUN -> [TIMEOUT][55] ([i915#1319])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl7/igt@kms_content_protection@legacy.html

  * igt@kms_content_protection@uevent:
    - shard-tglb:         NOTRUN -> [SKIP][56] ([fdo#111828])
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb1/igt@kms_content_protection@uevent.html
    - shard-iclb:         NOTRUN -> [SKIP][57] ([fdo#109300] / [fdo#111066])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb2/igt@kms_content_protection@uevent.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-skl:          [PASS][58] -> [INCOMPLETE][59] ([i915#146] / [i915#2828] / [i915#300])
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl4/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl8/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-b-cursor-max-size-onscreen:
    - shard-tglb:         NOTRUN -> [SKIP][60] ([i915#3359]) +2 similar issues
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb1/igt@kms_cursor_crc@pipe-b-cursor-max-size-onscreen.html

  * igt@kms_cursor_crc@pipe-d-cursor-512x512-onscreen:
    - shard-tglb:         NOTRUN -> [SKIP][61] ([fdo#109279] / [i915#3359]) +1 similar issue
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb7/igt@kms_cursor_crc@pipe-d-cursor-512x512-onscreen.html

  * igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic:
    - shard-iclb:         NOTRUN -> [SKIP][62] ([fdo#109274] / [fdo#109278])
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb2/igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-skl:          [PASS][63] -> [FAIL][64] ([i915#2346])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl9/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_dp_tiled_display@basic-test-pattern:
    - shard-tglb:         NOTRUN -> [SKIP][65] ([i915#426])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb7/igt@kms_dp_tiled_display@basic-test-pattern.html

  * igt@kms_flip@2x-absolute-wf_vblank-interruptible:
    - shard-iclb:         NOTRUN -> [SKIP][66] ([fdo#109274]) +1 similar issue
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb2/igt@kms_flip@2x-absolute-wf_vblank-interruptible.html

  * igt@kms_flip@2x-flip-vs-expired-vblank@ab-hdmi-a1-hdmi-a2:
    - shard-glk:          [PASS][67] -> [FAIL][68] ([i915#2122]) +1 similar issue
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-glk3/igt@kms_flip@2x-flip-vs-expired-vblank@ab-hdmi-a1-hdmi-a2.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-glk9/igt@kms_flip@2x-flip-vs-expired-vblank@ab-hdmi-a1-hdmi-a2.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1:
    - shard-skl:          [PASS][69] -> [FAIL][70] ([i915#79])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl2/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl6/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@b-edp1:
    - shard-skl:          [PASS][71] -> [INCOMPLETE][72] ([i915#146] / [i915#198])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl8/igt@kms_flip@flip-vs-suspend-interruptible@b-edp1.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl9/igt@kms_flip@flip-vs-suspend-interruptible@b-edp1.html

  * igt@kms_flip@flip-vs-suspend@c-dp1:
    - shard-apl:          [PASS][73] -> [DMESG-WARN][74] ([i915#180]) +1 similar issue
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-apl7/igt@kms_flip@flip-vs-suspend@c-dp1.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-apl2/igt@kms_flip@flip-vs-suspend@c-dp1.html

  * igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1:
    - shard-skl:          [PASS][75] -> [FAIL][76] ([i915#2122]) +2 similar issues
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl3/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl1/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs:
    - shard-apl:          NOTRUN -> [SKIP][77] ([fdo#109271] / [i915#2672])
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-apl2/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-blt:
    - shard-skl:          NOTRUN -> [SKIP][78] ([fdo#109271]) +15 similar issues
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl3/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-pwrite:
    - shard-tglb:         NOTRUN -> [SKIP][79] ([fdo#111825]) +13 similar issues
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb7/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][80] -> [DMESG-WARN][81] ([i915#180]) +7 similar issues
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-indfb-fliptrack-mmap-gtt:
    - shard-iclb:         NOTRUN -> [SKIP][82] ([fdo#109280]) +3 similar issues
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-2p-indfb-fliptrack-mmap-gtt.html

  * igt@kms_hdr@static-toggle-suspend:
    - shard-tglb:         NOTRUN -> [SKIP][83] ([i915#1187])
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb7/igt@kms_hdr@static-toggle-suspend.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - shard-apl:          NOTRUN -> [SKIP][84] ([fdo#109271] / [i915#533]) +2 similar issues
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-apl1/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-d:
    - shard-kbl:          NOTRUN -> [SKIP][85] ([fdo#109271] / [i915#533])
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl3/igt@kms_pipe_crc_basic@read-crc-pipe-d.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
    - shard-kbl:          NOTRUN -> [FAIL][86] ([fdo#108145] / [i915#265])
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl7/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-apl:          NOTRUN -> [FAIL][87] ([fdo#108145] / [i915#265])
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-apl2/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html
    - shard-skl:          NOTRUN -> [FAIL][88] ([fdo#108145] / [i915#265])
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl3/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-transparent-fb:
    - shard-apl:          NOTRUN -> [FAIL][89] ([i915#265]) +1 similar issue
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-apl2/igt@kms_plane_alpha_blend@pipe-b-alpha-transparent-fb.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][90] -> [FAIL][91] ([fdo#108145] / [i915#265]) +2 similar issues
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl6/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_scaling@scaler-with-clipping-clamping@pipe-c-scaler-with-clipping-clamping:
    - shard-apl:          NOTRUN -> [SKIP][92] ([fdo#109271] / [i915#2733])
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-apl2/igt@kms_plane_scaling@scaler-with-clipping-clamping@pipe-c-scaler-with-clipping-clamping.html

  * igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-2:
    - shard-apl:          NOTRUN -> [SKIP][93] ([fdo#109271] / [i915#658]) +3 similar issues
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-apl1/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-2.html

  * igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-1:
    - shard-kbl:          NOTRUN -> [SKIP][94] ([fdo#109271] / [i915#658]) +1 similar issue
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl3/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-1.html

  * igt@kms_psr@psr2_cursor_mmap_gtt:
    - shard-tglb:         NOTRUN -> [FAIL][95] ([i915#132] / [i915#3467]) +1 similar issue
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb7/igt@kms_psr@psr2_cursor_mmap_gtt.html

  * igt@kms_writeback@writeback-check-output:
    - shard-iclb:         NOTRUN -> [SKIP][96] ([i915#2437])
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb2/igt@kms_writeback@writeback-check-output.html
    - shard-skl:          NOTRUN -> [SKIP][97] ([fdo#109271] / [i915#2437])
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl8/igt@kms_writeback@writeback-check-output.html
    - shard-tglb:         NOTRUN -> [SKIP][98] ([i915#2437])
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb1/igt@kms_writeback@writeback-check-output.html

  * igt@kms_writeback@writeback-fb-id:
    - shard-kbl:          NOTRUN -> [SKIP][99] ([fdo#109271] / [i915#2437])
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl6/igt@kms_writeback@writeback-fb-id.html

  * igt@perf@mi-rpc:
    - shard-tglb:         NOTRUN -> [SKIP][100] ([fdo#109289])
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb1/igt@perf@mi-rpc.html

  * igt@prime_nv_test@nv_write_i915_gtt_mmap_read:
    - shard-tglb:         NOTRUN -> [SKIP][101] ([fdo#109291])
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb1/igt@prime_nv_test@nv_write_i915_gtt_mmap_read.html
    - shard-iclb:         NOTRUN -> [SKIP][102] ([fdo#109291])
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb2/igt@prime_nv_test@nv_write_i915_gtt_mmap_read.html

  * igt@runner@aborted:
    - shard-snb:          NOTRUN -> ([FAIL][103], [FAIL][104]) ([i915#3002])
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-snb6/igt@runner@aborted.html
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-snb7/igt@runner@aborted.html

  * igt@sysfs_clients@fair-7:
    - shard-iclb:         NOTRUN -> [SKIP][105] ([i915#2994])
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb2/igt@sysfs_clients@fair-7.html
    - shard-tglb:         NOTRUN -> [SKIP][106] ([i915#2994]) +1 similar issue
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb1/igt@sysfs_clients@fair-7.html

  * igt@sysfs_clients@pidname:
    - shard-apl:          NOTRUN -> [SKIP][107] ([fdo#109271] / [i915#2994])
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-apl2/igt@sysfs_clients@pidname.html

  * igt@sysfs_clients@split-50:
    - shard-kbl:          NOTRUN -> [SKIP][108] ([fdo#109271] / [i915#2994]) +3 similar issues
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl7/igt@sysfs_clients@split-50.html

  
#### Possible fixes ####

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - shard-iclb:         [FAIL][109] ([i915#2842]) -> [PASS][110] +1 similar issue
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-iclb2/igt@gem_exec_fair@basic-pace-solo@rcs0.html
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-iclb7/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@gem_exec_fair@basic-pace@bcs0:
    - shard-tglb:         [FAIL][111] ([i915#2842]) -> [PASS][112]
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-tglb5/igt@gem_exec_fair@basic-pace@bcs0.html
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb2/igt@gem_exec_fair@basic-pace@bcs0.html

  * igt@gem_exec_fair@basic-pace@vcs0:
    - shard-kbl:          [FAIL][113] ([i915#2842]) -> [PASS][114]
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-kbl1/igt@gem_exec_fair@basic-pace@vcs0.html
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl4/igt@gem_exec_fair@basic-pace@vcs0.html

  * igt@gem_exec_flush@basic-uc-prw-default:
    - shard-tglb:         [INCOMPLETE][115] -> [PASS][116]
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-tglb6/igt@gem_exec_flush@basic-uc-prw-default.html
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-tglb7/igt@gem_exec_flush@basic-uc-prw-default.html

  * igt@gem_pipe_control_store_loop@fresh-buffer:
    - shard-glk:          [DMESG-WARN][117] ([i915#118] / [i915#95]) -> [PASS][118]
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-glk8/igt@gem_pipe_control_store_loop@fresh-buffer.html
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-glk7/igt@gem_pipe_control_store_loop@fresh-buffer.html

  * igt@i915_pm_rpm@system-suspend:
    - shard-kbl:          [INCOMPLETE][119] ([i915#151] / [i915#155]) -> [PASS][120]
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-kbl4/igt@i915_pm_rpm@system-suspend.html
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl7/igt@i915_pm_rpm@system-suspend.html

  * igt@i915_suspend@forcewake:
    - shard-skl:          [INCOMPLETE][121] ([i915#636]) -> [PASS][122]
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl1/igt@i915_suspend@forcewake.html
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl3/igt@i915_suspend@forcewake.html

  * igt@kms_flip@flip-vs-expired-vblank@a-edp1:
    - shard-skl:          [FAIL][123] ([i915#79]) -> [PASS][124]
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl4/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl8/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@c-dp1:
    - shard-apl:          [DMESG-WARN][125] ([i915#180]) -> [PASS][126] +4 similar issues
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-apl2/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-apl8/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html
    - shard-kbl:          [INCOMPLETE][127] ([i915#636]) -> [PASS][128]
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-kbl2/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl6/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render:
    - shard-skl:          [DMESG-WARN][129] ([i915#1982]) -> [PASS][130] +1 similar issue
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl10/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render.html
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl5/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [FAIL][131] ([i915#1188]) -> [PASS][132]
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl2/igt@kms_hdr@bpc-switch-dpms.html
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl6/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes:
    - shard-kbl:          [DMESG-WARN][133] ([i915#180]) -> [PASS][134] +3 similar issues
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-kbl1/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes.html
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-kbl3/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes.html

  * igt@perf@polling-parameterized:
    - shard-skl:          [FAIL][135] ([i915#1542]) -> [PASS][136]
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10484/shard-skl3/igt@perf@polling-parameterized.html
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/shard-skl10/igt@perf@polling-parameterized.html

  
#### Warnings ####

  * igt@kms_flip@2x-absolute-wf_vblank:
    - shard-tglb:         [SKIP][137] ([fdo#111825]) -> [SKIP][138] ([fdo#111825] / [i915#3966])
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20822/index.html

[-- Attachment #2: Type: text/html, Size: 33638 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Intel-gfx] [PATCH 10/11] drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 10/11] drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups Daniel Vetter
@ 2021-08-31  9:29   ` Maarten Lankhorst
  2021-08-31 12:14   ` [Intel-gfx] [PATCH] " Daniel Vetter
  1 sibling, 0 replies; 33+ messages in thread
From: Maarten Lankhorst @ 2021-08-31  9:29 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Jon Bloomfield,
	Chris Wilson, Joonas Lahtinen, Thomas Hellström,
	Matthew Auld, Lionel Landwerlin, Dave Airlie, Jason Ekstrand

Op 13-08-2021 om 22:30 schreef Daniel Vetter:
> We don't need the absolute speed of rcu for this. And
> i915_address_space in general dont need rcu protection anywhere else,
> after we've made gem contexts and engines a lot more immutable.
>
> Note that this semantically reverts
>
> commit aabbe344dc3ca5f7d8263a02608ba6179e8a4499
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Fri Aug 30 19:03:25 2019 +0100
>
>     drm/i915: Use RCU for unlocked vm_idr lookup
>
> except we have the conversion from idr to xarray in between.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 005b1cec7007..e37fac8fac0c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1881,11 +1881,11 @@ i915_gem_vm_lookup(struct drm_i915_file_private *file_priv, u32 id)
>  {
>  	struct i915_address_space *vm;
>  
> -	rcu_read_lock();
> +	xa_lock(&file_priv->vm_xa);
>  	vm = xa_load(&file_priv->vm_xa, id);
>  	if (vm && !kref_get_unless_zero(&vm->ref))
>  		vm = NULL;
I think this could be a plain i915_vm_get now, kref_get_unless_zero is not guarded by RCU any more.
> -	rcu_read_unlock();
> +	xa_unlock(&file_priv->vm_xa);
>  
>  	return vm;
>  }

Apart from that, all looks good.

With this fix, for patch 2-11:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Release i915_gem_context from a worker
  2021-08-14 10:43 ` [Intel-gfx] [PATCH] " Daniel Vetter
@ 2021-08-31  9:38   ` Maarten Lankhorst
  2021-08-31 12:16     ` Daniel Vetter
  0 siblings, 1 reply; 33+ messages in thread
From: Maarten Lankhorst @ 2021-08-31  9:38 UTC (permalink / raw)
  To: Daniel Vetter, Intel Graphics Development
  Cc: DRI Development, Daniel Vetter, Jon Bloomfield, Chris Wilson,
	Joonas Lahtinen, Thomas Hellström, Matthew Auld,
	Lionel Landwerlin, Dave Airlie, Jason Ekstrand

Op 14-08-2021 om 12:43 schreef Daniel Vetter:
> The only reason for this really is the i915_gem_engines->fence
> callback engines_notify(), which exists purely as a fairly funky
> reference counting scheme for that. Otherwise all other callers are
> from process context, and generally fairly benign locking context.
>
> Unfortunately untangling that requires some major surgery, and we have
> a few i915_gem_context reference counting bugs that need fixing, and
> they blow in the current hardirq calling context, so we need a
> stop-gap measure.
>
> Put a FIXME comment in when this should be removable again.
>
> v2: Fix mock_context(), noticed by intel-gfx-ci.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_context.c       | 13 +++++++++++--
>  drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 12 ++++++++++++
>  drivers/gpu/drm/i915/gem/selftests/mock_context.c |  1 +
>  3 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index fd169cf2f75a..051bc357ff65 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -986,9 +986,10 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx,
>  	return err;
>  }
>  
> -void i915_gem_context_release(struct kref *ref)
> +static void i915_gem_context_release_work(struct work_struct *work)
>  {
> -	struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
> +	struct i915_gem_context *ctx = container_of(work, typeof(*ctx),
> +						    release_work);
>  
>  	trace_i915_context_free(ctx);
>  	GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
> @@ -1002,6 +1003,13 @@ void i915_gem_context_release(struct kref *ref)
>  	kfree_rcu(ctx, rcu);
>  }
>  
> +void i915_gem_context_release(struct kref *ref)
> +{
> +	struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
> +
> +	queue_work(ctx->i915->wq, &ctx->release_work);
> +}
> +
>  static inline struct i915_gem_engines *
>  __context_engines_static(const struct i915_gem_context *ctx)
>  {
> @@ -1303,6 +1311,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
>  	ctx->sched = pc->sched;
>  	mutex_init(&ctx->mutex);
>  	INIT_LIST_HEAD(&ctx->link);
> +	INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
>  
>  	spin_lock_init(&ctx->stale.lock);
>  	INIT_LIST_HEAD(&ctx->stale.engines);
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> index 94c03a97cb77..0c38789bd4a8 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> @@ -288,6 +288,18 @@ struct i915_gem_context {
>  	 */
>  	struct kref ref;
>  
> +	/**
> +	 * @release_work:
> +	 *
> +	 * Work item for deferred cleanup, since i915_gem_context_put() tends to
> +	 * be called from hardirq context.
> +	 *
> +	 * FIXME: The only real reason for this is &i915_gem_engines.fence, all
> +	 * other callers are from process context and need at most some mild
> +	 * shuffling to pull the i915_gem_context_put() call out of a spinlock.
> +	 */
> +	struct work_struct release_work;
> +
>  	/**
>  	 * @rcu: rcu_head for deferred freeing.
>  	 */
> diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> index fee070df1c97..067d68a6fe4c 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> @@ -23,6 +23,7 @@ mock_context(struct drm_i915_private *i915,
>  	kref_init(&ctx->ref);
>  	INIT_LIST_HEAD(&ctx->link);
>  	ctx->i915 = i915;
> +	INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
>  
>  	mutex_init(&ctx->mutex);
>  

----
Is the workqueue really needed? I'm not sure you could still race in drm_syncobj_free when refcount is zero, so in that case removing locking from _release would work as well as a workqueue.

Something like below would keep the drm_sync_obj_put hardirq safe.

I assume when freeing, the  cb list is supposed to be empty, so I added a WARN_ON just to be sure, otherwise we should just tear down the list without locking too.

This should be a better alternative for patch 1.
----8<-------
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index c9a9d74f338c..9d561decd97e 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -462,7 +462,13 @@ void drm_syncobj_free(struct kref *kref)
 	struct drm_syncobj *syncobj = container_of(kref,
 						   struct drm_syncobj,
 						   refcount);
-	drm_syncobj_replace_fence(syncobj, NULL);
+	struct dma_fence *old_fence;
+
+	old_fence = rcu_dereference_protected(syncobj->fence, !kref_read(&syncobj->refcount));
+	dma_fence_put(old_fence);
+
+	WARN_ON(!list_empty(&syncobj->cb_list));
+
 	kfree(syncobj);
 }
 EXPORT_SYMBOL(drm_syncobj_free);



^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with drm/i915: Release i915_gem_context from a worker (rev3)
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (18 preceding siblings ...)
  2021-08-14 12:38 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
@ 2021-08-31 10:16 ` Patchwork
  2021-08-31 12:18 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with drm/i915: Release i915_gem_context from a worker (rev4) Patchwork
  2021-09-02 12:42 ` [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Tvrtko Ursulin
  21 siblings, 0 replies; 33+ messages in thread
From: Patchwork @ 2021-08-31 10:16 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

== Series Details ==

Series: series starting with drm/i915: Release i915_gem_context from a worker (rev3)
URL   : https://patchwork.freedesktop.org/series/93693/
State : failure

== Summary ==

Applying: drm/i915: Release i915_gem_context from a worker
Applying: drm/i915: Release ctx->syncobj on final put, not on ctx close
Applying: drm/i915: Keep gem ctx->vm alive until the final put
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/gem/i915_gem_context.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/gem/i915_gem_context.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/gem/i915_gem_context.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0003 drm/i915: Keep gem ctx->vm alive until the final put
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".



^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Intel-gfx] [PATCH] drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups
  2021-08-13 20:30 ` [Intel-gfx] [PATCH 10/11] drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups Daniel Vetter
  2021-08-31  9:29   ` Maarten Lankhorst
@ 2021-08-31 12:14   ` Daniel Vetter
  1 sibling, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2021-08-31 12:14 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Daniel Vetter,
	Jon Bloomfield, Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

We don't need the absolute speed of rcu for this. And
i915_address_space in general dont need rcu protection anywhere else,
after we've made gem contexts and engines a lot more immutable.

Note that this semantically reverts

commit aabbe344dc3ca5f7d8263a02608ba6179e8a4499
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Aug 30 19:03:25 2019 +0100

    drm/i915: Use RCU for unlocked vm_idr lookup

except we have the conversion from idr to xarray in between.

v2: kref_get_unless_zero is no longer required (Maarten)

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jason Ekstrand <jason@jlekstrand.net>
---
 drivers/gpu/drm/i915/i915_drv.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index be2392bbcecc..d89ff55d8fc8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1874,11 +1874,11 @@ i915_gem_vm_lookup(struct drm_i915_file_private *file_priv, u32 id)
 {
 	struct i915_address_space *vm;
 
-	rcu_read_lock();
+	xa_lock(&file_priv->vm_xa);
 	vm = xa_load(&file_priv->vm_xa, id);
-	if (vm && !kref_get_unless_zero(&vm->ref))
-		vm = NULL;
-	rcu_read_unlock();
+	if (vm)
+		kref_get(&vm->ref);
+	xa_unlock(&file_priv->vm_xa);
 
 	return vm;
 }
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Release i915_gem_context from a worker
  2021-08-31  9:38   ` Maarten Lankhorst
@ 2021-08-31 12:16     ` Daniel Vetter
  2021-08-31 15:14       ` Daniel Vetter
  0 siblings, 1 reply; 33+ messages in thread
From: Daniel Vetter @ 2021-08-31 12:16 UTC (permalink / raw)
  To: Maarten Lankhorst
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	Daniel Vetter, Jon Bloomfield, Chris Wilson, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

On Tue, Aug 31, 2021 at 11:38:27AM +0200, Maarten Lankhorst wrote:
> Op 14-08-2021 om 12:43 schreef Daniel Vetter:
> > The only reason for this really is the i915_gem_engines->fence
> > callback engines_notify(), which exists purely as a fairly funky
> > reference counting scheme for that. Otherwise all other callers are
> > from process context, and generally fairly benign locking context.
> >
> > Unfortunately untangling that requires some major surgery, and we have
> > a few i915_gem_context reference counting bugs that need fixing, and
> > they blow in the current hardirq calling context, so we need a
> > stop-gap measure.
> >
> > Put a FIXME comment in when this should be removable again.
> >
> > v2: Fix mock_context(), noticed by intel-gfx-ci.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Jason Ekstrand <jason@jlekstrand.net>
> > ---
> >  drivers/gpu/drm/i915/gem/i915_gem_context.c       | 13 +++++++++++--
> >  drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 12 ++++++++++++
> >  drivers/gpu/drm/i915/gem/selftests/mock_context.c |  1 +
> >  3 files changed, 24 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > index fd169cf2f75a..051bc357ff65 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > @@ -986,9 +986,10 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx,
> >  	return err;
> >  }
> >  
> > -void i915_gem_context_release(struct kref *ref)
> > +static void i915_gem_context_release_work(struct work_struct *work)
> >  {
> > -	struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
> > +	struct i915_gem_context *ctx = container_of(work, typeof(*ctx),
> > +						    release_work);
> >  
> >  	trace_i915_context_free(ctx);
> >  	GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
> > @@ -1002,6 +1003,13 @@ void i915_gem_context_release(struct kref *ref)
> >  	kfree_rcu(ctx, rcu);
> >  }
> >  
> > +void i915_gem_context_release(struct kref *ref)
> > +{
> > +	struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
> > +
> > +	queue_work(ctx->i915->wq, &ctx->release_work);
> > +}
> > +
> >  static inline struct i915_gem_engines *
> >  __context_engines_static(const struct i915_gem_context *ctx)
> >  {
> > @@ -1303,6 +1311,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
> >  	ctx->sched = pc->sched;
> >  	mutex_init(&ctx->mutex);
> >  	INIT_LIST_HEAD(&ctx->link);
> > +	INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
> >  
> >  	spin_lock_init(&ctx->stale.lock);
> >  	INIT_LIST_HEAD(&ctx->stale.engines);
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> > index 94c03a97cb77..0c38789bd4a8 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> > @@ -288,6 +288,18 @@ struct i915_gem_context {
> >  	 */
> >  	struct kref ref;
> >  
> > +	/**
> > +	 * @release_work:
> > +	 *
> > +	 * Work item for deferred cleanup, since i915_gem_context_put() tends to
> > +	 * be called from hardirq context.
> > +	 *
> > +	 * FIXME: The only real reason for this is &i915_gem_engines.fence, all
> > +	 * other callers are from process context and need at most some mild
> > +	 * shuffling to pull the i915_gem_context_put() call out of a spinlock.
> > +	 */
> > +	struct work_struct release_work;
> > +
> >  	/**
> >  	 * @rcu: rcu_head for deferred freeing.
> >  	 */
> > diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> > index fee070df1c97..067d68a6fe4c 100644
> > --- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> > +++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> > @@ -23,6 +23,7 @@ mock_context(struct drm_i915_private *i915,
> >  	kref_init(&ctx->ref);
> >  	INIT_LIST_HEAD(&ctx->link);
> >  	ctx->i915 = i915;
> > +	INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
> >  
> >  	mutex_init(&ctx->mutex);
> >  
> 
> ----
> Is the workqueue really needed? I'm not sure you could still race in
> drm_syncobj_free when refcount is zero, so in that case removing locking
> from _release would work as well as a workqueue.
> 
> Something like below would keep the drm_sync_obj_put hardirq safe.
> 
> I assume when freeing, the  cb list is supposed to be empty, so I added a WARN_ON just to be sure, otherwise we should just tear down the list without locking too.
> 
> This should be a better alternative for patch 1.

This isn't enough, because the problem isn't just the syncobj put. It's
also the i915_vm_put, and if we dercuify the intel_context stuff too, then
there will be more intel_context_put on top.

So we really need the worker here I think. Trying to make every _unpin() and
_put() work from hardirq context with clever locking tricks is why the
current code is so incomprehensible.

Also vms are rare enough that we really don't care about some
overhead/delay here.
-Daniel

> ----8<-------
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index c9a9d74f338c..9d561decd97e 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -462,7 +462,13 @@ void drm_syncobj_free(struct kref *kref)
>  	struct drm_syncobj *syncobj = container_of(kref,
>  						   struct drm_syncobj,
>  						   refcount);
> -	drm_syncobj_replace_fence(syncobj, NULL);
> +	struct dma_fence *old_fence;
> +
> +	old_fence = rcu_dereference_protected(syncobj->fence, !kref_read(&syncobj->refcount));
> +	dma_fence_put(old_fence);
> +
> +	WARN_ON(!list_empty(&syncobj->cb_list));
> +
>  	kfree(syncobj);
>  }
>  EXPORT_SYMBOL(drm_syncobj_free);
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with drm/i915: Release i915_gem_context from a worker (rev4)
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (19 preceding siblings ...)
  2021-08-31 10:16 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with drm/i915: Release i915_gem_context from a worker (rev3) Patchwork
@ 2021-08-31 12:18 ` Patchwork
  2021-09-02 12:42 ` [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Tvrtko Ursulin
  21 siblings, 0 replies; 33+ messages in thread
From: Patchwork @ 2021-08-31 12:18 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: series starting with drm/i915: Release i915_gem_context from a worker (rev4)
URL   : https://patchwork.freedesktop.org/series/93693/
State : failure

== Summary ==

Applying: drm/i915: Release i915_gem_context from a worker
Applying: drm/i915: Release ctx->syncobj on final put, not on ctx close
Applying: drm/i915: Keep gem ctx->vm alive until the final put
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/gem/i915_gem_context.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/gem/i915_gem_context.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/gem/i915_gem_context.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0003 drm/i915: Keep gem ctx->vm alive until the final put
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Release i915_gem_context from a worker
  2021-08-31 12:16     ` Daniel Vetter
@ 2021-08-31 15:14       ` Daniel Vetter
  2021-09-02 10:04         ` Maarten Lankhorst
  0 siblings, 1 reply; 33+ messages in thread
From: Daniel Vetter @ 2021-08-31 15:14 UTC (permalink / raw)
  To: Maarten Lankhorst
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	Daniel Vetter, Jon Bloomfield, Chris Wilson, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

On Tue, Aug 31, 2021 at 02:16:56PM +0200, Daniel Vetter wrote:
> On Tue, Aug 31, 2021 at 11:38:27AM +0200, Maarten Lankhorst wrote:
> > Op 14-08-2021 om 12:43 schreef Daniel Vetter:
> > > The only reason for this really is the i915_gem_engines->fence
> > > callback engines_notify(), which exists purely as a fairly funky
> > > reference counting scheme for that. Otherwise all other callers are
> > > from process context, and generally fairly benign locking context.
> > >
> > > Unfortunately untangling that requires some major surgery, and we have
> > > a few i915_gem_context reference counting bugs that need fixing, and
> > > they blow in the current hardirq calling context, so we need a
> > > stop-gap measure.
> > >
> > > Put a FIXME comment in when this should be removable again.
> > >
> > > v2: Fix mock_context(), noticed by intel-gfx-ci.
> > >
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
> > > Cc: Matthew Auld <matthew.auld@intel.com>
> > > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > > Cc: Dave Airlie <airlied@redhat.com>
> > > Cc: Jason Ekstrand <jason@jlekstrand.net>
> > > ---
> > >  drivers/gpu/drm/i915/gem/i915_gem_context.c       | 13 +++++++++++--
> > >  drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 12 ++++++++++++
> > >  drivers/gpu/drm/i915/gem/selftests/mock_context.c |  1 +
> > >  3 files changed, 24 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > index fd169cf2f75a..051bc357ff65 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > > @@ -986,9 +986,10 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx,
> > >  	return err;
> > >  }
> > >  
> > > -void i915_gem_context_release(struct kref *ref)
> > > +static void i915_gem_context_release_work(struct work_struct *work)
> > >  {
> > > -	struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
> > > +	struct i915_gem_context *ctx = container_of(work, typeof(*ctx),
> > > +						    release_work);
> > >  
> > >  	trace_i915_context_free(ctx);
> > >  	GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
> > > @@ -1002,6 +1003,13 @@ void i915_gem_context_release(struct kref *ref)
> > >  	kfree_rcu(ctx, rcu);
> > >  }
> > >  
> > > +void i915_gem_context_release(struct kref *ref)
> > > +{
> > > +	struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
> > > +
> > > +	queue_work(ctx->i915->wq, &ctx->release_work);
> > > +}
> > > +
> > >  static inline struct i915_gem_engines *
> > >  __context_engines_static(const struct i915_gem_context *ctx)
> > >  {
> > > @@ -1303,6 +1311,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
> > >  	ctx->sched = pc->sched;
> > >  	mutex_init(&ctx->mutex);
> > >  	INIT_LIST_HEAD(&ctx->link);
> > > +	INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
> > >  
> > >  	spin_lock_init(&ctx->stale.lock);
> > >  	INIT_LIST_HEAD(&ctx->stale.engines);
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> > > index 94c03a97cb77..0c38789bd4a8 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> > > @@ -288,6 +288,18 @@ struct i915_gem_context {
> > >  	 */
> > >  	struct kref ref;
> > >  
> > > +	/**
> > > +	 * @release_work:
> > > +	 *
> > > +	 * Work item for deferred cleanup, since i915_gem_context_put() tends to
> > > +	 * be called from hardirq context.
> > > +	 *
> > > +	 * FIXME: The only real reason for this is &i915_gem_engines.fence, all
> > > +	 * other callers are from process context and need at most some mild
> > > +	 * shuffling to pull the i915_gem_context_put() call out of a spinlock.
> > > +	 */
> > > +	struct work_struct release_work;
> > > +
> > >  	/**
> > >  	 * @rcu: rcu_head for deferred freeing.
> > >  	 */
> > > diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> > > index fee070df1c97..067d68a6fe4c 100644
> > > --- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> > > +++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
> > > @@ -23,6 +23,7 @@ mock_context(struct drm_i915_private *i915,
> > >  	kref_init(&ctx->ref);
> > >  	INIT_LIST_HEAD(&ctx->link);
> > >  	ctx->i915 = i915;
> > > +	INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
> > >  
> > >  	mutex_init(&ctx->mutex);
> > >  
> > 
> > ----
> > Is the workqueue really needed? I'm not sure you could still race in
> > drm_syncobj_free when refcount is zero, so in that case removing locking
> > from _release would work as well as a workqueue.
> > 
> > Something like below would keep the drm_sync_obj_put hardirq safe.
> > 
> > I assume when freeing, the  cb list is supposed to be empty, so I added a WARN_ON just to be sure, otherwise we should just tear down the list without locking too.
> > 
> > This should be a better alternative for patch 1.
> 
> This isn't enough, because the problem isn't just the syncobj put. It's
> also the i915_vm_put, and if we dercuify the intel_context stuff too, then
> there will be more intel_context_put on top.
> 
> So we really need the worker here I think. Trying to make every _unpin() and
> _put() work from hardirq context with clever locking tricks is why the
> current code is so incomprehensible.
> 
> Also vms are rare enough that we really don't care about some
> overhead/delay here.

Other reason is the one I explained in the commit message: Aside from the
engines i915_active there's no reason why anyone should call
i915_gem_context_put outside of process context. And I plan to fix that as
the next step. Or at least I'll try to untangle the context/engine
lifetime rules a bit.
-Daniel

> -Daniel
> 
> > ----8<-------
> > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> > index c9a9d74f338c..9d561decd97e 100644
> > --- a/drivers/gpu/drm/drm_syncobj.c
> > +++ b/drivers/gpu/drm/drm_syncobj.c
> > @@ -462,7 +462,13 @@ void drm_syncobj_free(struct kref *kref)
> >  	struct drm_syncobj *syncobj = container_of(kref,
> >  						   struct drm_syncobj,
> >  						   refcount);
> > -	drm_syncobj_replace_fence(syncobj, NULL);
> > +	struct dma_fence *old_fence;
> > +
> > +	old_fence = rcu_dereference_protected(syncobj->fence, !kref_read(&syncobj->refcount));
> > +	dma_fence_put(old_fence);
> > +
> > +	WARN_ON(!list_empty(&syncobj->cb_list));
> > +
> >  	kfree(syncobj);
> >  }
> >  EXPORT_SYMBOL(drm_syncobj_free);
> > 
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Release i915_gem_context from a worker
  2021-08-31 15:14       ` Daniel Vetter
@ 2021-09-02 10:04         ` Maarten Lankhorst
  0 siblings, 0 replies; 33+ messages in thread
From: Maarten Lankhorst @ 2021-09-02 10:04 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	Daniel Vetter, Jon Bloomfield, Chris Wilson, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

Op 31-08-2021 om 17:14 schreef Daniel Vetter:
> On Tue, Aug 31, 2021 at 02:16:56PM +0200, Daniel Vetter wrote:
>> On Tue, Aug 31, 2021 at 11:38:27AM +0200, Maarten Lankhorst wrote:
>>> Op 14-08-2021 om 12:43 schreef Daniel Vetter:
>>>> The only reason for this really is the i915_gem_engines->fence
>>>> callback engines_notify(), which exists purely as a fairly funky
>>>> reference counting scheme for that. Otherwise all other callers are
>>>> from process context, and generally fairly benign locking context.
>>>>
>>>> Unfortunately untangling that requires some major surgery, and we have
>>>> a few i915_gem_context reference counting bugs that need fixing, and
>>>> they blow in the current hardirq calling context, so we need a
>>>> stop-gap measure.
>>>>
>>>> Put a FIXME comment in when this should be removable again.
>>>>
>>>> v2: Fix mock_context(), noticed by intel-gfx-ci.
>>>>
>>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>>> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
>>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
>>>> Cc: Matthew Auld <matthew.auld@intel.com>
>>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>> Cc: Dave Airlie <airlied@redhat.com>
>>>> Cc: Jason Ekstrand <jason@jlekstrand.net>
>>>> ---
>>>>  drivers/gpu/drm/i915/gem/i915_gem_context.c       | 13 +++++++++++--
>>>>  drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 12 ++++++++++++
>>>>  drivers/gpu/drm/i915/gem/selftests/mock_context.c |  1 +
>>>>  3 files changed, 24 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>>> index fd169cf2f75a..051bc357ff65 100644
>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>>> @@ -986,9 +986,10 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx,
>>>>  	return err;
>>>>  }
>>>>  
>>>> -void i915_gem_context_release(struct kref *ref)
>>>> +static void i915_gem_context_release_work(struct work_struct *work)
>>>>  {
>>>> -	struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
>>>> +	struct i915_gem_context *ctx = container_of(work, typeof(*ctx),
>>>> +						    release_work);
>>>>  
>>>>  	trace_i915_context_free(ctx);
>>>>  	GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
>>>> @@ -1002,6 +1003,13 @@ void i915_gem_context_release(struct kref *ref)
>>>>  	kfree_rcu(ctx, rcu);
>>>>  }
>>>>  
>>>> +void i915_gem_context_release(struct kref *ref)
>>>> +{
>>>> +	struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
>>>> +
>>>> +	queue_work(ctx->i915->wq, &ctx->release_work);
>>>> +}
>>>> +
>>>>  static inline struct i915_gem_engines *
>>>>  __context_engines_static(const struct i915_gem_context *ctx)
>>>>  {
>>>> @@ -1303,6 +1311,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
>>>>  	ctx->sched = pc->sched;
>>>>  	mutex_init(&ctx->mutex);
>>>>  	INIT_LIST_HEAD(&ctx->link);
>>>> +	INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
>>>>  
>>>>  	spin_lock_init(&ctx->stale.lock);
>>>>  	INIT_LIST_HEAD(&ctx->stale.engines);
>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
>>>> index 94c03a97cb77..0c38789bd4a8 100644
>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
>>>> @@ -288,6 +288,18 @@ struct i915_gem_context {
>>>>  	 */
>>>>  	struct kref ref;
>>>>  
>>>> +	/**
>>>> +	 * @release_work:
>>>> +	 *
>>>> +	 * Work item for deferred cleanup, since i915_gem_context_put() tends to
>>>> +	 * be called from hardirq context.
>>>> +	 *
>>>> +	 * FIXME: The only real reason for this is &i915_gem_engines.fence, all
>>>> +	 * other callers are from process context and need at most some mild
>>>> +	 * shuffling to pull the i915_gem_context_put() call out of a spinlock.
>>>> +	 */
>>>> +	struct work_struct release_work;
>>>> +
>>>>  	/**
>>>>  	 * @rcu: rcu_head for deferred freeing.
>>>>  	 */
>>>> diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_context.c b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
>>>> index fee070df1c97..067d68a6fe4c 100644
>>>> --- a/drivers/gpu/drm/i915/gem/selftests/mock_context.c
>>>> +++ b/drivers/gpu/drm/i915/gem/selftests/mock_context.c
>>>> @@ -23,6 +23,7 @@ mock_context(struct drm_i915_private *i915,
>>>>  	kref_init(&ctx->ref);
>>>>  	INIT_LIST_HEAD(&ctx->link);
>>>>  	ctx->i915 = i915;
>>>> +	INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
>>>>  
>>>>  	mutex_init(&ctx->mutex);
>>>>  
>>> ----
>>> Is the workqueue really needed? I'm not sure you could still race in
>>> drm_syncobj_free when refcount is zero, so in that case removing locking
>>> from _release would work as well as a workqueue.
>>>
>>> Something like below would keep the drm_sync_obj_put hardirq safe.
>>>
>>> I assume when freeing, the  cb list is supposed to be empty, so I added a WARN_ON just to be sure, otherwise we should just tear down the list without locking too.
>>>
>>> This should be a better alternative for patch 1.
>> This isn't enough, because the problem isn't just the syncobj put. It's
>> also the i915_vm_put, and if we dercuify the intel_context stuff too, then
>> there will be more intel_context_put on top.
>>
>> So we really need the worker here I think. Trying to make every _unpin() and
>> _put() work from hardirq context with clever locking tricks is why the
>> current code is so incomprehensible.
>>
>> Also vms are rare enough that we really don't care about some
>> overhead/delay here.
> Other reason is the one I explained in the commit message: Aside from the
> engines i915_active there's no reason why anyone should call
> i915_gem_context_put outside of process context. And I plan to fix that as
> the next step. Or at least I'll try to untangle the context/engine
> lifetime rules a bit.
> -Daniel


That would definitely help me a lot too, so Acked-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> for patch 1.


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker
  2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
                   ` (20 preceding siblings ...)
  2021-08-31 12:18 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with drm/i915: Release i915_gem_context from a worker (rev4) Patchwork
@ 2021-09-02 12:42 ` Tvrtko Ursulin
  2021-09-02 15:05   ` Daniel Vetter
  21 siblings, 1 reply; 33+ messages in thread
From: Tvrtko Ursulin @ 2021-09-02 12:42 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Jon Bloomfield,
	Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand


On 13/08/2021 21:30, Daniel Vetter wrote:
> The only reason for this really is the i915_gem_engines->fence
> callback engines_notify(), which exists purely as a fairly funky
> reference counting scheme for that. Otherwise all other callers are
> from process context, and generally fairly benign locking context.

There is reset which definitely isn't process context.

Otherwise I did not really get from the commit message is this patch 
fixing an existing problem or preparing something for the future. If the 
former then as I wrote above - I am pretty sure there are call sites 
from the tasklet already.

Regards,

Tvrtko

> Unfortunately untangling that requires some major surgery, and we have
> a few i915_gem_context reference counting bugs that need fixing, and
> they blow in the current hardirq calling context, so we need a
> stop-gap measure.
> 
> Put a FIXME comment in when this should be removable again.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_context.c       | 13 +++++++++++--
>   drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 12 ++++++++++++
>   2 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index fd169cf2f75a..051bc357ff65 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -986,9 +986,10 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx,
>   	return err;
>   }
>   
> -void i915_gem_context_release(struct kref *ref)
> +static void i915_gem_context_release_work(struct work_struct *work)
>   {
> -	struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
> +	struct i915_gem_context *ctx = container_of(work, typeof(*ctx),
> +						    release_work);
>   
>   	trace_i915_context_free(ctx);
>   	GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
> @@ -1002,6 +1003,13 @@ void i915_gem_context_release(struct kref *ref)
>   	kfree_rcu(ctx, rcu);
>   }
>   
> +void i915_gem_context_release(struct kref *ref)
> +{
> +	struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
> +
> +	queue_work(ctx->i915->wq, &ctx->release_work);
> +}
> +
>   static inline struct i915_gem_engines *
>   __context_engines_static(const struct i915_gem_context *ctx)
>   {
> @@ -1303,6 +1311,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
>   	ctx->sched = pc->sched;
>   	mutex_init(&ctx->mutex);
>   	INIT_LIST_HEAD(&ctx->link);
> +	INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
>   
>   	spin_lock_init(&ctx->stale.lock);
>   	INIT_LIST_HEAD(&ctx->stale.engines);
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> index 94c03a97cb77..0c38789bd4a8 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> @@ -288,6 +288,18 @@ struct i915_gem_context {
>   	 */
>   	struct kref ref;
>   
> +	/**
> +	 * @release_work:
> +	 *
> +	 * Work item for deferred cleanup, since i915_gem_context_put() tends to
> +	 * be called from hardirq context.
> +	 *
> +	 * FIXME: The only real reason for this is &i915_gem_engines.fence, all
> +	 * other callers are from process context and need at most some mild
> +	 * shuffling to pull the i915_gem_context_put() call out of a spinlock.
> +	 */
> +	struct work_struct release_work;
> +
>   	/**
>   	 * @rcu: rcu_head for deferred freeing.
>   	 */
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker
  2021-09-02 12:42 ` [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Tvrtko Ursulin
@ 2021-09-02 15:05   ` Daniel Vetter
  2021-09-02 16:20     ` Tvrtko Ursulin
  0 siblings, 1 reply; 33+ messages in thread
From: Daniel Vetter @ 2021-09-02 15:05 UTC (permalink / raw)
  To: Tvrtko Ursulin
  Cc: DRI Development, Intel Graphics Development, Daniel Vetter,
	Jon Bloomfield, Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

On Thu, Sep 2, 2021 at 2:42 PM Tvrtko Ursulin
<tvrtko.ursulin@linux.intel.com> wrote:
>
>
> On 13/08/2021 21:30, Daniel Vetter wrote:
> > The only reason for this really is the i915_gem_engines->fence
> > callback engines_notify(), which exists purely as a fairly funky
> > reference counting scheme for that. Otherwise all other callers are
> > from process context, and generally fairly benign locking context.
>
> There is reset which definitely isn't process context.

gpu reset runs in process context. The tasklet context is the
engines_notify I'm talking about above.

> Otherwise I did not really get from the commit message is this patch
> fixing an existing problem or preparing something for the future. If the
> former then as I wrote above - I am pretty sure there are call sites
> from the tasklet already.
>
> Regards,
>
> Tvrtko
>
> > Unfortunately untangling that requires some major surgery, and we have
> > a few i915_gem_context reference counting bugs that need fixing, and
> > they blow in the current hardirq calling context, so we need a
> > stop-gap measure.

I guess this para wasn't clear, but subsequent patches fix the
refcount bugs and need this prep patch here.
-Daniel

> >
> > Put a FIXME comment in when this should be removable again.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Jason Ekstrand <jason@jlekstrand.net>
> > ---
> >   drivers/gpu/drm/i915/gem/i915_gem_context.c       | 13 +++++++++++--
> >   drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 12 ++++++++++++
> >   2 files changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > index fd169cf2f75a..051bc357ff65 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > @@ -986,9 +986,10 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx,
> >       return err;
> >   }
> >
> > -void i915_gem_context_release(struct kref *ref)
> > +static void i915_gem_context_release_work(struct work_struct *work)
> >   {
> > -     struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
> > +     struct i915_gem_context *ctx = container_of(work, typeof(*ctx),
> > +                                                 release_work);
> >
> >       trace_i915_context_free(ctx);
> >       GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
> > @@ -1002,6 +1003,13 @@ void i915_gem_context_release(struct kref *ref)
> >       kfree_rcu(ctx, rcu);
> >   }
> >
> > +void i915_gem_context_release(struct kref *ref)
> > +{
> > +     struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
> > +
> > +     queue_work(ctx->i915->wq, &ctx->release_work);
> > +}
> > +
> >   static inline struct i915_gem_engines *
> >   __context_engines_static(const struct i915_gem_context *ctx)
> >   {
> > @@ -1303,6 +1311,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
> >       ctx->sched = pc->sched;
> >       mutex_init(&ctx->mutex);
> >       INIT_LIST_HEAD(&ctx->link);
> > +     INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
> >
> >       spin_lock_init(&ctx->stale.lock);
> >       INIT_LIST_HEAD(&ctx->stale.engines);
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> > index 94c03a97cb77..0c38789bd4a8 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> > @@ -288,6 +288,18 @@ struct i915_gem_context {
> >        */
> >       struct kref ref;
> >
> > +     /**
> > +      * @release_work:
> > +      *
> > +      * Work item for deferred cleanup, since i915_gem_context_put() tends to
> > +      * be called from hardirq context.
> > +      *
> > +      * FIXME: The only real reason for this is &i915_gem_engines.fence, all
> > +      * other callers are from process context and need at most some mild
> > +      * shuffling to pull the i915_gem_context_put() call out of a spinlock.
> > +      */
> > +     struct work_struct release_work;
> > +
> >       /**
> >        * @rcu: rcu_head for deferred freeing.
> >        */
> >



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker
  2021-09-02 15:05   ` Daniel Vetter
@ 2021-09-02 16:20     ` Tvrtko Ursulin
  2021-09-02 20:02       ` Daniel Vetter
  0 siblings, 1 reply; 33+ messages in thread
From: Tvrtko Ursulin @ 2021-09-02 16:20 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Intel Graphics Development, Daniel Vetter,
	Jon Bloomfield, Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand


On 02/09/2021 16:05, Daniel Vetter wrote:
> On Thu, Sep 2, 2021 at 2:42 PM Tvrtko Ursulin
> <tvrtko.ursulin@linux.intel.com> wrote:
>>
>>
>> On 13/08/2021 21:30, Daniel Vetter wrote:
>>> The only reason for this really is the i915_gem_engines->fence
>>> callback engines_notify(), which exists purely as a fairly funky
>>> reference counting scheme for that. Otherwise all other callers are
>>> from process context, and generally fairly benign locking context.
>>
>> There is reset which definitely isn't process context.
> 
> gpu reset runs in process context. The tasklet context is the
> engines_notify I'm talking about above.

I haven't looked very deeply but please double check the path from 
execlists_submission_tasklet -> execlists_reset -> intel_engine_reset -> 
__intel_engine_reset -> execlists_reset_rewind -> execlists_reset_csb -> 
execlists_reset_active -> __i915_request_reset -> mark_guilty -> 
i915_gem_context_put.

>> Otherwise I did not really get from the commit message is this patch
>> fixing an existing problem or preparing something for the future. If the
>> former then as I wrote above - I am pretty sure there are call sites
>> from the tasklet already.
>>
>> Regards,
>>
>> Tvrtko
>>
>>> Unfortunately untangling that requires some major surgery, and we have
>>> a few i915_gem_context reference counting bugs that need fixing, and
>>> they blow in the current hardirq calling context, so we need a
>>> stop-gap measure.
> 
> I guess this para wasn't clear, but subsequent patches fix the
> refcount bugs and need this prep patch here.

So up to where in the series are those fixes and where other stuff 
follows? Worth spliting and having cover letters perhaps? Is the fixing 
part applicable to the existing code or only comes to play with the 
syncobj single timeline changes?

Regards,

Tvrtko

> -Daniel
> 
>>>
>>> Put a FIXME comment in when this should be removable again.
>>>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
>>> Cc: Matthew Auld <matthew.auld@intel.com>
>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>> Cc: Dave Airlie <airlied@redhat.com>
>>> Cc: Jason Ekstrand <jason@jlekstrand.net>
>>> ---
>>>    drivers/gpu/drm/i915/gem/i915_gem_context.c       | 13 +++++++++++--
>>>    drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 12 ++++++++++++
>>>    2 files changed, 23 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>> index fd169cf2f75a..051bc357ff65 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>> @@ -986,9 +986,10 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx,
>>>        return err;
>>>    }
>>>
>>> -void i915_gem_context_release(struct kref *ref)
>>> +static void i915_gem_context_release_work(struct work_struct *work)
>>>    {
>>> -     struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
>>> +     struct i915_gem_context *ctx = container_of(work, typeof(*ctx),
>>> +                                                 release_work);
>>>
>>>        trace_i915_context_free(ctx);
>>>        GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
>>> @@ -1002,6 +1003,13 @@ void i915_gem_context_release(struct kref *ref)
>>>        kfree_rcu(ctx, rcu);
>>>    }
>>>
>>> +void i915_gem_context_release(struct kref *ref)
>>> +{
>>> +     struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
>>> +
>>> +     queue_work(ctx->i915->wq, &ctx->release_work);
>>> +}
>>> +
>>>    static inline struct i915_gem_engines *
>>>    __context_engines_static(const struct i915_gem_context *ctx)
>>>    {
>>> @@ -1303,6 +1311,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
>>>        ctx->sched = pc->sched;
>>>        mutex_init(&ctx->mutex);
>>>        INIT_LIST_HEAD(&ctx->link);
>>> +     INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
>>>
>>>        spin_lock_init(&ctx->stale.lock);
>>>        INIT_LIST_HEAD(&ctx->stale.engines);
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
>>> index 94c03a97cb77..0c38789bd4a8 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
>>> @@ -288,6 +288,18 @@ struct i915_gem_context {
>>>         */
>>>        struct kref ref;
>>>
>>> +     /**
>>> +      * @release_work:
>>> +      *
>>> +      * Work item for deferred cleanup, since i915_gem_context_put() tends to
>>> +      * be called from hardirq context.
>>> +      *
>>> +      * FIXME: The only real reason for this is &i915_gem_engines.fence, all
>>> +      * other callers are from process context and need at most some mild
>>> +      * shuffling to pull the i915_gem_context_put() call out of a spinlock.
>>> +      */
>>> +     struct work_struct release_work;
>>> +
>>>        /**
>>>         * @rcu: rcu_head for deferred freeing.
>>>         */
>>>
> 
> 
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker
  2021-09-02 16:20     ` Tvrtko Ursulin
@ 2021-09-02 20:02       ` Daniel Vetter
  2021-09-03 10:40         ` Tvrtko Ursulin
  0 siblings, 1 reply; 33+ messages in thread
From: Daniel Vetter @ 2021-09-02 20:02 UTC (permalink / raw)
  To: Tvrtko Ursulin
  Cc: DRI Development, Intel Graphics Development, Daniel Vetter,
	Jon Bloomfield, Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand

On Thu, Sep 2, 2021 at 6:20 PM Tvrtko Ursulin
<tvrtko.ursulin@linux.intel.com> wrote:
> On 02/09/2021 16:05, Daniel Vetter wrote:
> > On Thu, Sep 2, 2021 at 2:42 PM Tvrtko Ursulin
> > <tvrtko.ursulin@linux.intel.com> wrote:
> >>
> >>
> >> On 13/08/2021 21:30, Daniel Vetter wrote:
> >>> The only reason for this really is the i915_gem_engines->fence
> >>> callback engines_notify(), which exists purely as a fairly funky
> >>> reference counting scheme for that. Otherwise all other callers are
> >>> from process context, and generally fairly benign locking context.
> >>
> >> There is reset which definitely isn't process context.
> >
> > gpu reset runs in process context. The tasklet context is the
> > engines_notify I'm talking about above.
>
> I haven't looked very deeply but please double check the path from
> execlists_submission_tasklet -> execlists_reset -> intel_engine_reset ->
> __intel_engine_reset -> execlists_reset_rewind -> execlists_reset_csb ->
> execlists_reset_active -> __i915_request_reset -> mark_guilty ->
> i915_gem_context_put.

Thanks for pointing this out, I'll add it to the commit message.

More stuff to fix, yay.

> >> Otherwise I did not really get from the commit message is this patch
> >> fixing an existing problem or preparing something for the future. If the
> >> former then as I wrote above - I am pretty sure there are call sites
> >> from the tasklet already.
> >>
> >> Regards,
> >>
> >> Tvrtko
> >>
> >>> Unfortunately untangling that requires some major surgery, and we have
> >>> a few i915_gem_context reference counting bugs that need fixing, and
> >>> they blow in the current hardirq calling context, so we need a
> >>> stop-gap measure.
> >
> > I guess this para wasn't clear, but subsequent patches fix the
> > refcount bugs and need this prep patch here.
>
> So up to where in the series are those fixes and where other stuff
> follows? Worth spliting and having cover letters perhaps? Is the fixing
> part applicable to the existing code or only comes to play with the
> syncobj single timeline changes?

There's Fixes: lines. One is timeline syncobj, the other is 2 years old.
-Daniel

>
> Regards,
>
> Tvrtko
>
> > -Daniel
> >
> >>>
> >>> Put a FIXME comment in when this should be removable again.
> >>>
> >>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >>> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> >>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >>> Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
> >>> Cc: Matthew Auld <matthew.auld@intel.com>
> >>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> >>> Cc: Dave Airlie <airlied@redhat.com>
> >>> Cc: Jason Ekstrand <jason@jlekstrand.net>
> >>> ---
> >>>    drivers/gpu/drm/i915/gem/i915_gem_context.c       | 13 +++++++++++--
> >>>    drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 12 ++++++++++++
> >>>    2 files changed, 23 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >>> index fd169cf2f75a..051bc357ff65 100644
> >>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> >>> @@ -986,9 +986,10 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx,
> >>>        return err;
> >>>    }
> >>>
> >>> -void i915_gem_context_release(struct kref *ref)
> >>> +static void i915_gem_context_release_work(struct work_struct *work)
> >>>    {
> >>> -     struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
> >>> +     struct i915_gem_context *ctx = container_of(work, typeof(*ctx),
> >>> +                                                 release_work);
> >>>
> >>>        trace_i915_context_free(ctx);
> >>>        GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
> >>> @@ -1002,6 +1003,13 @@ void i915_gem_context_release(struct kref *ref)
> >>>        kfree_rcu(ctx, rcu);
> >>>    }
> >>>
> >>> +void i915_gem_context_release(struct kref *ref)
> >>> +{
> >>> +     struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
> >>> +
> >>> +     queue_work(ctx->i915->wq, &ctx->release_work);
> >>> +}
> >>> +
> >>>    static inline struct i915_gem_engines *
> >>>    __context_engines_static(const struct i915_gem_context *ctx)
> >>>    {
> >>> @@ -1303,6 +1311,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
> >>>        ctx->sched = pc->sched;
> >>>        mutex_init(&ctx->mutex);
> >>>        INIT_LIST_HEAD(&ctx->link);
> >>> +     INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
> >>>
> >>>        spin_lock_init(&ctx->stale.lock);
> >>>        INIT_LIST_HEAD(&ctx->stale.engines);
> >>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> >>> index 94c03a97cb77..0c38789bd4a8 100644
> >>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> >>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
> >>> @@ -288,6 +288,18 @@ struct i915_gem_context {
> >>>         */
> >>>        struct kref ref;
> >>>
> >>> +     /**
> >>> +      * @release_work:
> >>> +      *
> >>> +      * Work item for deferred cleanup, since i915_gem_context_put() tends to
> >>> +      * be called from hardirq context.
> >>> +      *
> >>> +      * FIXME: The only real reason for this is &i915_gem_engines.fence, all
> >>> +      * other callers are from process context and need at most some mild
> >>> +      * shuffling to pull the i915_gem_context_put() call out of a spinlock.
> >>> +      */
> >>> +     struct work_struct release_work;
> >>> +
> >>>        /**
> >>>         * @rcu: rcu_head for deferred freeing.
> >>>         */
> >>>
> >
> >
> >



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker
  2021-09-02 20:02       ` Daniel Vetter
@ 2021-09-03 10:40         ` Tvrtko Ursulin
  0 siblings, 0 replies; 33+ messages in thread
From: Tvrtko Ursulin @ 2021-09-03 10:40 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: DRI Development, Intel Graphics Development, Daniel Vetter,
	Jon Bloomfield, Chris Wilson, Maarten Lankhorst, Joonas Lahtinen,
	Thomas Hellström, Matthew Auld, Lionel Landwerlin,
	Dave Airlie, Jason Ekstrand


On 02/09/2021 21:02, Daniel Vetter wrote:
> On Thu, Sep 2, 2021 at 6:20 PM Tvrtko Ursulin
> <tvrtko.ursulin@linux.intel.com> wrote:
>> On 02/09/2021 16:05, Daniel Vetter wrote:
>>> On Thu, Sep 2, 2021 at 2:42 PM Tvrtko Ursulin
>>> <tvrtko.ursulin@linux.intel.com> wrote:
>>>>
>>>>
>>>> On 13/08/2021 21:30, Daniel Vetter wrote:
>>>>> The only reason for this really is the i915_gem_engines->fence
>>>>> callback engines_notify(), which exists purely as a fairly funky
>>>>> reference counting scheme for that. Otherwise all other callers are
>>>>> from process context, and generally fairly benign locking context.
>>>>
>>>> There is reset which definitely isn't process context.
>>>
>>> gpu reset runs in process context. The tasklet context is the
>>> engines_notify I'm talking about above.
>>
>> I haven't looked very deeply but please double check the path from
>> execlists_submission_tasklet -> execlists_reset -> intel_engine_reset ->
>> __intel_engine_reset -> execlists_reset_rewind -> execlists_reset_csb ->
>> execlists_reset_active -> __i915_request_reset -> mark_guilty ->
>> i915_gem_context_put.
> 
> Thanks for pointing this out, I'll add it to the commit message.
> 
> More stuff to fix, yay.
> 
>>>> Otherwise I did not really get from the commit message is this patch
>>>> fixing an existing problem or preparing something for the future. If the
>>>> former then as I wrote above - I am pretty sure there are call sites
>>>> from the tasklet already.
>>>>
>>>> Regards,
>>>>
>>>> Tvrtko
>>>>
>>>>> Unfortunately untangling that requires some major surgery, and we have
>>>>> a few i915_gem_context reference counting bugs that need fixing, and
>>>>> they blow in the current hardirq calling context, so we need a
>>>>> stop-gap measure.
>>>
>>> I guess this para wasn't clear, but subsequent patches fix the
>>> refcount bugs and need this prep patch here.
>>
>> So up to where in the series are those fixes and where other stuff
>> follows? Worth spliting and having cover letters perhaps? Is the fixing
>> part applicable to the existing code or only comes to play with the
>> syncobj single timeline changes?
> 
> There's Fixes: lines. One is timeline syncobj, the other is 2 years old.

So first two patches are standalone and fix the immediate bug? Could you 
describe the composition and doings of the series in a cover letter so 
it's possible to have an overview of chunk of work tackled?

Regards,

Tvrtko

> -Daniel
> 
>>
>> Regards,
>>
>> Tvrtko
>>
>>> -Daniel
>>>
>>>>>
>>>>> Put a FIXME comment in when this should be removable again.
>>>>>
>>>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>>>> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
>>>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>> Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
>>>>> Cc: Matthew Auld <matthew.auld@intel.com>
>>>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>>> Cc: Dave Airlie <airlied@redhat.com>
>>>>> Cc: Jason Ekstrand <jason@jlekstrand.net>
>>>>> ---
>>>>>     drivers/gpu/drm/i915/gem/i915_gem_context.c       | 13 +++++++++++--
>>>>>     drivers/gpu/drm/i915/gem/i915_gem_context_types.h | 12 ++++++++++++
>>>>>     2 files changed, 23 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>>>> index fd169cf2f75a..051bc357ff65 100644
>>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
>>>>> @@ -986,9 +986,10 @@ static struct i915_gem_engines *user_engines(struct i915_gem_context *ctx,
>>>>>         return err;
>>>>>     }
>>>>>
>>>>> -void i915_gem_context_release(struct kref *ref)
>>>>> +static void i915_gem_context_release_work(struct work_struct *work)
>>>>>     {
>>>>> -     struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
>>>>> +     struct i915_gem_context *ctx = container_of(work, typeof(*ctx),
>>>>> +                                                 release_work);
>>>>>
>>>>>         trace_i915_context_free(ctx);
>>>>>         GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
>>>>> @@ -1002,6 +1003,13 @@ void i915_gem_context_release(struct kref *ref)
>>>>>         kfree_rcu(ctx, rcu);
>>>>>     }
>>>>>
>>>>> +void i915_gem_context_release(struct kref *ref)
>>>>> +{
>>>>> +     struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
>>>>> +
>>>>> +     queue_work(ctx->i915->wq, &ctx->release_work);
>>>>> +}
>>>>> +
>>>>>     static inline struct i915_gem_engines *
>>>>>     __context_engines_static(const struct i915_gem_context *ctx)
>>>>>     {
>>>>> @@ -1303,6 +1311,7 @@ i915_gem_create_context(struct drm_i915_private *i915,
>>>>>         ctx->sched = pc->sched;
>>>>>         mutex_init(&ctx->mutex);
>>>>>         INIT_LIST_HEAD(&ctx->link);
>>>>> +     INIT_WORK(&ctx->release_work, i915_gem_context_release_work);
>>>>>
>>>>>         spin_lock_init(&ctx->stale.lock);
>>>>>         INIT_LIST_HEAD(&ctx->stale.engines);
>>>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
>>>>> index 94c03a97cb77..0c38789bd4a8 100644
>>>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
>>>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h
>>>>> @@ -288,6 +288,18 @@ struct i915_gem_context {
>>>>>          */
>>>>>         struct kref ref;
>>>>>
>>>>> +     /**
>>>>> +      * @release_work:
>>>>> +      *
>>>>> +      * Work item for deferred cleanup, since i915_gem_context_put() tends to
>>>>> +      * be called from hardirq context.
>>>>> +      *
>>>>> +      * FIXME: The only real reason for this is &i915_gem_engines.fence, all
>>>>> +      * other callers are from process context and need at most some mild
>>>>> +      * shuffling to pull the i915_gem_context_put() call out of a spinlock.
>>>>> +      */
>>>>> +     struct work_struct release_work;
>>>>> +
>>>>>         /**
>>>>>          * @rcu: rcu_head for deferred freeing.
>>>>>          */
>>>>>
>>>
>>>
>>>
> 
> 
> 

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2021-09-03 10:40 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-13 20:30 [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Daniel Vetter
2021-08-13 20:30 ` [Intel-gfx] [PATCH 02/11] drm/i915: Release ctx->syncobj on final put, not on ctx close Daniel Vetter
2021-08-13 20:30 ` [Intel-gfx] [PATCH 03/11] drm/i915: Keep gem ctx->vm alive until the final put Daniel Vetter
2021-08-13 20:30 ` [Intel-gfx] [PATCH 04/11] drm/i915: Drop code to handle set-vm races from execbuf Daniel Vetter
2021-08-13 20:30 ` [Intel-gfx] [PATCH 05/11] drm/i915: Rename i915_gem_context_get_vm_rcu to i915_gem_context_get_eb_vm Daniel Vetter
2021-08-13 20:30 ` [Intel-gfx] [PATCH 06/11] drm/i915: Use i915_gem_context_get_eb_vm in ctx_getparam Daniel Vetter
2021-08-13 20:30 ` [Intel-gfx] [PATCH 07/11] drm/i915: Add i915_gem_context_is_full_ppgtt Daniel Vetter
2021-08-13 20:30 ` [Intel-gfx] [PATCH 08/11] drm/i915: Use i915_gem_context_get_eb_vm in intel_context_set_gem Daniel Vetter
2021-08-13 20:30 ` [Intel-gfx] [PATCH 09/11] drm/i915: Drop __rcu from gem_context->vm Daniel Vetter
2021-08-13 20:30 ` [Intel-gfx] [PATCH 10/11] drm/i915: use xa_lock/unlock for fpriv->vm_xa lookups Daniel Vetter
2021-08-31  9:29   ` Maarten Lankhorst
2021-08-31 12:14   ` [Intel-gfx] [PATCH] " Daniel Vetter
2021-08-13 20:30 ` [Intel-gfx] [PATCH 11/11] drm/i915: Stop rcu support for i915_address_space Daniel Vetter
2021-08-13 21:48 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/11] drm/i915: Release i915_gem_context from a worker Patchwork
2021-08-13 21:49 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-08-13 22:18 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-08-14  1:26 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2021-08-14 10:43 ` [Intel-gfx] [PATCH] " Daniel Vetter
2021-08-31  9:38   ` Maarten Lankhorst
2021-08-31 12:16     ` Daniel Vetter
2021-08-31 15:14       ` Daniel Vetter
2021-09-02 10:04         ` Maarten Lankhorst
2021-08-14 10:55 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with drm/i915: Release i915_gem_context from a worker (rev2) Patchwork
2021-08-14 10:56 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-08-14 11:19 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-08-14 12:38 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2021-08-31 10:16 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with drm/i915: Release i915_gem_context from a worker (rev3) Patchwork
2021-08-31 12:18 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for series starting with drm/i915: Release i915_gem_context from a worker (rev4) Patchwork
2021-09-02 12:42 ` [Intel-gfx] [PATCH 01/11] drm/i915: Release i915_gem_context from a worker Tvrtko Ursulin
2021-09-02 15:05   ` Daniel Vetter
2021-09-02 16:20     ` Tvrtko Ursulin
2021-09-02 20:02       ` Daniel Vetter
2021-09-03 10:40         ` Tvrtko Ursulin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).