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 --- .../selftests/cgroup/lib/include/cgroup_util.h | 3 ++ tools/testing/selftests/cgroup/test_kmem.c | 2 +- tools/testing/selftests/cgroup/test_memcontrol.c | 47 +++++++++++++--------- 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/tools/testing/selftests/cgroup/lib/include/cgroup_util.h b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h index 5d39c709ac7a4..5307f3bf32a09 100644 --- a/tools/testing/selftests/cgroup/lib/include/cgroup_util.h +++ b/tools/testing/selftests/cgroup/lib/include/cgroup_util.h @@ -21,6 +21,9 @@ #define DEFAULT_WAIT_INTERVAL_US (100 * 1000) /* 100 ms */ +/* 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 437f2d35f205e..30af47ad69793 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 (sysconf(_SC_PAGESIZE) * 64 * get_nprocs()) +#define MAX_VMSTAT_ERROR (sysconf(_SC_PAGESIZE) * MEMCG_CHARGE_BATCH * get_nprocs()) #define KMEM_DEAD_WAIT_RETRIES 80 diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c index a24c1e07b2035..e9d3a04ecf7fc 100644 --- a/tools/testing/selftests/cgroup/test_memcontrol.c +++ b/tools/testing/selftests/cgroup/test_memcontrol.c @@ -18,11 +18,14 @@ #include #include #include +#include #include "kselftest.h" #include "cgroup_util.h" -#define MEMCG_SOCKSTAT_WAIT_RETRIES 30 +/* 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; @@ -1333,7 +1336,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; @@ -1347,17 +1351,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 "); @@ -1366,7 +1378,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; } @@ -1411,6 +1423,7 @@ static int test_memcg_sock(const char *root) unsigned short port; char *memcg; long sock_post = -1; + int flush_rounds = 3; if (memcg_sockets_disabled()) return KSFT_SKIP; @@ -1468,21 +1481,15 @@ static int test_memcg_sock(const char *root) if (cg_read_long(memcg, "memory.current") < 0) goto cleanup; - /* - * memory.stat is updated asynchronously via the memcg rstat - * flushing worker, which runs periodically (every 2 seconds, - * see FLUSH_TIME). On a busy system, the "sock " counter may - * stay non-zero for a short period of time after the TCP - * connection is closed and all socket memory has been - * uncharged. - * - * Poll memory.stat for up to 3 seconds (~FLUSH_TIME plus some - * scheduling slack) and require that the "sock " counter - * eventually drops to zero. - */ - sock_post = cg_read_key_long_poll(memcg, "memory.stat", "sock ", 0, - MEMCG_SOCKSTAT_WAIT_RETRIES, - DEFAULT_WAIT_INTERVAL_US); + /* 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; -- 2.43.0