Re: [PATCH v6 1/7] drm/xe/gt: Use GT ordered workqueue for wedging

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/xe: Infinite TDR loop when device wedged during GT init
--

commit 6009cd9a52688d1928f2b5ef571d3108edd7243e
Author: Raag Jadav <raag.jadav@intel.com>

drm/xe/gt: Use GT ordered workqueue for wedging

This commit introduces a worker for GT specific wedge handling and queues the
teardown on the GT ordered workqueue. This prevents scheduler disruptions
while jobs are still in-flight. It also introduces the
for_each_initialized_gt() macro and uses it when declaring the device
wedged.

> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index e1979bbcf7bc6..014bc5b0216dd 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -1425,7 +1425,7 @@ void xe_device_declare_wedged(struct xe_device *xe)
>  			    WEDGED_URL, XE_BUG_URL);
>  	}
>  
> -	for_each_gt(gt, xe, id)
> +	for_each_initialized_gt(gt, xe, id)
>  		xe_gt_declare_wedged(gt);

[Severity: High]
Does this create a race condition if the device is wedged concurrently with
xe_gt_init()?

If xe_device_declare_wedged() runs while a GT is initializing, the GT is
skipped here because gt->initialized is false.

> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index 775c826b68b42..d8bf538ceafff 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
[ ... ]
> @@ -769,6 +773,8 @@ int xe_gt_init(struct xe_gt *gt)
>  			return err;
>  	}
>  
> +	/* Must be last */
> +	WRITE_ONCE(gt->initialized, true);
>  	return 0;
>  }

Then xe_gt_init() sets gt->initialized to true without checking if a global
wedge event occurred during initialization.

This allows the GT to become active even though its GuC was never wedged
(since xe_guc_submit_wedge() was skipped during xe_device_declare_wedged()).

If jobs submitted to this GT (like migration jobs) hang, the TDR in
guc_exec_queue_timedout_job() runs. Since the TDR detects the global wedge
via xe_wedged(), it skips the hardware reset but returns NO_HANG while
immediately re-queueing the TDR work:

drivers/gpu/drm/xe/xe_guc_submit.c:guc_exec_queue_timedout_job() {
    ...
    xe_guc_exec_queue_trigger_cleanup(q);

    if (wedge_device)
        xe_device_declare_wedged(gt_to_xe(q->gt));

    /*
     * We want the job added back to the pending list so it gets freed; this
     * is what DRM_GPU_SCHED_STAT_NO_HANG does.
     */
    return DRM_GPU_SCHED_STAT_NO_HANG;
}

Can this lead to an infinite TDR spinloop (100% CPU deadlock) since the job
is never completed nor is the hardware reset?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014055215.330408-1-raag.jadav@intel.com?part=1