On Mon, Mar 09, 2026 at 10:46:03AM +0000, Tvrtko Ursulin wrote:
>
> On 06/03/2026 18:00, Thadeu Lima de Souza Cascardo wrote:
> > Currently, ttm_bo_swapout is not exercised by the TTM KUnit tests.
> >
> > It used to be exercised until commit 76689eb52667 ("drm/ttm: remove
> > ttm_bo_validate_swapout test"), but that test was removed as it was
> > unreliable due to requiring to allocate half of the system memory.
> >
> > Calling ttm_bo_swapout directly with a single allocated BO, however, does
> > not suffer from that problem, and was able to detect a UAF introduced by
> > commit c06da4b3573a ("drm/ttm: Tidy usage of local variables a little
> > bit"), when built with KASAN.
> >
> > When applying a fix to that UAF, the test passed without any issues.
>
> Thank you for closing this gap and again for fixing up after me. I have some
> minor comments below, but the test looks good to me and either way:
>
> Reviewed-by: Tvrtko Ursulin <[email protected]>
>
> >
> > Cc: Karolina Stolarek <[email protected]>
> > Cc: Tvrtko Ursulin <[email protected]>
> > Cc: Christian König <[email protected]>
> > Signed-off-by: Thadeu Lima de Souza Cascardo <[email protected]>
> > ---
> > drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c | 42
> > ++++++++++++++++++++++++
> > 1 file changed, 42 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> > b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> > index
> > 6d95447a989d20d60227025be874265b2b491f59..9848c008a443d70ad16c6e018381878f0898e487
> > 100644
> > --- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> > +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
> > @@ -759,6 +759,47 @@ static void
> > ttm_bo_validate_move_fence_not_signaled(struct kunit *test)
> > ttm_mock_manager_fini(priv->ttm_dev, snd_mem);
> > }
> > +static void ttm_bo_validate_swapout(struct kunit *test)
> > +{
> > + u32 mem_type = TTM_PL_TT;
> > + struct ttm_test_devices *priv = test->priv;
> > + struct ttm_operation_ctx ctx_init = { };
> > + enum ttm_bo_type bo_type = ttm_bo_type_device;
> > + struct ttm_resource_manager *man;
> > + struct ttm_placement *placement;
> > + struct ttm_buffer_object *bo;
> > + struct ttm_place *place;
> > + int err;
> > + s64 swapped;
> > +
> > + ttm_mock_manager_init(priv->ttm_dev, mem_type, MANAGER_SIZE);
> > + man = ttm_manager_type(priv->ttm_dev, mem_type);
> > + KUNIT_ASSERT_NOT_NULL(test, man);
> > +
> > + place = ttm_place_kunit_init(test, mem_type, 0);
> > + KUNIT_ASSERT_NOT_NULL(test, place);
> > + placement = ttm_placement_kunit_init(test, place, 1);
> > + KUNIT_ASSERT_NOT_NULL(test, placement);
>
> Above two seem to have internal asserts so you could drop them from the
> test.
>
Thanks for pointing that out. I have fixed that and also the need to export
ttm_bo_swapout for tests in case the tests are built as modules.
> > +
> > + bo = kunit_kzalloc(test, sizeof(*bo), GFP_KERNEL);
> > + KUNIT_ASSERT_NOT_NULL(test, bo);
> > +
> > + drm_gem_private_object_init(priv->drm, &bo->base, MANAGER_SIZE);
> > + err = ttm_bo_init_reserved(priv->ttm_dev, bo, bo_type, placement,
> > + PAGE_SIZE, &ctx_init, NULL, NULL,
> > + &dummy_ttm_bo_destroy);
> > + KUNIT_EXPECT_EQ(test, err, 0);
> > + dma_resv_unlock(bo->base.resv);
> > +
> > + swapped = ttm_bo_swapout(priv->ttm_dev, &ctx_init, man, GFP_KERNEL, 1);
>
> Is there a particular reason to ask for 1 page to swap out instead of
> matching the bo size? Might be more obvious (I dare not to say future proof)
> to match it with the expectation from the below assert.
>
Just that I found that all callers for ttm_lru_walk_for_evict use 1 as the
target except for ttm_bo_swapout, which uses its parameter as the argument
but its only callers uses 1 as the target too. :-D
> > + KUNIT_EXPECT_EQ(test, swapped, MANAGER_SIZE / PAGE_SIZE);
Another future proof way to do it would be to test for swapped >= 1. Let me
try using MANAGER_SIZE / PAGE_SIZE and keep the expectation here.
>
> Side note, it looks like ttm_bo_swapout() kerneldoc should be fixed to
> correctly say return is number of pages, not bytes.
Nice catch. target too should be number of pages. Let me send the fix for
that too.
Thanks.
Cascardo.
>
> Regards,
>
> Tvrtko
>
> > + KUNIT_EXPECT_EQ(test, bo->resource->mem_type, TTM_PL_SYSTEM);
> > + KUNIT_EXPECT_TRUE(test, bo->ttm->page_flags & TTM_TT_FLAG_SWAPPED);
> > +
> > + ttm_bo_fini(bo);
> > + ttm_mock_manager_fini(priv->ttm_dev, mem_type);
> > +}
> > +
> > static void ttm_bo_validate_happy_evict(struct kunit *test)
> > {
> > u32 mem_type = TTM_PL_VRAM, mem_multihop = TTM_PL_TT,
> > @@ -1153,6 +1194,7 @@ static struct kunit_case ttm_bo_validate_test_cases[]
> > = {
> > KUNIT_CASE(ttm_bo_validate_move_fence_signaled),
> > KUNIT_CASE_PARAM(ttm_bo_validate_move_fence_not_signaled,
> > ttm_bo_validate_wait_gen_params),
> > + KUNIT_CASE(ttm_bo_validate_swapout),
> > KUNIT_CASE(ttm_bo_validate_happy_evict),
> > KUNIT_CASE(ttm_bo_validate_all_pinned_evict),
> > KUNIT_CASE(ttm_bo_validate_allowed_only_evict),
> >
> > ---
> > base-commit: 11439c4635edd669ae435eec308f4ab8a0804808
> > change-id: 20260306-ttm_bo_swapout_test-ad130eac0bcf
> >
> > Best regards,
>
