Re: [PATCH v3 2/3] drm/xe/vf: Fix fs_reclaim warning with CCS save/restore BB allocation

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 2/10/2026 11:59 AM, Satyanarayana K V P wrote:
> CCS save/restore batch buffers are attached during BO allocation and
> detached during BO teardown. The shrinker triggers xe_bo_move(), which is
> used for both allocation and deletion paths.
> 
> When BO allocation and shrinking occur concurrently, a circular locking
> dependency involving fs_reclaim and swap_guard can occur, leading to a
> deadlock such as:
> 
> ======================================================
> WARNING: possible circular locking dependency detected
> ------------------------------------------------------
> 
>       CPU0                    CPU1
>       ----                    ----
>  lock(fs_reclaim);
>                               lock(&sa_manager->swap_guard);
>                               lock(fs_reclaim);
>  lock(&sa_manager->swap_guard);
> 
>  *** DEADLOCK ***
> =====================================================
> 
> To avoid this, the BB pointer and SA are allocated using xe_bb_alloc()
> before taking lock and SA is initialized using xe_bb_init() preventing
> reclaim from being invoked in this context.
> 
> Fixes: 864690cf4dd62 ("drm/xe/vf: Attach and detach CCS copy commands with BO")
> Signed-off-by: Satyanarayana K V P <satyanarayana.k.v.p@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> 
> ---
> V2 -> V3:
> - Created new functions xe_sa_bo_alloc(), xe_sa_bo_release() and
> xe_sa_bo_init(). (Thomas)
> - Created new functions xe_bb_alloc(), xe_bb_release and
> xe_bb_init(). (Thomas)
> - Updated guard() to scoped_guard() in xe_migrate_ccs_rw_copy(). (Thomas)
> 
> V1 -> V2:
> - Used drm_suballoc_alloc() and drm_suballoc_init() for BB allocation
> (Thomas).
> ---
>  drivers/gpu/drm/xe/xe_bb.c      | 49 +++++++++++------
>  drivers/gpu/drm/xe/xe_bb.h      |  7 ++-
>  drivers/gpu/drm/xe/xe_migrate.c | 96 ++++++++++++++++++---------------
>  drivers/gpu/drm/xe/xe_sa.c      | 40 ++++++++++++++
>  drivers/gpu/drm/xe/xe_sa.h      |  3 ++
>  5 files changed, 135 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bb.c b/drivers/gpu/drm/xe/xe_bb.c
> index 8b678297aaa2..631ae564e719 100644
> --- a/drivers/gpu/drm/xe/xe_bb.c
> +++ b/drivers/gpu/drm/xe/xe_bb.c
> @@ -59,16 +59,43 @@ struct xe_bb *xe_bb_new(struct xe_gt *gt, u32 dwords, bool usm)
>  	return ERR_PTR(err);
>  }
>  
> -struct xe_bb *xe_bb_ccs_new(struct xe_gt *gt, u32 dwords,
> -			    enum xe_sriov_vf_ccs_rw_ctxs ctx_id)

shouldn't we add kernel-doc for all new/updated public functions?

> +struct xe_bb *xe_bb_alloc(struct xe_gt *gt)
>  {
>  	struct xe_bb *bb = kmalloc(sizeof(*bb), GFP_KERNEL);
>  	struct xe_device *xe = gt_to_xe(gt);
> -	struct xe_sa_manager *bb_pool;
>  	int err;
>  
>  	if (!bb)
>  		return ERR_PTR(-ENOMEM);
> +
> +	bb->bo = xe_sa_bo_alloc(GFP_KERNEL);
> +	if (IS_ERR(bb->bo)) {
> +		drm_err(&xe->drm, "Sub-allocator memory allocation failed with %ld\n",

nit: there is xe_err(xe, ...)

nit: we try to print errors using %pe, which here is even more appropriate

nit: maybe "Failed to allocate SA object for BO (%pe)\n" ?

nit: or maybe any error message should be in xe_sa_bo_alloc()

> +			PTR_ERR(bb->bo));
> +		err = PTR_ERR(bb->bo);
> +		goto err;
> +	}
> +
> +	return bb;
> +
> +err:
> +	kfree(bb);
> +	return ERR_PTR(err);
> +}
> +

kernel-doc

> +void xe_bb_release(struct xe_bb *bb)
> +{
> +	if (bb->bo)

do we need this?
in xe_bb_alloc() we guarantee that bb will either have valid .bo or no bb at all

> +		xe_sa_bo_release(bb->bo);
> +
> +	kfree(bb);
> +}
> +

kernel-doc

> +int xe_bb_init(struct xe_gt *gt, struct xe_bb *bb,

as this is xe_bb function, it should take xe_bb as first param

> +	       struct xe_sa_manager *bb_pool, u32 dwords)
> +{
> +	int err;
> +
>  	/*
>  	 * We need to allocate space for the requested number of dwords &
>  	 * one additional MI_BATCH_BUFFER_END dword. Since the whole SA
> @@ -76,22 +103,14 @@ struct xe_bb *xe_bb_ccs_new(struct xe_gt *gt, u32 dwords,
>  	 * is not over written when the last chunk of SA is allocated for BB.
>  	 * So, this extra DW acts as a guard here.
>  	 */
> -
> -	bb_pool = xe->sriov.vf.ccs.contexts[ctx_id].mem.ccs_bb_pool;
> -	bb->bo = xe_sa_bo_new(bb_pool, 4 * (dwords + 1));
> -
> -	if (IS_ERR(bb->bo)) {
> -		err = PTR_ERR(bb->bo);
> -		goto err;
> -	}
> +	err = xe_sa_bo_init(bb_pool, bb->bo, 4 * (dwords + 1));
> +	if (err)
> +		return err;
>  
>  	bb->cs = xe_sa_bo_cpu_addr(bb->bo);
>  	bb->len = 0;
>  
> -	return bb;
> -err:
> -	kfree(bb);
> -	return ERR_PTR(err);
> +	return 0;
>  }
>  
>  static struct xe_sched_job *
> diff --git a/drivers/gpu/drm/xe/xe_bb.h b/drivers/gpu/drm/xe/xe_bb.h
> index 2a8adc9a6dee..3eb80925bfd1 100644
> --- a/drivers/gpu/drm/xe/xe_bb.h
> +++ b/drivers/gpu/drm/xe/xe_bb.h
> @@ -13,11 +13,14 @@ struct dma_fence;
>  struct xe_gt;
>  struct xe_exec_queue;
>  struct xe_sched_job;
> +struct xe_sa_manager;

wrong order

>  enum xe_sriov_vf_ccs_rw_ctxs;
>  
>  struct xe_bb *xe_bb_new(struct xe_gt *gt, u32 dwords, bool usm);
> -struct xe_bb *xe_bb_ccs_new(struct xe_gt *gt, u32 dwords,
> -			    enum xe_sriov_vf_ccs_rw_ctxs ctx_id);
> +struct xe_bb *xe_bb_alloc(struct xe_gt *gt);
> +void xe_bb_release(struct xe_bb *bb);
> +int xe_bb_init(struct xe_gt *gt, struct xe_bb *bb,
> +	       struct xe_sa_manager *bb_pool, u32 dwords);
>  struct xe_sched_job *xe_bb_create_job(struct xe_exec_queue *q,
>  				      struct xe_bb *bb);
>  struct xe_sched_job *xe_bb_create_migration_job(struct xe_exec_queue *q,
> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index 078a9bc2821d..c858eaa70e3e 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
> @@ -1148,65 +1148,75 @@ int xe_migrate_ccs_rw_copy(struct xe_tile *tile, struct xe_exec_queue *q,
>  		size -= src_L0;
>  	}
>  
> -	bb_pool = ctx->mem.ccs_bb_pool;
> -	guard(mutex) (xe_sa_bo_swap_guard(bb_pool));
> -	xe_sa_bo_swap_shadow(bb_pool);
> -
> -	bb = xe_bb_ccs_new(gt, batch_size, read_write);
> +	bb = xe_bb_alloc(gt);
>  	if (IS_ERR(bb)) {
> -		drm_err(&xe->drm, "BB allocation failed.\n");
>  		err = PTR_ERR(bb);
>  		return err;

nit: this could be

		return PTR_ERR(bb);

>  	}
>  
> -	batch_size_allocated = batch_size;
> -	size = xe_bo_size(src_bo);
> -	batch_size = 0;
> +	bb_pool = ctx->mem.ccs_bb_pool;
> +	scoped_guard(mutex, xe_sa_bo_swap_guard(bb_pool)) {
> +		xe_sa_bo_swap_shadow(bb_pool);
> +
> +		err = xe_bb_init(gt, bb, bb_pool, batch_size);
> +		if (err) {
> +			drm_err(&xe->drm, "BB allocation failed.\n");

nit: there is xe_err() but since there is a gt maybe it should be xe_gt_err() ?

nit: or maybe move that message to xe_bb_init() ?

> +			xe_bb_release(bb);
> +			return err;
> +		}
>  
> -	/*
> -	 * Emit PTE and copy commands here.
> -	 * The CCS copy command can only support limited size. If the size to be
> -	 * copied is more than the limit, divide copy into chunks. So, calculate
> -	 * sizes here again before copy command is emitted.
> -	 */
> -	while (size) {
> -		batch_size += 10; /* Flush + ggtt addr + 2 NOP */
> -		u32 flush_flags = 0;
> -		u64 ccs_ofs, ccs_size;
> -		u32 ccs_pt;
> +		batch_size_allocated = batch_size;
> +		size = xe_bo_size(src_bo);
> +		batch_size = 0;
>  
> -		u32 avail_pts = max_mem_transfer_per_pass(xe) / LEVEL0_PAGE_TABLE_ENCODE_SIZE;
> +		/*
> +		 * Emit PTE and copy commands here.
> +		 * The CCS copy command can only support limited size. If the size to be
> +		 * copied is more than the limit, divide copy into chunks. So, calculate
> +		 * sizes here again before copy command is emitted.
> +		 */
>  
> -		src_L0 = xe_migrate_res_sizes(m, &src_it);
> +		while (size) {
> +			batch_size += 10; /* Flush + ggtt addr + 2 NOP */
> +			u32 flush_flags = 0;
> +			u64 ccs_ofs, ccs_size;
> +			u32 ccs_pt;
>  
> -		batch_size += pte_update_size(m, false, src, &src_it, &src_L0,
> -					      &src_L0_ofs, &src_L0_pt, 0, 0,
> -					      avail_pts);
> +			u32 avail_pts = max_mem_transfer_per_pass(xe) /
> +					LEVEL0_PAGE_TABLE_ENCODE_SIZE;
>  
> -		ccs_size = xe_device_ccs_bytes(xe, src_L0);
> -		batch_size += pte_update_size(m, 0, NULL, &ccs_it, &ccs_size, &ccs_ofs,
> -					      &ccs_pt, 0, avail_pts, avail_pts);
> -		xe_assert(xe, IS_ALIGNED(ccs_it.start, PAGE_SIZE));
> -		batch_size += EMIT_COPY_CCS_DW;
> +			src_L0 = xe_migrate_res_sizes(m, &src_it);
> +
> +			batch_size += pte_update_size(m, false, src, &src_it, &src_L0,
> +						      &src_L0_ofs, &src_L0_pt, 0, 0,
> +						      avail_pts);
> +
> +			ccs_size = xe_device_ccs_bytes(xe, src_L0);
> +			batch_size += pte_update_size(m, 0, NULL, &ccs_it, &ccs_size, &ccs_ofs,
> +						      &ccs_pt, 0, avail_pts, avail_pts);
> +			xe_assert(xe, IS_ALIGNED(ccs_it.start, PAGE_SIZE));
> +			batch_size += EMIT_COPY_CCS_DW;
>  
> -		emit_pte(m, bb, src_L0_pt, false, true, &src_it, src_L0, src);
> +			emit_pte(m, bb, src_L0_pt, false, true, &src_it, src_L0, src);
>  
> -		emit_pte(m, bb, ccs_pt, false, false, &ccs_it, ccs_size, src);
> +			emit_pte(m, bb, ccs_pt, false, false, &ccs_it, ccs_size, src);
>  
> -		bb->len = emit_flush_invalidate(bb->cs, bb->len, flush_flags);
> -		flush_flags = xe_migrate_ccs_copy(m, bb, src_L0_ofs, src_is_pltt,
> -						  src_L0_ofs, dst_is_pltt,
> -						  src_L0, ccs_ofs, true);
> -		bb->len = emit_flush_invalidate(bb->cs, bb->len, flush_flags);
> +			bb->len = emit_flush_invalidate(bb->cs, bb->len, flush_flags);
> +			flush_flags = xe_migrate_ccs_copy(m, bb, src_L0_ofs, src_is_pltt,
> +							  src_L0_ofs, dst_is_pltt,
> +							  src_L0, ccs_ofs, true);
> +			bb->len = emit_flush_invalidate(bb->cs, bb->len, flush_flags);
>  
> -		size -= src_L0;
> -	}
> +			size -= src_L0;
> +		}
>  
> -	xe_assert(xe, (batch_size_allocated == bb->len));
> -	src_bo->bb_ccs[read_write] = bb;
> +		xe_assert(xe, (batch_size_allocated == bb->len));
> +		src_bo->bb_ccs[read_write] = bb;
> +
> +		xe_sriov_vf_ccs_rw_update_bb_addr(ctx);
> +		xe_sa_bo_sync_shadow(bb->bo);
> +	}
>  
> -	xe_sriov_vf_ccs_rw_update_bb_addr(ctx);
> -	xe_sa_bo_sync_shadow(bb->bo);
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/xe/xe_sa.c b/drivers/gpu/drm/xe/xe_sa.c
> index b738102575d4..59d0187b3e82 100644
> --- a/drivers/gpu/drm/xe/xe_sa.c
> +++ b/drivers/gpu/drm/xe/xe_sa.c
> @@ -175,6 +175,46 @@ struct drm_suballoc *__xe_sa_bo_new(struct xe_sa_manager *sa_manager, u32 size,
>  	return drm_suballoc_new(&sa_manager->base, size, gfp, true, 0);
>  }
>  
> +/**
> + * xe_sa_bo_alloc - Allocate uninitialized suballoc object.
> + * @gfp: gfp flags used for memory allocation.
> + *
> + * Allocate memory for an uninitialized suballoc object. Intended usage is
> + * allocate memory for suballoc object outside of a reclaim tainted context
> + * and then be initialized at a later time in a reclaim tainted context.
> + *
> + * Return: a new uninitialized suballoc object, or an ERR_PTR(-ENOMEM).
> + */
> +

extra \n

> +struct drm_suballoc *xe_sa_bo_alloc(gfp_t gfp)
> +{
> +	return drm_suballoc_alloc(gfp);
> +}
> +
> +/**
> + * xe_sa_bo_release - Release memory for suballocation.

nit: add () to function name

    * xe_sa_bo_release() - ...


> + * @sa: The struct drm_suballoc.
> + */
> +void xe_sa_bo_release(struct drm_suballoc *sa)
> +{
> +	drm_suballoc_release(sa);
> +}
> +
> +/**
> + * xe_sa_bo_init - Initialize a suballocation.

ditto

> + * @sa_manager: pointer to the sa_manager
> + * @sa: The struct drm_suballoc.
> + * @size: number of bytes we want to suballocate.
> + *
> + * Try to make a suballocation on a pre-allocated suballoc object of size @size.
> + *
> + * Return: zero on success, errno on failure.
> + */
> +int xe_sa_bo_init(struct xe_sa_manager *sa_manager, struct drm_suballoc *sa, u32 size)

why size is u32 ? drm_suballoc_init() takes size_t

> +{
> +	return drm_suballoc_init(&sa_manager->base, sa, size, true, 0);
> +}
> +
>  /**
>   * xe_sa_bo_flush_write() - Copy the data from the sub-allocation to the GPU memory.
>   * @sa_bo: the &drm_suballoc to flush
> diff --git a/drivers/gpu/drm/xe/xe_sa.h b/drivers/gpu/drm/xe/xe_sa.h
> index 05e9a4e00e78..19d4b698a7d7 100644
> --- a/drivers/gpu/drm/xe/xe_sa.h
> +++ b/drivers/gpu/drm/xe/xe_sa.h
> @@ -38,6 +38,9 @@ static inline struct drm_suballoc *xe_sa_bo_new(struct xe_sa_manager *sa_manager
>  	return __xe_sa_bo_new(sa_manager, size, GFP_KERNEL);
>  }
>  
> +struct drm_suballoc *xe_sa_bo_alloc(gfp_t gfp);
> +void xe_sa_bo_release(struct drm_suballoc *sa);
> +int xe_sa_bo_init(struct xe_sa_manager *sa_manager, struct drm_suballoc *sa, u32 size);
>  void xe_sa_bo_flush_write(struct drm_suballoc *sa_bo);
>  void xe_sa_bo_sync_read(struct drm_suballoc *sa_bo);
>  void xe_sa_bo_free(struct drm_suballoc *sa_bo, struct dma_fence *fence);