Re: [PATCH v3 3/4] mm: add system wide stats items category

David Hildenbrand
2024-08-08T18:20:22Z

On 08.08.24 17:42, Pasha Tatashin wrote:
> /proc/vmstat contains events and stats, events can only grow, but stats
> can grow and srhink.

s/shrink/

I think we discussed exposing this in /proc/meminfo. There, it would be
much easier to simply have a global variable, print it, and be done with
it. Like we do with TotalCma.

For /proc/vmstat that's likely the right approach what you have here.

>
> vmstat has the following:
> -------------------------
> NR_VM_ZONE_STAT_ITEMS: per-zone stats
> NR_VM_NUMA_EVENT_ITEMS: per-numa events
> NR_VM_NODE_STAT_ITEMS: per-numa stats
> NR_VM_WRITEBACK_STAT_ITEMS: system-wide background-writeback and
> dirty-throttling tresholds.
> NR_VM_EVENT_ITEMS: system-wide events
> -------------------------
>
> Rename NR_VM_WRITEBACK_STAT_ITEMS to NR_VM_STAT_ITEMS, to track the
> system-wide stats, we are going to add per-page metadata stats to this
> category in the next patch.
>
> Also delete unused writeback_stat_name() function.
>
> Suggested-by: Yosry Ahmed <yosryahmed@google.com>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
> include/linux/vmstat.h | 15 ++++-----------
> mm/vmstat.c | 6 +++---
> 2 files changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
> index 23cd17942036..9ab4fa5e09b5 100644
> --- a/include/linux/vmstat.h
> +++ b/include/linux/vmstat.h
> @@ -34,10 +34,11 @@ struct reclaim_stat {
> unsigned nr_lazyfree_fail;
> };
>
> -enum writeback_stat_item {
> +/* Stat data for system wide items */
> +enum vm_stat_item {
> NR_DIRTY_THRESHOLD,
> NR_DIRTY_BG_THRESHOLD,
> - NR_VM_WRITEBACK_STAT_ITEMS,
> + NR_VM_STAT_ITEMS,
> };
>
> #ifdef CONFIG_VM_EVENT_COUNTERS
> @@ -514,21 +515,13 @@ static inline const char *lru_list_name(enum lru_list lru)
> return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
> }
>
> -static inline const char *writeback_stat_name(enum writeback_stat_item item)
> -{
> - return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
> - NR_VM_NUMA_EVENT_ITEMS +
> - NR_VM_NODE_STAT_ITEMS +
> - item];
> -}
> -
> #if defined(CONFIG_VM_EVENT_COUNTERS) || defined(CONFIG_MEMCG)
> static inline const char *vm_event_name(enum vm_event_item item)
> {
> return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
> NR_VM_NUMA_EVENT_ITEMS +
> NR_VM_NODE_STAT_ITEMS +
> - NR_VM_WRITEBACK_STAT_ITEMS +
> + NR_VM_STAT_ITEMS +
> item];
> }
> #endif /* CONFIG_VM_EVENT_COUNTERS || CONFIG_MEMCG */
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index 04a1cb6cc636..6f8aa4766f16 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1257,7 +1257,7 @@ const char * const vmstat_text[] = {
> "pgdemote_khugepaged",
> "nr_memmap",
> "nr_memmap_boot",
> - /* enum writeback_stat_item counters */
> + /* system-wide enum vm_stat_item counters */
> "nr_dirty_threshold",
> "nr_dirty_background_threshold",
>
> @@ -1790,7 +1790,7 @@ static const struct seq_operations zoneinfo_op = {
> #define NR_VMSTAT_ITEMS (NR_VM_ZONE_STAT_ITEMS + \
> NR_VM_NUMA_EVENT_ITEMS + \
> NR_VM_NODE_STAT_ITEMS + \
> - NR_VM_WRITEBACK_STAT_ITEMS + \
> + NR_VM_STAT_ITEMS + \
> (IS_ENABLED(CONFIG_VM_EVENT_COUNTERS) ? \
> NR_VM_EVENT_ITEMS : 0))
>
> @@ -1827,7 +1827,7 @@ static void *vmstat_start(struct seq_file *m, loff_t *pos)
>
> global_dirty_limits(v + NR_DIRTY_BG_THRESHOLD,
> v + NR_DIRTY_THRESHOLD);
> - v += NR_VM_WRITEBACK_STAT_ITEMS;
> + v += NR_VM_STAT_ITEMS;
>
> #ifdef CONFIG_VM_EVENT_COUNTERS
> all_vm_events(v);

--
Cheers,

David / dhildenb