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=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 89300C433E0 for ; Wed, 3 Mar 2021 11:40:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 45B7064E05 for ; Wed, 3 Mar 2021 11:40:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1357900AbhCCLhG (ORCPT ); Wed, 3 Mar 2021 06:37:06 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33048 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234386AbhCCDO1 (ORCPT ); Tue, 2 Mar 2021 22:14:27 -0500 Received: from mail-pl1-x636.google.com (mail-pl1-x636.google.com [IPv6:2607:f8b0:4864:20::636]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 745DFC06178B for ; Tue, 2 Mar 2021 19:13:03 -0800 (PST) Received: by mail-pl1-x636.google.com with SMTP id d8so2821836plg.10 for ; Tue, 02 Mar 2021 19:13:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=jPtyv7FWNE5afHArRbhNorqbEBt/90YtJ2/j1QcFJFk=; b=L2HVGMBbxOe1DUQxG0VZsngFSjQuWnhYhNGZmw4MHVlJRyqbmUwS8iL9WGQ/8pWBei /XN3a1460NKQsgVyKnsD5Rq2n++ewQ8EQadRMPs4XpnfqYtazzjce6/qqHHzc46N0xHq gVJQSelHZ5nrFMSUA9sD63r6gRTOwkJsMZB4jurQlq3ntGcBN8jZX2bjdBpAyUtNdZ+B iWByPp5+QhBBfOJtdyYLc8rJVM7qGBTPuYeo3JQBHH2bq5cnDef+ycCNtasjW1zNaLRQ YLr8n/VbJGaPj3r5lV0lvW1y6J1scJUg3LG23obPzPSolQIMtVfzNp5hpsgsnuyOrM4e 6bkw== 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=jPtyv7FWNE5afHArRbhNorqbEBt/90YtJ2/j1QcFJFk=; b=W50ePuK6C2tPLcwRILvj7jnLAAJiPns0Gyt8/0fFd3dcFoEMqqA8xIkDpbOFUz1am5 gkt6uTrWctQbblPhCoSHSmQKbNP4tCsarGNnzKg+8bPzpqTW/ArQPnKxpps0CBwYAjIQ 4lV7SJqoKwGiMFR8wgCPp/cbqRqE9Kp3KD241bK14GMggTu8guPqC8qNOyM7BDycS4hO D8qfUejOAP5gPX7QRzjH3OLTvxYIVL4IfYKyqgNabdd5js4Nk2Qk1li0aNM7BRXC9xIm 9BJDrDbS//jYmznWcTyHirYU4EvjNHkGDUZkuqdOPuE6QDdxhaLim7vg8ICEIH2RUEbk 17LQ== X-Gm-Message-State: AOAM530ULOKx4ls4VyLpNW8OCSoDa0El0opv28AHgI79g+YJcb2Q8Fcw +cy37sKBTyJv0stjZHrD13QCVqgArP5D29Iy8IxBhw== X-Google-Smtp-Source: ABdhPJx3chnssos306qpL9AmHwbrb44kCCmB0MEYUU84BwvstDSlD/D91l1sBkVKXSYBrKUHOk9rCIv3wyp3pOsPqyM= X-Received: by 2002:a17:90a:f008:: with SMTP id bt8mr1365042pjb.13.1614741183030; Tue, 02 Mar 2021 19:13:03 -0800 (PST) MIME-Version: 1.0 References: <20210302081823.9849-1-songmuchun@bytedance.com> In-Reply-To: From: Muchun Song Date: Wed, 3 Mar 2021 11:12:26 +0800 Message-ID: Subject: Re: [External] Re: [PATCH] mm: memcontrol: fix root_mem_cgroup charging To: Roman Gushchin Cc: Johannes Weiner , Michal Hocko , Andrew Morton , Shakeel Butt , LKML , Linux Memory Management List Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Mar 3, 2021 at 2:58 AM Roman Gushchin wrote: > > On Tue, Mar 02, 2021 at 04:18:23PM +0800, Muchun Song wrote: > > CPU0: CPU1: > > > > objcg = get_obj_cgroup_from_current(); > > obj_cgroup_charge(objcg); > > memcg_reparent_objcgs(); > > xchg(&objcg->memcg, root_mem_cgroup); > > // memcg == root_mem_cgroup > > memcg = obj_cgroup_memcg(objcg); > > __memcg_kmem_charge(memcg); > > // Do not charge to the root memcg > > try_charge(memcg); > > > > If the objcg->memcg is reparented to the root_mem_cgroup, > > obj_cgroup_charge() can pass root_mem_cgroup as the first > > parameter to here. The root_mem_cgroup is skipped in the > > try_charge(). So the page counters of it do not update. > > > > When we uncharge this, we will decrease the page counters > > (e.g. memory and memsw) of the root_mem_cgroup. This will > > cause the page counters of the root_mem_cgroup to be out > > of balance. Fix it by charging the page to the > > root_mem_cgroup unconditional. > > Is this a problem? It seems that we do not expose root memcg's counters > except kmem and tcp. In the page_counter_cancel(), we can see a WARN_ON_ONCE() to catch this issue. Yeah, it is very hard to trigger this warn for root memcg. But it actually can. Right? If we do not care about the root memcg counter, we should not warn for the root memcg. > It seems that the described problem is not > applicable to the kmem counter. Please, explain. The kmem counter of the root memcg is updated unconditionally. Because we do not check whether the memcg is root when we charge pages to the kmem counter. Thanks. > > Thanks! > > > > > Fixes: bf4f059954dc ("mm: memcg/slab: obj_cgroup API") > > Signed-off-by: Muchun Song > > --- > > mm/memcontrol.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 2db2aeac8a9e..edf604824d63 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -3078,6 +3078,19 @@ static int __memcg_kmem_charge(struct mem_cgroup *memcg, gfp_t gfp, > > if (ret) > > return ret; > > > > + /* > > + * If the objcg->memcg is reparented to the root_mem_cgroup, > > + * obj_cgroup_charge() can pass root_mem_cgroup as the first > > + * parameter to here. We should charge the page to the > > + * root_mem_cgroup unconditional to keep it's page counters > > + * balance. > > + */ > > + if (unlikely(mem_cgroup_is_root(memcg))) { > > + page_counter_charge(&memcg->memory, nr_pages); > > + if (do_memsw_account()) > > + page_counter_charge(&memcg->memsw, nr_pages); > > + } > > + > > if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) && > > !page_counter_try_charge(&memcg->kmem, nr_pages, &counter)) { > > > > -- > > 2.11.0 > > 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=-13.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 94544C433DB for ; Wed, 3 Mar 2021 03:13:08 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id E3DA2600EF for ; Wed, 3 Mar 2021 03:13:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E3DA2600EF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=bytedance.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 6B71B8D011D; Tue, 2 Mar 2021 22:13:07 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 68CC58D00FA; Tue, 2 Mar 2021 22:13:07 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 52F6D8D011D; Tue, 2 Mar 2021 22:13:07 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0165.hostedemail.com [216.40.44.165]) by kanga.kvack.org (Postfix) with ESMTP id 358248D00FA for ; Tue, 2 Mar 2021 22:13:07 -0500 (EST) Received: from smtpin16.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id DDA621801870B for ; Wed, 3 Mar 2021 03:13:06 +0000 (UTC) X-FDA: 77877091572.16.0D49D5F Received: from mail-pl1-f170.google.com (mail-pl1-f170.google.com [209.85.214.170]) by imf18.hostedemail.com (Postfix) with ESMTP id D03DB2000383 for ; Wed, 3 Mar 2021 03:13:03 +0000 (UTC) Received: by mail-pl1-f170.google.com with SMTP id ba1so13257719plb.1 for ; Tue, 02 Mar 2021 19:13:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=jPtyv7FWNE5afHArRbhNorqbEBt/90YtJ2/j1QcFJFk=; b=L2HVGMBbxOe1DUQxG0VZsngFSjQuWnhYhNGZmw4MHVlJRyqbmUwS8iL9WGQ/8pWBei /XN3a1460NKQsgVyKnsD5Rq2n++ewQ8EQadRMPs4XpnfqYtazzjce6/qqHHzc46N0xHq gVJQSelHZ5nrFMSUA9sD63r6gRTOwkJsMZB4jurQlq3ntGcBN8jZX2bjdBpAyUtNdZ+B iWByPp5+QhBBfOJtdyYLc8rJVM7qGBTPuYeo3JQBHH2bq5cnDef+ycCNtasjW1zNaLRQ YLr8n/VbJGaPj3r5lV0lvW1y6J1scJUg3LG23obPzPSolQIMtVfzNp5hpsgsnuyOrM4e 6bkw== 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=jPtyv7FWNE5afHArRbhNorqbEBt/90YtJ2/j1QcFJFk=; b=E3T0MDESrXS2TFSFMaiG0hi2u7pUjAwVFs4ZRwbVc4YdFnwiD855Q+bG8/tLZrbCdR AG13dugzNWikaa02ySBlz+biUF2Wnxm//vJq93B2p22Ui4r6dcGZEcubnDcvF0GDzqTC bIAJcW8HwCL3zgmfH+mdhIIPlJfaLkpyte3qrzBuXgCX2A7n13BisXP0vefXRL/U7ili AYMn8lK4oWMVm+trXVP0Pse9v7pXzJzRMFNyHuCmxb+pzpItc/fDI6KNBe3gYw+HaADK h92oTbWSw4LJErFIGlg18cQY2PVhvfJ2syKrJpVwHtNkKf1dpvLwmJANNt80w9vFh3W/ 9mCg== X-Gm-Message-State: AOAM531iNYYij1A7GGnKR1m0d/CqB0rrONkn/YbhnJh9RL2SzXKpcdDV GAthCc/0c4/sx3I9d3Xpg1jJf/qEqb056vM9vx4X9g== X-Google-Smtp-Source: ABdhPJx3chnssos306qpL9AmHwbrb44kCCmB0MEYUU84BwvstDSlD/D91l1sBkVKXSYBrKUHOk9rCIv3wyp3pOsPqyM= X-Received: by 2002:a17:90a:f008:: with SMTP id bt8mr1365042pjb.13.1614741183030; Tue, 02 Mar 2021 19:13:03 -0800 (PST) MIME-Version: 1.0 References: <20210302081823.9849-1-songmuchun@bytedance.com> In-Reply-To: From: Muchun Song Date: Wed, 3 Mar 2021 11:12:26 +0800 Message-ID: Subject: Re: [External] Re: [PATCH] mm: memcontrol: fix root_mem_cgroup charging To: Roman Gushchin Cc: Johannes Weiner , Michal Hocko , Andrew Morton , Shakeel Butt , LKML , Linux Memory Management List Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: D03DB2000383 X-Stat-Signature: yedy5gqrbi5urasbfaq5cwgk8h3uzrdh Received-SPF: none (bytedance.com>: No applicable sender policy available) receiver=imf18; identity=mailfrom; envelope-from=""; helo=mail-pl1-f170.google.com; client-ip=209.85.214.170 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1614741183-337252 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 Wed, Mar 3, 2021 at 2:58 AM Roman Gushchin wrote: > > On Tue, Mar 02, 2021 at 04:18:23PM +0800, Muchun Song wrote: > > CPU0: CPU1: > > > > objcg = get_obj_cgroup_from_current(); > > obj_cgroup_charge(objcg); > > memcg_reparent_objcgs(); > > xchg(&objcg->memcg, root_mem_cgroup); > > // memcg == root_mem_cgroup > > memcg = obj_cgroup_memcg(objcg); > > __memcg_kmem_charge(memcg); > > // Do not charge to the root memcg > > try_charge(memcg); > > > > If the objcg->memcg is reparented to the root_mem_cgroup, > > obj_cgroup_charge() can pass root_mem_cgroup as the first > > parameter to here. The root_mem_cgroup is skipped in the > > try_charge(). So the page counters of it do not update. > > > > When we uncharge this, we will decrease the page counters > > (e.g. memory and memsw) of the root_mem_cgroup. This will > > cause the page counters of the root_mem_cgroup to be out > > of balance. Fix it by charging the page to the > > root_mem_cgroup unconditional. > > Is this a problem? It seems that we do not expose root memcg's counters > except kmem and tcp. In the page_counter_cancel(), we can see a WARN_ON_ONCE() to catch this issue. Yeah, it is very hard to trigger this warn for root memcg. But it actually can. Right? If we do not care about the root memcg counter, we should not warn for the root memcg. > It seems that the described problem is not > applicable to the kmem counter. Please, explain. The kmem counter of the root memcg is updated unconditionally. Because we do not check whether the memcg is root when we charge pages to the kmem counter. Thanks. > > Thanks! > > > > > Fixes: bf4f059954dc ("mm: memcg/slab: obj_cgroup API") > > Signed-off-by: Muchun Song > > --- > > mm/memcontrol.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 2db2aeac8a9e..edf604824d63 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -3078,6 +3078,19 @@ static int __memcg_kmem_charge(struct mem_cgroup *memcg, gfp_t gfp, > > if (ret) > > return ret; > > > > + /* > > + * If the objcg->memcg is reparented to the root_mem_cgroup, > > + * obj_cgroup_charge() can pass root_mem_cgroup as the first > > + * parameter to here. We should charge the page to the > > + * root_mem_cgroup unconditional to keep it's page counters > > + * balance. > > + */ > > + if (unlikely(mem_cgroup_is_root(memcg))) { > > + page_counter_charge(&memcg->memory, nr_pages); > > + if (do_memsw_account()) > > + page_counter_charge(&memcg->memsw, nr_pages); > > + } > > + > > if (!cgroup_subsys_on_dfl(memory_cgrp_subsys) && > > !page_counter_try_charge(&memcg->kmem, nr_pages, &counter)) { > > > > -- > > 2.11.0 > >