* [PATCH] drm/ttm: fix swapped-out resources never leaving their bulk_move range
@ 2026-09-09 20:50 Vadim Nikitushkin
2026-09-10 7:14 ` Thomas Hellström
0 siblings, 1 reply; 6+ messages in thread
From: Vadim Nikitushkin @ 2026-09-09 20:50 UTC (permalink / raw)
To: christian.koenig, ray.huang, matthew.auld, matthew.brost,
thomas.hellstrom
Cc: dri-devel, linux-kernel, stable, skainsworth, alexander.deucher,
bernardomagri21, 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/ttm: fix swapped-out resources never leaving their bulk_move range
2026-09-09 20:50 [PATCH] drm/ttm: fix swapped-out resources never leaving their bulk_move range Vadim Nikitushkin
@ 2026-09-10 7:14 ` Thomas Hellström
2026-09-10 7:46 ` Christian König
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Hellström @ 2026-09-10 7:14 UTC (permalink / raw)
To: Vadim Nikitushkin, christian.koenig, ray.huang, matthew.auld,
matthew.brost
Cc: dri-devel, linux-kernel, stable, skainsworth, alexander.deucher,
bernardomagri21
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);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/ttm: fix swapped-out resources never leaving their bulk_move range
2026-09-10 7:14 ` Thomas Hellström
@ 2026-09-10 7:46 ` Christian König
2026-09-10 14:34 ` [PATCH] drm/ttm: apply the swapout bulk_move fix to the intended condition Vadim Nikitushkin
2026-09-11 16:08 ` [PATCH] drm/ttm: fix swapped-out resources never leaving their bulk_move range Samuel Ainsworth
0 siblings, 2 replies; 6+ messages in thread
From: Christian König @ 2026-09-10 7:46 UTC (permalink / raw)
To: Thomas Hellström, Vadim Nikitushkin, ray.huang, matthew.auld,
matthew.brost
Cc: dri-devel, linux-kernel, stable, skainsworth, alexander.deucher,
bernardomagri21, Natalie Vock
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);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] drm/ttm: apply the swapout bulk_move fix to the intended condition
2026-09-10 7:46 ` Christian König
@ 2026-09-10 14:34 ` Vadim Nikitushkin
2026-09-10 16:07 ` Christian König
2026-09-11 16:08 ` [PATCH] drm/ttm: fix swapped-out resources never leaving their bulk_move range Samuel Ainsworth
1 sibling, 1 reply; 6+ messages in thread
From: Vadim Nikitushkin @ 2026-09-10 14:34 UTC (permalink / raw)
To: christian.koenig, thomas.hellstrom
Cc: ray.huang, matthew.auld, matthew.brost, dri-devel, linux-kernel,
stable, skainsworth, alexander.deucher, bernardomagri21,
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/ttm: apply the swapout bulk_move fix to the intended condition
2026-09-10 14:34 ` [PATCH] drm/ttm: apply the swapout bulk_move fix to the intended condition Vadim Nikitushkin
@ 2026-09-10 16:07 ` Christian König
0 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2026-09-10 16:07 UTC (permalink / raw)
To: Vadim Nikitushkin, thomas.hellstrom
Cc: ray.huang, matthew.auld, matthew.brost, dri-devel, linux-kernel,
stable, skainsworth, alexander.deucher, bernardomagri21
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/ttm: fix swapped-out resources never leaving their bulk_move range
2026-09-10 7:46 ` Christian König
2026-09-10 14:34 ` [PATCH] drm/ttm: apply the swapout bulk_move fix to the intended condition Vadim Nikitushkin
@ 2026-09-11 16:08 ` Samuel Ainsworth
1 sibling, 0 replies; 6+ messages in thread
From: Samuel Ainsworth @ 2026-09-11 16:08 UTC (permalink / raw)
To: Christian König
Cc: Thomas Hellström, Vadim Nikitushkin, ray.huang, matthew.auld,
matthew.brost, dri-devel, linux-kernel, stable, alexander.deucher,
bernardomagri21, Natalie Vock
I left a comment on the commit in GitLab
(https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/3db7d7d583419f7b1f2e141e36418802dbb25cf8#note_3658149)
but it occurred to me that it may be closer to convention to reply
here. In short, AFAIU the commit description describes fixing this by
checking `ttm_tt_swapout()` inside `ttm_bo_swapout_cb()` but the
actual diff is modifying `ttm_resource_try_charge()` checking inside
`ttm_bo_alloc_at_place()`.
Perhaps I'm missing something?
Sam
On Thu, Sep 10, 2026 at 3:46 AM Christian König
<christian.koenig@amd.com> wrote:
>
> 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);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-11 16:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-09 20:50 [PATCH] drm/ttm: fix swapped-out resources never leaving their bulk_move range Vadim Nikitushkin
2026-09-10 7:14 ` Thomas Hellström
2026-09-10 7:46 ` Christian König
2026-09-10 14:34 ` [PATCH] drm/ttm: apply the swapout bulk_move fix to the intended condition Vadim Nikitushkin
2026-09-10 16:07 ` Christian König
2026-09-11 16:08 ` [PATCH] drm/ttm: fix swapped-out resources never leaving their bulk_move range Samuel Ainsworth
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.