On Wed, Nov 23, 2022 at 09:21:31AM +0000, Yosry Ahmed wrote:
> 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.
>
> This will be used in a following patch in another test.
>
> Signed-off-by: Yosry Ahmed <yosryahmed@xxxxxxxxxx>
> ---
> .../selftests/cgroup/test_memcontrol.c | 85 +++++++++++--------
> 1 file changed, 49 insertions(+), 36 deletions(-)
>
> diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
> index 8833359556f3..d4182e94945e 100644
> --- a/tools/testing/selftests/cgroup/test_memcontrol.c
> +++ b/tools/testing/selftests/cgroup/test_memcontrol.c
> @@ -645,6 +645,53 @@ static int test_memcg_max(const char *root)
> return ret;
The code below looks correct, but can be simplified a bit.
And btw thank you for adding a test!
Reviewed-by: Roman Gushchin <roman.gushchin@xxxxxxxxx>
(idk if you want invest your time in further simplication of this code,
it was this way before this patch, so up to you).
> }
>
> +/* Reclaim from @memcg until usage reaches @goal_usage */
> +static bool reclaim_until(const char *memcg, long goal_usage)
> +{
> + char buf[64];
> + int retries = 5;
> + int err;
> + long current, to_reclaim;
> +
> + /* Nothing to do here */
> + if (cg_read_long(memcg, "memory.current") <= goal_usage)
> + return true;
> +
> + while (true) {
> + current = cg_read_long(memcg, "memory.current");
> + to_reclaim = current - goal_usage;
> +
> + /*
> + * We only keep looping if we get -EAGAIN, which means we could
> + * not reclaim the full amount. This means we got -EAGAIN when
> + * we actually reclaimed the requested amount, so fail.
> + */
> + if (to_reclaim <= 0)
> + break;
> +
> + 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, goal_usage, 3) && current > goal_usage)
> + break;
There are 3 places in this function where memory.current is read and compared
to goal_usage. I believe only one can be left.
> + return true;
> + }
> +
> + /* 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. */
> + break;
if (err != -EAGAIN || retries--)
break;
Thanks!
