[v2,0/3] Resolve suspend-resume racing with GuC destroy-context-worker

Message

Alan Previn Aug. 15, 2023, 1:12 a.m. UTC

This series is the result of debugging issues root caused to
races between the GuC's destroyed_worker_func being triggered
vs repeating suspend-resume cycles with concurrent delayed
fence signals for engine-freeing.

The reproduction steps require that an app is launched right
before the start of the suspend cycle where it creates a
new gem context and submits a tiny workload that would
complete in the middle of the suspend cycle. However this
app uses dma-buffer sharing or dma-fence with non-GPU
objects or signals that eventually triggers a FENCE_FREE
via__i915_sw_fence_notify that connects to engines_notify ->
free_engines_rcu -> intel_context_put ->
kref_put(&ce->ref..) that queues the worker after the GuCs
CTB has been disabled (i.e. after i915-gem's suspend-late).

This sequence is a corner-case and required repeating this
app->suspend->resume cycle ~1500 times across 4 identical
systems to see it once. That said, based on above callstack,
it is clear that merely flushing the context destruction worker,
which is obviously missing and needed, isn't sufficient.

Because of that, this series adds additional patches besides
the obvious (Patch #1) flushing of the worker during the
suspend flows. It also includes (Patch #2) closing a race
between sending the context-deregistration H2G vs the CTB
getting disabled in the midst of it (by detecing the failure
and unrolling the guc-lrc-unpin flow) and (Patch #32) not
infinitely waiting in intel_gt_pm_wait_timeout_for_idle
when in the suspend-flow.

Alan Previn (3):
  drm/i915/guc: Flush context destruction worker at suspend
  drm/i915/guc: Close deregister-context race against CT-loss
  drm/i915/gt: Timeout when waiting for idle in suspending

 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  2 +-
 drivers/gpu/drm/i915/gt/intel_gt_pm.c         |  7 ++-
 drivers/gpu/drm/i915/gt/intel_gt_pm.h         |  7 ++-
 .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 45 +++++++++++++++++--
 .../gpu/drm/i915/gt/uc/intel_guc_submission.h |  2 +
 drivers/gpu/drm/i915/gt/uc/intel_uc.c         |  2 +
 drivers/gpu/drm/i915/intel_wakeref.c          | 14 ++++--
 drivers/gpu/drm/i915/intel_wakeref.h          |  5 ++-
 8 files changed, 71 insertions(+), 13 deletions(-)


base-commit: 85f20fb339f05ec4221bb295c13e46061c5c566f

Comments

Alan Previn Aug. 15, 2023, 1:20 a.m. UTC | #1

On Mon, 2023-08-14 at 18:12 -0700, Teres Alexis, Alan Previn wrote:
> This series is the result of debugging issues root caused to
> races between the GuC's destroyed_worker_func being triggered
> vs repeating suspend-resume cycles with concurrent delayed
> fence signals for engine-freeing.
alan: forgot credit:
Tested-by: Mousumi Jana <[e-mail]>

alan:snip.
> 
> 
> Alan Previn (3):
>   drm/i915/guc: Flush context destruction worker at suspend
>   drm/i915/guc: Close deregister-context race against CT-loss
>   drm/i915/gt: Timeout when waiting for idle in suspending
> 
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c     |  2 +-
>  drivers/gpu/drm/i915/gt/intel_gt_pm.c         |  7 ++-
>  drivers/gpu/drm/i915/gt/intel_gt_pm.h         |  7 ++-
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 45 +++++++++++++++++--
>  .../gpu/drm/i915/gt/uc/intel_guc_submission.h |  2 +
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c         |  2 +
>  drivers/gpu/drm/i915/intel_wakeref.c          | 14 ++++--
>  drivers/gpu/drm/i915/intel_wakeref.h          |  5 ++-
>  8 files changed, 71 insertions(+), 13 deletions(-)
> 
> 
> base-commit: 85f20fb339f05ec4221bb295c13e46061c5c566f