Received-SPF: pass (google.com: domain of linux-kernel+bounces-139552-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 10 Apr 2024 16:58:26 -0700 (PDT)
From: David Rientjes <rientjes@google.com>
To: Andrew Morton <akpm@linux-foundation.org>
cc: Sourav Panda <souravpanda@google.com>, corbet@lwn.net, 
    gregkh@linuxfoundation.org, rafael@kernel.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, 
    Vlastimil Babka <vbabka@suse.cz>, 
    "Liam R. Howlett" <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, 
    Matthew Wilcox <willy@infradead.org>, weixugc@google.com
Subject: Re: [PATCH v9 1/1] mm: report per-page metadata information
In-Reply-To: <20240319143320.d1b1ef7f6fa77b748579ba59@linux-foundation.org>
Message-ID: <65b77d3e-d683-1e90-ebb0-5c7758143048@google.com>
References: <20240220214558.3377482-1-souravpanda@google.com> <20240220214558.3377482-2-souravpanda@google.com> <20240319143320.d1b1ef7f6fa77b748579ba59@linux-foundation.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII

On Tue, 19 Mar 2024, Andrew Morton wrote:

> On Tue, 20 Feb 2024 13:45:58 -0800 Sourav Panda <souravpanda@google.com> wrote:
> 
> > Adds two new per-node fields, namely nr_memmap and nr_memmap_boot,
> > to /sys/devices/system/node/nodeN/vmstat and a global Memmap field
> > to /proc/meminfo. This information can be used by users to see how
> > much memory is being used by per-page metadata, which can vary
> > depending on build configuration, machine architecture, and system
> > use.
> 
> I yield to no man in my admiration of changelogging but boy, that's a
> lot of changelogging.  Would it be possible to consolidate the [0/N]
> coverletter and the [1/N] changelog into a single thing please?
> 
> >  Documentation/filesystems/proc.rst |  3 +++
> >  fs/proc/meminfo.c                  |  4 ++++
> >  include/linux/mmzone.h             |  4 ++++
> >  include/linux/vmstat.h             |  4 ++++
> >  mm/hugetlb_vmemmap.c               | 17 ++++++++++++----
> >  mm/mm_init.c                       |  3 +++
> >  mm/page_alloc.c                    |  1 +
> >  mm/page_ext.c                      | 32 +++++++++++++++++++++---------
> >  mm/sparse-vmemmap.c                |  8 ++++++++
> >  mm/sparse.c                        |  7 ++++++-
> >  mm/vmstat.c                        | 26 +++++++++++++++++++++++-
> >  11 files changed, 94 insertions(+), 15 deletions(-)
> 
> And yet we offer the users basically no documentation.  The new sysfs
> file should be documented under Documentation/ABI somewhere and
> perhaps we could prepare some more expansive user-facing documentation
> elsewhere?
> 

Sourav, is it possible to refresh this series into a v10 on top of the 
latest upstream kernel with a single condensed changelog that details the 
current behavior, what extension this is adding, and how it is generally 
useful?

As noted here, the cover letter has great material that discusses the 
rationale for this change but would be lost if only this patch is merged.  
So typically the cover letter material gets concatenated into the 
changelog, but in this case there's a lot of overlap.

A single patch that includes a succinct changelog would be awesome.

And then the requested documentation in Documentation/ABI either included 
in the same patch or as a second patch in the series?

I don't think the resulting patch series will actually need a cover letter 
after that, it will be able to stand on its own.

> I'd like to hear others' views on the overall usefulness/utility of this
> change, please?
> 

Likely true for all hyperscalers, the immediate use case that this could 
be applied to is to track boot memory overhead and any regression over 
time (across kernel upgrades, firmware upgrades, etc) that may change the 
amount of total memory available.  We'd want to subtract out the boot 
overhead that we know about (like struct page here) and then alert on any 
regression where we're losing memory from reboot to reboot for any reason.

This increased visibility into boot memory overhead allows us to create a 
mechanism to track changes over time when otherwise that attribution of 
that memory is not available.