[PATCH] drm/ttm/tests: Remove checks from ttm_pool_free_no_dma_alloc

[PATCH] drm/ttm/tests: Remove checks from ttm_pool_free_no_dma_alloc

[Maarten Lankhorst]

On !x86, the pool type is never initialised, and the pages are freed
back to the system.

The test broke on the list_lru rewrite, but I'm not sure how that it was
supposed to work previously. In the meantime CI is broken so reverting
for now.

Fixes: 444e2a19d7fd ("ttm/pool: port to list_lru. (v2)")
Cc: Christian Koenig <christian.koenig@amd.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Dave Chinner <david@fromorbit.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Dave Airlie <airlied@redhat.com>
Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
---
 drivers/gpu/drm/ttm/tests/ttm_pool_test.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/ttm/tests/ttm_pool_test.c b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c
index 01197014b83f0..be75c8abf3883 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_pool_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_pool_test.c
@@ -368,7 +368,6 @@ static void ttm_pool_free_no_dma_alloc(struct kunit *test)
 	struct ttm_test_devices *devs = priv->devs;
 	struct ttm_tt *tt;
 	struct ttm_pool *pool;
-	struct ttm_pool_type *pt;
 	enum ttm_caching caching = ttm_uncached;
 	unsigned int order = 2;
 	size_t size = (1 << order) * PAGE_SIZE;
@@ -382,14 +381,9 @@ static void ttm_pool_free_no_dma_alloc(struct kunit *test)
 	ttm_pool_init(pool, devs->dev, NUMA_NO_NODE, 0);
 	ttm_pool_alloc(pool, tt, &simple_ctx);
 
-	pt = &pool->caching[caching].orders[order];
-	KUNIT_ASSERT_TRUE(test, list_lru_count(&pt->pages) == 1);
-
 	ttm_pool_free(pool, tt);
 	ttm_tt_fini(tt);
 
-	KUNIT_ASSERT_TRUE(test, list_lru_count(&pt->pages) == 1);
-
 	ttm_pool_fini(pool);
 }
 
-- 
2.53.0

Claude review: drm/ttm/tests: Remove checks from ttm_pool_free_no_dma_alloc

[Claude Code Review Bot]

Overall Series Review

Subject: drm/ttm/tests: Remove checks from ttm_pool_free_no_dma_alloc
Author: Maarten Lankhorst <dev@lankhorst.se>
Patches: 1
Reviewed: 2026-04-12T11:03:53.106940

---

This is a single-patch fix that removes broken assertions from the `ttm_pool_free_no_dma_alloc` kunit test. The fix is correct and appropriate as a CI-unblocking measure. The root cause is well-identified: without `TTM_ALLOCATION_POOL_USE_DMA_ALLOC`, `ttm_pool_select_type()` does not return the pool's own `pool->caching[caching].orders[order]` entry — on x86 it returns a global pool, and on !x86 it returns NULL. In both cases, the per-pool `ttm_pool_type` is never initialized via `ttm_pool_type_init()`, so calling `list_lru_count()` on it is invalid.

The patch is a reasonable quick fix to unbreak CI. The test still exercises the alloc/free path itself, but it no longer verifies any postconditions.

**Recommendation: Accept**, with minor nits below.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/ttm/tests: Remove checks from ttm_pool_free_no_dma_alloc

[Claude Code Review Bot]

Patch Review

**Correctness: Good.** The removed assertions were fundamentally wrong for this test case. Looking at `ttm_pool_select_type()` (`ttm_pool.c:401`):

```c
static struct ttm_pool_type *ttm_pool_select_type(struct ttm_pool *pool,
                                                  enum ttm_caching caching,
                                                  unsigned int order)
{
    if (ttm_pool_uses_dma_alloc(pool))
        return &pool->caching[caching].orders[order];

#ifdef CONFIG_X86
    switch (caching) {
    case ttm_write_combined:
        ...
        return &global_write_combined[order];
    case ttm_uncached:
        ...
        return &global_uncached[order];
```

When `alloc_flags=0` (no DMA alloc), `ttm_pool_init()` skips `ttm_pool_type_init()` for `pool->caching[caching].orders[order]` because `ttm_pool_select_type()` returns either a global pool (x86) or NULL (!x86) — never the pool's own entry. The test was accessing an uninitialized `list_lru`, which is undefined behavior on any architecture.

Note that the sibling test `ttm_pool_free_dma_alloc` (which passes `TTM_ALLOCATION_POOL_USE_DMA_ALLOC`) correctly accesses `pool->caching[caching].orders[order]` because `ttm_pool_select_type()` *does* return the per-pool entry in that case, and `ttm_pool_type_init()` initializes it.

**Minor nits:**

1. **Commit message grammar:** "I'm not sure how that it was supposed to work previously" — the word "that" appears extraneous. Should be "I'm not sure how it was supposed to work previously."

2. **Duplicate Cc:** Christian König is Cc'd twice (once with the ö, once as "Koenig"). This is harmless but unnecessary.

3. **Test value:** After this patch, the test performs `ttm_pool_alloc` + `ttm_pool_free` + `ttm_pool_fini` with no assertions beyond the initial setup checks. It still has value as a crash/leak regression test, but it might be worth adding a TODO comment or a follow-up to add meaningful assertions — e.g., verifying pages were returned to the global pool (on x86) or freed to the system (on !x86). The commit message says "reverting for now" which implies this is intended as temporary, but in practice such things tend to persist.

4. **Pre-existing issue in the original test:** Even before the `list_lru` rewrite, the original test's assertions were conceptually wrong — they were checking the pool's own `ttm_pool_type` entry, but without DMA alloc, pages never go through that entry. On x86 they go to `global_uncached[order]`, and on !x86 they're freed directly. This suggests the original test (from the `list_lru` port commit `444e2a19d7fd`) had a latent bug that was masked by the old `list_head`-based pool implementation. This is consistent with the commit message's observation.

Overall this is a straightforward and correct CI fix. No functional concerns.

---
Generated by Claude Code Patch Reviewer