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 9E22FC433F5 for ; Sat, 23 Apr 2022 11:36:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F31A86B0073; Sat, 23 Apr 2022 07:36:56 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EE0416B0074; Sat, 23 Apr 2022 07:36:56 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DA86F6B0075; Sat, 23 Apr 2022 07:36:56 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.25]) by kanga.kvack.org (Postfix) with ESMTP id CBAF76B0073 for ; Sat, 23 Apr 2022 07:36:56 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 9700460EDB for ; Sat, 23 Apr 2022 11:36:56 +0000 (UTC) X-FDA: 79387942032.22.5489395 Received: from mail-qk1-f182.google.com (mail-qk1-f182.google.com [209.85.222.182]) by imf09.hostedemail.com (Postfix) with ESMTP id 0AE2514000E for ; Sat, 23 Apr 2022 11:36:53 +0000 (UTC) Received: by mail-qk1-f182.google.com with SMTP id q75so7590181qke.6 for ; Sat, 23 Apr 2022 04:36:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=BcOqG7n5x1u/kJMqwh39Y5RSFSmbFCI9uw/6+AwfTFk=; b=xND5criTdVqz9SC+NDtV3JmsEYg8egVjOe6XMAJcq8D42T0p9Gyew+ThrMgmEDW/zG C4b8Fbv94SWIuceJ8oVZ1yruqT9U/tRUHdRby0JeQ77CB6lULe6IUwRDJDdiC8O5nqQ0 XVDKi3k9Bdh7tNI5tpPJKJFlHMdCEJDVB+vraAUjQhgMJWwj5m3K1oqm5K+0cu87jDhz tNxfHOm8z8CMQtW55uSxsRRGbtWvx24P1HP2jrUyPVoXJ2cUHUOGJ7UlrcP+KO/7ZDES CFgcGDQhzi7+XOgYttgAawdS+98X/lzuSV5jfvQmqPB6ebug4Jk+9QzvsqrflZANZxDG rf+g== X-Gm-Message-State: AOAM533nXEdh1sNah7XedAWS5tHKG1RLLjtXjb+56oLMqW3/yNq9EVFE Tc6bXX6R+bkbYtTv6Dy3vjE= X-Google-Smtp-Source: ABdhPJxuD1TVwkZ0HbifP6E6S0QwJLLQMTOA4Qo7MnFnODcBZD0xZcU2q1vPD/m7S/aMeB9RyKrfQw== X-Received: by 2002:a05:620a:2495:b0:69e:e047:c6e7 with SMTP id i21-20020a05620a249500b0069ee047c6e7mr5094904qkn.737.1650713815298; Sat, 23 Apr 2022 04:36:55 -0700 (PDT) Received: from dev0025.ash9.facebook.com (fwdproxy-ash-119.fbsv.net. [2a03:2880:20ff:77::face:b00c]) by smtp.gmail.com with ESMTPSA id a63-20020ae9e842000000b0069e6722632bsm2246042qkg.39.2022.04.23.04.36.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 23 Apr 2022 04:36:55 -0700 (PDT) Date: Sat, 23 Apr 2022 04:36:52 -0700 From: David Vernet To: Roman Gushchin Cc: akpm@linux-foundation.org, tj@kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, cgroups@vger.kernel.org, hannes@cmpxchg.org, mhocko@kernel.org, shakeelb@google.com, kernel-team@fb.com Subject: Re: [PATCH 3/5] cgroup: Account for memory_localevents in test_memcg_oom_group_leaf_events() Message-ID: <20220423113652.ys5gd7vvwkvbdte4@dev0025.ash9.facebook.com> References: <20220422155728.3055914-1-void@manifault.com> <20220422155728.3055914-4-void@manifault.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20211029 X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 0AE2514000E Authentication-Results: imf09.hostedemail.com; dkim=none; spf=pass (imf09.hostedemail.com: domain of dcvernet@gmail.com designates 209.85.222.182 as permitted sender) smtp.mailfrom=dcvernet@gmail.com; dmarc=none X-Rspam-User: X-Stat-Signature: uoswt7zbnewxwizjnzkiici5yy44hy47 X-HE-Tag: 1650713813-506673 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 Fri, Apr 22, 2022 at 04:14:49PM -0700, Roman Gushchin wrote: > On Fri, Apr 22, 2022 at 08:57:27AM -0700, David Vernet wrote: > > The test_memcg_oom_group_leaf_events() testcase in the cgroup memcg tests > > validates that processes in a group that perform allocations exceeding > > memory.oom.group are killed. It also validates that the > > memory.events.oom_kill events are properly propagated in this case. Commit > > 06e11c907ea4 ("kselftests: memcg: update the oom group leaf events test") > > fixed test_memcg_oom_group_leaf_events() to account for the fact that the > > memory.events.oom_kill events in a child cgroup is propagated up to its > > parent. This behavior can actually be configured by the memory_localevents > > mount option, so this patch updates the testcase to properly account for > > the possible presence of this mount option. > > > > Signed-off-by: David Vernet > > --- > > .../testing/selftests/cgroup/test_memcontrol.c | 17 ++++++++++++++++- > > 1 file changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c > > index ea2fd27e52df..d88e0ca3f3d1 100644 > > --- a/tools/testing/selftests/cgroup/test_memcontrol.c > > +++ b/tools/testing/selftests/cgroup/test_memcontrol.c > > @@ -21,6 +21,7 @@ > > #include "../kselftest.h" > > #include "cgroup_util.h" > > > > +static bool has_localevents; > > static bool has_recursiveprot; > > > > /* > > @@ -1091,6 +1092,7 @@ static int test_memcg_oom_group_leaf_events(const char *root) > > { > > int ret = KSFT_FAIL; > > char *parent, *child; > > + long parent_oom_events; > > > > parent = cg_name(root, "memcg_test_0"); > > child = cg_name(root, "memcg_test_0/memcg_test_1"); > > @@ -1128,7 +1130,15 @@ static int test_memcg_oom_group_leaf_events(const char *root) > > if (cg_read_key_long(child, "memory.events", "oom_kill ") <= 0) > > goto cleanup; > > > > - if (cg_read_key_long(parent, "memory.events", "oom_kill ") <= 0) > > + parent_oom_events = cg_read_key_long( > > + parent, "memory.events", "oom_kill "); > > + // If memory_localevents is not enabled (the default), the parent should > > + // count OOM events in its children groups. Otherwise, it should not > > + // have observed any events. > > Please, use /* */ style comments, it's a generic kernel style. Ack, will fix in a follow-on patch. > > > + if (has_localevents) { > > + if (parent_oom_events != 0) > > + goto cleanup; > > + } else if (parent_oom_events <= 0) > > goto cleanup; > > How about something like this? IMO a bit more clear what's going on. > if ((has_local_events && parent_oom_events == 0) || > parent_oom_events > 0) > ret = KSFT_PASS; Agreed that's a bit clearer, I'll include this in the follow-on patch.