Hello Petr,
On Wed, 15 Oct 2025 at 17:02, Petr Vorel pvorel@suse.cz wrote:
On Wed, Oct 15, 2025 at 04:42:47PM +0200, Petr Vorel wrote:
The overcommit test uses sum_total, the sum (memory and swap) to test the overcommit settings.
This fixes two problems on 32-bit systems. The first is seen with a integer overflow can occur when calculating sum_total * 2, if the sum_total is larger than 2GB. The second is limited virtual address
You still mention GB ...
Yep. It is GB.
space (2-3GB) means the test can fail from address space exhaustion before overcommit has been tested.
Now the test runs correctly on low-memory 32-bit systems while avoiding both the overflow bug and virtual address space issues.
Signed-off-by: Ben Copeland ben.copeland@linaro.org Acked-by: Arnd Bergmann arnd@arndb.de Reviewed-by: Petr Vorel pvorel@suse.cz
.../kernel/mem/tunable/overcommit_memory.c | 33 +++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/testcases/kernel/mem/tunable/overcommit_memory.c b/testcases/kernel/mem/tunable/overcommit_memory.c index 9b2cb479d..7ff5a98d0 100644 --- a/testcases/kernel/mem/tunable/overcommit_memory.c +++ b/testcases/kernel/mem/tunable/overcommit_memory.c @@ -131,24 +131,45 @@ static void overcommit_memory_test(void) TST_SYS_CONF_LONG_SET(OVERCOMMIT_MEMORY, 2, 1);
update_mem_commit();
- alloc_and_check(commit_left * 2, EXPECT_FAIL);
- alloc_and_check(commit_limit + total_batch_size, EXPECT_FAIL);
- /* Skip tests that would overflow or exceed 32-bit address space */
- if (tst_kernel_bits() == 64 || (unsigned long)commit_left <= TST_GB / TST_KB) {
... but TST_GB / TST_KB is actually MB (you could use TST_MB).
The sizes in this test are measured in KB, so it's 1GB but measured in terms of KB not bytes. Using TST_MB would work mathematically but it's misleading.
Ah, I'm sorry to overlook an obvious point.
Implementation details: thinking about the code twice, shouldn't be the check for overflow in alloc_and_check() instead of outside (to keep the condition on single place)?
Excellent suggestion. It makes sense to simplify it like that. I will spin up a v3 patch with those changes.
Ben
Also, if kept outside the 1st and 2nd if could be joined:
/* Skip tests that would overflow or exceed 32-bit address space */ if (tst_kernel_bits() == 64 || (unsigned long)commit_left <= TST_GB / TST_KB) { alloc_and_check(commit_left * 2, EXPECT_FAIL); alloc_and_check(commit_limit + total_batch_size, EXPECT_FAIL); update_mem_commit(); alloc_and_check(commit_left / 2, EXPECT_PASS); } else { tst_res(TCONF, "Skipping large allocation tests due to address space limits"); }instead of your proposal:
update_mem_commit(); /* Skip tests that would overflow or exceed 32-bit address space */ if (tst_kernel_bits() == 64 || (unsigned long)commit_left <= TST_GB / TST_KB) { alloc_and_check(commit_left * 2, EXPECT_FAIL); alloc_and_check(commit_limit + total_batch_size, EXPECT_FAIL); } else { tst_res(TCONF, "Skipping large allocation tests due to address space limits"); } update_mem_commit(); if (tst_kernel_bits() == 64 || (unsigned long)commit_left <= TST_GB / TST_KB) { alloc_and_check(commit_left / 2, EXPECT_PASS); } else { tst_res(TCONF, "Skipping commit_left/2 allocation test due to address space limits"); }because update_mem_commit() IMHO just evaluates /proc/meminfo values, but when alloc_and_check() is skipped nothing has changed.
Kind regards, Petr
regards, dan carpenter