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 X-Spam-Level: X-Spam-Status: No, score=-17.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6377EC433E1 for ; Thu, 20 Aug 2020 21:25:56 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 0CDBA214F1 for ; Thu, 20 Aug 2020 21:25:55 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="WYASPIx5" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0CDBA214F1 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 6F47C8D0005; Thu, 20 Aug 2020 17:25:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 67D5D6B002A; Thu, 20 Aug 2020 17:25:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 546638D0005; Thu, 20 Aug 2020 17:25:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0045.hostedemail.com [216.40.44.45]) by kanga.kvack.org (Postfix) with ESMTP id 39AE96B0029 for ; Thu, 20 Aug 2020 17:25:55 -0400 (EDT) Received: from smtpin10.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 0016D180AD81D for ; Thu, 20 Aug 2020 21:25:54 +0000 (UTC) X-FDA: 77172229470.10.flag18_1f0d57727033 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin10.hostedemail.com (Postfix) with ESMTP id C731716A047 for ; Thu, 20 Aug 2020 21:25:54 +0000 (UTC) X-HE-Tag: flag18_1f0d57727033 X-Filterd-Recvd-Size: 5734 Received: from mail-lj1-f195.google.com (mail-lj1-f195.google.com [209.85.208.195]) by imf33.hostedemail.com (Postfix) with ESMTP for ; Thu, 20 Aug 2020 21:25:54 +0000 (UTC) Received: by mail-lj1-f195.google.com with SMTP id w25so3669531ljo.12 for ; Thu, 20 Aug 2020 14:25:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=qg7ruuG7drWC0btRbr1uOGU4H3EwWSnI1pvnZe6oIgQ=; b=WYASPIx5hMDjYGgzF+G0RR6vw6melkP0mguy36kGh/wXS2V3ohz4Yii1y0Dtu6iU+C lgRRuCa0Yz5B35a2ewWAKf33bxKi/ZqAeyD8rQWw85LdFb2z8gOVCfCrvbQvZJodRNNh 93t7iBP1COzAj3azTPFpmNT9k287RMuDfF0QdK0dNPN9GZd/Pm2A1vVPjU1lJWZkZaHf nU6xPpk8TwGTW75Mc7AKO0NSOXk++jx1+LU8HnsQ1FWGYEidvRi3cymONp0dCcu0QeUG 4+nCMyIA/qqZc1mDImkpEjw/YDWEjMczSHz8TTIH2lTkWLwJ5jB54ykxBxVdOEh8OdXx YVpw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=qg7ruuG7drWC0btRbr1uOGU4H3EwWSnI1pvnZe6oIgQ=; b=iVrsigaRh/3ceiplHYF/tynvy6LVvztkQ0qXg8vtcOeYlqTEtfpietKQC8LuuCnKPw Bw1Ia6qWf2y72Rc5KChpnfpJEbccKH0f3bRF1ZhzehE58JM/rANyI0kH+eM+Xe7HMM6f HLOHdVpzE2P0bYP9sk7aRSDQDQMkRuaWcoEovMcvpJNPyrrGz8C1akC/JzgDLeC2a4kB w6KBtk6Wms8F2uXW1A3SWiNHO2sCwfzOglS0vu4lkq9f/Ie79gN6o7ZSxWtbz5JY2O+Y 7lR/e2Y/6++EcVqpJ3gJjFdMXq0kJERmJdAuWoEuDt4WRbSUxEGlQ3+wOQI4k5NQvt91 0xEQ== X-Gm-Message-State: AOAM530gYIMwwlqTrbLc57cspgsYb8vUNr/PCyFusMt8AtjfyIf2hOKo gYkYzOLbhpfar4uauOz9K2LG3tziKk+OL1qZBqODWQ== X-Google-Smtp-Source: ABdhPJzqYADzY46O//FIwuGauKEriUJHxgBT1Pgq+Qn+KFzEpsr1NawCQfd1pK50vIf1JrU/YAnSLFtnkF/7XxmoXBo= X-Received: by 2002:a2e:9396:: with SMTP id g22mr121322ljh.446.1597958752359; Thu, 20 Aug 2020 14:25:52 -0700 (PDT) MIME-Version: 1.0 References: <20200820130350.3211-1-longman@redhat.com> <20200820130350.3211-3-longman@redhat.com> <20200820173546.GB912520@cmpxchg.org> In-Reply-To: From: Shakeel Butt Date: Thu, 20 Aug 2020 14:25:41 -0700 Message-ID: Subject: Re: [PATCH 2/3] mm/memcg: Simplify mem_cgroup_get_max() To: Waiman Long Cc: Johannes Weiner , Michal Hocko , Vladimir Davydov , Andrew Morton , Tejun Heo , LKML , Cgroups , Linux MM , Chris Down , Roman Gushchin , Yafang Shao Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: C731716A047 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam02 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 Thu, Aug 20, 2020 at 1:29 PM Waiman Long wrote: > > On 8/20/20 1:35 PM, Johannes Weiner wrote: > > On Thu, Aug 20, 2020 at 09:03:49AM -0400, Waiman Long wrote: > >> The mem_cgroup_get_max() function used to get memory+swap max from > >> both the v1 memsw and v2 memory+swap page counters & return the maximum > >> of these 2 values. This is redundant and it is more efficient to just > >> get either the v1 or the v2 values depending on which one is currently > >> in use. > >> > >> Signed-off-by: Waiman Long > >> --- > >> mm/memcontrol.c | 14 +++++--------- > >> 1 file changed, 5 insertions(+), 9 deletions(-) > >> > >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c > >> index 26b7a48d3afb..d219dca5239f 100644 > >> --- a/mm/memcontrol.c > >> +++ b/mm/memcontrol.c > >> @@ -1633,17 +1633,13 @@ void mem_cgroup_print_oom_meminfo(struct mem_cgroup *memcg) > >> */ > >> unsigned long mem_cgroup_get_max(struct mem_cgroup *memcg) > >> { > >> - unsigned long max; > >> + unsigned long max = READ_ONCE(memcg->memory.max); > >> > >> - max = READ_ONCE(memcg->memory.max); > >> if (mem_cgroup_swappiness(memcg)) { > >> - unsigned long memsw_max; > >> - unsigned long swap_max; > >> - > >> - memsw_max = memcg->memsw.max; > >> - swap_max = READ_ONCE(memcg->swap.max); > >> - swap_max = min(swap_max, (unsigned long)total_swap_pages); > >> - max = min(max + swap_max, memsw_max); > >> + if (cgroup_subsys_on_dfl(memory_cgrp_subsys)) > >> + max += READ_ONCE(memcg->swap.max); > >> + else > >> + max = memcg->memsw.max; > > I agree with the premise of the patch, but v1 and v2 have sufficiently > > different logic, and the way v1 overrides max from the innermost > > branch again also doesn't help in understanding what's going on. > > > > Can you please split out the v1 and v2 code? > > > > if (cgroup_subsys_on_dfl(memory_cgrp_subsys)) { > > max = READ_ONCE(memcg->memory.max); > > if (mem_cgroup_swappiness(memcg)) > > max += READ_ONCE(memcg->swap.max); > > } else { > > if (mem_cgroup_swappiness(memcg)) > > max = memcg->memsw.max; > > else > > max = READ_ONCE(memcg->memory.max); > > } > > > > It's slightly repetitive, but IMO much more readable. > > > Sure. That makes it even better. > Can you please also add in the commit message why it is ok to drop total_swap_pages comparison from mem_cgroup_get_max()?