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: [email protected] # v7.1+
> Signed-off-by: Vadim Nikitushkin <[email protected]>
> ---
> 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
>
