Re: [PATCH] drm/ttm: apply the swapout bulk_move fix to the intended condition

[Christian König]

Hi Vadim,

On 9/10/26 16:34, Vadim Nikitushkin wrote:
> Commit 3db7d7d58341 ("drm/ttm: fix swapped-out resources never leaving
> their bulk_move range") landed in drm-misc-fixes with its one-line
> change applied to the wrong "if": the "if (ret)" after
> ttm_resource_try_charge() in ttm_bo_alloc_at_place() became
> "if (ret > 0)", while the "if (!ret)" after ttm_tt_swapout() in
> ttm_bo_swapout_cb() that the patch targeted was left untouched.
> 
> ttm_resource_try_charge() returns 0 or a negative error code, so with
> "ret > 0" a failed dmem cgroup charge no longer fails the allocation.
> Restore that check and apply the intended change: ttm_tt_swapout()
> returns the number of pages swapped out on success, so the bulk_move
> removal must run for ret > 0.
> 
> Fixes: 3db7d7d58341 ("drm/ttm: fix swapped-out resources never leaving their bulk_move range")
> Cc: stable@xxxxxxxxxxxxxxx # v7.1+
> Signed-off-by: Vadim Nikitushkin <bub4z0r@xxxxxxxxx>
> ---
> Christian, the commit in drm-misc-fixes (3db7d7d58341) ended up with the
> change applied to the try_charge condition at line 532 instead of the
> swapout one at line 1434; the resulting tree still has "if (!ret)" in
> ttm_bo_swapout_cb(). This is a fix-up on top of 3db7d7d58341 since
> drm-misc-fixes does not rebase. Sorry for the noise.

Mea culpa.

AMDs mail servers convert incoming mail to a different encoding which prevents the patch from applying cleanly (yeah everybody knows how braindead that is).

Usually I pick up patches from patchwork now, but this time I though I could apply the one liner manually.

Well long story short that didn't worked the way it should.

I've just reviewed and pushed this patch to drm-misc-fixes as well.

Thanks a lot for helping out here.

Sorry,
Christian.

> 
>  drivers/gpu/drm/ttm/ttm_bo.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index a12af5b..9b85b5f 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -532,7 +532,7 @@ static int ttm_bo_alloc_at_place(struct ttm_buffer_object *bo,
>                 ret = ttm_resource_try_charge(bo, place, &alloc_state->charge_pool,
>                                               force_space ? &alloc_state->limit_pool
>                                                           : NULL);
> -               if (ret > 0) {
> +               if (ret) {
>                         /*
>                          * -EAGAIN means the charge failed, which we treat
>                          * like an allocation failure. Therefore, return an
> @@ -1434,7 +1434,7 @@ ttm_bo_swapout_cb(struct ttm_lru_walk *walk, struct ttm_buffer_object *bo)
> 
>         if (ttm_tt_is_populated(tt)) {
>                 ret = ttm_tt_swapout(bdev, tt, swapout_walk->gfp_flags);
> -               if (!ret) {
> +               if (ret > 0) {
>                         spin_lock(&bdev->lru_lock);
>                         ttm_resource_del_bulk_move_unevictable(bo->resource, bo);
>                         ttm_resource_move_to_lru_tail(bo->resource);
> --
> 2.53.0
>