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 695C3C6369E for ; Thu, 3 Dec 2020 03:37:35 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 84562208A9 for ; Thu, 3 Dec 2020 03:37:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 84562208A9 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 B96606B0068; Wed, 2 Dec 2020 22:37:33 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B1EF56B006C; Wed, 2 Dec 2020 22:37:33 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9E6EA6B006E; Wed, 2 Dec 2020 22:37:33 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0040.hostedemail.com [216.40.44.40]) by kanga.kvack.org (Postfix) with ESMTP id 837AC6B0068 for ; Wed, 2 Dec 2020 22:37:33 -0500 (EST) Received: from smtpin20.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 3BE001EE6 for ; Thu, 3 Dec 2020 03:37:33 +0000 (UTC) X-FDA: 77550561186.20.patch05_2f0d553273b8 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin20.hostedemail.com (Postfix) with ESMTP id 1168B180C0609 for ; Thu, 3 Dec 2020 03:37:33 +0000 (UTC) X-HE-Tag: patch05_2f0d553273b8 X-Filterd-Recvd-Size: 6351 Received: from mail-pg1-f195.google.com (mail-pg1-f195.google.com [209.85.215.195]) by imf42.hostedemail.com (Postfix) with ESMTP for ; Thu, 3 Dec 2020 03:37:32 +0000 (UTC) Received: by mail-pg1-f195.google.com with SMTP id t37so554541pga.7 for ; Wed, 02 Dec 2020 19:37:32 -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=/0xr/gzJjOZ5SODRA3FMIk6kL0F9ClCll/zV7XChFAo=; b=HY2GSLdnb+9L/xS5afBWxuK06/8OF0WB8Qry7TbjprPX+g82CLCtaIoansVJ6iP/yN Ly853xzIqhMgFPVTtOfOjPyHjCOa3hhkpMiX9/Y3F4EK5nr+t+i9vozAlDJK6uoQ4ELH GVgnmB8YgiD51Z9qYxSouucBkt/rlpBxAybh8vXXH+gQPOsW427MFn2gxo//2sqJyGsK +RkVDzianrbJxsJfoftjDclsHuCEuD+V4NV3Y/6K4/uCf3QRW1a0qYyIoSon6l/IzlCy l8jan0Jx2lIH02lSY+cnqeHhz/64Y1MUpjYQG/8CBH9U77Ru5hcIKjKApgRNH5xZY5ec L2Bw== 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=/0xr/gzJjOZ5SODRA3FMIk6kL0F9ClCll/zV7XChFAo=; b=cXRVEerXY+OtGTQT9PCwD9ypR8w7XiYg2qapkGsHbQjIm2O+x2SP8qatrfGlaTY/nr z9IrRhczW0C2Hy03KiP0AWQVZBRx9hvuwJB6tX+wwmcKURAeFDzT3UvhMiGro+Hj07eq kllFYJTS/HyROA/o3INAvElagI4awmTKaKE/HsSuc3hsMkmL18A2kl8OvHhvx+hoUiml cjAXtA7EqmbJFQSQjUONANJ1nYhOp/PpdMqf3c3qOtYdOMpnFBqzt3+0AVxoxmZW2SeX Ug4/51EKlWyUjVMOFKrpjJINvZiHXJHZuvQHxUYyyAv5USUqYJge6Op4MZ7neiOliM2J lFSQ== X-Gm-Message-State: AOAM533Z6RA3q/0MiyJpzpOhn7g96MRFIos47iupxQjfFHu6F0GpE4tc LP4mXL+dsAFZxfuFDup41VK/aRyx7p+fPmz6gZnHlg== X-Google-Smtp-Source: ABdhPJz3faoAVseY/IdtEUhGbr3UHiCxbeFjTsQ9mGrZYO95xXKjwSHdndVR49F8o3DvOqYgNRe2bLhirgEKKFAJVHo= X-Received: by 2002:a63:ff18:: with SMTP id k24mr1286059pgi.273.1606966651315; Wed, 02 Dec 2020 19:37:31 -0800 (PST) MIME-Version: 1.0 References: <20201202121434.75099-1-songmuchun@bytedance.com> <20201202211646.GA1517142@carbon.lan> <20201203032052.GA1568874@carbon.DHCP.thefacebook.com> In-Reply-To: <20201203032052.GA1568874@carbon.DHCP.thefacebook.com> From: Muchun Song Date: Thu, 3 Dec 2020 11:36:54 +0800 Message-ID: Subject: Re: [External] Re: [PATCH] mm/memcontrol: make the slab calculation consistent To: Roman Gushchin Cc: Johannes Weiner , Michal Hocko , Vladimir Davydov , Andrew Morton , Cgroups , Linux Memory Management List , LKML Content-Type: text/plain; charset="UTF-8" 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, Dec 3, 2020 at 11:21 AM Roman Gushchin wrote: > > On Thu, Dec 03, 2020 at 10:53:33AM +0800, Muchun Song wrote: > > On Thu, Dec 3, 2020 at 5:16 AM Roman Gushchin wrote: > > > > > > On Wed, Dec 02, 2020 at 08:14:34PM +0800, Muchun Song wrote: > > > > Although the ratio of the slab is one, we also should read the ratio > > > > from the related memory_stats instead of hard-coding. And the local > > > > variable of size is already the value of slab_unreclaimable. So we > > > > do not need to read again. Simplify the code here. > > > > > > > > Signed-off-by: Muchun Song > > > > --- > > > > mm/memcontrol.c | 22 +++++++++++++++++----- > > > > 1 file changed, 17 insertions(+), 5 deletions(-) > > > > > > Hi Muchun! > > > > > > > > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > > > index 9922f1510956..03a9c64560f6 100644 > > > > --- a/mm/memcontrol.c > > > > +++ b/mm/memcontrol.c > > > > @@ -1545,12 +1545,22 @@ static int __init memory_stats_init(void) > > > > int i; > > > > > > > > for (i = 0; i < ARRAY_SIZE(memory_stats); i++) { > > > > + switch (memory_stats[i].idx) { > > > > #ifdef CONFIG_TRANSPARENT_HUGEPAGE > > > > - if (memory_stats[i].idx == NR_ANON_THPS || > > > > - memory_stats[i].idx == NR_FILE_THPS || > > > > - memory_stats[i].idx == NR_SHMEM_THPS) > > > > + case NR_ANON_THPS: > > > > + case NR_FILE_THPS: > > > > + case NR_SHMEM_THPS: > > > > memory_stats[i].ratio = HPAGE_PMD_SIZE; > > > > + break; > > > > #endif > > > > + case NR_SLAB_UNRECLAIMABLE_B: > > > > + VM_BUG_ON(i < 1); > > > > + VM_BUG_ON(memory_stats[i - 1].idx != NR_SLAB_RECLAIMABLE_B); > > > > > > Please, convert these to BUILD_BUG_ON(), they don't have to be runtime checks. > > > > Agree. But here we cannot use BUILD_BUG_ON(). The compiler will > > complain about it. > > We can! > > We just need to change the condition. All we really need to check is that > NR_SLAB_UNRECLAIMABLE_B immediately following NR_SLAB_RECLAIMABLE_B. But I think that we need to check that memory_stats[i] immediately following memory_stats[j] where i is the index of NR_SLAB_UNRECLAIMABLE_B and j is the index of NR_SLAB_RECLAIMABLE_B. > > Something like BUILD_BUG_ON(NR_SLAB_UNRECLAIMABLE_B != NR_SLAB_RECLAIMABLE_B + 1) So this cannot work. Thanks. > should work (completely untested). > > > > > > > > > > > > > + break; > > > > + default: > > > > + break; > > > > + } > > > > + > > > > VM_BUG_ON(!memory_stats[i].ratio); > > > > VM_BUG_ON(memory_stats[i].idx >= MEMCG_NR_STAT); > > > > } > > > > @@ -1587,8 +1597,10 @@ static char *memory_stat_format(struct mem_cgroup *memcg) > > > > seq_buf_printf(&s, "%s %llu\n", memory_stats[i].name, size); > > > > > > > > > > Can you, please, add a small comment here stating that we're printing > > > unreclaimable, reclaimable and the sum of both? It will simplify the reading of the code. > > > > Will do. > > Thank you! -- Yours, Muchun