All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org, John.C.Harrison@Intel.com,
	matthew.brost@intel.com
Subject: [Intel-gfx] [PATCH v5 13/25] drm/i915/guc: Don't touch guc_state.sched_state without a lock
Date: Wed,  1 Sep 2021 17:50:10 -0700	[thread overview]
Message-ID: <20210902005022.711767-14-daniele.ceraolospurio@intel.com> (raw)
In-Reply-To: <20210902005022.711767-1-daniele.ceraolospurio@intel.com>

From: Matthew Brost <matthew.brost@intel.com>

Before we did some clever tricks to not use the a lock when touching
guc_state.sched_state in certain cases. Don't do that, enforce the use
of the lock.

v2:
 (kernel test robo )
  - Add __maybe_unused to sched_state_is_init()

v3: rebase after the unused code path removal has been moved to an
earlier patch.

Signed-off-by: Matthew Brost <matthew.brost@intel.com>
Reported-by: kernel test robot <lkp@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 22 ++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index e036a171ff17..ca73128d7b4d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -151,11 +151,23 @@ static inline void clr_context_registered(struct intel_context *ce)
 
 static inline void init_sched_state(struct intel_context *ce)
 {
-	/* Only should be called from guc_lrc_desc_pin() */
+	lockdep_assert_held(&ce->guc_state.lock);
 	atomic_set(&ce->guc_sched_state_no_lock, 0);
 	ce->guc_state.sched_state &= SCHED_STATE_BLOCKED_MASK;
 }
 
+__maybe_unused
+static bool sched_state_is_init(struct intel_context *ce)
+{
+	/*
+	 * XXX: Kernel contexts can have SCHED_STATE_NO_LOCK_REGISTERED after
+	 * suspend.
+	 */
+	return !(atomic_read(&ce->guc_sched_state_no_lock) &
+		 ~SCHED_STATE_NO_LOCK_REGISTERED) &&
+		!(ce->guc_state.sched_state &= ~SCHED_STATE_BLOCKED_MASK);
+}
+
 static inline bool
 context_wait_for_deregister_to_register(struct intel_context *ce)
 {
@@ -166,7 +178,7 @@ context_wait_for_deregister_to_register(struct intel_context *ce)
 static inline void
 set_context_wait_for_deregister_to_register(struct intel_context *ce)
 {
-	/* Only should be called from guc_lrc_desc_pin() without lock */
+	lockdep_assert_held(&ce->guc_state.lock);
 	ce->guc_state.sched_state |=
 		SCHED_STATE_WAIT_FOR_DEREGISTER_TO_REGISTER;
 }
@@ -607,9 +619,7 @@ static void scrub_guc_desc_for_outstanding_g2h(struct intel_guc *guc)
 	bool pending_disable, pending_enable, deregister, destroyed, banned;
 
 	xa_for_each(&guc->context_lookup, index, ce) {
-		/* Flush context */
 		spin_lock_irqsave(&ce->guc_state.lock, flags);
-		spin_unlock_irqrestore(&ce->guc_state.lock, flags);
 
 		/*
 		 * Once we are at this point submission_disabled() is guaranteed
@@ -625,6 +635,8 @@ static void scrub_guc_desc_for_outstanding_g2h(struct intel_guc *guc)
 		banned = context_banned(ce);
 		init_sched_state(ce);
 
+		spin_unlock_irqrestore(&ce->guc_state.lock, flags);
+
 		if (pending_enable || destroyed || deregister) {
 			decr_outstanding_submission_g2h(guc);
 			if (deregister)
@@ -1324,6 +1336,7 @@ static int guc_lrc_desc_pin(struct intel_context *ce, bool loop)
 	int ret = 0;
 
 	GEM_BUG_ON(!engine->mask);
+	GEM_BUG_ON(!sched_state_is_init(ce));
 
 	/*
 	 * Ensure LRC + CT vmas are is same region as write barrier is done
@@ -1352,7 +1365,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce, bool loop)
 	desc->priority = ce->guc_prio;
 	desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD;
 	guc_context_policy_init(engine, desc);
-	init_sched_state(ce);
 
 	/*
 	 * The context_lookup xarray is used to determine if the hardware
-- 
2.25.1


WARNING: multiple messages have this Message-ID (diff)
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org, John.C.Harrison@Intel.com,
	matthew.brost@intel.com
Subject: [PATCH v5 13/25] drm/i915/guc: Don't touch guc_state.sched_state without a lock
Date: Wed,  1 Sep 2021 17:50:10 -0700	[thread overview]
Message-ID: <20210902005022.711767-14-daniele.ceraolospurio@intel.com> (raw)
In-Reply-To: <20210902005022.711767-1-daniele.ceraolospurio@intel.com>

From: Matthew Brost <matthew.brost@intel.com>

Before we did some clever tricks to not use the a lock when touching
guc_state.sched_state in certain cases. Don't do that, enforce the use
of the lock.

v2:
 (kernel test robo )
  - Add __maybe_unused to sched_state_is_init()

v3: rebase after the unused code path removal has been moved to an
earlier patch.

Signed-off-by: Matthew Brost <matthew.brost@intel.com>
Reported-by: kernel test robot <lkp@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 22 ++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index e036a171ff17..ca73128d7b4d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -151,11 +151,23 @@ static inline void clr_context_registered(struct intel_context *ce)
 
 static inline void init_sched_state(struct intel_context *ce)
 {
-	/* Only should be called from guc_lrc_desc_pin() */
+	lockdep_assert_held(&ce->guc_state.lock);
 	atomic_set(&ce->guc_sched_state_no_lock, 0);
 	ce->guc_state.sched_state &= SCHED_STATE_BLOCKED_MASK;
 }
 
+__maybe_unused
+static bool sched_state_is_init(struct intel_context *ce)
+{
+	/*
+	 * XXX: Kernel contexts can have SCHED_STATE_NO_LOCK_REGISTERED after
+	 * suspend.
+	 */
+	return !(atomic_read(&ce->guc_sched_state_no_lock) &
+		 ~SCHED_STATE_NO_LOCK_REGISTERED) &&
+		!(ce->guc_state.sched_state &= ~SCHED_STATE_BLOCKED_MASK);
+}
+
 static inline bool
 context_wait_for_deregister_to_register(struct intel_context *ce)
 {
@@ -166,7 +178,7 @@ context_wait_for_deregister_to_register(struct intel_context *ce)
 static inline void
 set_context_wait_for_deregister_to_register(struct intel_context *ce)
 {
-	/* Only should be called from guc_lrc_desc_pin() without lock */
+	lockdep_assert_held(&ce->guc_state.lock);
 	ce->guc_state.sched_state |=
 		SCHED_STATE_WAIT_FOR_DEREGISTER_TO_REGISTER;
 }
@@ -607,9 +619,7 @@ static void scrub_guc_desc_for_outstanding_g2h(struct intel_guc *guc)
 	bool pending_disable, pending_enable, deregister, destroyed, banned;
 
 	xa_for_each(&guc->context_lookup, index, ce) {
-		/* Flush context */
 		spin_lock_irqsave(&ce->guc_state.lock, flags);
-		spin_unlock_irqrestore(&ce->guc_state.lock, flags);
 
 		/*
 		 * Once we are at this point submission_disabled() is guaranteed
@@ -625,6 +635,8 @@ static void scrub_guc_desc_for_outstanding_g2h(struct intel_guc *guc)
 		banned = context_banned(ce);
 		init_sched_state(ce);
 
+		spin_unlock_irqrestore(&ce->guc_state.lock, flags);
+
 		if (pending_enable || destroyed || deregister) {
 			decr_outstanding_submission_g2h(guc);
 			if (deregister)
@@ -1324,6 +1336,7 @@ static int guc_lrc_desc_pin(struct intel_context *ce, bool loop)
 	int ret = 0;
 
 	GEM_BUG_ON(!engine->mask);
+	GEM_BUG_ON(!sched_state_is_init(ce));
 
 	/*
 	 * Ensure LRC + CT vmas are is same region as write barrier is done
@@ -1352,7 +1365,6 @@ static int guc_lrc_desc_pin(struct intel_context *ce, bool loop)
 	desc->priority = ce->guc_prio;
 	desc->context_flags = CONTEXT_REGISTRATION_FLAG_KMD;
 	guc_context_policy_init(engine, desc);
-	init_sched_state(ce);
 
 	/*
 	 * The context_lookup xarray is used to determine if the hardware
-- 
2.25.1


  parent reply	other threads:[~2021-09-02  0:53 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-02  0:49 [PATCH v5 00/25] Clean up GuC CI failures, simplify locking, and kernel DOC Daniele Ceraolo Spurio
2021-09-02  0:49 ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:49 ` [PATCH v5 01/25] drm/i915/guc: Fix blocked context accounting Daniele Ceraolo Spurio
2021-09-02  0:49   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:49 ` [PATCH v5 02/25] drm/i915/guc: Fix outstanding G2H accounting Daniele Ceraolo Spurio
2021-09-02  0:49   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02 22:11   ` John Harrison
2021-09-02 22:11     ` [Intel-gfx] " John Harrison
2021-09-02  0:50 ` [PATCH v5 03/25] drm/i915/guc: Unwind context requests in reverse order Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 04/25] drm/i915/guc: Don't drop ce->guc_active.lock when unwinding context Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 05/25] drm/i915/guc: Process all G2H message at once in work queue Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 06/25] drm/i915/guc: Workaround reset G2H is received after schedule done G2H Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 07/25] Revert "drm/i915/gt: Propagate change in error status to children on unhold" Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-03 23:49   ` Daniele Ceraolo Spurio
2021-09-03 23:49     ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 08/25] drm/i915/guc: Kick tasklet after queuing a request Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 09/25] drm/i915/guc: Don't enable scheduling on a banned context, guc_id invalid, not registered Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 10/25] drm/i915/guc: Copy whole golden context, set engine state size of subset Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02 19:25   ` John Harrison
2021-09-02 19:25     ` [Intel-gfx] " John Harrison
2021-09-02  0:50 ` [Intel-gfx] [PATCH v5 11/25] drm/i915/selftests: Add initial GuC selftest for scrubbing lost G2H Daniele Ceraolo Spurio
2021-09-02  0:50   ` Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 12/25] drm/i915/guc: Take context ref when cancelling request Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` Daniele Ceraolo Spurio [this message]
2021-09-02  0:50   ` [PATCH v5 13/25] drm/i915/guc: Don't touch guc_state.sched_state without a lock Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 14/25] drm/i915/guc: Reset LRC descriptor if register returns -ENODEV Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [Intel-gfx] [PATCH v5 15/25] drm/i915: Allocate error capture in nowait context Daniele Ceraolo Spurio
2021-09-02  0:50   ` Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 16/25] drm/i915/guc: Flush G2H work queue during reset Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 17/25] drm/i915/guc: Release submit fence from an irq_work Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 18/25] drm/i915/guc: Move guc_blocked fence to struct guc_state Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 19/25] drm/i915/guc: Rework and simplify locking Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 20/25] drm/i915/guc: Proper xarray usage for contexts_lookup Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 21/25] drm/i915/guc: Drop pin count check trick between sched_disable and re-pin Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 22/25] drm/i915/guc: Move GuC priority fields in context under guc_active Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 23/25] drm/i915/guc: Move fields protected by guc->contexts_lock into sub structure Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 24/25] drm/i915/guc: Drop guc_active move everything into guc_state Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02  0:50 ` [PATCH v5 25/25] drm/i915/guc: Add GuC kernel doc Daniele Ceraolo Spurio
2021-09-02  0:50   ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-02 17:01   ` John Harrison
2021-09-02 17:01     ` [Intel-gfx] " John Harrison
2021-09-02 17:15     ` Daniele Ceraolo Spurio
2021-09-02 17:15       ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-03 19:59   ` Daniele Ceraolo Spurio
2021-09-03 19:59     ` [Intel-gfx] " Daniele Ceraolo Spurio
2021-09-08 23:38     ` John Harrison
2021-09-08 23:38       ` [Intel-gfx] " John Harrison
2021-09-02  1:04 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Clean up GuC CI failures, simplify locking, and kernel DOC (rev8) Patchwork
2021-09-02  1:05 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-09-02  1:32 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-09-02  2:46 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2021-09-03 20:12 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Clean up GuC CI failures, simplify locking, and kernel DOC (rev9) Patchwork
2021-09-03 20:13 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-09-03 20:42 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2021-09-04  0:15 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Clean up GuC CI failures, simplify locking, and kernel DOC (rev10) Patchwork
2021-09-04  0:17 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-09-04  0:46 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210902005022.711767-14-daniele.ceraolospurio@intel.com \
    --to=daniele.ceraolospurio@intel.com \
    --cc=John.C.Harrison@Intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=matthew.brost@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.