[RFC PATCH 1/1] drm/xe: keep VM-bound WC BOs resident during reclaim

[RFC PATCH 1/1] drm/xe: keep VM-bound WC BOs resident during reclaim

[Neil Zhong]

On x86, restoring a backed-up write-combined (WC) buffer object can be
expensive. The restore path allocates WB pages and converts them with
set_pages_array_wc(), which performs synchronous cache and TLB flushes.

Xe currently allows its shrinker to back up the pages of a WC BO while
the BO still has GPUVA mappings created by VM_BIND. A later validation
restores the pages while holding the BO's dma-resv. The cache-attribute
conversion then serializes EXEC, VM_BIND and dma-buf users on that
reservation object.

This was observed as intermittent HDR 4K60 playback stalls on two
Panther Lake systems with 8 GiB of memory. In a pre-change reproducer,
the maximum ioctl latencies were 549 ms for XE_EXEC, 291 ms for
XE_VM_BIND and 343 ms for DMA-BUF IMPORT. ttm_tt_restore reached 80.6 ms.

Keep a non-purgeable WC BO resident while it has at least one GPUVA
mapping. Purgeable BOs are still discarded, and after the last
VM_UNBIND the BO becomes reclaimable again. Add an A/B module parameter
which can restore the old behavior.

With the change, a 21-minute capture had no XE_EXEC, XE_VM_BIND or
DMA-BUF ioctl over the 16.7 ms frame interval. Their respective maxima
were 348 us, 136 us and 20 us. The ttm_tt_restore maximum was 8.33 ms,
and the set_pages_array_wc call rate fell from 14.64/s to 0.179/s.

The change intentionally trades reclaimable memory for latency while a
WC BO remains mapped. It does not take an additional BO reference or
change teardown: VM destruction and process exit remove the GPUVA
mappings and drop their existing references. During testing,
MemAvailable remained near 3 GiB and the dma-buf working set released
five 24 MiB surfaces while playback continued.

Signed-off-by: Neil Zhong <neil.zhong@ugreen.com>
---
 drivers/gpu/drm/xe/xe_defaults.h |  1 +
 drivers/gpu/drm/xe/xe_module.c   |  6 ++++++
 drivers/gpu/drm/xe/xe_module.h   |  2 +-
 drivers/gpu/drm/xe/xe_shrinker.c | 28 ++++++++++++++++++++++++++++
 4 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_defaults.h b/drivers/gpu/drm/xe/xe_defaults.h
index c8ae1d5f..645e289f 100644
--- a/drivers/gpu/drm/xe/xe_defaults.h
+++ b/drivers/gpu/drm/xe/xe_defaults.h
@@ -22,5 +22,6 @@
 #define XE_DEFAULT_WEDGED_MODE			XE_WEDGED_MODE_UPON_CRITICAL_ERROR
 #define XE_DEFAULT_WEDGED_MODE_STR		"upon-critical-error"
 #define XE_DEFAULT_SVM_NOTIFIER_SIZE		512
+#define XE_DEFAULT_ALLOW_BOUND_WC_SHRINK	false
 
 #endif
diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c
index 848d6526..67bcaf54 100644
--- a/drivers/gpu/drm/xe/xe_module.c
+++ b/drivers/gpu/drm/xe/xe_module.c
@@ -22,6 +22,7 @@
 #include "xe_sched_job.h"
 
 struct xe_modparam xe_modparam = {
+	.allow_bound_wc_shrink = XE_DEFAULT_ALLOW_BOUND_WC_SHRINK,
 	.probe_display =	XE_DEFAULT_PROBE_DISPLAY,
 	.guc_log_level =	XE_DEFAULT_GUC_LOG_LEVEL,
 	.force_probe =		XE_DEFAULT_FORCE_PROBE,
@@ -33,6 +34,11 @@ struct xe_modparam xe_modparam = {
 	/* the rest are 0 by default */
 };
 
+module_param_named(allow_bound_wc_shrink, xe_modparam.allow_bound_wc_shrink,
+		   bool, 0600);
+MODULE_PARM_DESC(allow_bound_wc_shrink,
+		 "Permit reclaim of VM-bound write-combined BOs");
+
 module_param_named(svm_notifier_size, xe_modparam.svm_notifier_size, uint, 0600);
 MODULE_PARM_DESC(svm_notifier_size, "Set the svm notifier size in MiB, must be power of 2 "
 		 "[default=" __stringify(XE_DEFAULT_SVM_NOTIFIER_SIZE) "]");
diff --git a/drivers/gpu/drm/xe/xe_module.h b/drivers/gpu/drm/xe/xe_module.h
index a0eb7db0..bf2c49ce 100644
--- a/drivers/gpu/drm/xe/xe_module.h
+++ b/drivers/gpu/drm/xe/xe_module.h
@@ -12,6 +12,7 @@ struct work_struct;
 
 /* Module modprobe variables */
 struct xe_modparam {
+	bool allow_bound_wc_shrink;
 	bool probe_display;
 	int force_vram_bar_size;
 	int guc_log_level;
@@ -32,4 +33,3 @@ bool xe_destroy_wq_queue(struct work_struct *work);
 void xe_destroy_wq_flush(void);
 
 #endif
-
diff --git a/drivers/gpu/drm/xe/xe_shrinker.c b/drivers/gpu/drm/xe/xe_shrinker.c
index 83374cd5..caa39f39 100644
--- a/drivers/gpu/drm/xe/xe_shrinker.c
+++ b/drivers/gpu/drm/xe/xe_shrinker.c
@@ -11,6 +11,7 @@
 #include <drm/ttm/ttm_tt.h>
 
 #include "xe_bo.h"
+#include "xe_module.h"
 #include "xe_pm.h"
 #include "xe_shrinker.h"
 
@@ -54,6 +55,30 @@ xe_shrinker_mod_pages(struct xe_shrinker *shrinker, long shrinkable, long purgea
 	write_unlock(&shrinker->lock);
 }
 
+static bool xe_shrinker_skip_bound_wc(struct ttm_buffer_object *ttm_bo,
+				      const struct xe_bo_shrink_flags flags)
+{
+	struct xe_bo *bo;
+
+	if (flags.purge || xe_modparam.allow_bound_wc_shrink ||
+	    !xe_bo_is_xe_bo(ttm_bo))
+		return false;
+
+	bo = ttm_to_xe_bo(ttm_bo);
+
+	/*
+	 * Restoring a backed-up WC BO changes freshly allocated WB pages to WC.
+	 * On x86 that runs CPA cache/TLB flushes synchronously while validation
+	 * holds this BO's dma-resv. A VM-bound BO is also likely to be reused by
+	 * a following EXEC, so reclaiming it can turn moderate memory pressure
+	 * into a multi-client reservation-lock stall. Keep that working set
+	 * resident; purgeable objects and objects after VM_UNBIND remain
+	 * reclaimable.
+	 */
+	return ttm_bo->ttm->caching == ttm_write_combined &&
+		xe_bo_is_vm_bound(bo);
+}
+
 static s64 __xe_shrinker_walk(struct xe_device *xe,
 			      struct ttm_operation_ctx *ctx,
 			      const struct xe_bo_shrink_flags flags,
@@ -75,6 +100,9 @@ static s64 __xe_shrinker_walk(struct xe_device *xe,
 			continue;
 
 		ttm_bo_lru_for_each_reserved_guarded(&curs, man, &arg, ttm_bo) {
+			if (xe_shrinker_skip_bound_wc(ttm_bo, flags))
+				continue;
+
 			if (!ttm_bo_shrink_suitable(ttm_bo, ctx))
 				continue;
 
-- 
2.50.1 (Apple Git-155)

Re: [RFC PATCH 1/1] drm/xe: keep VM-bound WC BOs resident during reclaim

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional dereference of `ttm_bo->ttm` in `xe_shrinker_skip_bound_wc` can cause a kernel panic due to a NULL pointer dereference.
--

--- Patch [1]: [RFC PATCH 1/1] drm/xe: keep VM-bound WC BOs resident during reclaim ---
commit 1cd578b99da153c47f4b384eb842462aba8d14d3
Author: Neil Zhong <neil.zhong@ugreen.com>

drm/xe: keep VM-bound WC BOs resident during reclaim

This commit adds logic to prevent the shrinker from reclaiming VM-bound
write-combined (WC) buffer objects to avoid expensive restore operations.
It introduces a module parameter to control this behavior and trades
reclaimable memory for lower latency.

> diff --git a/drivers/gpu/drm/xe/xe_shrinker.c b/drivers/gpu/drm/xe/xe_shrinker.c
> index 83374cd576608..caa39f39cd155 100644
> --- a/drivers/gpu/drm/xe/xe_shrinker.c
> +++ b/drivers/gpu/drm/xe/xe_shrinker.c
[ ... ]
> @@ -54,6 +55,30 @@ xe_shrinker_mod_pages(struct xe_shrinker *shrinker, long shrinkable, long purgea
>  	write_unlock(&shrinker->lock);
>  }
>  
> +static bool xe_shrinker_skip_bound_wc(struct ttm_buffer_object *ttm_bo,
> +				      const struct xe_bo_shrink_flags flags)
> +{
> +	struct xe_bo *bo;
> +
> +	if (flags.purge || xe_modparam.allow_bound_wc_shrink ||
> +	    !xe_bo_is_xe_bo(ttm_bo))
> +		return false;
> +
> +	bo = ttm_to_xe_bo(ttm_bo);
> +
> +	/*
> +	 * Restoring a backed-up WC BO changes freshly allocated WB pages to WC.
> +	 * On x86 that runs CPA cache/TLB flushes synchronously while validation
> +	 * holds this BO's dma-resv. A VM-bound BO is also likely to be reused by
> +	 * a following EXEC, so reclaiming it can turn moderate memory pressure
> +	 * into a multi-client reservation-lock stall. Keep that working set
> +	 * resident; purgeable objects and objects after VM_UNBIND remain
> +	 * reclaimable.
> +	 */
> +	return ttm_bo->ttm->caching == ttm_write_combined &&
> +		xe_bo_is_vm_bound(bo);
> +}
> +
>  static s64 __xe_shrinker_walk(struct xe_device *xe,
>  			      struct ttm_operation_ctx *ctx,
>  			      const struct xe_bo_shrink_flags flags,
> @@ -75,6 +100,9 @@ static s64 __xe_shrinker_walk(struct xe_device *xe,
>  			continue;
>  
>  		ttm_bo_lru_for_each_reserved_guarded(&curs, man, &arg, ttm_bo) {
> +			if (xe_shrinker_skip_bound_wc(ttm_bo, flags))
> +				continue;
> +
>  			if (!ttm_bo_shrink_suitable(ttm_bo, ctx))
>  				continue;

[Severity: High]
Does this code risk a NULL pointer dereference when accessing ttm_bo->ttm
inside xe_shrinker_skip_bound_wc()?

A buffer object can be on the LRU list with a NULL ttm pointer under
certain conditions, such as during early initialization or if pipeline
gutting removes it.

While ttm_bo_shrink_suitable() safely handles this by checking if
ttm_bo->ttm is populated, the new xe_shrinker_skip_bound_wc() check is
invoked prior to ttm_bo_shrink_suitable(). This leads to an unconditional
dereference of ttm_bo->ttm in xe_shrinker_skip_bound_wc() before the safety
check occurs.

Could the check be moved after ttm_bo_shrink_suitable(), or should
xe_shrinker_skip_bound_wc() explicitly verify ttm_bo->ttm before
dereferencing it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/4BF27CE161436C73+20260728061443.33616-2-neil.zhong@ugreen.com?part=1