From a8c457e56ce15babb0e9640364e699c995f05b23 Mon Sep 17 00:00:00 2001 From: "randomizedcoder dave.seddon.ca@gmail.com" Date: Fri, 18 Sep 2026 10:23:02 -0700 Subject: [PATCH] fix(tx_pool): compute tx page-pool floor in 64-bit to avoid overflow homa_tx_pool_gc() turns the tx_page_pool_min_kb floor into a page count with `homa->tx_page_pool_min_kb * 1000`. tx_page_pool_min_kb is a plain int, so for large floors (a multi-GB tx reserve is a legitimate sysctl setting) the multiply overflows 32-bit int. The resulting min_pages is garbage -- typically negative -- which makes `release = max_low_mark - min_pages` huge, so gc reclaims pages that the floor was supposed to keep. Promote the multiply to u64 (and cast the shifted result back to int). Adds a table-driven regression test, homa_tx_pool_gc__min_kb_floor_no_overflow, whose corner row sets a floor whose `* 1000` overflows a 32-bit int: before the fix gc frees the whole pool (avail 10 -> 0); after the fix the floor is honored and the pool is left intact. Existing tx_pool suite stays green (30/30). Co-Authored-By: Claude Opus 4.8 --- homa_tx_pool.c | 4 ++-- test/unit_homa_tx_pool.c | 47 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/homa_tx_pool.c b/homa_tx_pool.c index 740ecb73..f622a856 100644 --- a/homa_tx_pool.c +++ b/homa_tx_pool.c @@ -417,8 +417,8 @@ void homa_tx_pool_gc(struct homa *homa) * releasing the lock, since freeing is expensive). */ spin_lock_bh(&max_pool->mutex); - min_pages = ((homa->tx_page_pool_min_kb * 1000) - + (HOMA_TX_PAGE_SIZE - 1)) >> HOMA_TX_PAGE_SHIFT; + min_pages = (int)((((u64)homa->tx_page_pool_min_kb * 1000) + + (HOMA_TX_PAGE_SIZE - 1)) >> HOMA_TX_PAGE_SHIFT); /* Note: may need to adjust max_low_mark to reflect changes made * while lock wasn't held. diff --git a/test/unit_homa_tx_pool.c b/test/unit_homa_tx_pool.c index 4769fc62..c12c5988 100644 --- a/test/unit_homa_tx_pool.c +++ b/test/unit_homa_tx_pool.c @@ -575,6 +575,53 @@ TEST_F(homa_tx_pool, homa_tx_pool_gc__empty_pool) homa_tx_pool_gc(&self->homa); EXPECT_EQ(0, get_tx_pool_core(0)->pool->avail); } +TEST_F(homa_tx_pool, homa_tx_pool_gc__min_kb_floor_no_overflow) +{ + /* homa_tx_pool_gc() converts the tx_page_pool_min_kb floor (an int, in + * KB) into a page count. Computing it as `min_kb * 1000` in 32-bit int + * overflows for large floors, yielding a bogus (often negative) + * min_pages and freeing pages that should have been kept. The floor + * must be computed in 64-bit. + * + * Table-driven: each row sets a floor and the resulting post-gc avail. + * The corner row's floor overflows a 32-bit `* 1000` but is harmless in + * 64-bit, so the pool must be left intact. + */ + static const struct { + const char *name; + int min_kb; /* tx_page_pool_min_kb (floor, in KB) */ + int pages; /* pages loaded into core 0's pool */ + int low_mark; + int exp_avail; /* expected avail after gc */ + } cases[] = { + {"modest_floor_frees_down_to_floor", + (5 * HOMA_TX_PAGE_SIZE) / 1000, 10, 9, 6}, + {"floor_exceeds_pool_frees_none", + (50 * HOMA_TX_PAGE_SIZE) / 1000, 10, 10, 10}, + {"huge_floor_no_int_overflow", + 3000000, 10, 10, 10}, + }; + int i; + + for (i = 0; i < (int)ARRAY_SIZE(cases); i++) { + TH_LOG("case: %s", cases[i].name); + + homa_tx_pool_cleanup(&self->homa); + EXPECT_EQ(0, homa_tx_pool_init(&self->homa)); + + mock_clock = 1000000; + self->homa.tx_page_free_time = 0; + self->homa.tx_page_frees_per_sec = 20; + self->homa.tx_page_pool_min_kb = cases[i].min_kb; + + add_to_pool(&self->homa, cases[i].pages, 0); + get_tx_pool_core(0)->pool->low_mark = cases[i].low_mark; + + homa_tx_pool_gc(&self->homa); + EXPECT_EQ(cases[i].exp_avail, + get_tx_pool_core(0)->pool->avail); + } +} TEST_F(homa_tx_pool, homa_copy_to_frags) {