On 31/10/2022 10:09, Tvrtko Ursulin wrote:
On 28/10/2022 20:46, John.C.Harrison@xxxxxxxxx wrote:
From: John Harrison <John.C.Harrison@xxxxxxxxx>
The engine busyness stats has a worker function to do things like
64bit extend the 32bit hardware counters. The GuC's reset prepare
function flushes out this worker function to ensure no corruption
happens during the reset. Unforunately, the worker function has an
infinite wait for active resets to finish before doing its work. Thus
a deadlock would occur if the worker function had actually started
just as the reset starts.
Update the worker to abort if a reset is in progress rather than
waiting for it to complete. It will still acquire the reset lock in
the case where a reset was not already in progress. So the processing
is still safe from corruption, but the deadlock can no longer occur.
Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
---
drivers/gpu/drm/i915/gt/intel_reset.c | 15 ++++++++++++++-
drivers/gpu/drm/i915/gt/intel_reset.h | 1 +
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 6 ++++--
3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c
b/drivers/gpu/drm/i915/gt/intel_reset.c
index 3159df6cdd492..2f48c6e4420ea 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -1407,7 +1407,7 @@ void intel_gt_handle_error(struct intel_gt *gt,
intel_runtime_pm_put(gt->uncore->rpm, wakeref);
}
-int intel_gt_reset_trylock(struct intel_gt *gt, int *srcu)
+static int _intel_gt_reset_trylock(struct intel_gt *gt, int *srcu,
bool retry)
{
might_lock(>->reset.backoff_srcu);
might_sleep();
@@ -1416,6 +1416,9 @@ int intel_gt_reset_trylock(struct intel_gt *gt,
int *srcu)
while (test_bit(I915_RESET_BACKOFF, >->reset.flags)) {
rcu_read_unlock();
+ if (!retry)
+ return -EBUSY;
+
if (wait_event_interruptible(gt->reset.queue,
!test_bit(I915_RESET_BACKOFF,
>->reset.flags)))
Would it be more obvious to rename the existing semantics to
intel_gt_reset_interruptible(), while the flavour you add in this patch
truly is trylock? I am not sure, since it's all a bit special, but
trylock sure feels confusing if it can sleep forever...
Oh and might_sleep() shouldn't be there with the trylock version - I
mean any flavour of the real trylock.
Regards,
Tvrtko
