All of 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 4/5] drm/xe: Introduce xe_wedge
Date: Tue, 13 Oct 2026 11:16:25 +0000	[thread overview]
Message-ID: <sashiko-outbox-169599@kernel.org> (raw)
In-Reply-To: <20261013110439.181447-5-raag.jadav@intel.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] workqueue: deadlock in xe_gt_wedge_declare via ordered_wq self-blocking
- [High] locking: flush_work() called from hardirq context in xe_mert_irq_handler
- [Low] build: incomplete enum xe_wedged_mode passed by value in header
--

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

drm/xe: Introduce xe_wedge

Consolidate wedging implementation into a dedicated xe_wedge component
for better maintainability. While at it, rename all wedge helpers
to match with component name.

> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index 775c826b68b42..ba10434763381 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
[ ... ]
> @@ -986,7 +993,7 @@ static void gt_reset_worker(struct work_struct *w)
>  
>  err_fail:
>  	xe_log_err_fatal(gt, GT, err, "reset failed\n");
> -	xe_device_declare_wedged(gt_to_xe(gt));
> +	xe_wedge_declare(gt_to_xe(gt), DRM_WEDGE_RECOVERY_REBIND | DRM_WEDGE_RECOVERY_BUS_RESET);

[Severity: High]
Can this sequence cause a deadlock? The gt_reset_worker is already executing
on the gt->ordered_wq workqueue. Calling xe_wedge_declare() eventually calls
xe_gt_wedge_declare() which queues gt->wedge.worker to the same ordered_wq
and then calls flush_work(). If the ordered workqueue has max_active=1, won't
it block permanently waiting for the newly queued item? A similar situation
exists in vf_post_migration_recovery().

>  err_pm_put:
>  	xe_pm_runtime_put(gt_to_xe(gt));
>  }
[ ... ]
> @@ -1190,16 +1205,16 @@ struct xe_hw_engine *xe_gt_any_hw_engine(struct xe_gt *gt)
>  }
>  
>  /**
> - * xe_gt_declare_wedged() - Declare GT wedged
> + * xe_gt_wedge_declare() - Declare GT wedged
>   * @gt: the GT object
>   *
>   * Wedge the GT which stops all submission, saves desired debug state, and
>   * cleans up anything which could timeout.
>   */
> -void xe_gt_declare_wedged(struct xe_gt *gt)
> +void xe_gt_wedge_declare(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]
Does this flush_work() execute in hardirq context? The function
xe_mert_irq_handler() in xe_mert.c runs in hardirq context and calls
mert_handle_cat_error(), which calls xe_wedge_declare(), ultimately leading
here. Calling a sleeping function from atomic context will cause issues.

>  }
> diff --git a/drivers/gpu/drm/xe/xe_wedge.h b/drivers/gpu/drm/xe/xe_wedge.h
> new file mode 100644
> index 0000000000000..5fda8ce015d49
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_wedge.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2026 Intel Corporation
> + */
> +
> +#ifndef _XE_WEDGE_H_
> +#define _XE_WEDGE_H_
> +
> +#include <linux/types.h>
> +
> +struct xe_device;
> +enum xe_wedged_mode;
> +
> +bool xe_wedged(struct xe_device *xe);
> +void xe_wedge_declare(struct xe_device *xe, unsigned long method);
> +int xe_wedge_init(struct xe_device *xe);
> +int xe_wedge_set_mode(struct xe_device *xe, enum xe_wedged_mode mode);

[Severity: Low]
Is this prototype valid according to the C standard? The incomplete enum
xe_wedged_mode is being passed by value. Including xe_wedge_types.h directly
should resolve this build issue.

> +
> +#endif

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

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

Thread overview: 18+ 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
2026-10-13 18:29   ` Rodrigo Vivi
2026-10-13 18:49     ` Matthew Brost
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 [this message]
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-169599@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.