On Tue, Jan 17, 2023 at 01:36:26PM -0800, john.c.harri...@intel.com wrote:
> From: John Harrison <john.c.harri...@intel.com>
>
> 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.
> Fixes: 573ba126aef3 ("drm/i915/guc: Capture error state on context reset")
> Cc: Matthew Brost <matthew.br...@intel.com>
> Cc: John Harrison <john.c.harri...@intel.com>
> Cc: Jani Nikula <jani.nik...@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursu...@linux.intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com>
> Cc: Andrzej Hajda <andrzej.ha...@intel.com>
> Cc: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.a...@intel.com>
> Cc: Matt Roper <matthew.d.ro...@intel.com>
> Cc: Umesh Nerlige Ramappa <umesh.nerlige.rama...@intel.com>
> Cc: Michael Cheng <michael.ch...@intel.com>
> Cc: Lucas De Marchi <lucas.demar...@intel.com>
> Cc: Tejas Upadhyay <tejaskumarx.surendrakumar.upadh...@intel.com>
> Cc: Andy Shevchenko <andriy.shevche...@linux.intel.com>
> Cc: Aravind Iddamsetty <aravind.iddamse...@intel.com>
> Cc: Alan Previn <alan.previn.teres.ale...@intel.com>
> Cc: Bruce Chang <yu.bruce.ch...@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
Is it possible to utilize --to --cc parameters to git send-email instead of
noisy Cc list?
...
> + 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.
...
> @@ -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.
--
With Best Regards,
Andy Shevchenko
