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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3D4C7EEAA78 for ; Thu, 14 Sep 2023 21:04:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A1BCC6B02EE; Thu, 14 Sep 2023 17:04:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9CC656B02EF; Thu, 14 Sep 2023 17:04:37 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 893386B02F0; Thu, 14 Sep 2023 17:04:37 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 75BC46B02EE for ; Thu, 14 Sep 2023 17:04:37 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 41C50806D9 for ; Thu, 14 Sep 2023 21:04:37 +0000 (UTC) X-FDA: 81236431794.01.16296F6 Received: from mail-ua1-f50.google.com (mail-ua1-f50.google.com [209.85.222.50]) by imf01.hostedemail.com (Postfix) with ESMTP id 71A484001A for ; Thu, 14 Sep 2023 21:04:35 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=S9dxz2Xf; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of souravpanda@google.com designates 209.85.222.50 as permitted sender) smtp.mailfrom=souravpanda@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1694725475; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=RfrxHwoyl0QJP9rZkoBCvgGoU6cT/XzkWLNDxXM8VB0=; b=AxIyNjsme18lq8dMCq7Q3boy1xGAd9tyMGnqMb3YBeFzhe2l10qR0+jgHQhG2cHyl9qGKK Z6cifyTbj7tKKPRqPiIm7HxrX1tFqH2dZ+cfnii+KMEpDk9W12DgU9mZH92hYafT+ysHXP e5D6xl95ZyZxx3ECqJFlQhtM6Jr2aqQ= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=S9dxz2Xf; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of souravpanda@google.com designates 209.85.222.50 as permitted sender) smtp.mailfrom=souravpanda@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1694725475; a=rsa-sha256; cv=none; b=WmJun95dEWhqFxdv1/XOFhF57GMgsKkKSnAdF/3YPgScFdsgZ61G/X/TC+SjTPjqeE4U7c c9tPx2IxJkeN5roK+dSw0t8mbN/fmZxkvY/8Z0Er1IDXPVFSMWCCoOM6NLJCT5kpABTtvQ 0j2J+rNhZi7i4EW7rYhz0bb4NmShmeM= Received: by mail-ua1-f50.google.com with SMTP id a1e0cc1a2514c-7a52db1e4bbso529129241.3 for ; Thu, 14 Sep 2023 14:04:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1694725474; x=1695330274; darn=kvack.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=RfrxHwoyl0QJP9rZkoBCvgGoU6cT/XzkWLNDxXM8VB0=; b=S9dxz2XfKzo2fR3iGJZ7wyO/Na/M0yMcpf/2TzgK3s/XovBBWJGCV0F3vvQcgqUS4p wy9hhh9d9/YFFSxJl0CUYFFUqQJU0FhXqZh43xOPeA63FbRTXIGgmyM6S/eWheLnjOkU RXXVC7QWOMu9GkOeX3ZijD0NF7f6JlvzOxmvMd/F2P+jiebtqoSWAoHlZwwCstgmuqCc QgKnnmOG/kx0oU7i+N5nCgq3l0SRneyIXZZ/c1OxFvfwUhLdV31eROqyVzd2lg9itCbk 29Ht+CXTKqCCxp7oYfaKUGA6EJDvQRGjLlsOBvn5F9+c9ejLJq02HWxlfO3vONp2as92 HHvA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694725474; x=1695330274; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=RfrxHwoyl0QJP9rZkoBCvgGoU6cT/XzkWLNDxXM8VB0=; b=PwpcEDHXAnMTP2pHS7BKq0yj7k2zB/RByQVPVok+tjLDGxU7GIzxLmb88LqFMa+0yT 7hFjfSbyoWtbPLXrwXOkKgNQXiKZMpwoUyon6JgHDYSfOtj1tH/jMVP0ik3X6ocN2DZ9 pGUwDPAgvpFdXJUcovLyXl3HGvWNwKZPRHY0u41reK6ezHImzbTP28L/hnC/vC8MNa0A 3ilmT9wSiyRi9xlhFshgHwyrEwfxMuesTIh9Cg/qMb9aDwRbhJR9FBfsgpCsICbI16lF TN2WgO2FPMRV2MWsnxc8WrgrRXHvPrOZoPGP1zA90JDhl+0FzfK8kQc+QzxYEDUyAVgB liiQ== X-Gm-Message-State: AOJu0Yx3zRAaN54mOUk+1gFZDnE82S4x1ZNfLJRZTgI4DnIjjgIvhoPx m+sH4sCe7nUYFQT5KSlyLXVuOQ8ScS+DA3CRqBqYOw== X-Google-Smtp-Source: AGHT+IF1itLK0GMRl1XPvYFojjC+58a+1RKFs8fG8hNv49OgWTa0WZ+2mciXxPXus4cZL8ebVaIFhVky0hOZJ9/9vEw= X-Received: by 2002:a1f:c943:0:b0:48c:fc50:4292 with SMTP id z64-20020a1fc943000000b0048cfc504292mr6636164vkf.15.1694725474276; Thu, 14 Sep 2023 14:04:34 -0700 (PDT) MIME-Version: 1.0 References: <20230913173000.4016218-1-souravpanda@google.com> <20230913173000.4016218-2-souravpanda@google.com> In-Reply-To: From: Sourav Panda Date: Thu, 14 Sep 2023 14:04:22 -0700 Message-ID: Subject: Re: [PATCH v1 1/1] mm: report per-page metadata information To: Matthew Wilcox Cc: corbet@lwn.net, gregkh@linuxfoundation.org, rafael@kernel.org, akpm@linux-foundation.org, mike.kravetz@oracle.com, muchun.song@linux.dev, rppt@kernel.org, david@redhat.com, rdunlap@infradead.org, chenlinxuan@uniontech.com, yang.yang29@zte.com.cn, tomas.mudrunka@gmail.com, bhelgaas@google.com, ivan@cloudflare.com, pasha.tatashin@soleen.com, yosryahmed@google.com, hannes@cmpxchg.org, shakeelb@google.com, kirill.shutemov@linux.intel.com, wangkefeng.wang@huawei.com, adobriyan@gmail.com, vbabka@suse.cz, Liam.Howlett@oracle.com, surenb@google.com, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-doc@vger.kernel.org, linux-mm@kvack.org Content-Type: multipart/alternative; boundary="000000000000e34a230605580597" X-Rspamd-Queue-Id: 71A484001A X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: p66pxix5ykstp88h1p48p195umus5f9x X-HE-Tag: 1694725475-927446 X-HE-Meta: U2FsdGVkX1+sPsWSxO37Shbt1lXC99042w3+xNDtxxwbSZWKPbg0vmEi45hi9Pc7cWFgOa9jX7djFV2tDoElS0cLVhv1TTUABJNBcFC4MGR4TZRl4Y1MMCnGq+dwZ8YkOn4yHTi/FdqMp3oHDOsoSP/CDIMdtp0y8u4FxpaDjpt9WhOvY1ycm4zToQXOld3Ct2fA6rbJrXaDgPQlIivMB8yCS1rEKYzfH7gkp68GH+7EEcz6bdr52Baicz+yoYW9Rz6f/WFcLO82JUsqP32r/B/TfZt3CskCnujK3cz5iVB0kW+SCBn2rok01G9R/N8WDaTeSF5Db6QqSv/CzvVQkdLwT98hxu8SjIkItbziUBeJkKaAddCTFY3HeSrGPBCP1NZ1IiGRgHeYtm5ARiL68hVHDDwnoof7deHpRzoH79OJ9wU88RcqapkrtcgMlFYu6IFxIHJZJtFYAJZ6fuXWSAHXi8P/sAbkIVgxd6ZUz65RI5eAMABaWa2L+LSnwdQr5fB/sytEwvs+wJ7vbnaqxo203vfFGJNnubVOU2tS++C8/m1IGA4E6NElxnwkB6PCrPcvgHFfe0Zhv5FyHA9DuGNXOfsC6vMWSK7CWZX8NqckZ9Vd7zLl4FnRZOZiTiizsMpWSbKKge8OR+aqpo34eBBEjcIYj1JvHmIQVk9EnTqbrmfAPJFencSXpuQKr0XZScbNlW0IzadoTmptFa4ItAY0U7nXG8ZUVi4qTRuUl7zRJSKPqg3oWv2LTEe/puYuiTIA+gNT8499NYGxJoidRvCaqAhnYeieqrQRTslnE1IVU5HS9MI3ND+5Na6dg/u8MJ8mJSs0LkTvQMhlcpPVG4n3FvmS9aOmbWke/O6wr1JkZQWoGGRA0L1E4dUV+q5NLBJCZdo9I1VEJBPArcu0slyi/3fNozfGxGz0olsaNKttsLZtP2lTztzvrANn9H2wAdM9nT0vGRz8YufGfXB b/i6HlwY Juv/SApcTxc1vJjVS/9J21ThmBNbFxVO2etkIpTSuS1JTGTq6cM8K8PIdgDbCJAAGkY4P93qEPWxqZwxnRHvVmViPI3IGVXxPYVfaYMWJ5s1HJQJILdiMFiY4Uw5NtaI5+Q26n4O96K5GdVn1+Z8I/nxwYQNaDoaInG4VBRv7u47eQLoD2Bjyq8GbUaUwuiu/WxHeesuLTRLK+GkM/0506N1oZYKaLeidgrbmWkSEQpx0jMCIjsEdCN6LeT/uNdcmY9Jf33aP8BwZwMJ9qF268aG+P0BLBlP6RQlfmNotERM//d6iwayU+Z9YzPtBe5HG297T4VAZk0kRUofqo1o6rmqGpDGLRC1+LRpVLkjKT9jpwG4rpAynq0TG6A== 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: --000000000000e34a230605580597 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Matthew Wilcox, Thank you very much for reviewing my patch. Please find my responses below. On Wed, Sep 13, 2023 at 10:56=E2=80=AFAM Matthew Wilcox wrote: > On Wed, Sep 13, 2023 at 10:30:00AM -0700, Sourav Panda wrote: > > @@ -387,8 +390,12 @@ static int alloc_vmemmap_page_list(unsigned long > start, unsigned long end, > > > > while (nr_pages--) { > > page =3D alloc_pages_node(nid, gfp_mask, 0); > > - if (!page) > > + if (!page) { > > goto out; > > + } else { > > + __mod_node_page_state(NODE_DATA(page_to_nid(page)= ), > > + NR_PAGE_METADATA, 1); > > + } > > list_add_tail(&page->lru, list); > > What a strange way of writing this. Why not simply: > > if (!page) > goto out; > + __mod_node_page_state(NODE_DATA(page_to_nid(page)), > + NR_PAGE_METADATA, 1); > list_add_tail(&page->lru, list); > Thank you Matthew Wilcox for your comment. I agree with you and will make the corresponding change. > > > @@ -314,6 +319,10 @@ static void free_page_ext(void *addr) > > BUG_ON(PageReserved(page)); > > kmemleak_free(addr); > > free_pages_exact(addr, table_size); > > + > > + __mod_node_page_state(NODE_DATA(page_to_nid(page)), > NR_PAGE_METADATA, > > + (long)-1 * (PAGE_ALIGN(table_size) > >> PAGE_SHIFT)); > > Why not spell that as "-1L"? > > And while I'm asking questions, why NODE_DATA(page_to_nid(page)) instead > of page_pgdat(page)? > Yes, thank you! I shall make both the suggested changes. > > > @@ -2274,4 +2275,24 @@ static int __init extfrag_debug_init(void) > > } > > > > module_init(extfrag_debug_init); > > + > > +// Page metadata size (struct page and page_ext) in pages > > Don't use // comments. > Thank you. I shall replace them with /* text */ to be uniform with the document. > > > +void __init writeout_early_perpage_metadata(void) > > "writeout" is something swap does. I'm sure this has a better name, > though I can't think what it might be. > > > +{ > > + int nid; > > + struct pglist_data *pgdat; > > + > > + for_each_online_pgdat(pgdat) { > > + nid =3D pgdat->node_id; > > + __mod_node_page_state(NODE_DATA(nid), NR_PAGE_METADATA, > > + early_perpage_metadata[nid]); > > + } > > +} > > #endif > > -- > > 2.42.0.283.g2d96d420d3-goog > > Yep, thank you! Does store_early_perpage_metadata seem better to you? --000000000000e34a230605580597 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi=C2=A0Matthew Wilcox,

Than= k you very much for reviewing my patch. Please find my responses below.
On W= ed, Sep 13, 2023 at 10:56=E2=80=AFAM Matthew Wilcox <willy@infradead.org> wrote:
On Wed, Sep 13, 2023 at 10:30:00AM -= 0700, Sourav Panda wrote:
> @@ -387,8 +390,12 @@ static int alloc_vmemmap_page_list(unsigned long = start, unsigned long end,
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0while (nr_pages--) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0page =3D alloc_p= ages_node(nid, gfp_mask, 0);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!page)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!page) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0goto out;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} else {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0__mod_node_page_state(NODE_DATA(page_to_nid(page)),
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0NR_PAGE_METADATA, 1);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0list_add_tail(&a= mp;page->lru, list);

What a strange way of writing this.=C2=A0 Why not simply:

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!page)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 goto out;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0__mod_node_page_sta= te(NODE_DATA(page_to_nid(page)),
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0NR_PAGE_METADATA, 1);
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 list_add_tail(&= page->lru, list);

Thank you=C2=A0Mat= thew Wilcox for your comment. I agree with you and will make the correspond= ing change.
=C2=A0

> @@ -314,6 +319,10 @@ static void free_page_ext(void *addr)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0BUG_ON(PageReser= ved(page));
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0kmemleak_free(ad= dr);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0free_pages_exact= (addr, table_size);
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0__mod_node_page_state= (NODE_DATA(page_to_nid(page)), NR_PAGE_METADATA,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(long)-1 * (PAGE_AL= IGN(table_size) >> PAGE_SHIFT));

Why not spell that as "-1L"?

And while I'm asking questions, why NODE_DATA(page_to_nid(page)) instea= d
of page_pgdat(page)?

Yes, thank you! I = shall make both the suggested changes.
=C2=A0

> @@ -2274,4 +2275,24 @@ static int __init extfrag_debug_init(void)
>=C2=A0 }
>=C2=A0
>=C2=A0 module_init(extfrag_debug_init);
> +
> +// Page metadata size (struct page and page_ext) in pages

Don't use // comments.

Thank you. I= shall replace them with /* text */ to be uniform with the document.
<= div>=C2=A0

> +void __init writeout_early_perpage_metadata(void)

"writeout" is something swap does.=C2=A0 I'm sure this has a = better name,
though I can't think what it might be.

> +{
> +=C2=A0 =C2=A0 =C2=A0int nid;
> +=C2=A0 =C2=A0 =C2=A0struct pglist_data *pgdat;
> +
> +=C2=A0 =C2=A0 =C2=A0for_each_online_pgdat(pgdat) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0nid =3D pgdat->nod= e_id;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0__mod_node_page_state= (NODE_DATA(nid), NR_PAGE_METADATA,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0early_perpage_metad= ata[nid]);
> +=C2=A0 =C2=A0 =C2=A0}
> +}
>=C2=A0 #endif
> --
> 2.42.0.283.g2d96d420d3-goog
>

Yep, thank you! Does store_early_perpa= ge_metadata seem=C2=A0better to you?
--000000000000e34a230605580597--