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;
Hello Guopeng.
+Cc Leon Huang Fu leon.huangfu@shopee.com
On Thu, Nov 20, 2025 at 02:04:06PM +0800, Guopeng Zhang zhangguopeng@kylinos.cn wrote:
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).
(FTR, I remember there is `echo 1 > /sys/module/rcutree/parameters/do_rcu_barrier`, however, I'm not sure it works always as expected (a reader may actually wait for multi-stage RCU pipeline), so plain timeout is more reliable.)
- 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.
I like the approach of adaptive waiting to settle in such tests.
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.
BTW Have you looked into the number of retries until success? Was it in accordance with the flushing interval?
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);- }
I think this may be useful also for othe tests (at least other memory.stat checks), so some encapsulated implementation like a macro with parameters cg_read_assert_gt_with_retries(cg, file, field, exp, timeout, retries) WDYT?
Michal
On 11/20/25 23:35, Michal Koutný wrote:
Hello Guopeng.
+Cc Leon Huang Fu leon.huangfu@shopee.com
On Thu, Nov 20, 2025 at 02:04:06PM +0800, Guopeng Zhang zhangguopeng@kylinos.cn wrote:
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).
(FTR, I remember there is `echo 1 > /sys/module/rcutree/parameters/do_rcu_barrier`, however, I'm not sure it works always as expected (a reader may actually wait for multi-stage RCU pipeline), so plain timeout is more reliable.)
Hi Michal,
Thank you for the suggestion.
I tested using `echo 1 > /sys/module/rcutree/parameters/do_rcu_barrier`, but unfortunately the effect was not very good on my setup. As you mentioned, a reader may actually wait for the multi-stage RCU pipeline, so a plain timeout seems more reliable here.
- 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.
I like the approach of adaptive waiting to settle in such tests.
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.
BTW Have you looked into the number of retries until success? Was it in accordance with the flushing interval?
Yes. From my observations, it usually succeeds after about 10–15 retries on average (roughly 1–1.5 seconds), and occasionally it takes more than 20 retries (>2 seconds). This looks broadly in line with the periodic rstat flushing interval (~2 seconds) plus some scheduling slack.
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);- }
I think this may be useful also for othe tests (at least other memory.stat checks), so some encapsulated implementation like a macro with parameters cg_read_assert_gt_with_retries(cg, file, field, exp, timeout, retries) WDYT?
Michal
That’s a great idea. I agree this pattern could be useful for other `memory.stat` checks as well, and I will implement an encapsulated helper/macro along those lines as per your suggestion.
Thanks, Guopeng
linux-kselftest-mirror@lists.linaro.org
-
Guopeng Zhang -
Michal Koutný