| Message ID | 20231205223118.3575485-1-souravpanda@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mm: report per-page metadata information | expand |
On Tue, Dec 05, 2023 at 02:31:17PM -0800, Sourav Panda wrote: > Changelog: > v6: > - Interface changes > - Added per-node nr_page_metadata and > nr_page_metadata_boot fields that are exported > in /sys/devices/system/node/nodeN/vmstat Again, please do not add any new fields to existing sysfs files, that's not going to work well. You can add a new sysfs file, that's file, but do not continue the abuse of the sysfs api in this file. thanks, greg k-h
Hi Greg, Sourav removed the new field from sys/device../nodeN/meminfo as you requested; however, in nodeN/vmstat fields still get appended, as there is code that displays every item in zone_stat_item, node_stat_item without option to opt-out. I mentioned it to you at LPC. In my IOMMU [1] series, there are also fields that are added to node_stat_item that as result are appended to nodeN/vmstat. Pasha [1] https://lore.kernel.org/all/20231130201504.2322355-1-pasha.tatashin@soleen.com On Tue, Dec 5, 2023 at 9:36 PM Greg KH <gregkh@linuxfoundation.org> wrote: > > On Tue, Dec 05, 2023 at 02:31:17PM -0800, Sourav Panda wrote: > > Changelog: > > v6: > > - Interface changes > > - Added per-node nr_page_metadata and > > nr_page_metadata_boot fields that are exported > > in /sys/devices/system/node/nodeN/vmstat > > Again, please do not add any new fields to existing sysfs files, that's > not going to work well. You can add a new sysfs file, that's file, but > do not continue the abuse of the sysfs api in this file. > > thanks, > > greg k-h
On Tue, Dec 05, 2023 at 09:57:36PM -0500, Pasha Tatashin wrote: > Hi Greg, > > Sourav removed the new field from sys/device../nodeN/meminfo as you > requested; however, in nodeN/vmstat fields still get appended, as > there is code that displays every item in zone_stat_item, > node_stat_item without option to opt-out. I mentioned it to you at > LPC. Sorry, I thought that was a proc file, not a sysfs file. Don't grow that file please, it should not be that way and adding to it will just cause others to also want to add to it and we end up with the whole proc file mess... > In my IOMMU [1] series, there are also fields that are added to > node_stat_item that as result are appended to nodeN/vmstat. I missed that, that too shouldn't be done please. Again, sysfs is "one value per file" for a reason, don't abuse the fact that we missed this abuse of the rules for that file by adding more things to it. thanks, greg k-h
On Wed, 6 Dec 2023 12:12:10 +0900 Greg KH <gregkh@linuxfoundation.org> wrote: > On Tue, Dec 05, 2023 at 09:57:36PM -0500, Pasha Tatashin wrote: > > Hi Greg, > > > > Sourav removed the new field from sys/device../nodeN/meminfo as you > > requested; however, in nodeN/vmstat fields still get appended, as > > there is code that displays every item in zone_stat_item, > > node_stat_item without option to opt-out. I mentioned it to you at > > LPC. > > Sorry, I thought that was a proc file, not a sysfs file. Don't grow > that file please, it should not be that way and adding to it will just > cause others to also want to add to it and we end up with the whole proc > file mess... > > > In my IOMMU [1] series, there are also fields that are added to > > node_stat_item that as result are appended to nodeN/vmstat. > > I missed that, that too shouldn't be done please. > > Again, sysfs is "one value per file" for a reason, don't abuse the fact > that we missed this abuse of the rules for that file by adding more > things to it. I'm afraid that horse has bolted. hp2:/usr/src/25> wc /sys/devices/system/node/node0/vmstat 61 122 1309 /sys/devices/system/node/node0/vmstat We're never going to unpick this into 61 separate files so adding new files at this stage is pointless.
On Wed, Dec 06, 2023 at 07:59:13AM -0800, Andrew Morton wrote: > On Wed, 6 Dec 2023 12:12:10 +0900 Greg KH <gregkh@linuxfoundation.org> wrote: > > > On Tue, Dec 05, 2023 at 09:57:36PM -0500, Pasha Tatashin wrote: > > > Hi Greg, > > > > > > Sourav removed the new field from sys/device../nodeN/meminfo as you > > > requested; however, in nodeN/vmstat fields still get appended, as > > > there is code that displays every item in zone_stat_item, > > > node_stat_item without option to opt-out. I mentioned it to you at > > > LPC. > > > > Sorry, I thought that was a proc file, not a sysfs file. Don't grow > > that file please, it should not be that way and adding to it will just > > cause others to also want to add to it and we end up with the whole proc > > file mess... > > > > > In my IOMMU [1] series, there are also fields that are added to > > > node_stat_item that as result are appended to nodeN/vmstat. > > > > I missed that, that too shouldn't be done please. > > > > Again, sysfs is "one value per file" for a reason, don't abuse the fact > > that we missed this abuse of the rules for that file by adding more > > things to it. > > I'm afraid that horse has bolted. > > hp2:/usr/src/25> wc /sys/devices/system/node/node0/vmstat > 61 122 1309 /sys/devices/system/node/node0/vmstat > > We're never going to unpick this into 61 separate files so adding new > files at this stage is pointless. But if it keeps growing, it will eventually overflow and start crashing the kernel and you will then have to do the horrid thing of turning it into a binary sysfs file. So I can please ask that no new entries be added to the file please, let's not keep making things worse. For new items, just add new files, don't add to the existing mess. thanks, greg k-h
Changelog: v6: - Interface changes - Added per-node nr_page_metadata and nr_page_metadata_boot fields that are exported in /sys/devices/system/node/nodeN/vmstat - nr_page_metadata exclusively tracks buddy allocations while nr_page_metadata_boot exclusively tracks memblock allocations. - Modified PageMetadata in /proc/meminfo to only include buddy allocations so that it is comparable against MemTotal in /proc/meminfo - Removed the PageMetadata field added in /sys/devices/system/node/nodeN/meminfo - Addressed bugs reported by the kernel test bot. - All occurences of __mod_node_page_state have been replaced by mod_node_page_state. - Addressed comments from Muchun Song. - Removed page meta data accouting from mm/hugetlb.c. When CONFIG_SPARSEMEM_VMEMMAP is disabled struct pages should not be returned to buddy. - Modified the cover letter with the results and analysis - From when memory_hotplug.memmap_on_memory is alternated between 0 and 1. - To account for the new interface changes. v5: - Addressed comments from Muchun Song. - Fixed errors introduced in v4 when CONFIG_SPARSEMEM_VMEMMAP is disabled by testing against FLATMEM and SPARSEMEM memory models. - Handled the condition wherein the allocation of walk.reuse_page fails, by moving NR_PAGE_METADATA update into the clause if( walk.reuse_page ). - Removed the usage of DIV_ROUND_UP from alloc_vmemmap_page_list since "end - start" is always a multiple of PAGE_SIZE. - Modified alloc_vmemmap_page_list to update NR_PAGE_METADATA once instead of every loop. v4: - Addressed comment from Matthew Wilcox. - Used __node_stat_sub_folio and __node_stat_add_folio instead of __mod_node_page_state in mm/hugetlb.c. - Used page_pgdat wherever possible in the entire patch. - Used DIV_ROUND_UP() wherever possible in the entire patch. v3: - Addressed one comment from Matthew Wilcox. - In free_page_ext, page_pgdat() is now extracted prior to freeing the memory. v2: - Fixed the three bugs reported by kernel test robot. - Enhanced the commit message as recommended by David Hildenbrand. - Addressed comments from Matthew Wilcox: - Simplified alloc_vmemmap_page_list() and free_page_ext() as recommended. - Used the appropriate comment style in mm/vmstat.c. - Replaced writeout_early_perpage_metadata() with store_early_perpage_metadata() to reduce ambiguity with what swap does. - Addressed comments from Mike Rapoport: - Simplified the loop in alloc_vmemmap_page_list(). - Could NOT address a comment to move store_early_perpage_metadata() near where nodes and page allocator are initialized. - Included the vmalloc()ed page_ext in accounting within free_page_ext(). - Made early_perpage_metadata[MAX_NUMNODES] static. Previous approaches and discussions ----------------------------------- v5: https://lore.kernel.org/all/20231101230816.1459373-1-souravpanda@google.com v4: https://lore.kernel.org/all/20231031223846.827173-1-souravpanda@google.com v3: https://lore.kernel.org/all/20231031174459.459480-1-souravpanda@google.com v2: https://lore.kernel.org/all/20231018005548.3505662-1-souravpanda@google.com v1: https://lore.kernel.org/r/20230913173000.4016218-2-souravpanda@google.com Hi! This patch adds two new per-node fields, namely nr_page_metadata and nr_page_metadata_boot to /sys/devices/system/node/nodeN/vmstat and a global PageMetadata 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. Per-page metadata is the amount of memory that Linux needs in order to manage memory at the page granularity. The majority of such memory is used by "struct page" and "page_ext" data structures. Background ---------- Kernel overhead observability is missing some of the largest allocations during runtime, including vmemmap (struct pages) and page_ext. This patch aims to address this problem by exporting a new metric PageMetadata. On the contrary, the kernel does provide observibility for boot memory allocations. For example, the metric reserved_pages depicts the pages allocated by the bootmem allocator. This can be simply calculated as present_pages - managed_pages, which are both exported in /proc/zoneinfo. The metric reserved_pages is primarily composed of struct pages and page_ext. What about the struct pages (allocated by bootmem allocator) that are free'd during hugetlbfs allocations and then allocated by buddy-allocator once hugtlbfs pages are free'd? /proc/meminfo MemTotal changes: MemTotal does not include memblock allocations but includes buddy allocations. However, during runtime memblock allocations can be shifted into buddy allocations, and therefore become part of MemTotal. Once the struct pages get allocated by buddy allocator, we lose track of these struct page allocations overhead accounting. Therefore, we must export new metrics. nr_page_metadata and nr_page_metadata_boot exclusively track the struct page and page ext allocations made by the buddy allocator and memblock allocator, respectively for each node. PageMetadata in /proc/meminfo would report the struct page and page ext allocations made by the buddy allocator. Results and analysis -------------------- Memory model: Sparsemem-vmemmap $ echo 1 > /proc/sys/vm/hugetlb_optimize_vmemmap $ cat /proc/meminfo | grep MemTotal MemTotal: 32919124 kB $ cat /proc/meminfo | grep Meta PageMetadata: 65536 kB $ cat /sys/devices/system/node/node0/vmstat | grep "nr_page_metadata" nr_page_metadata 8192 nr_page_metadata_boot 65536 $ cat /sys/devices/system/node/node1/vmstat | grep "nr_page_metadata" nr_page_metadata 8192 nr_page_metadata_boot 65536 AFTER HUGTLBFS RESERVATION $ echo 512 > /proc/sys/vm/nr_hugepages $ cat /proc/meminfo | grep MemTotal MemTotal: 32935508 kB $ cat /proc/meminfo | grep Meta PageMetadata: 67584 kB $ cat /sys/devices/system/node/node0/vmstat | grep "nr_page_metadata" nr_page_metadata 8448 nr_page_metadata_boot 63488 $ cat /sys/devices/system/node/node1/vmstat | grep "nr_page_metadata" nr_page_metadata 8448 nr_page_metadata_booy 63488 AFTER FREEING HUGTLBFS RESERVATION $ echo 0 > /proc/sys/vm/nr_hugepages $ cat /proc/meminfo | grep MemTotal MemTotal: 32935508 kB $ cat /proc/meminfo | grep Meta PageMetadata: 81920 kB $ cat /sys/devices/system/node/node0/vmstat | grep "nr_page_metadata" nr_page_metadata 10240 nr_page_metadata_boot 63488 $ cat /sys/devices/system/node/node1/vmstat | grep "nr_page_metadata" nr_page_metadata 10240 nr_page_metadata_boot 63488 ------------------------ Memory Hotplug with memory_hotplug.memmap_on_memory=1 BEFORE HOTADD $ cat /proc/meminfo | grep MemTotal MemTotal: 32919124 kB $ cat /proc/meminfo | grep PageMeta PageMetadata: 65536 kB $ cat /sys/devices/system/node/node0/vmstat | grep "nr_page_metadata" nr_page_metadata 8192 nr_page_metadata_boot 65536 $ cat /sys/devices/system/node/node1/vmstat | grep "nr_page_metadata" nr_page_metadata 8192 nr_page_metadata_boot 65536 AFTER HOTADDING 1GB $ cat /proc/meminfo | grep MemTotal MemTotal: 33951316 kB $ cat /proc/meminfo | grep PageMeta PageMetadata: 83968 kB $ cat /sys/devices/system/node/node0/vmstat | grep "nr_page_metadata" nr_page_metadata 12800 nr_page_metadata_boot 65536 $ cat /sys/devices/system/node/node1/vmstat | grep "nr_page_metadata" nr_page_metadata 8192 nr_page_metadata_boot 65536 -------------------------- Memory Hotplug with memory_hotplug.memmap_on_memory=0 BEFORE HOTADD $ cat /proc/meminfo | grep MemTotal MemTotal: 32919124 kB $ cat /proc/meminfo | grep PageMeta PageMetadata: 65536 kB $ cat /sys/devices/system/node/node0/vmstat | grep "nr_page_metadata" nr_page_metadata 8192 nr_page_metadata_boot 65536 $ cat /sys/devices/system/node/node1/vmstat | grep "nr_page_metadata" nr_page_metadata 8192 nr_page_metadata_boot 65536 AFTER HOTADDING 1GB $ cat /proc/meminfo | grep MemTotal MemTotal: 33967700 kB $ cat /proc/meminfo | grep PageMeta PageMetadata: 83968 kB $ cat /sys/devices/system/node/node0/vmstat | grep "nr_page_metadata" nr_page_metadata 12800 nr_page_metadata_boot 65536 $ cat /sys/devices/system/node/node1/vmstat | grep "nr_page_metadata" nr_page_metadata 8192 nr_page_metadata_boot 65536 Sourav Panda (1): mm: report per-page metadata information Documentation/filesystems/proc.rst | 3 +++ fs/proc/meminfo.c | 7 +++++++ include/linux/mmzone.h | 4 ++++ include/linux/vmstat.h | 4 ++++ mm/hugetlb_vmemmap.c | 19 ++++++++++++++---- 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, 99 insertions(+), 15 deletions(-)