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

[Konstantin Khorenko <khorenko@virtuozzo.com>]

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