[PATCH] drm/ttm/tests: add a test to exercise ttm_bo_swapout

[PATCH] drm/ttm/tests: add a test to exercise ttm_bo_swapout

[Thadeu Lima de Souza Cascardo]

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.

Cc: Karolina Stolarek <karolina.stolarek@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: Christian König <christian.koenig@amd.com>
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
---
 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);
+
+	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);
+	KUNIT_EXPECT_EQ(test, swapped, MANAGER_SIZE / PAGE_SIZE);
+	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,
-- 
Thadeu Lima de Souza Cascardo <cascardo@igalia.com>

Claude review: drm/ttm/tests: add a test to exercise ttm_bo_swapout

[Claude Code Review Bot]

Patch Review

**Positive aspects:**
- Well-written commit message with clear motivation and historical context.
- The test follows the established patterns in `ttm_bo_validate_test.c` (same allocation style, same cleanup pattern with `ttm_bo_fini`/`ttm_mock_manager_fini`).
- Good validation: checks the return value, the resulting mem_type, and the swapped flag.

**Minor observations:**

1. **`KUNIT_EXPECT_EQ` vs `KUNIT_ASSERT_EQ` for `ttm_bo_init_reserved` failure** (line 791):
   ```c
   KUNIT_EXPECT_EQ(test, err, 0);
   ```
   If `ttm_bo_init_reserved` fails, the test continues and will likely crash on the subsequent `dma_resv_unlock` and `ttm_bo_swapout` calls with an uninitialized BO. Using `KUNIT_ASSERT_EQ` would abort the test on failure, preventing cascading issues. That said, the existing tests in this file all use `KUNIT_EXPECT_EQ` for this same check (e.g., lines 129, 178, 220, 252), so this is consistent with the existing code style. It could be argued the existing tests all have this same weakness, but that's a separate cleanup.

2. **Potential NULL dereference in assertions after swapout** (lines 796-797):
   ```c
   KUNIT_EXPECT_EQ(test, bo->resource->mem_type, TTM_PL_SYSTEM);
   KUNIT_EXPECT_TRUE(test, bo->ttm->page_flags & TTM_TT_FLAG_SWAPPED);
   ```
   If `ttm_bo_swapout` fails or doesn't behave as expected, `bo->resource` or `bo->ttm` could potentially be NULL. A defensive check or using `KUNIT_ASSERT_NOT_NULL` before dereferencing would be safer. However, in practice, a failed swapout would likely leave these pointers valid (just not in the expected state), so this is a minor robustness concern rather than a real bug.

3. **Checking swapped return value** (line 794-795):
   ```c
   swapped = ttm_bo_swapout(priv->ttm_dev, &ctx_init, man, GFP_KERNEL, 1);
   KUNIT_EXPECT_EQ(test, swapped, MANAGER_SIZE / PAGE_SIZE);
   ```
   The `MANAGER_SIZE / PAGE_SIZE` expectation is consistent with having initialized the GEM object with size `MANAGER_SIZE` at line 787. This looks correct.

4. **Cleanup ordering**: The test calls `ttm_bo_fini(bo)` before `ttm_mock_manager_fini(priv->ttm_dev, mem_type)`, which is the correct order (destroy BO before tearing down the manager). This matches the pattern in other tests like `ttm_bo_validate_move_fence_not_signaled` at lines 757-759.

**Overall:** The patch is straightforward, well-structured, and follows the existing conventions. The observations above are all minor. The test provides useful coverage for `ttm_bo_swapout` that was previously missing.

**Reviewed-by worthy** with no blocking issues.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/ttm/tests: add a test to exercise ttm_bo_swapout

[Claude Code Review Bot]

Overall Series Review

Subject: drm/ttm/tests: add a test to exercise ttm_bo_swapout
Author: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
Patches: 1
Reviewed: 2026-03-09T08:23:09.150307

---

This is a single-patch series that adds a KUnit test for `ttm_bo_swapout`. The test is well-motivated: the commit message explains that a previous swapout test was removed (commit 76689eb52667) because it was unreliable, and this new test takes a simpler, more direct approach by calling `ttm_bo_swapout` on a single BO rather than trying to trigger it indirectly through memory pressure. The test was validated by detecting a real UAF bug. The patch is clean and follows the existing test patterns in the file.

**Verdict: Looks good overall, with a couple of minor observations.**

---
Generated by Claude Code Patch Reviewer

[PATCH v2] drm/ttm/tests: add a test to exercise ttm_bo_swapout

[Thadeu Lima de Souza Cascardo]

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.

Cc: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: Christian König <christian.koenig@amd.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
---
Changes in v2:
- Removed extra asserts that are already internally used.
- Export ttm_bo_swapout with EXPORT_SYMBOL_FOR_TESTS_ONLY to build test as
  module.
- Request all pages of the BO to be swapped out.
- Link to v1: https://lore.kernel.org/r/20260306-ttm_bo_swapout_test-v1-1-aaab11091ee0@igalia.com
---
 drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c | 41 ++++++++++++++++++++++++
 drivers/gpu/drm/ttm/ttm_bo.c                     |  2 ++
 2 files changed, 43 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..4d458fa427da3e6272913d7836a860bc4956edb8 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,46 @@ 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);
+	placement = ttm_placement_kunit_init(test, place, 1);
+
+	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,
+				 MANAGER_SIZE / PAGE_SIZE);
+	KUNIT_EXPECT_EQ(test, swapped, MANAGER_SIZE / PAGE_SIZE);
+	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 +1193,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),
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 0765d69423d2f4028bf2effd21af53e117ee7a29..89008e5eb1c7d5412fde7771409da04279414f6d 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -32,6 +32,7 @@
 #define pr_fmt(fmt) "[TTM] " fmt
 
 #include <drm/drm_print.h>
+#include <drm/drm_util.h>
 #include <drm/ttm/ttm_allocation.h>
 #include <drm/ttm/ttm_bo.h>
 #include <drm/ttm/ttm_placement.h>
@@ -1232,6 +1233,7 @@ s64 ttm_bo_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
 
 	return ttm_lru_walk_for_evict(&swapout_walk.walk, bdev, man, target);
 }
+EXPORT_SYMBOL_FOR_TESTS_ONLY(ttm_bo_swapout);
 
 void ttm_bo_tt_destroy(struct ttm_buffer_object *bo)
 {

---
base-commit: 1f318b96cc84d7c2ab792fcc0bfd42a7ca890681
change-id: 20260306-ttm_bo_swapout_test-ad130eac0bcf

Best regards,
-- 
Thadeu Lima de Souza Cascardo <cascardo@igalia.com>

Claude review: drm/ttm/tests: add a test to exercise ttm_bo_swapout

[Claude Code Review Bot]

Patch Review

**Overall assessment: Good patch, minor observations only.**

**Positive aspects:**

- The test follows the established patterns in `ttm_bo_validate_test.c` well — using `ttm_mock_manager_init`, `ttm_place_kunit_init`, `ttm_placement_kunit_init`, `kunit_kzalloc`, `drm_gem_private_object_init`, `ttm_bo_init_reserved`, and `ttm_bo_fini` in the same manner as other tests.
- Using `EXPORT_SYMBOL_FOR_TESTS_ONLY` is the correct approach for making `ttm_bo_swapout` available to the test module, and it already has precedent (`ttm_tt_swapout` at `ttm_tt.c:368` uses the same macro).
- The commit message is well-written with good context about why the previous test was removed and why this new approach is better.
- The v2 changes (removing redundant asserts, requesting all pages) are good improvements.

**Observations:**

1. **`KUNIT_EXPECT_EQ` vs `KUNIT_ASSERT_EQ` for `ttm_bo_init_reserved`**: At line 135-136 of the test:
   ```c
   KUNIT_EXPECT_EQ(test, err, 0);
   dma_resv_unlock(bo->base.resv);
   ```
   If `ttm_bo_init_reserved` fails, `err != 0` and the BO may not be properly initialized, yet execution continues to `dma_resv_unlock` and `ttm_bo_swapout`. This should arguably be `KUNIT_ASSERT_EQ` to bail out on failure, since the subsequent code depends on successful initialization. That said, the v2 changelog says "Removed extra asserts that are already internally used," so this may have been a deliberate reviewer-requested change. The existing tests in this file use `KUNIT_EXPECT_EQ` in similar positions, so this is consistent with the file's convention.

2. **Documentation vs implementation mismatch (pre-existing)**: The `ttm_bo_swapout` doc says `@target: The desired number of bytes to swap out` and the return is described as "The number of bytes actually swapped out." However, `ttm_bo_swapout_cb` returns `ttm->num_pages` (pages, not bytes), and the test correctly uses `MANAGER_SIZE / PAGE_SIZE` (i.e., pages) for both target and expected return:
   ```c
   swapped = ttm_bo_swapout(priv->ttm_dev, &ctx_init, man, GFP_KERNEL,
                            MANAGER_SIZE / PAGE_SIZE);
   KUNIT_EXPECT_EQ(test, swapped, MANAGER_SIZE / PAGE_SIZE);
   ```
   This is correct behavior matching the actual implementation, but the doc comment on `ttm_bo_swapout` is misleading (pre-existing issue, not introduced by this patch).

3. **The `#include <drm/drm_util.h>` addition**: Adding this include to `ttm_bo.c` is correct since `EXPORT_SYMBOL_FOR_TESTS_ONLY` is defined in `drm/drm_util.h`. It's clean and minimal.

4. **Cleanup ordering**: The test calls `ttm_bo_fini(bo)` before `ttm_mock_manager_fini()`, which is the correct ordering — the BO's resource (now in SYSTEM) should be released before tearing down the mock manager. This matches the pattern used elsewhere in the file.

**Verdict**: The patch is well-crafted and addresses a real testing gap. The EXPECT vs ASSERT question in point 1 is the only potential concern, and it follows the file's existing convention. Reviewed-by from Tvrtko is already present. This looks good to merge.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/ttm/tests: add a test to exercise ttm_bo_swapout

[Claude Code Review Bot]

Overall Series Review

Subject: drm/ttm/tests: add a test to exercise ttm_bo_swapout
Author: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
Patches: 1
Reviewed: 2026-03-10T12:02:40.219911

---

This is a single-patch series that adds a KUnit test for `ttm_bo_swapout`. The motivation is sound: the function lost test coverage when the previous test was removed (commit 76689eb52667) due to reliability issues with allocating half of system memory. The new approach of directly calling `ttm_bo_swapout` on a single BO avoids that problem entirely and also detected a real UAF bug. The patch is clean, well-structured, and follows the existing test patterns in the file.

The patch looks **good and ready to merge**, with only minor observations below.

---
Generated by Claude Code Patch Reviewer