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=-3.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no 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 2C83FC43460 for ; Wed, 21 Apr 2021 13:39:45 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 81B40613B6 for ; Wed, 21 Apr 2021 13:39:44 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 81B40613B6 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 DEA796B0036; Wed, 21 Apr 2021 09:39:43 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D98966B006E; Wed, 21 Apr 2021 09:39:43 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C13826B0070; Wed, 21 Apr 2021 09:39:43 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0117.hostedemail.com [216.40.44.117]) by kanga.kvack.org (Postfix) with ESMTP id A1EBB6B0036 for ; Wed, 21 Apr 2021 09:39:43 -0400 (EDT) Received: from smtpin25.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 5D54E3643 for ; Wed, 21 Apr 2021 13:39:43 +0000 (UTC) X-FDA: 78056481846.25.D421EA7 Received: from mail-pl1-f175.google.com (mail-pl1-f175.google.com [209.85.214.175]) by imf02.hostedemail.com (Postfix) with ESMTP id 98DB140002D7 for ; Wed, 21 Apr 2021 13:39:17 +0000 (UTC) Received: by mail-pl1-f175.google.com with SMTP id s20so5820923plr.13 for ; Wed, 21 Apr 2021 06:39:40 -0700 (PDT) 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=YhMLOnWO5qZngJMN7NKdOR6yuHZHGJTjvv7soDc2Xpo=; b=RZp8y013plb9zCPZP4ZesknGmTm7X/DyZwCOfFUmolq8UrIKRF6QqyYbwiZ2rX17OO BOiNITK1rollxoeIVzXwfCl4VrRFd99nWbYP6pNxQJJkUHmSeOsEToXPvHYFW/rG/bpj tMWnS0KX2n0EEf518JKKUi9Wedl3JTIGinQ1pJhACYXJo8Lv3fVoa6bfTDTBlyK0pTPs rbTZaZxuRD2Rw4xLBUOw/IvfvcdF62IyhwoUkSxgYX4fJQjJym2uFDhE6qzOInnsk4SG OXuUog/rFGw4HCA3/5JhHcLKv/Phj4HRSobca4JP0k9YprJHRuRLjCCRwhr3QkDoxUTj 7hyw== 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=YhMLOnWO5qZngJMN7NKdOR6yuHZHGJTjvv7soDc2Xpo=; b=KA06opoG4jPw5wgIOdvA6SqCVPav/OnvU/xatG6vKG0DA3cphWT374nDbeTjc2HQ7n a7yBcmdYFrkRn3w0S9NQZdyV8WJoTW2JBGznX7rHmXi9o8KaWLojAfe0v06OPWdv5H19 kzO84cX9NbUxkFPXlw6zbWN2y+SOgjPnncH6S46ynHkMmWt+fQQZ6Uqyr5dw8cWPiPS3 DA39nvP3Xqx3cHng3Mg1SZdV5WhHWuYyt6+WkXm//Pexa6lEVsE+rT6KFqO4weSxYlva CJ/j2Qn88atUDEjnwvZncTHtIvCubs5flBIhE51vfiMu+tJnjnbG/UBpZND6qdVyoGnU HlbA== X-Gm-Message-State: AOAM533BN3a1DZ6SOzM9iq/cCXl05KzrTz02GkUqkONzfEfJG5qcmLsW t6eU4aayV2EKFN4DLX9/SzVPZY+DVGv4+0SipX8A5A== X-Google-Smtp-Source: ABdhPJzOl9yYrt/64p9ziBPGD3/TvD8+f3yKe8pN6dxoOUcsz3QU7eI4rPq8zKTMwRC1YjgRIwoslaT8CuBxnlAbvwM= X-Received: by 2002:a17:902:d482:b029:ec:9091:d099 with SMTP id c2-20020a170902d482b02900ec9091d099mr21410029plg.34.1619012379567; Wed, 21 Apr 2021 06:39:39 -0700 (PDT) MIME-Version: 1.0 References: <20210421062644.68331-1-songmuchun@bytedance.com> In-Reply-To: From: Muchun Song Date: Wed, 21 Apr 2021 21:39:03 +0800 Message-ID: Subject: Re: [External] Re: [PATCH] mm: memcontrol: fix root_mem_cgroup charging To: Michal Hocko Cc: Roman Gushchin , Johannes Weiner , Andrew Morton , Shakeel Butt , Vladimir Davydov , LKML , Linux Memory Management List , Xiongchun duan , fam.zheng@bytedance.com Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: xmf3pt45w61buqo6wwzmquzhoefp9aki X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 98DB140002D7 Received-SPF: none (bytedance.com>: No applicable sender policy available) receiver=imf02; identity=mailfrom; envelope-from=""; helo=mail-pl1-f175.google.com; client-ip=209.85.214.175 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1619012357-695471 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, Apr 21, 2021 at 9:03 PM Michal Hocko wrote: > > On Wed 21-04-21 17:50:06, Muchun Song wrote: > > On Wed, Apr 21, 2021 at 3:34 PM Michal Hocko wrote: > > > > > > On Wed 21-04-21 14:26:44, Muchun Song wrote: > > > > The below scenario can cause the page counters of the root_mem_cgroup > > > > to be out of balance. > > > > > > > > CPU0: CPU1: > > > > > > > > objcg = get_obj_cgroup_from_current() > > > > obj_cgroup_charge_pages(objcg) > > > > memcg_reparent_objcgs() > > > > // reparent to root_mem_cgroup > > > > WRITE_ONCE(iter->memcg, parent) > > > > // memcg == root_mem_cgroup > > > > memcg = get_mem_cgroup_from_objcg(objcg) > > > > // do not charge to the root_mem_cgroup > > > > try_charge(memcg) > > > > > > > > obj_cgroup_uncharge_pages(objcg) > > > > memcg = get_mem_cgroup_from_objcg(objcg) > > > > // uncharge from the root_mem_cgroup > > > > page_counter_uncharge(&memcg->memory) > > > > > > > > This can cause the page counter to be less than the actual value, > > > > Although we do not display the value (mem_cgroup_usage) so there > > > > shouldn't be any actual problem, but there is a WARN_ON_ONCE in > > > > the page_counter_cancel(). Who knows if it will trigger? So it > > > > is better to fix it. > > > > > > The changelog doesn't explain the fix and why you have chosen to charge > > > kmem objects to root memcg and left all other try_charge users intact. > > > > The object cgroup is special (because the page can reparent). Only the > > user of objcg APIs should be fixed. > > > > > The reason is likely that those are not reparented now but that just > > > adds an inconsistency. > > > > > > Is there any reason you haven't simply matched obj_cgroup_uncharge_pages > > > to check for the root memcg and bail out early? > > > > Because obj_cgroup_uncharge_pages() uncharges pages from the > > root memcg unconditionally. Why? Because some pages can be > > reparented to root memcg, in order to ensure the correctness of > > page counter of root memcg. We have to uncharge pages from > > root memcg. So we do not check whether the page belongs to > > the root memcg when it uncharges. > > I am not sure I follow. Let me ask differently. Wouldn't you > achieve the same if you simply didn't uncharge root memcg in > obj_cgroup_charge_pages? I'm afraid not. Some pages should uncharge root memcg, some pages should not uncharge root memcg. But all those pages belong to the root memcg. We cannot distinguish between the two. I believe Roman is very familiar with this mechanism (objcg APIs). Hi Roman, Any thoughts on this? > > Btw. which tree is this patch based on? The current linux-next doesn't > uncharge from memcg->memory inside obj_cgroup_uncharge_pages (nor does > the Linus tree). Sorry. I should expose more details. obj_cgroup_uncharge_pages refill_stock->drain_stock page_counter_uncharge // uncharging is here Thanks. > -- > Michal Hocko > SUSE Labs