On 1/18/2023 00:29, Andy Shevchenko wrote:
On Tue, Jan 17, 2023 at 01:36:26PM -0800, John.C.Harrison@xxxxxxxxx wrote:
From: John Harrison <John.C.Harrison@xxxxxxxxx>
When GuC support was added to error capture, the locking around the
request object was broken. Fix it up.
The context based search manages the spinlocking around the search
internally. So it needs to grab the reference count internally as
well. The execlist only request based search relies on external
locking, so it needs an external reference count. So no change to that
code itself but the context version does change.
The only other caller is the code for dumping engine state to debugfs.
That code wasn't previously getting an explicit reference at all as it
does everything while holding the execlist specific spinlock. So that
needs updaing as well as that spinlock doesn't help when using GuC
submission. Rather than trying to conditionally get/put depending on
submission model, just change it to always do the get/put.
In addition, intel_guc_find_hung_context() was not acquiring the
correct spinlock before searching the request list. So fix that up too.
Fixes: dc0dad365c5e ("drm/i915/guc: Fix for error capture after full GPU reset
with GuC")
Must be one line.
In my tree it is one line. git itself does the line wrap when creating
the email. I missed that I need to manually unwrap it again before
actually sending the email. Although the CI checkpatch also pointed this
out in it's own obscure manner.
Fixes: 573ba126aef3 ("drm/i915/guc: Capture error state on context reset")
Cc: Matthew Brost <matthew.brost@xxxxxxxxx>
Cc: John Harrison <John.C.Harrison@xxxxxxxxx>
Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Andrzej Hajda <andrzej.hajda@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Matthew Auld <matthew.auld@xxxxxxxxx>
Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx>
Cc: Michael Cheng <michael.cheng@xxxxxxxxx>
Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
Cc: Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@xxxxxxxxx>
Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Cc: Aravind Iddamsetty <aravind.iddamsetty@xxxxxxxxx>
Cc: Alan Previn <alan.previn.teres.alexis@xxxxxxxxx>
Cc: Bruce Chang <yu.bruce.chang@xxxxxxxxx>
Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
Is it possible to utilize --to --cc parameters to git send-email instead of
noisy Cc list?
This is the list auto-generated by the 'dim fixes' tool. I am told this
is the officially correct way to create a fixes patch - copy the output
from 'dim fixes' as is into the patch headers.
...
+ if (hung_rq)
+ i915_request_put(hung_rq);
In Linux kernel the idiom is that freeing resources APIs should be NULL-aware
(or ERR_PTR aware or both). Does i915 follows that? If so, the test should be
inside i915_request_put() rather than in any of the callers.
That's as maybe. But this is how i915_request_put() currently works - it
is simply a wrapper around 'dma_fence_put(&rq->fence);'. So passing in a
null pointer will immediately cause a null pointer deref. If you want
the put implementation to change and to re-work all its callers, that
should be done in a separate patch and not piled on top of other changes.
...
@@ -4847,6 +4857,7 @@ void intel_guc_find_hung_context(struct intel_engine_cs *engine)
xa_lock(&guc->context_lookup);
goto done;
}
+
next:
intel_context_put(ce);
xa_lock(&guc->context_lookup);
Stray change.
Intentional change to improve the readability of a function that is
being modified by other changes in this patch.
John.
