Michal Koutný <[email protected]> writes: > On Tue, Sep 01, 2026 at 05:22:28AM +0000, Wilson Felipe Pereira > <[email protected]> wrote: >> In test_no_kmem_bypass(), delta (stored_pages * page_size - zswapped) is >> checked against stored_pages * page_size / 4 to verify that the pages >> pushed to zswap belong to the test memory cgroup. >> >> Due to slight stat update timing differences, delta can evaluate to a small >> negative number (e.g. -5MB out of 1GB). Because delta is declared as a >> signed int and stored_pages is an unsigned size_t, C's usual arithmetic >> conversions implicitly promote a negative delta to a large unsigned 64-bit >> integer, causing `delta < stored_pages * page_size / 4` to falsely evaluate >> to 0 and fail the test. >> >> Fix this by declaring zswapped and delta as signed long long and comparing >> against a signed threshold, ensuring negative deltas correctly evaluate >> to true. >> >> Fixes: a549f9f31561a ("selftests: cgroup: add test_zswap with no kmem bypass >> test") >> Signed-off-by: Wilson Felipe Pereira <[email protected]> >> Acked-by: Michal Koutný <[email protected]> > > (The long long helper is a non-trivial change, so the ack should be > stripped on this version (unless you can convince me ;-).)
I'm sorry for that! Thanks for the review and for the patience. > > I'm afraid this bitness propagated too broadly. > > The long vs long long difference is only relevant on 32b, right? > And the bit-width issue only appears because of the product, not the > values read from memory.stat [1]. > > I'd consider three ways forward: > a) Consolidate the helpers into one cg_read_key_s64() (same explicit size > regardless of arch), > b) keep single arch-dependent helper and tackle product(s) only, > c) do not bother with 32b in these selftests (I have a hunch that > they're not so relevant on such archs). > > WDYT? I like this option a), any thoughts on the sequence here? I thought the following steps: 1) Add cg_read_key_s64() here and make use of it; 2) In another patch (probably 3/3), replace all uses of cg_read_key_long by cg_read_key_s64. There are 41 uses currently. I believe the tests could be flaky in 32b already (just a guess), at least option a) would make all be more deterministic. > > Michal > > > [1] At least the values derived from available memory, memory events > could grow indefinitely. It's true that kernel uses explicit u64 for > all of the entries.
