[Devel] [PATCH hci-8.1 0/2] selftests: cgroup: fix the flaky and vacuous test_memcg_sock

[Devel] [PATCH hci-8.1 0/2] selftests: cgroup: fix the flaky and vacuous test_memcg_sock

[Konstantin Khorenko]

test_memcg_sock compares memory.current of its cgroup with the "sock "
entry of memory.stat. memory.current moves at charge time, "sock " only
when the memcg stats are flushed, and nothing in the test makes a flush
happen. So the test either compares 0 with 0 and passes without having
checked anything, or fails at random.

This tree is worse off than mainline was before the fix: the final
check reads "sock " once, right after the connection is closed, with no
wait at all. Mainline polled for 3 seconds there (6360d444ae32), this
tree never got that commit.

It fails here
=============

10.193.16.26 (4 CPUs), test_memcontrol run 100 times before and after
the series, interleaved:

  kernel                                  before      after
  6.12.0-211.52.1.22.8.vz10 (release)     0 failed    0 failed
  6.12.0-211.52.1.22.8.vz10 (debug)       5 failed    0 failed

Every failure on the debug kernel is the final check: the client part
of the test had passed, the single read still saw the socket memory.

The release kernel passes here only because the machine is small. A
memory.stat read flushes once MEMCG_CHARGE_BATCH pages per online CPU
of updates are pending, so on 4 CPUs the test traffic crosses the
threshold by itself. On a 48-CPU host mainline's test failed 8 of 30
runs on a debug kernel, and most of its passes compared 0 with 0.

The patches
===========

Both are in mainline's cgroup tree, cgroup/for-7.4:
https://lore.kernel.org/r/20261012-b4-prep-vstor-149854-let-test_memcg_sock-compare-flushed-sock-stats-v1-0-bc864b1950ad@virtuozzo.com

- 1/2 skips the test under cgroup.memory=nosocket, where no socket
  memory is charged and the test can only pass on 0 == 0.
- 2/2 makes the test cause the flushes it needs instead of waiting for
  them: the client keeps the data in the server's socket and reads until
  enough socket memory has moved through the cgroup that a memory.stat
  read must flush, and the final check allocates and frees anonymous
  memory in the cgroup before it reads "sock ".

The backport differs from mainline only as the Conflict: notes say:

- 1/2 sizes its buffer with PAGE_SIZE, mainline renamed it to BUF_SIZE
- 2/2 replaces the single "sock " read, not mainline's poll

https://virtuozzo.atlassian.net/browse/VSTOR-149854

---
Konstantin Khorenko (2):
      ms/selftests: cgroup: skip test_memcg_sock under cgroup.memory=nosocket
      ms/selftests: cgroup: make test_memcg_sock check flushed sock stats

 .../selftests/cgroup/lib/include/cgroup_util.h     |  3 ++
 tools/testing/selftests/cgroup/test_kmem.c         |  2 +-
 tools/testing/selftests/cgroup/test_memcontrol.c   | 56 ++++++++++++++++++++--
 3 files changed, 55 insertions(+), 6 deletions(-)
---
base-commit: 8026fb5599f945a36a6911f8febad3ab569176da
change-id: 20261019-b4-prep-khorenko-vstor-149854-selftests-test_memcg_sock-6c864da317a3

Best regards,
--  
Konstantin Khorenko <khorenko@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 1/2] ms/selftests: cgroup: skip test_memcg_sock under cgroup.memory=nosocket

[Konstantin Khorenko]

With cgroup.memory=nosocket the memory controller does not charge
socket memory, so the "sock " entry of memory.stat stays 0 and
memory.current moves only by the kernel objects accept() allocates.
tcp_client() then passes on 0 == 0 without having checked any
accounting, or fails if those objects took a fresh charge batch.

Look for nosocket among the cgroup.memory= options on the kernel command
line and skip the test there.

Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
(cherry picked from commit 216a983640bce942e25c62e9d2f901dfba888cca)

Conflict: cgroup_util.h here still calls the buffer size PAGE_SIZE, the
rename to BUF_SIZE in 6e9f5c2eecd1 ("selftests/cgroup: rename PAGE_SIZE
to BUF_SIZE in cgroup_util") is not in the tree, so
memcg_sockets_disabled() sizes its buffer with PAGE_SIZE. The value is
the same.

https://virtuozzo.atlassian.net/browse/VSTOR-149854

Feature: fix selftests
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 tools/testing/selftests/cgroup/test_memcontrol.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index 7519aeff4dbd1..003c865d8db93 100644
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c
@@ -1356,6 +1356,24 @@ static int tcp_client(const char *cgroup, unsigned short port)
 	return ret;
 }
 
+static bool memcg_sockets_disabled(void)
+{
+	char buf[4 * PAGE_SIZE], *opt;
+
+	if (read_text("/proc/cmdline", buf, sizeof(buf)) <= 0)
+		return false;
+
+	for (opt = strstr(buf, "cgroup.memory="); opt;
+	     opt = strstr(opt + 1, "cgroup.memory=")) {
+		char *end = strchrnul(opt, ' ');
+		char *ns = strstr(opt, "nosocket");
+
+		if (ns && ns < end)
+			return true;
+	}
+	return false;
+}
+
 /*
  * This test checks socket memory accounting.
  * The test forks a TCP server listens on a random port between 1000
@@ -1370,6 +1388,9 @@ static int test_memcg_sock(const char *root)
 	unsigned short port;
 	char *memcg;
 
+	if (memcg_sockets_disabled())
+		return KSFT_SKIP;
+
 	memcg = cg_name(root, "memcg_test");
 	if (!memcg)
 		goto cleanup;

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 2/2] ms/selftests: cgroup: make test_memcg_sock check flushed sock stats

[Konstantin Khorenko]

test_memcg_sock compares memory.current with the "sock " entry of
memory.stat. memory.current changes at charge time, "sock " only when
the memcg stats are flushed: by a memory.stat read once enough updates
are pending (MEMCG_CHARGE_BATCH per online CPU), or by a periodic,
deferrable worker. Nothing in the test makes sure a flush happens:

- tcp_client() takes 16 readings right after connect(). If no flush
  lands among them, "sock " reads 0, and the test either passes on
  0 == 0 without checking anything or, once memory.current shows the
  first charge, cannot pass any more and fails:

    not ok 11 test_memcg_sock

- The final check waits 3 seconds for "sock " to drop to 0, counting on
  the deferrable worker, which can run later than that on an idle
  machine.

Make the test cause the flushes instead of waiting for them. Every page
of socket or anonymous memory that comes and goes in the cgroup counts
towards the flush threshold, and MEMCG_FLUSH_BYTES (twice the vmstat error
test_kmem allows for) is enough to cross it. After that much, either
another flush has reset the count or the next memory.stat read flushes.

- tcp_client() sets a small receive buffer before it connects. Only
  the server's socket is charged to the test cgroup, and with a large
  receive window its data moves straight on to the client; with few
  CPUs the client then drains it between reads and "sock " stays near 0.
  A small window keeps the data queued in the server's socket.
- tcp_client() then keeps reading, comparing after every read, until
  "sock " is nonzero and matches memory.current, for up to four times
  MEMCG_FLUSH_BYTES. A comparison can still miss after a flush: one
  that came midway leaves "sock " behind, and memory.current also holds
  the per-CPU charge stock. The budget leaves room for several flushes.
- After the server has exited, the test allocates and frees
  MEMCG_FLUSH_BYTES of anonymous memory in the cgroup and reads "sock ",
  for up to three rounds until it reads 0. The socket memory is
  uncharged before the first round, so any flush that has gone through
  all CPUs since sees "sock " at 0. A flush still running in parallel
  can reset the count before it reaches the CPU that did the uncharge,
  and the read then misses the uncharge. That flush only has to finish
  its walk over the CPUs for the next read to see it, and each round
  gives it the time of another allocation. Three rounds are a margin for
  a flusher that gets rescheduled midway, not a guarantee: nothing
  bounds how long it can be held up.

MEMCG_CHARGE_BATCH goes to cgroup_util.h, and test_kmem's
MAX_VMSTAT_ERROR uses it too.

Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
(cherry picked from commit eb69e955de7bdea6aa932fc4cce413ca439a4a5d)

Conflict: 6360d444ae32 ("selftests: cgroup: make test_memcg_sock robust
against delayed sock stats") is not in the tree, so the final check here
reads "sock " once instead of polling it. The flush rounds replace that
single read, and test_memcg_sock() gets the sock_post declaration that
commit added. The rest is context:
- cgroup_util.h has no DEFAULT_WAIT_INTERVAL_US, MEMCG_CHARGE_BATCH goes
  after TEST_UID
- test_kmem.c's MAX_VMSTAT_ERROR still uses 4096 for the page size
- test_memcontrol.c includes "../kselftest.h"

https://virtuozzo.atlassian.net/browse/VSTOR-149854

Feature: fix selftests
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 .../selftests/cgroup/lib/include/cgroup_util.h     |  3 ++
 tools/testing/selftests/cgroup/test_kmem.c         |  2 +-
 tools/testing/selftests/cgroup/test_memcontrol.c   | 35 ++++++++++++++++++----
 3 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/cgroup/lib/include/cgroup_util.h b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
index 0096bdbb757bd..4faebce101255 100644
--- a/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
+++ b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h
@@ -13,6 +13,9 @@
 
 #define TEST_UID	65534 /* usually nobody, any !root is fine */
 
+/* from include/linux/memcontrol.h, which userspace cannot include */
+#define MEMCG_CHARGE_BATCH	64
+
 /*
  * Checks if two given values differ by less than err% of their sum.
  */
diff --git a/tools/testing/selftests/cgroup/test_kmem.c b/tools/testing/selftests/cgroup/test_kmem.c
index 96693d8772be2..56255771667f5 100644
--- a/tools/testing/selftests/cgroup/test_kmem.c
+++ b/tools/testing/selftests/cgroup/test_kmem.c
@@ -24,7 +24,7 @@
  * the maximum discrepancy between charge and vmstat entries is number
  * of cpus multiplied by 64 pages.
  */
-#define MAX_VMSTAT_ERROR (4096 * 64 * get_nprocs())
+#define MAX_VMSTAT_ERROR (4096 * MEMCG_CHARGE_BATCH * get_nprocs())
 
 
 static int alloc_dcache(const char *cgroup, void *arg)
diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index 003c865d8db93..88960a9331c00 100644
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c
@@ -17,10 +17,15 @@
 #include <netdb.h>
 #include <errno.h>
 #include <sys/mman.h>
+#include <sys/sysinfo.h>
 
 #include "../kselftest.h"
 #include "cgroup_util.h"
 
+/* enough page updates for a memory.stat read to flush, see MAX_VMSTAT_ERROR in test_kmem.c */
+#define MEMCG_FLUSH_BYTES	\
+	(2 * sysconf(_SC_PAGESIZE) * MEMCG_CHARGE_BATCH * get_nprocs())
+
 static bool has_localevents;
 static bool has_recursiveprot;
 
@@ -1310,7 +1315,8 @@ static int tcp_client(const char *cgroup, unsigned short port)
 	const char server[] = "localhost";
 	struct addrinfo *ai;
 	char servport[6];
-	int retries = 0x10; /* nice round number */
+	long budget = 4 * MEMCG_FLUSH_BYTES;
+	int rcvbuf = 64 * 1024;
 	int sk, ret;
 	long allocated;
 
@@ -1324,17 +1330,25 @@ static int tcp_client(const char *cgroup, unsigned short port)
 	if (sk < 0)
 		goto free_ainfo;
 
+	/* keep the data in the server's socket, the one charged to the cgroup */
+	ret = setsockopt(sk, SOL_SOCKET, SO_RCVBUF, &rcvbuf, sizeof(rcvbuf));
+	if (ret < 0)
+		goto close_sk;
+
 	ret = connect(sk, ai->ai_addr, ai->ai_addrlen);
 	if (ret < 0)
 		goto close_sk;
 
 	ret = KSFT_FAIL;
-	while (retries--) {
+	while (budget > 0) {
 		uint8_t buf[0x100000];
 		long current, sock;
+		ssize_t len;
 
-		if (read(sk, buf, sizeof(buf)) <= 0)
+		len = read(sk, buf, sizeof(buf));
+		if (len <= 0)
 			goto close_sk;
+		budget -= len;
 
 		current = cg_read_long(cgroup, "memory.current");
 		sock = cg_read_key_long(cgroup, "memory.stat", "sock ");
@@ -1343,7 +1357,7 @@ static int tcp_client(const char *cgroup, unsigned short port)
 			goto close_sk;
 
 		/* exclude the memory not related to socket connection */
-		if (values_close(current - allocated, sock, 10)) {
+		if (sock > 0 && values_close(current - allocated, sock, 10)) {
 			ret = KSFT_PASS;
 			break;
 		}
@@ -1387,6 +1401,8 @@ static int test_memcg_sock(const char *root)
 	int bind_retries = 5, ret = KSFT_FAIL, pid, err;
 	unsigned short port;
 	char *memcg;
+	long sock_post = -1;
+	int flush_rounds = 3;
 
 	if (memcg_sockets_disabled())
 		return KSFT_SKIP;
@@ -1438,7 +1454,16 @@ static int test_memcg_sock(const char *root)
 	if (cg_read_long(memcg, "memory.current") < 0)
 		goto cleanup;
 
-	if (cg_read_key_long(memcg, "memory.stat", "sock "))
+	/* the uncharge is done, but a flush still walking the CPUs can hide it from a read */
+	while (flush_rounds--) {
+		if (cg_run(memcg, alloc_anon, (void *)MEMCG_FLUSH_BYTES))
+			goto cleanup;
+
+		sock_post = cg_read_key_long(memcg, "memory.stat", "sock ");
+		if (!sock_post)
+			break;
+	}
+	if (sock_post)
 		goto cleanup;
 
 	ret = KSFT_PASS;

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 2/2] ms/selftests: cgroup: make test_memcg_sock check flushed sock stats

[no-reply]

> test_memcg_sock compares memory.current with the "sock " entry of
> memory.stat. memory.current changes at charge time, "sock " only when
> the memcg stats are flushed: by a memory.stat read once enough updates
> are pending (MEMCG_CHARGE_BATCH per online CPU), or by a periodic,
> deferrable worker. Nothing in the test makes sure a flush happens:
> 
> - tcp_client() takes 16 readings right after connect(). If no flush
>   lands among them, "sock " reads 0, and the test either passes on
>   0 == 0 without checking anything or, once memory.current shows the
>   first charge, cannot pass any more and fails:
> 
>     not ok 11 test_memcg_sock
> 
> - The final check waits 3 seconds for "sock " to drop to 0, counting on
>   the deferrable worker, which can run later than that on an idle
> [ ... ]
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-149854
> 
> Feature: fix selftests
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261019-b4-prep-khorenko-vstor-149854-selftests-test_memcg_sock-v1-0-efbfa45dc967@virtuozzo.com?part=2


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 1/2] ms/selftests: cgroup: skip test_memcg_sock under cgroup.memory=nosocket

[no-reply]

> With cgroup.memory=nosocket the memory controller does not charge
> socket memory, so the "sock " entry of memory.stat stays 0 and
> memory.current moves only by the kernel objects accept() allocates.
> tcp_client() then passes on 0 == 0 without having checked any
> accounting, or fails if those objects took a fresh charge batch.
> 
> Look for nosocket among the cgroup.memory= options on the kernel command
> line and skip the test there.
> 
> Assisted-by: LLM
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> (cherry picked from commit 216a983640bce942e25c62e9d2f901dfba888cca)
> 
> Conflict: cgroup_util.h here still calls the buffer size PAGE_SIZE, the
> rename to BUF_SIZE in 6e9f5c2eecd1 ("selftests/cgroup: rename PAGE_SIZE
> to BUF_SIZE in cgroup_util") is not in the tree, so
> memcg_sockets_disabled() sizes its buffer with PAGE_SIZE. The value is
> the same.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-149854
> 
> Feature: fix selftests
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261019-b4-prep-khorenko-vstor-149854-selftests-test_memcg_sock-v1-0-efbfa45dc967@virtuozzo.com?part=1


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 0/2] selftests: cgroup: fix the flaky and vacuous test_memcg_sock

[Konstantin Khorenko]

On Mon, 19 Oct 2026 21:47:32 +0200, Konstantin Khorenko wrote:
> selftests: cgroup: fix the flaky and vacuous test_memcg_sock
> 
> test_memcg_sock compares memory.current of its cgroup with the "sock "
> entry of memory.stat. memory.current moves at charge time, "sock " only
> when the memcg stats are flushed, and nothing in the test makes a flush
> happen. So the test either compares 0 with 0 and passes without having
> checked anything, or fails at random.
> 
> [...]

Applied to branch-rh10-6.12.0-211.52.1.22.x.vz10-ovz, will be released in rh10-6.12.0-211.52.1.22.9.vz10, thanks!

[1/2] ms/selftests: cgroup: skip test_memcg_sock under cgroup.memory=nosocket
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/f7931ffaaea2
[2/2] ms/selftests: cgroup: make test_memcg_sock check flushed sock stats
      https://bitbucket.org/virtuozzocore/vzkernel.vzs/commits/8d450687314f

-- 
Konstantin Khorenko <khorenko@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel