From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 87902C433F5 for ; Sat, 23 Apr 2022 21:43:52 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9EC406B0074; Sat, 23 Apr 2022 17:43:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 99B066B0075; Sat, 23 Apr 2022 17:43:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8143E6B0078; Sat, 23 Apr 2022 17:43:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.28]) by kanga.kvack.org (Postfix) with ESMTP id 6DBC46B0074 for ; Sat, 23 Apr 2022 17:43:51 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay11.hostedemail.com (Postfix) with ESMTP id 3EAC2803D9 for ; Sat, 23 Apr 2022 21:43:51 +0000 (UTC) X-FDA: 79389471462.24.DF5A544 Received: from mail-pg1-f172.google.com (mail-pg1-f172.google.com [209.85.215.172]) by imf21.hostedemail.com (Postfix) with ESMTP id B51591C002B for ; Sat, 23 Apr 2022 21:43:48 +0000 (UTC) Received: by mail-pg1-f172.google.com with SMTP id i63so10195158pge.11 for ; Sat, 23 Apr 2022 14:43:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=NjdKMlafojykRWM3mLmTI0beW/EaYpJclIaGul4FrUE=; b=DPReL9r/tgtBxHjkNlFUr3vLePZV3WBKBCdqSzMfebUFeNdYbJ/ENvRK/DMKBq2S8Q gLdxMYTD/r0rB1oM71uJkyNGfmhAkNRJP9KfLeZV3hYSi73cL81ApuqK0mqT6jU8IAYf 0N56sADpN3Xkq48jtOg+tuJsm6VA3nAl921pZ+CQQfh6DaY3UIMoGmlqcaEkFaBuGsUF BmDd0YcJAuwAjyIfL3UniTtX7SPXUThVYlELl3Y5perm06zDnXZ+rV3AKMixc7Q8pZKz YvoRorV6wbnVw4L1iQwmzoC7meonzJsnBHwcBNixrZaEBPik4w4UydYvWeCWdLPI2kXz vTug== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=NjdKMlafojykRWM3mLmTI0beW/EaYpJclIaGul4FrUE=; b=DdzN5lHbZjmeij7Wm1nxqFJIhrFYrwfOOhh3+/b66VXBm9ZjuTQD5cMiEfwQBSGXft X6t6q+jYtqdy1RSIUzf4Twr5TkPTeTWnVyVLEw4r2+fWDC5ExY4rLtsbxs2bFIxXGCSe S/MMeYmKy2XAv+nC+C7wHrs1jXf1MDrsy84YvvzyIxRELtsXCVSTJHCwieL21YEzfdep xWbVQKx25QJZ6oDKpJ0HBzCZOIfMeHs6urQ+KODDF33/4FRxGEHxZLXRTdnNHZtYSp29 uls9foMaT4NhqTaq6lAESbP9W7DFtwOX0IGr1fTatD3BNQm2HcAtKNF4UnFV7Sfs0NlQ k6fg== X-Gm-Message-State: AOAM530dMyGyANa5cl3M4GgeDxLhbWfYBQMCCsD+0NNAu8I6owtENFmQ evBOqhLnV8OOEWYcK4AV8tdbIJumV27CX35jvHjwjw== X-Google-Smtp-Source: ABdhPJw4oCtdWDvodWpsyA5xVH+DH+MnprCEJol9U/OWK04/udNMPZ6L4XEE7Ab6fQOB4kMsx/PUTEbHKYrLAOv6N3o= X-Received: by 2002:a63:88c8:0:b0:3ab:1871:13ac with SMTP id l191-20020a6388c8000000b003ab187113acmr1557536pgd.561.1650750229438; Sat, 23 Apr 2022 14:43:49 -0700 (PDT) MIME-Version: 1.0 References: <20220421234426.3494842-1-yosryahmed@google.com> <20220421234426.3494842-5-yosryahmed@google.com> <20220423142801.gnvd42cdcsz4hpon@google.com> In-Reply-To: <20220423142801.gnvd42cdcsz4hpon@google.com> From: Yosry Ahmed Date: Sat, 23 Apr 2022 14:43:13 -0700 Message-ID: Subject: Re: [PATCH v4 4/4] selftests: cgroup: add a selftest for memory.reclaim To: Shakeel Butt Cc: Johannes Weiner , Michal Hocko , Andrew Morton , Roman Gushchin , David Rientjes , Tejun Heo , Zefan Li , Jonathan Corbet , Shuah Khan , Yu Zhao , Dave Hansen , Wei Xu , Greg Thelen , Chen Wandun , Vaibhav Jain , =?UTF-8?Q?Michal_Koutn=C3=BD?= , Tim Chen , Dan Schatzberg , cgroups@vger.kernel.org, linux-doc@vger.kernel.org, Linux Kernel Mailing List , Linux-MM , linux-kselftest@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: B51591C002B X-Stat-Signature: daj5gf8u4kgo1wy3w3k8j5wfwyargfwp Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b="DPReL9r/"; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf21.hostedemail.com: domain of yosryahmed@google.com designates 209.85.215.172 as permitted sender) smtp.mailfrom=yosryahmed@google.com X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1650750228-774557 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sat, Apr 23, 2022 at 7:28 AM Shakeel Butt wrote: > > On Thu, Apr 21, 2022 at 11:44:26PM +0000, Yosry Ahmed wrote: > > Add a new test for memory.reclaim that verifies that the interface > > correctly reclaims memory as intended, from both anon and file pages. > > > > Signed-off-by: Yosry Ahmed > > Acked-by: Roman Gushchin > > --- > > .../selftests/cgroup/test_memcontrol.c | 86 +++++++++++++++++++ > > 1 file changed, 86 insertions(+) > > > > diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c > > index f2ffb3a30194..5f7c20de2426 100644 > > --- a/tools/testing/selftests/cgroup/test_memcontrol.c > > +++ b/tools/testing/selftests/cgroup/test_memcontrol.c > > @@ -760,6 +760,91 @@ static int test_memcg_max(const char *root) > > return ret; > > } > > > > +/* > > + * This test checks that memory.reclaim reclaims the given > > + * amount of memory (from both anon and file). > > + */ > > +static int test_memcg_reclaim(const char *root) > > +{ > > + int ret = KSFT_FAIL, fd, retries; > > + char *memcg; > > + long current, to_reclaim; > > + char buf[64]; > > + > > + memcg = cg_name(root, "memcg_test"); > > + if (!memcg) > > + goto cleanup; > > + > > + if (cg_create(memcg)) > > + goto cleanup; > > + > > + current = cg_read_long(memcg, "memory.current"); > > + if (current != 0) > > + goto cleanup; > > + > > + cg_run_nowait(memcg, alloc_anon_noexit, (void *) MB(50)); > > Don't you need is_swap_enabled() check before deciding to do the anon > allocations? Yes you are right. In the next version will check whether or not swap is enabled and modify the test accordingly. > > + sleep(1); > > + > > + fd = get_temp_fd(); > > + if (fd < 0) > > + goto cleanup; > > + > > + cg_run_nowait(memcg, alloc_pagecache_50M_noexit, (void *)(long)fd); > > + sleep(1); > > These sleep(1)s do not seem robust. Since kernel keeps the page cache > around, you can convert anon to use tmpfs and use simple cg_run to > trigger the allocations of anon (tmpfs) and file which will remain in > memory even after return from cg_run. Other tests in the file are also using sleep approach (see test_memcg_min, although it retries for multiple times until memory.current reaches an expected amount). In my experience it hasn't been flaky running for multiple times on different machines, but I agree it can be flaky (false negative). I am not sure about the allocating file pages with cg_run, is it guaranteed that the page cache will remain in memory until the test ends? If it doesn't, it can also flake, but it would produce false positives (the test could pass because the kernel drained page cache for some other reason although the interface is not working correctly). In my personal opinion, false negative flakes are better than false positives. At least currently the test explicitly and clearly fails if the allocations are not successful. If we rely on the page cache remaining until the test finishes then it could silently pass if the interface is not working correctly. There are a few ways we can go forward with this: 1) Keep everything as-is, but print a message if the test fails due to memory.current not reaching 100MB to make it clear that it didn't fail due to a problem with the interface. 2) Add a sleep/retry loop similar to test_memcg_min instead of sleeping once. 3) Send a signal from forked children when they are done with the allocation, and wait to receive this signal in the test to make sure the allocation is completed. In my opinion we should do (1) (and maybe (2)) for now as (3) could be an overkill if the test is normal passing. Maybe add a comment about (3) being an option in the future if the test flakes. Let me know what you think?