Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Medium] The beneficial-order reclaim backoff optimization is inadvertently 
skipped when restoring backed-up buffer objects (swap-ins).
--

--- Patch [16]: [PATCH v3 16/33] drm/xe: Back off beneficial-order reclaim 
under defrag pressure ---
commit 2099c6d8da1f6677d9db38b2ffe92ba3eb96f8f0
Author: Matthew Brost <[email protected]>

drm/xe: Back off beneficial-order reclaim under defrag pressure

Once a meaningful number of BOs are tracked as being backed at a
sub-optimal order, this commit configures the TTM pool to back off from
aggressive reclaim during populate. This allows allocations to make
forward progress, leaving upgrades to beneficial orders for a deferred
defrag pass.

> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c

[ ... ]

> @@ -598,6 +606,12 @@ static int xe_ttm_tt_populate(struct ttm_device 
> *ttm_dev, struct ttm_tt *tt,
>       if (ttm_tt_is_backed_up(tt) && !xe_tt->purgeable) {
>               err = ttm_tt_restore(ttm_dev, tt, ctx);
>       } else {
> +             struct xe_device *xe = ttm_to_xe_device(ttm_dev);
> +
> +             if (atomic_read(&xe->mem.defrag.count) >=
> +                 XE_BO_DEFRAG_RECLAIM_BACKOFF_THRESHOLD)
> +                     ctx->beneficial_reclaim_backoff = true;
> +

[Severity: Medium]
Does this unintentionally skip the backoff optimization for swap-ins?

Because the beneficial_reclaim_backoff flag is only set inside the else
branch, any swapped-out buffer object being restored via ttm_tt_restore()
under fragmentation pressure will miss this optimization.

Without the flag, ttm_tt_restore() will continue pushing hard for
beneficial-order pages, potentially causing the exact reclaim and
compaction stalls this commit intended to avoid.

Would it make sense to move the device defrag count check and flag
evaluation above the ttm_tt_is_backed_up() check so both paths benefit
from the optimization?

>               ttm_tt_clear_backed_up(tt);
>               err = ttm_pool_alloc(&ttm_dev->pool, tt, ctx);
>       }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=16

Reply via email to