From: "Michal Koutný" <mkoutny@suse.com>
To: Wilson Felipe Pereira <wfelipe@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
Yosry Ahmed <yosry@kernel.org>, Nhat Pham <nphamcs@gmail.com>,
Chengming Zhou <chengming.zhou@linux.dev>,
Tejun Heo <tj@kernel.org>, Shuah Khan <shuah@kernel.org>,
linux-mm@kvack.org, cgroups@vger.kernel.org,
linux-kselftest@vger.kernel.org
Subject: Re: [PATCH 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass
Date: Thu, 6 Aug 2026 10:32:49 +0200 [thread overview]
Message-ID: <anRFukpcm9Pb97AQ@localhost.localdomain> (raw)
In-Reply-To: <20260804042053.56940-3-wfelipe@google.com>
[-- Attachment #1: Type: text/plain, Size: 1316 bytes --]
On Tue, Aug 04, 2026 at 04:20:45AM +0000, Wilson Felipe Pereira <wfelipe@google.com> 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.
Thanks for the breakdown.
> Fix this by checking abs(delta), ensuring the test correctly compares the
> absolute difference between system zswap and cgroup zswapped bytes.
I still think the delta quantity has some meaning here and taking the
abs() changes the semantics of the checked inequality.
What about making both zswapped and delta `long`s? (Under similar
reasoning, there's already a possible loss after squashing
cg_read_key_long() result into the `int`.)
In any way, also
Fixes: a549f9f31561a ("selftests: cgroup: add test_zswap with no kmem bypass test")
Thanks,
Michal
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]
prev parent reply other threads:[~2026-08-06 8:32 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-04 4:20 [PATCH 0/2] selftests/cgroup: test_zswap: fix single-core flakes Wilson Felipe Pereira
2026-08-04 4:20 ` [PATCH 1/2] selftests/cgroup: test_zswap: retry subtree_control write on EBUSY in test_zswap_writeback Wilson Felipe Pereira
[not found] ` <CAO9r8zM9H8iMOE2FE-H47CMUAExf3VcCf04anT=_Ja=m=sbr4g@mail.gmail.com>
2026-08-06 8:33 ` Michal Koutný
2026-08-04 4:20 ` [PATCH 2/2] selftests/cgroup: test_zswap: fix implicit unsigned promotion bug in test_no_kmem_bypass Wilson Felipe Pereira
2026-08-06 8:32 ` Michal Koutný [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=anRFukpcm9Pb97AQ@localhost.localdomain \
--to=mkoutny@suse.com \
--cc=cgroups@vger.kernel.org \
--cc=chengming.zhou@linux.dev \
--cc=hannes@cmpxchg.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=nphamcs@gmail.com \
--cc=shuah@kernel.org \
--cc=tj@kernel.org \
--cc=wfelipe@google.com \
--cc=yosry@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox