Re: [PATCH v2 2/3] drm/xe/sa: Add lockdep annotations for SA manager swap_guard

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

Hi

On Wed, 2026-02-04 at 16:46 +0000, Satyanarayana K V P wrote:
> Annotate the SA manager init path to model taking swap_guard while
> under
> reclaim context. This helps lockdep catch potential circular
> dependencies
> between fs_reclaim and swap_guard in debug builds.
> 
> Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> Suggested-by: Matthew Brost <matthew.brost@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> 
> ---
> V1 -> V2:
> - None.
> ---
>  drivers/gpu/drm/xe/xe_sa.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_sa.c b/drivers/gpu/drm/xe/xe_sa.c
> index b738102575d4..5efbb5a09f77 100644
> --- a/drivers/gpu/drm/xe/xe_sa.c
> +++ b/drivers/gpu/drm/xe/xe_sa.c
> @@ -89,6 +89,12 @@ struct xe_sa_manager
> *__xe_sa_bo_manager_init(struct xe_tile *tile, u32 size,
>   if (ret)
>   return ERR_PTR(ret);
>  
> + if (IS_ENABLED(CONFIG_PROVE_LOCKING)) {
> + fs_reclaim_acquire(GFP_KERNEL);
> + might_lock(&sa_manager->swap_guard);
> + fs_reclaim_release(GFP_KERNEL);
> + }
> +
>   shadow = xe_managed_bo_create_pin_map(xe, tile, size,
>         XE_BO_FLAG_VRAM_IF_DGFX(tile) |
>         XE_BO_FLAG_GGTT |

Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

In addition to this, a couple of comments to the code that is already
in the driver:

It would be beneficial for understanding if a document section was
added for the typical usage flow of the shadow buffer, something like
the below (hope I got this correct).

*) Clearly stating the use-case: That the whole buffer is bound to HW
and can execute at any time. The shadow buffer is part of a double
buffering scheme so that updates are visible to the hardware
atomically.

*) The flow:
-lock()
-Swap buffers: The buffers are identical. The buffer bound to hardware
becomes the shadow.
-Update the primary buffer.
-Flush the cpu buffer to primary on DGFX (BTW IIRC this was missing in
the code). -Point the HW to the primary buffer.
-sync the shadow to the primary.
-unlock()

In addition perhaps more lockdep asserts an also perhaps pin the lock
in swap buffers and unpin in sync to shadow so that if anybody releases
the lock in between you'd get a warning.

But this can be done as a follow-up, (beware the possibly missing cpu
buffer flush, though). I think it's worth spending some time on this.

Thanks,
Thomas
   A.