test_memcg_sock() currently requires that memory.stat's "sock " counter is exactly zero immediately after the TCP server exits. On a busy system this assumption is too strict:
- Socket memory may be freed with a small delay (e.g. RCU callbacks). - memcg statistics are updated asynchronously via the rstat flushing worker, so the "sock " value in memory.stat can stay non-zero for a short period of time even after all socket memory has been uncharged.
As a result, test_memcg_sock() can intermittently fail even though socket memory accounting is working correctly.
Make the test more robust by polling memory.stat for the "sock " counter and allowing it some time to drop to zero instead of checking it only once. The timeout is set to 3 seconds to cover the periodic rstat flush interval (FLUSH_TIME = 2*HZ by default) plus some scheduling slack. If the counter does not become zero within the timeout, the test still fails as before.
On my test system, running test_memcontrol 50 times produced:
- Before this patch: 6/50 runs passed. - After this patch: 50/50 runs passed.
Suggested-by: Lance Yang lance.yang@linux.dev Reviewed-by: Lance Yang lance.yang@linux.dev Signed-off-by: Guopeng Zhang zhangguopeng@kylinos.cn --- v3: - Move MEMCG_SOCKSTAT_WAIT_* defines after the #include block as suggested. v2: - Mention the periodic rstat flush interval (FLUSH_TIME = 2*HZ) in the comment and clarify the rationale for the 3s timeout. - Replace the hard-coded retry count and wait interval with macros to avoid magic numbers and make the 3s timeout calculation explicit. --- .../selftests/cgroup/test_memcontrol.c | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c index 4e1647568c5b..8ff7286fc80b 100644 --- a/tools/testing/selftests/cgroup/test_memcontrol.c +++ b/tools/testing/selftests/cgroup/test_memcontrol.c @@ -21,6 +21,9 @@ #include "kselftest.h" #include "cgroup_util.h"
+#define MEMCG_SOCKSTAT_WAIT_RETRIES 30 /* 3s total */ +#define MEMCG_SOCKSTAT_WAIT_INTERVAL_US (100 * 1000) /* 100 ms */ + static bool has_localevents; static bool has_recursiveprot;
@@ -1384,6 +1387,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 i;
memcg = cg_name(root, "memcg_test"); if (!memcg) @@ -1432,7 +1437,30 @@ 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 ")) + /* + * 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. + */ + for (i = 0; i < MEMCG_SOCKSTAT_WAIT_RETRIES; i++) { + sock_post = cg_read_key_long(memcg, "memory.stat", "sock "); + if (sock_post < 0) + goto cleanup; + + if (!sock_post) + break; + + usleep(MEMCG_SOCKSTAT_WAIT_INTERVAL_US); + } + + if (sock_post) goto cleanup;
ret = KSFT_PASS;