Message ID | 20221215091907.763801-1-haifeng.xu@shopee.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:e747:0:0:0:0:0 with SMTP id c7csp245531wrn; Thu, 15 Dec 2022 01:34:08 -0800 (PST) X-Google-Smtp-Source: AA0mqf5UXYUNzt8OkbeUOi50pc6MpKVWeoRhyimGslqbW3qwqZBCV0AbJZs0T1Q/W6/Bmbrhf33i X-Received: by 2002:a05:6402:12c9:b0:46c:55ef:8d50 with SMTP id k9-20020a05640212c900b0046c55ef8d50mr23693802edx.24.1671096847969; Thu, 15 Dec 2022 01:34:07 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1671096847; cv=none; d=google.com; s=arc-20160816; b=S+eBnvwoqqPAoGxJ2e1XrRV4cWwTiwX6CYHr3TfESwfJjncESEktVw7ODf1J94gnNy rGYyZWix5Yc+MnEhOdCS57hOQhPg11mp9ozJRc12w7MLBUZtBp2cePNGaI4OQ6YZvtXz KSIzxBfT99SD39U7I7Mrunl4IA3emSAAmM3OzpZHUyBl3N/yryc3OClX997o7LKRezrl mAzcA3E40URkxP2kRh75ImNF00jipXQLS0ny8wZF40lrJuSPak/AbM99vAqz1vv6o71j I7o+3nc30lrirndITc2W/8L8Cya6sMl6aI0JDdZf/lJzlMt/qDSxxRtqgKx4GHSfHhfJ 0q1A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=0Wu5pKKt/tYg5j4ptIlawG89Bmd9AY8kHRaAGqPqkAs=; b=HK53W3679uSwNydJjP+H4xnuQfE7+5OVSIzROcAt4LaLJTTDsZ2jy1c3TMCPq5SVSV B04r/Kp1HONcZ+NvqvWgOlOkROISFsFDDt/MDakjp9pJCpxvc+pzNejJotWh8Z/xiWTF WFMmsXt4K1i3uMzpncu8hyWPIXLtTSyUN7tyWABGNBSFQECa/FAB9AwH2FWhT6wT87Yo +HmHD3TrRFocnoAeE+AFmUjHL/HVQ99ZEZ439V1Lnwh7E6koJsGGDGDPK5aIjQZwmKgE 5X/KDX8NpX2IeRG5PCn30spLCvyVDLhfHoT3ZezzS/LWw8xDHjA6RolW0NnGU17fSGcG 2roQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@shopee.com header.s=shopee.com header.b=QFKuTxlm; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=shopee.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id eb11-20020a0564020d0b00b0046ccce3279csi17362539edb.178.2022.12.15.01.33.44; Thu, 15 Dec 2022 01:34:07 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@shopee.com header.s=shopee.com header.b=QFKuTxlm; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=shopee.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229908AbiLOJUR (ORCPT <rfc822;jeantsuru.cumc.mandola@gmail.com> + 99 others); Thu, 15 Dec 2022 04:20:17 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58058 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229785AbiLOJUM (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 15 Dec 2022 04:20:12 -0500 Received: from mail-pj1-x1036.google.com (mail-pj1-x1036.google.com [IPv6:2607:f8b0:4864:20::1036]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 007733F04A for <linux-kernel@vger.kernel.org>; Thu, 15 Dec 2022 01:20:10 -0800 (PST) Received: by mail-pj1-x1036.google.com with SMTP id v13-20020a17090a6b0d00b00219c3be9830so2080626pjj.4 for <linux-kernel@vger.kernel.org>; Thu, 15 Dec 2022 01:20:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=shopee.com; s=shopee.com; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=0Wu5pKKt/tYg5j4ptIlawG89Bmd9AY8kHRaAGqPqkAs=; b=QFKuTxlmPSvSiGvceKI4jRcxQaPyAjRxEIe0jeAsm9mJMiBq4DitPskSJWUUIb4B3w Smq+oC/Sz4TApQ7i4RMCOwudRKNzT/ErbL+9E8ebSRVa9h1nO25UAc4SIBSSFhssMpa1 Dun3NPFKzDk4ylkMUEPnMux4UJCxWM3rpzY8klpS57EKwJG15KH41hXr96Ojped9mj9S IKt3vhb7yUx46syZ8OB4D6Eiz7vxQvN2kF5HpaamIlywYdVBGiW234HXFT9RGq8DYvaS T8pIp7dw91J9GUx1aHr67DFfP+V6xKoqmakiltZ33fGFh6NaYmwwQ7At/x8CXsL3RXe4 w9KA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=0Wu5pKKt/tYg5j4ptIlawG89Bmd9AY8kHRaAGqPqkAs=; b=AiX7QgBwvgyqkeuRe7JobAbSG29Iy/NJBx6BET1KFydjkRfbH8NUCVGRjVo1cMXg0Z H54zGqrQLfQ6sM9Wt2eILsQRC0B4XQSUC48gCflnFR/5p4oRpulJN4dBXnRDcFuWNaUb GBecNrCZtr0ITJ2q1/rVsY2/uH8yVpw1Vb9mcA1Wj87SMbhOABOjPYCNXwIQzmUcBcA6 x8fiRRcgugQ25QAfawamADiOzl5OZbxFV0UASxGOv9IxWPzan0sCzhaFSYrBn0DU6IXQ cZAQXvYadLJ7rSME/sKkm8Bpvi9V+pBu7FwMdtWJBrfHPMHHcSMDKiC831WoJ25Wn/G2 MEHQ== X-Gm-Message-State: ANoB5plJ8Ya0DP6MvyvL6eAdeT1jThRruhj/jNKbkks3pqQ2mu2CWd2a 4vnAs3jWZhgIWBZYDvuomjezvA== X-Received: by 2002:a17:903:4283:b0:189:7100:c50e with SMTP id ju3-20020a170903428300b001897100c50emr29499311plb.48.1671096010440; Thu, 15 Dec 2022 01:20:10 -0800 (PST) Received: from ubuntu-haifeng.default.svc.cluster.local ([101.127.248.173]) by smtp.gmail.com with ESMTPSA id n15-20020a170903110f00b00188f8badbcdsm3302643plh.137.2022.12.15.01.20.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Dec 2022 01:20:10 -0800 (PST) From: Haifeng Xu <haifeng.xu@shopee.com> To: akpm@linux-foundation.org Cc: shakeelb@google.com, roman.gushchin@linux.dev, songmuchun@bytedance.com, hannes@cmpxchg.org, vbabka@suse.cz, willy@infradead.org, vasily.averin@linux.dev, linux-kernel@vger.kernel.org, Haifeng Xu <haifeng.xu@shopee.com> Subject: [PATCH] mm/memcontrol: Skip root memcg in memcg_memory_event_mm Date: Thu, 15 Dec 2022 09:19:07 +0000 Message-Id: <20221215091907.763801-1-haifeng.xu@shopee.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1752272048817029354?= X-GMAIL-MSGID: =?utf-8?q?1752272048817029354?= |
Series |
mm/memcontrol: Skip root memcg in memcg_memory_event_mm
|
|
Commit Message
Haifeng Xu
Dec. 15, 2022, 9:19 a.m. UTC
The memory events aren't supported on root cgroup, so there is no need
to account MEMCG_OOM_KILL on root memcg.
Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com>
---
include/linux/memcontrol.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On Thu, Dec 15, 2022 at 09:19:07AM +0000, Haifeng Xu wrote: > The memory events aren't supported on root cgroup, so there is no need > to account MEMCG_OOM_KILL on root memcg. > Can you explain the scenario where this is happening and causing issue for you? > Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com> > --- > include/linux/memcontrol.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index 567f12323f55..09f75161a3bc 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -1142,7 +1142,7 @@ static inline void memcg_memory_event_mm(struct mm_struct *mm, > > rcu_read_lock(); > memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); > - if (likely(memcg)) > + if (likely(memcg && !mem_cgroup_is_root(memcg))) Even if we need this additional check, this is not the right place. > memcg_memory_event(memcg, event); > rcu_read_unlock(); > } > -- > 2.25.1 >
On 2022/12/16 02:18, Shakeel Butt wrote: > On Thu, Dec 15, 2022 at 09:19:07AM +0000, Haifeng Xu wrote: >> The memory events aren't supported on root cgroup, so there is no need >> to account MEMCG_OOM_KILL on root memcg. >> > > Can you explain the scenario where this is happening and causing issue > for you? > If the victim selected by oom killer belongs to root memcg, memcg_memory_event_mm still counts the MEMCG_OOM_KILL event. This behavior is meaningless because the flag of events/events.local in memory_files is CFTYPE_NOT_ON_ROOT. The root memcg does not count any memory event. >> Signed-off-by: Haifeng Xu <haifeng.xu@shopee.com> >> --- >> include/linux/memcontrol.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h >> index 567f12323f55..09f75161a3bc 100644 >> --- a/include/linux/memcontrol.h >> +++ b/include/linux/memcontrol.h >> @@ -1142,7 +1142,7 @@ static inline void memcg_memory_event_mm(struct mm_struct *mm, >> >> rcu_read_lock(); >> memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); >> - if (likely(memcg)) >> + if (likely(memcg && !mem_cgroup_is_root(memcg))) > > Even if we need this additional check, this is not the right place. > >> memcg_memory_event(memcg, event); >> rcu_read_unlock(); >> } >> -- >> 2.25.1 >>
On Fri, Dec 16, 2022 at 09:43:02AM +0800, Haifeng Xu wrote: > > > On 2022/12/16 02:18, Shakeel Butt wrote: > > On Thu, Dec 15, 2022 at 09:19:07AM +0000, Haifeng Xu wrote: > >> The memory events aren't supported on root cgroup, so there is no need > >> to account MEMCG_OOM_KILL on root memcg. > >> > > > > Can you explain the scenario where this is happening and causing issue > > for you? > > > If the victim selected by oom killer belongs to root memcg, memcg_memory_event_mm > still counts the MEMCG_OOM_KILL event. This behavior is meaningless because the > flag of events/events.local in memory_files is CFTYPE_NOT_ON_ROOT. The root memcg > does not count any memory event. > What about v1's memory.oom_control?
On 2022/12/16 14:42, Shakeel Butt wrote: > On Fri, Dec 16, 2022 at 09:43:02AM +0800, Haifeng Xu wrote: >> >> >> On 2022/12/16 02:18, Shakeel Butt wrote: >>> On Thu, Dec 15, 2022 at 09:19:07AM +0000, Haifeng Xu wrote: >>>> The memory events aren't supported on root cgroup, so there is no need >>>> to account MEMCG_OOM_KILL on root memcg. >>>> >>> >>> Can you explain the scenario where this is happening and causing issue >>> for you? >>> >> If the victim selected by oom killer belongs to root memcg, memcg_memory_event_mm >> still counts the MEMCG_OOM_KILL event. This behavior is meaningless because the >> flag of events/events.local in memory_files is CFTYPE_NOT_ON_ROOT. The root memcg >> does not count any memory event. >> > > What about v1's memory.oom_control? > The memory.oom_control doesn't set the CFTYPE_NOT_ON_ROOT flag. But oom_kill_disable or under_oom actually only support non-root memcg, so the memory_events should be consistent with them.
On Fri, Dec 16, 2022 at 03:28:53PM +0800, Haifeng Xu wrote: > > > On 2022/12/16 14:42, Shakeel Butt wrote: > > On Fri, Dec 16, 2022 at 09:43:02AM +0800, Haifeng Xu wrote: > >> > >> > >> On 2022/12/16 02:18, Shakeel Butt wrote: > >>> On Thu, Dec 15, 2022 at 09:19:07AM +0000, Haifeng Xu wrote: > >>>> The memory events aren't supported on root cgroup, so there is no need > >>>> to account MEMCG_OOM_KILL on root memcg. > >>>> > >>> > >>> Can you explain the scenario where this is happening and causing issue > >>> for you? > >>> > >> If the victim selected by oom killer belongs to root memcg, memcg_memory_event_mm > >> still counts the MEMCG_OOM_KILL event. This behavior is meaningless because the > >> flag of events/events.local in memory_files is CFTYPE_NOT_ON_ROOT. The root memcg > >> does not count any memory event. > >> > > > > What about v1's memory.oom_control? > > > > The memory.oom_control doesn't set the CFTYPE_NOT_ON_ROOT flag. But oom_kill_disable or > under_oom actually only support non-root memcg, so the memory_events should be consistent > with them. Did you take a look at mem_cgroup_oom_control_read()? It is displaying MEMCG_OOM_KILL for root memcg. Irrespective it makes sense or not, you want to change behavior of user visible interface. If you really want to then propose for the deprecation of that interface.
On 2022/12/16 15:36, Shakeel Butt wrote: > On Fri, Dec 16, 2022 at 03:28:53PM +0800, Haifeng Xu wrote: >> >> >> On 2022/12/16 14:42, Shakeel Butt wrote: >>> On Fri, Dec 16, 2022 at 09:43:02AM +0800, Haifeng Xu wrote: >>>> >>>> >>>> On 2022/12/16 02:18, Shakeel Butt wrote: >>>>> On Thu, Dec 15, 2022 at 09:19:07AM +0000, Haifeng Xu wrote: >>>>>> The memory events aren't supported on root cgroup, so there is no need >>>>>> to account MEMCG_OOM_KILL on root memcg. >>>>>> >>>>> >>>>> Can you explain the scenario where this is happening and causing issue >>>>> for you? >>>>> >>>> If the victim selected by oom killer belongs to root memcg, memcg_memory_event_mm >>>> still counts the MEMCG_OOM_KILL event. This behavior is meaningless because the >>>> flag of events/events.local in memory_files is CFTYPE_NOT_ON_ROOT. The root memcg >>>> does not count any memory event. >>>> >>> >>> What about v1's memory.oom_control? >>> >> >> The memory.oom_control doesn't set the CFTYPE_NOT_ON_ROOT flag. But oom_kill_disable or >> under_oom actually only support non-root memcg, so the memory_events should be consistent >> with them. > > Did you take a look at mem_cgroup_oom_control_read()? It is displaying > MEMCG_OOM_KILL for root memcg. Irrespective it makes sense or not, you > want to change behavior of user visible interface. If you really want to > then propose for the deprecation of that interface. Yes, I have see it in mem_cgroup_oom_control_read() and I think that showing MEMCG_OOM_KILL for root memcg doesn't make much sense. Shoud I add the CFTYPE_NOT_ON_ROOT flag for cgroup v1?
On Fri, Dec 16, 2022 at 03:50:49PM +0800, Haifeng Xu wrote: > > > On 2022/12/16 15:36, Shakeel Butt wrote: > > On Fri, Dec 16, 2022 at 03:28:53PM +0800, Haifeng Xu wrote: > >> > >> > >> On 2022/12/16 14:42, Shakeel Butt wrote: > >>> On Fri, Dec 16, 2022 at 09:43:02AM +0800, Haifeng Xu wrote: > >>>> > >>>> > >>>> On 2022/12/16 02:18, Shakeel Butt wrote: > >>>>> On Thu, Dec 15, 2022 at 09:19:07AM +0000, Haifeng Xu wrote: > >>>>>> The memory events aren't supported on root cgroup, so there is no need > >>>>>> to account MEMCG_OOM_KILL on root memcg. > >>>>>> > >>>>> > >>>>> Can you explain the scenario where this is happening and causing issue > >>>>> for you? > >>>>> > >>>> If the victim selected by oom killer belongs to root memcg, memcg_memory_event_mm > >>>> still counts the MEMCG_OOM_KILL event. This behavior is meaningless because the > >>>> flag of events/events.local in memory_files is CFTYPE_NOT_ON_ROOT. The root memcg > >>>> does not count any memory event. > >>>> > >>> > >>> What about v1's memory.oom_control? > >>> > >> > >> The memory.oom_control doesn't set the CFTYPE_NOT_ON_ROOT flag. But oom_kill_disable or > >> under_oom actually only support non-root memcg, so the memory_events should be consistent > >> with them. > > > > Did you take a look at mem_cgroup_oom_control_read()? It is displaying > > MEMCG_OOM_KILL for root memcg. Irrespective it makes sense or not, you > > want to change behavior of user visible interface. If you really want to > > then propose for the deprecation of that interface. > > Yes, I have see it in mem_cgroup_oom_control_read() and I think that > showing MEMCG_OOM_KILL for root memcg doesn't make much sense. > It doesn't matter as there might already be users using it. > Shoud I add the CFTYPE_NOT_ON_ROOT flag for cgroup v1? > Before doing anything, I am still not seeing why we really need this patch? What exactly is the issue this patch is trying to solve? To me this patch is negatively impacting the readability of the code. Unless you are seeing some real production issues, I don't think we need to add any special casing for MEMCG_OOM_KILL here.
On 2022/12/16 16:17, Shakeel Butt wrote: > On Fri, Dec 16, 2022 at 03:50:49PM +0800, Haifeng Xu wrote: >> >> >> On 2022/12/16 15:36, Shakeel Butt wrote: >>> On Fri, Dec 16, 2022 at 03:28:53PM +0800, Haifeng Xu wrote: >>>> >>>> >>>> On 2022/12/16 14:42, Shakeel Butt wrote: >>>>> On Fri, Dec 16, 2022 at 09:43:02AM +0800, Haifeng Xu wrote: >>>>>> >>>>>> >>>>>> On 2022/12/16 02:18, Shakeel Butt wrote: >>>>>>> On Thu, Dec 15, 2022 at 09:19:07AM +0000, Haifeng Xu wrote: >>>>>>>> The memory events aren't supported on root cgroup, so there is no need >>>>>>>> to account MEMCG_OOM_KILL on root memcg. >>>>>>>> >>>>>>> >>>>>>> Can you explain the scenario where this is happening and causing issue >>>>>>> for you? >>>>>>> >>>>>> If the victim selected by oom killer belongs to root memcg, memcg_memory_event_mm >>>>>> still counts the MEMCG_OOM_KILL event. This behavior is meaningless because the >>>>>> flag of events/events.local in memory_files is CFTYPE_NOT_ON_ROOT. The root memcg >>>>>> does not count any memory event. >>>>>> >>>>> >>>>> What about v1's memory.oom_control? >>>>> >>>> >>>> The memory.oom_control doesn't set the CFTYPE_NOT_ON_ROOT flag. But oom_kill_disable or >>>> under_oom actually only support non-root memcg, so the memory_events should be consistent >>>> with them. >>> >>> Did you take a look at mem_cgroup_oom_control_read()? It is displaying >>> MEMCG_OOM_KILL for root memcg. Irrespective it makes sense or not, you >>> want to change behavior of user visible interface. If you really want to >>> then propose for the deprecation of that interface. >> >> Yes, I have see it in mem_cgroup_oom_control_read() and I think that >> showing MEMCG_OOM_KILL for root memcg doesn't make much sense. >> > > It doesn't matter as there might already be users using it. > >> Shoud I add the CFTYPE_NOT_ON_ROOT flag for cgroup v1? >> > > Before doing anything, I am still not seeing why we really need this > patch? What exactly is the issue this patch is trying to solve? To me > this patch is negatively impacting the readability of the code. Unless > you are seeing some real production issues, I don't think we need to add > any special casing for MEMCG_OOM_KILL here. As we can see in memcg_memory_event(), memory event never be count in root memcg. Passing the root memcg to it seems somewhat self-contradictory. Also, cgroup v2 doesn't need this.
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 567f12323f55..09f75161a3bc 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -1142,7 +1142,7 @@ static inline void memcg_memory_event_mm(struct mm_struct *mm, rcu_read_lock(); memcg = mem_cgroup_from_task(rcu_dereference(mm->owner)); - if (likely(memcg)) + if (likely(memcg && !mem_cgroup_is_root(memcg))) memcg_memory_event(memcg, event); rcu_read_unlock(); }