Re: [Intel-gfx] [PATCH] drm/i915/guc: Use correct context lock when callig clr_context_registered

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 09/12/2021 19:14, Daniele Ceraolo Spurio wrote:


On 12/9/2021 10:48 AM, Matthew Brost wrote:
s/ce/cn/ when grabbing guc_state.lock before calling
clr_context_registered.

Fixes: 0f7976506de61 ("drm/i915/guc: Rework and simplify locking")
Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx>

I think Cc: stable is not needed here:

$ git tag --contains 0f7976506de61
drm-intel-fixes-2021-11-18
drm-intel-gt-next-2021-10-08
drm-intel-gt-next-2021-10-21
drm-intel-gt-next-2021-11-22
drm-intel-next-2021-10-15
drm-intel-next-fixes-2021-11-09
v5.16-rc1
v5.16-rc2
v5.16-rc3
v5.16-rc4

So still can hit 5.16 via fixes. Rodrigo, did I get this right and you will be able to pick it up next week or so?

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>

I'm assuming we didn't see any splat from the lockdep assert in clr_context_registered in our CI runs because we never hit this case as it requires 64k+ contexts. Maybe we can add a selftest to purposely exercise this path? Not a blocker for merging this fix.

Was the bug found by inspection or reported?

Given the buggy function is called steal_guc_id, so if the implication is there is no testing for guc id stealing, then it indeed please add some coverage ASAP.

Regards,

Tvrtko


Daniele

---
  drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 4 ++--
  1 file changed, 2 insertions(+), 2 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 1f9d4fde421f..9b7b4f4e0d91 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1937,9 +1937,9 @@ static int steal_guc_id(struct intel_guc *guc, struct intel_context *ce)
          list_del_init(&cn->guc_id.link);
          ce->guc_id = cn->guc_id;
-        spin_lock(&ce->guc_state.lock);
+        spin_lock(&cn->guc_state.lock);
          clr_context_registered(cn);
-        spin_unlock(&ce->guc_state.lock);
+        spin_unlock(&cn->guc_state.lock);
          set_context_guc_id_invalid(cn);




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux