selftests: cgroup: refactor proactive reclaim code to reclaim_until()

Refactor the code that drives writing to memory.reclaim (retrying, error
handling, etc) from test_memcg_reclaim() to a helper called
reclaim_until(), which proactively reclaims from a memcg until its usage
reaches a certain value.

While we are at it, refactor and simplify the reclaim loop.

This will be used in a following patch in another test.

Link: https://lkml.kernel.org/r/20221202031512.1365483-3-yosryahmed@google.com
Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
Suggested-by: Roman Gushchin <roman.gushchin@linux.dev>
Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>
Cc: Chris Down <chris@chrisdown.name>
Cc: David Rientjes <rientjes@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Muchun Song <songmuchun@bytedance.com>
Cc: Shakeel Butt <shakeelb@google.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Vasily Averin <vasily.averin@linux.dev>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Yu Zhao <yuzhao@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index 8833359..a8f4700 100644 (file)
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c
@@ -645,6 +645,48 @@ cleanup:
        return ret;
 }
 
+/*
+ * Reclaim from @memcg until usage reaches @goal by writing to
+ * memory.reclaim.
+ *
+ * This function will return false if the usage is already below the
+ * goal.
+ *
+ * This function assumes that writing to memory.reclaim is the only
+ * source of change in memory.current (no concurrent allocations or
+ * reclaim).
+ *
+ * This function makes sure memory.reclaim is sane. It will return
+ * false if memory.reclaim's error codes do not make sense, even if
+ * the usage goal was satisfied.
+ */
+static bool reclaim_until(const char *memcg, long goal)
+{
+       char buf[64];
+       int retries, err;
+       long current, to_reclaim;
+       bool reclaimed = false;
+
+       for (retries = 5; retries > 0; retries--) {
+               current = cg_read_long(memcg, "memory.current");
+
+               if (current < goal || values_close(current, goal, 3))
+                       break;
+               /* Did memory.reclaim return 0 incorrectly? */
+               else if (reclaimed)
+                       return false;
+
+               to_reclaim = current - goal;
+               snprintf(buf, sizeof(buf), "%ld", to_reclaim);
+               err = cg_write(memcg, "memory.reclaim", buf);
+               if (!err)
+                       reclaimed = true;
+               else if (err != -EAGAIN)
+                       return false;
+       }
+       return reclaimed;
+}
+
 /*
  * This test checks that memory.reclaim reclaims the given
  * amount of memory (from both anon and file, if possible).
@@ -653,8 +695,7 @@ static int test_memcg_reclaim(const char *root)
 {
        int ret = KSFT_FAIL, fd, retries;
        char *memcg;
-       long current, expected_usage, to_reclaim;
-       char buf[64];
+       long current, expected_usage;
 
        memcg = cg_name(root, "memcg_test");
        if (!memcg)
@@ -705,41 +746,8 @@ static int test_memcg_reclaim(const char *root)
         * Reclaim until current reaches 30M, this makes sure we hit both anon
         * and file if swap is enabled.
         */
-       retries = 5;
-       while (true) {
-               int err;
-
-               current = cg_read_long(memcg, "memory.current");
-               to_reclaim = current - MB(30);
-
-               /*
-                * We only keep looping if we get EAGAIN, which means we could
-                * not reclaim the full amount.
-                */
-               if (to_reclaim <= 0)
-                       goto cleanup;
-
-
-               snprintf(buf, sizeof(buf), "%ld", to_reclaim);
-               err = cg_write(memcg, "memory.reclaim", buf);
-               if (!err) {
-                       /*
-                        * If writing succeeds, then the written amount should have been
-                        * fully reclaimed (and maybe more).
-                        */
-                       current = cg_read_long(memcg, "memory.current");
-                       if (!values_close(current, MB(30), 3) && current > MB(30))
-                               goto cleanup;
-                       break;
-               }
-
-               /* The kernel could not reclaim the full amount, try again. */
-               if (err == -EAGAIN && retries--)
-                       continue;
-
-               /* We got an unexpected error or ran out of retries. */
+       if (!reclaim_until(memcg, MB(30)))
                goto cleanup;
-       }
 
        ret = KSFT_PASS;
 cleanup: