Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Raag Jadav" <raag.jadav@intel.com>, intel-xe@lists.freedesktop.org
Subject: Re: [PATCH v5 1/5] drm/xe/gt: Use GT ordered workqueue for wedging
Date: Tue, 13 Oct 2026 11:19:03 +0000	[thread overview]
Message-ID: <sashiko-outbox-169604@kernel.org> (raw)
In-Reply-To: <20261013110439.181447-2-raag.jadav@intel.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] workqueue: self-deadlock in xe_gt_declare_wedged() when invoked from ordered_wq workers
- [High] xe: sleeping in atomic context via flush_work in xe_gt_declare_wedged
- [High] drm/xe: missing memory barriers when publishing gt->initialized allows workqueue corruption
--

commit f7181376c2cff07d590f539fd01106cc19cb1c8f
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 it
on the GT ordered workqueue to prevent disrupting the scheduler while jobs are
in-flight. It also introduces the for_each_initialized_gt() macro to avoid
races during probe sequences.

> diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
> --- a/drivers/gpu/drm/xe/xe_device.h
> +++ b/drivers/gpu/drm/xe/xe_device.h
> @@ -139,6 +139,10 @@ static inline bool xe_device_uc_enabled(struct xe_device *xe)
>  	for_each_gt((gt__), (tile__)->xe, (id__)) \
>  		for_each_if((gt__)->tile == (tile__))
>  
> +#define for_each_initialized_gt(gt__, xe__, id__) \
> +	for_each_gt((gt__), (xe__), (id__)) \
> +		for_each_if(READ_ONCE((gt__)->initialized))

[Severity: High]
Does this missing acquire memory barrier allow workqueue corruption?

If a hardware error or AMC alert triggers device wedging on a different CPU
concurrently with xe_gt_init(), the for_each_initialized_gt() macro checks
this flag. However, READ_ONCE() does not emit memory barriers on all
architectures. 

On weakly-ordered CPUs (like ARM64), a concurrent reader's speculative loads
could be hoisted before the gt->initialized check completes, causing
xe_gt_declare_wedged() to invoke queue_work() on an uninitialized workqueue or
work struct.

Should this use smp_load_acquire() and smp_store_release() to guarantee
proper ordering?

> +
>  static inline struct xe_force_wake *gt_to_fw(struct xe_gt *gt)
>  {
>  	return &gt->pm.fw;

[ ... ]

> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> --- 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);

[Severity: High]
Similar to the read side, WRITE_ONCE() lacks a release memory barrier. The
CPU could reorder this store before the INIT_WORK(&gt->wedge.worker) store
earlier in xe_gt_init(). Replacing this with smp_store_release() would
ensure the GT state initialization is visible before the flag is published.

>  	return 0;
>  }

[ ... ]

> @@ -1200,6 +1214,6 @@ void xe_gt_declare_wedged(struct xe_gt *gt)
>  {
>  	xe_gt_assert(gt, gt_to_xe(gt)->wedged.mode);
>  
> -	xe_uc_declare_wedged(&gt->uc);
> -	xe_tlb_inval_reset(&gt->tlb_inval);
> +	queue_work(gt->ordered_wq, &gt->wedge.worker);
> +	flush_work(&gt->wedge.worker);

[Severity: High]
Can this flush_work() call create a self-deadlock?

If a worker thread already executing on gt->ordered_wq (such as
gt_reset_worker() upon a reset failure) invokes xe_device_declare_wedged(),
it will eventually call xe_gt_declare_wedged().

Since gt->ordered_wq has a concurrency level of 1, queue_work() will place
wedge.worker behind the currently executing worker. The executing worker
will then block in flush_work(), waiting for wedge.worker to complete,
resulting in an unresolvable circular dependency that hangs the workqueue.

[Severity: High]
Does this also introduce a sleeping in atomic context regression?

xe_device_declare_wedged() can be called from mert_handle_cat_error(), which
is executed by the xe_mert_irq_handler() in hardirq context (for example,
directly from dg1_irq_handler()).

The flush_work() function relies on might_sleep(). Calling a sleeping
function from an atomic hardirq context will cause a 'scheduling while
atomic' kernel panic.

>  }

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

  reply	other threads:[~2026-10-13 11:19 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-13 11:03 [PATCH v5 0/5] Introduce xe_wedge Raag Jadav
2026-10-13 11:03 ` [PATCH v5 1/5] drm/xe/gt: Use GT ordered workqueue for wedging Raag Jadav
2026-10-13 11:19   ` sashiko-bot [this message]
2026-10-13 18:29   ` Rodrigo Vivi
2026-10-13 18:49     ` Matthew Brost
2026-10-14  4:25       ` Raag Jadav
2026-10-13 11:03 ` [PATCH v5 2/5] drm/xe: Drop xe_device_set_wedged_method() Raag Jadav
2026-10-13 11:20   ` sashiko-bot
2026-10-13 18:30   ` Rodrigo Vivi
2026-10-13 11:03 ` [PATCH v5 3/5] drm/xe: Move xe_device_wedged_fini() registration to xe_device_probe_early() Raag Jadav
2026-10-13 11:03 ` [PATCH v5 4/5] drm/xe: Introduce xe_wedge Raag Jadav
2026-10-13 11:16   ` sashiko-bot
2026-10-13 18:32   ` Rodrigo Vivi
2026-10-13 11:03 ` [PATCH v5 5/5] drm/xe/ras: Move xe_ras_process_errors() to xe_ras Raag Jadav
2026-10-13 11:22   ` sashiko-bot
2026-10-13 11:12 ` ✗ CI.checkpatch: warning for Introduce xe_wedge (rev4) Patchwork
2026-10-13 11:14 ` ✓ CI.KUnit: success " Patchwork
2026-10-13 15:40 ` ✓ Xe.CI.BAT: " Patchwork
2026-10-13 23:38 ` ✗ Xe.CI.FULL: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=sashiko-outbox-169604@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=raag.jadav@intel.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox