[PATCH] drm/ttm: fix swapped-out resources never leaving their bulk_move range

[PATCH] drm/ttm: fix swapped-out resources never leaving their bulk_move range

[Vadim Nikitushkin]

ttm_tt_swapout() returns the number of pages swapped out on success and
a negative error code on failure; for a populated ttm it never returns
zero. Commit b2ed01e7ad3d ("drm/ttm: Fix ttm_bo_swapout() infinite LRU
walk on swapout failure") moved the bulk_move bookkeeping in
ttm_bo_swapout_cb() under "if (!ret)", so the
ttm_resource_del_bulk_move_unevictable() / ttm_resource_move_to_lru_tail()
pair is now skipped on every successful swapout. The equivalent change
for the shrinker in commit 1d59f36e95f7 ("drm/ttm: Fix ttm_bo_shrink()
infinite LRU walk on backup failure") tests "lret > 0", which is what
was intended here as well.

Before b2ed01e7ad3d the resource was taken off the bulk_move before the
swapout; since then a swapped-out resource stays inside its BO's
bulk_move range (and on the manager LRU) although it is unevictable.
When it is later freed or the BO leaves the bulk_move
(ttm_resource_free(), ttm_bo_set_bulk_move() via amdgpu_vm_bo_del()),
ttm_resource_del_bulk_move() skips it because of its
!ttm_resource_unevictable() guard, so a range endpoint in pos->first /
pos->last is left pointing at freed memory. The next
ttm_lru_bulk_move_tail() or ttm_resource_add_bulk_move() on that cursor
is a use-after-free, seen as the resv WARN in ttm_lru_bulk_move_add(),
"list_del corruption" in ttm_resource_move_to_lru_tail() or a NULL
dereference in ttm_resource_manager_next() -- minutes to hours after a
hibernation, or at process exit / reboot following one. Samuel
Ainsworth's analysis of drm/amd issue 5387 (see Link) identified the
dangling cursor; the missing removal at swapout time is the reason it
dangles.

Testing the condition for success restores the removal. On an AMD
Phoenix APU (ASUS UM3406GA, gfx1103) running suspend-then-hibernate on
a 7.0.y stable kernel carrying the backport (Ubuntu 7.0.0-31) the bug
crashed 5 of 18 hibernation cycles; a function profile of one
hibernation showed 336 ttm_tt_swapout() calls and zero
ttm_resource_del_bulk_move_unevictable() calls. With this change the
removal happens for every swapped-out resource and 12 further cycles
were clean.

Fixes: b2ed01e7ad3d ("drm/ttm: Fix ttm_bo_swapout() infinite LRU walk on swapout failure")
Cc: stable@vger.kernel.org # v7.1+
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5387
Link: https://lore.kernel.org/dri-devel/CAHYiNPa6aVacJoLOje-qZ1GyYx-9p0tN4NuP8D_eSL+UJeevXw@mail.gmail.com/
Signed-off-by: Vadim Nikitushkin <bub4z0r@gmail.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index ef56c18..9b85b5f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -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

Re: [PATCH] drm/ttm: fix swapped-out resources never leaving their bulk_move range

[Thomas Hellström]

On Wed, 2026-09-09 at 23:50 +0300, Vadim Nikitushkin wrote:
> ttm_tt_swapout() returns the number of pages swapped out on success
> and
> a negative error code on failure; for a populated ttm it never
> returns
> zero. Commit b2ed01e7ad3d ("drm/ttm: Fix ttm_bo_swapout() infinite
> LRU
> walk on swapout failure") moved the bulk_move bookkeeping in
> ttm_bo_swapout_cb() under "if (!ret)", so the
> ttm_resource_del_bulk_move_unevictable() /
> ttm_resource_move_to_lru_tail()
> pair is now skipped on every successful swapout. The equivalent
> change
> for the shrinker in commit 1d59f36e95f7 ("drm/ttm: Fix
> ttm_bo_shrink()
> infinite LRU walk on backup failure") tests "lret > 0", which is what
> was intended here as well.
> 
> Before b2ed01e7ad3d the resource was taken off the bulk_move before
> the
> swapout; since then a swapped-out resource stays inside its BO's
> bulk_move range (and on the manager LRU) although it is unevictable.
> When it is later freed or the BO leaves the bulk_move
> (ttm_resource_free(), ttm_bo_set_bulk_move() via amdgpu_vm_bo_del()),
> ttm_resource_del_bulk_move() skips it because of its
> !ttm_resource_unevictable() guard, so a range endpoint in pos->first
> /
> pos->last is left pointing at freed memory. The next
> ttm_lru_bulk_move_tail() or ttm_resource_add_bulk_move() on that
> cursor
> is a use-after-free, seen as the resv WARN in
> ttm_lru_bulk_move_add(),
> "list_del corruption" in ttm_resource_move_to_lru_tail() or a NULL
> dereference in ttm_resource_manager_next() -- minutes to hours after
> a
> hibernation, or at process exit / reboot following one. Samuel
> Ainsworth's analysis of drm/amd issue 5387 (see Link) identified the
> dangling cursor; the missing removal at swapout time is the reason it
> dangles.
> 
> Testing the condition for success restores the removal. On an AMD
> Phoenix APU (ASUS UM3406GA, gfx1103) running suspend-then-hibernate
> on
> a 7.0.y stable kernel carrying the backport (Ubuntu 7.0.0-31) the bug
> crashed 5 of 18 hibernation cycles; a function profile of one
> hibernation showed 336 ttm_tt_swapout() calls and zero
> ttm_resource_del_bulk_move_unevictable() calls. With this change the
> removal happens for every swapped-out resource and 12 further cycles
> were clean.
> 
> Fixes: b2ed01e7ad3d ("drm/ttm: Fix ttm_bo_swapout() infinite LRU walk
> on swapout failure")
> Cc: stable@vger.kernel.org # v7.1+
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5387
> Link:
> https://lore.kernel.org/dri-devel/CAHYiNPa6aVacJoLOje-qZ1GyYx-9p0tN4NuP8D_eSL+UJeevXw@mail.gmail.com/
> Signed-off-by: Vadim Nikitushkin <bub4z0r@gmail.com>

Nice catch.

This also explains why https://patchwork.freedesktop.org/series/170311/
appeared to fix the issue. But that series actually kept the resource
on the bulk sublist until someone bumped the LRU or removed it.

Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

> ---
>  drivers/gpu/drm/ttm/ttm_bo.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
> b/drivers/gpu/drm/ttm/ttm_bo.c
> index ef56c18..9b85b5f 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -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);

Re: [PATCH] drm/ttm: fix swapped-out resources never leaving their bulk_move range

[Christian König]

On 9/10/26 09:14, Thomas Hellström wrote:
> On Wed, 2026-09-09 at 23:50 +0300, Vadim Nikitushkin wrote:
>> ttm_tt_swapout() returns the number of pages swapped out on success
>> and
>> a negative error code on failure; for a populated ttm it never
>> returns
>> zero. Commit b2ed01e7ad3d ("drm/ttm: Fix ttm_bo_swapout() infinite
>> LRU
>> walk on swapout failure") moved the bulk_move bookkeeping in
>> ttm_bo_swapout_cb() under "if (!ret)", so the
>> ttm_resource_del_bulk_move_unevictable() /
>> ttm_resource_move_to_lru_tail()
>> pair is now skipped on every successful swapout. The equivalent
>> change
>> for the shrinker in commit 1d59f36e95f7 ("drm/ttm: Fix
>> ttm_bo_shrink()
>> infinite LRU walk on backup failure") tests "lret > 0", which is what
>> was intended here as well.
>>
>> Before b2ed01e7ad3d the resource was taken off the bulk_move before
>> the
>> swapout; since then a swapped-out resource stays inside its BO's
>> bulk_move range (and on the manager LRU) although it is unevictable.
>> When it is later freed or the BO leaves the bulk_move
>> (ttm_resource_free(), ttm_bo_set_bulk_move() via amdgpu_vm_bo_del()),
>> ttm_resource_del_bulk_move() skips it because of its
>> !ttm_resource_unevictable() guard, so a range endpoint in pos->first
>> /
>> pos->last is left pointing at freed memory. The next
>> ttm_lru_bulk_move_tail() or ttm_resource_add_bulk_move() on that
>> cursor
>> is a use-after-free, seen as the resv WARN in
>> ttm_lru_bulk_move_add(),
>> "list_del corruption" in ttm_resource_move_to_lru_tail() or a NULL
>> dereference in ttm_resource_manager_next() -- minutes to hours after
>> a
>> hibernation, or at process exit / reboot following one. Samuel
>> Ainsworth's analysis of drm/amd issue 5387 (see Link) identified the
>> dangling cursor; the missing removal at swapout time is the reason it
>> dangles.
>>
>> Testing the condition for success restores the removal. On an AMD
>> Phoenix APU (ASUS UM3406GA, gfx1103) running suspend-then-hibernate
>> on
>> a 7.0.y stable kernel carrying the backport (Ubuntu 7.0.0-31) the bug
>> crashed 5 of 18 hibernation cycles; a function profile of one
>> hibernation showed 336 ttm_tt_swapout() calls and zero
>> ttm_resource_del_bulk_move_unevictable() calls. With this change the
>> removal happens for every swapped-out resource and 12 further cycles
>> were clean.
>>
>> Fixes: b2ed01e7ad3d ("drm/ttm: Fix ttm_bo_swapout() infinite LRU walk
>> on swapout failure")
>> Cc: stable@vger.kernel.org # v7.1+
>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/5387
>> Link:
>> https://lore.kernel.org/dri-devel/CAHYiNPa6aVacJoLOje-qZ1GyYx-9p0tN4NuP8D_eSL+UJeevXw@mail.gmail.com/
>> Signed-off-by: Vadim Nikitushkin <bub4z0r@gmail.com>
> 
> Nice catch.

Agreed, that is a really good one. We had tons of people staring at the code without seeing that.

> 
> This also explains why https://patchwork.freedesktop.org/series/170311/
> appeared to fix the issue. But that series actually kept the resource
> on the bulk sublist until someone bumped the LRU or removed it.
> 
> Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

If nobody comes up with some last second objections I'm going to push that to drm-misc-fixes ASAP.

Thanks,
Christian.

> 
>> ---
>>  drivers/gpu/drm/ttm/ttm_bo.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
>> b/drivers/gpu/drm/ttm/ttm_bo.c
>> index ef56c18..9b85b5f 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -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);

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

[Vadim Nikitushkin]

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@vger.kernel.org # v7.1+
Signed-off-by: Vadim Nikitushkin <bub4z0r@gmail.com>
---
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.

 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

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@vger.kernel.org # v7.1+
> Signed-off-by: Vadim Nikitushkin <bub4z0r@gmail.com>
> ---
> 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
>