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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7BD7EC433EF for ; Thu, 18 Nov 2021 03:55:39 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id CCA7D61B42 for ; Thu, 18 Nov 2021 03:55:38 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org CCA7D61B42 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=bytedance.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 0D1A46B0072; Wed, 17 Nov 2021 22:55:28 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 081686B0073; Wed, 17 Nov 2021 22:55:28 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EB1516B0074; Wed, 17 Nov 2021 22:55:27 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0032.hostedemail.com [216.40.44.32]) by kanga.kvack.org (Postfix) with ESMTP id DD2EE6B0072 for ; Wed, 17 Nov 2021 22:55:27 -0500 (EST) Received: from smtpin09.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 9E086182C7849 for ; Thu, 18 Nov 2021 03:55:17 +0000 (UTC) X-FDA: 78820685874.09.C11ECD5 Received: from mail-yb1-f171.google.com (mail-yb1-f171.google.com [209.85.219.171]) by imf31.hostedemail.com (Postfix) with ESMTP id A7C8D1052985 for ; Thu, 18 Nov 2021 03:55:15 +0000 (UTC) Received: by mail-yb1-f171.google.com with SMTP id v64so13937427ybi.5 for ; Wed, 17 Nov 2021 19:55:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20210112.gappssmtp.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=7t2KOfThi8hwqtjS9xYJIJMpMs4r7+aUCTbqpqaQgB8=; b=VegwIcpX4FTIU4RKip0IPMIg6bX57jiY+9C8sQOQlBEqaM0YS6p0Xj1UBjhlrmsTws nFsgLGqhsql/oJaUZmT23eSUFDPYJKFEBLYMdg1sCfGd73WqCBwrljXGCjUDAyycs1Ui KiKS2L8yF5rQ4s0DOtrMCJQyrko1gAm1Be0794OMriv4R96ZDyOQwF3Ki6qUVRKGz/zl 0KsMERtV0Zk2+MG1lLyY2S/d2TIpwYf99h/8vuodrpwrJIDdDDY2rSzEXyJSryp2zD9Q JpMxhMN6GcLXDPQBFy86gmFjQ8pVlEXwfZzR0LT2a7bhXaOceJg0a2A/fBAKxn+o6p8k 5Bmg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=7t2KOfThi8hwqtjS9xYJIJMpMs4r7+aUCTbqpqaQgB8=; b=jNED0D4o12lKTyQUAAQ+FdKPuqj75LK28lN6G3u7huzaH5CcMRNCg5v0CJ1nT7UTMJ KEXFNfmaY03zqgGwHMa315b/9249outEjWprY/MxPTRYM54SWnmjiNqpYbuaU1O6CtRT 1M7rfZo2LnKZagYAPcqGz+UYaFPWjgf/VUkpmB/+P4l2UM6yNnzedovsHfEUn9E3aoYK 7tD8gLSQK9rvo9+8/9qqBR0Q2OAIEbqTCwnuvpFat+mzxdVR6byaqk/QhmL+GXmM247T EfKSd6vZ/0OCyDSGPVGvy6LGou+A6UhFgrL37fDidUh78voTe/9iPNNIWkEcOj7OnE/6 rIwg== X-Gm-Message-State: AOAM532x7CdE9MjFdg6xAdyHzP02HJSPD95qRXa91B3/hHbBVNMqFrGr DxwHY53EyY7eNeCHiPN7amvUxbRibX0yC/UQEPJxsw== X-Google-Smtp-Source: ABdhPJwDXNp4NAcyRT6V4FmOdCoZyUxOTi38EahqeDeNePoS+k/y6vFUW7lDd3FRU4FzdGYUTLVpGb8mv+7nIthpxFM= X-Received: by 2002:a05:6902:1149:: with SMTP id p9mr4795700ybu.404.1637207715886; Wed, 17 Nov 2021 19:55:15 -0800 (PST) MIME-Version: 1.0 References: <20211117201825.429650-1-almasrymina@google.com> <86d3ba7a-3706-d66c-cbf7-d2c39ad2cd4c@oracle.com> In-Reply-To: <86d3ba7a-3706-d66c-cbf7-d2c39ad2cd4c@oracle.com> From: Muchun Song Date: Thu, 18 Nov 2021 11:54:37 +0800 Message-ID: Subject: Re: [PATCH v7] hugetlb: Add hugetlb.*.numa_stat file To: Mike Kravetz , Mina Almasry Cc: Tejun Heo , Zefan Li , Johannes Weiner , Jonathan Corbet , Andrew Morton , Shuah Khan , Miaohe Lin , Oscar Salvador , Michal Hocko , David Rientjes , Shakeel Butt , Jue Wang , Yang Yao , Joanna Li , Cannon Matthews , Linux Memory Management List , LKML , Cgroups , linux-doc@vger.kernel.org Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: c7n66zxgckgreih87ewuo3tgg14t9x8g Authentication-Results: imf31.hostedemail.com; dkim=pass header.d=bytedance-com.20210112.gappssmtp.com header.s=20210112 header.b=VegwIcpX; dmarc=pass (policy=none) header.from=bytedance.com; spf=pass (imf31.hostedemail.com: domain of songmuchun@bytedance.com designates 209.85.219.171 as permitted sender) smtp.mailfrom=songmuchun@bytedance.com X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: A7C8D1052985 X-HE-Tag: 1637207715-267307 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, Nov 18, 2021 at 8:13 AM Mike Kravetz wrote: > > On 11/17/21 12:18, Mina Almasry wrote: > ... > > diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c > ... > > @@ -288,11 +317,21 @@ static void __hugetlb_cgroup_commit_charge(int idx, unsigned long nr_pages, > > struct hugetlb_cgroup *h_cg, > > struct page *page, bool rsvd) > > { > > + unsigned long *usage; > > + > > I assume the use of a pointer is just to make the following WRITE_ONCE > look better? I prefer the suggestion by Muchun: > > unsigned long usage = h_cg->nodeinfo[page_to_nid(page)]->usage[idx]; > > usage += nr_pages; > WRITE_ONCE(h_cg->nodeinfo[page_to_nid(page)]->usage[idx], usage); > > I had to think for just a second 'why are we using/passing a pointer?'. > Not insisting we use Muchun's suggestion, it just caused me to think > a little more than necessary. At least I have the same question here. For me I think it's unnecessary to use a pointer. > > In any case, I would move the variable usage inside the > 'if (!rsvd)' block. > > > if (hugetlb_cgroup_disabled() || !h_cg) > > return; > > > > __set_hugetlb_cgroup(page, h_cg, rsvd); > > - return; > > + if (!rsvd) { > > + usage = &h_cg->nodeinfo[page_to_nid(page)]->usage[idx]; > > + /* > > + * This write is not atomic due to fetching *usage and writing > > + * to it, but that's fine because we call this with > > + * hugetlb_lock held anyway. > > + */ > > + WRITE_ONCE(*usage, *usage + nr_pages); > > + } > > } > > > > void hugetlb_cgroup_commit_charge(int idx, unsigned long nr_pages, > > @@ -316,6 +355,7 @@ static void __hugetlb_cgroup_uncharge_page(int idx, unsigned long nr_pages, > > struct page *page, bool rsvd) > > { > > struct hugetlb_cgroup *h_cg; > > + unsigned long *usage; > > Same here. > > Otherwise, looks good to me. > -- > Mike Kravetz