Re: [PATCH v2 2/4] mm: memcg: read root lruvec stats from the node counters

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] mm: memcg: node_stat_includes_unaccounted() omits file and
  writeback stats
--

commit d2ba314724a8ba015e3bf2a30f155f1beba25a8c
Author: Joanne Koong <joannelkoong@gmail.com>

mm: memcg: read root lruvec stats from the node counters

This commit changes lruvec_page_state() and lruvec_page_state_monotonic()
to read per-node vmstat counters instead of rstat values for the root
memcg when querying stats that only track memory charged to a memcg.

> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 1ebc9330aeae5..2575bf2ba0ea6 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c

[ ... ]

> @@ -504,13 +511,52 @@ struct lruvec_stats {
>  	long state_pending[NR_MEMCG_NODE_STAT_ITEMS];
>  };
>  
> +/*
> + * Whether the node counter for @idx can also count memory that isn't charged
> + * to a memcg (eg kernel memory that isn't accounted). For these stats, the
> + * node counter can be larger than the root memcg's value, so the root memcg
> + * has to read its rstat value instead.
> + */
> +static bool node_stat_includes_unaccounted(enum node_stat_item idx)
> +{
> +	switch (idx) {
> +	case NR_SLAB_RECLAIMABLE_B:
> +	case NR_SLAB_UNRECLAIMABLE_B:
> +	case WORKINGSET_NODERECLAIM:
> +	case NR_VMALLOC:
> +	case NR_KERNEL_STACK_KB:
> +	case NR_PAGETABLE:
> +	case NR_SECONDARY_PAGETABLE:
> +#ifdef CONFIG_HUGETLB_PAGE
> +	case NR_HUGETLB:
> +#endif
> +		return true;
> +	default:
> +		return false;
> +	}
> +}

[Severity: Medium]
Should node_stat_includes_unaccounted() also include NR_FILE_PAGES,
NR_FILE_MAPPED, and NR_WRITEBACK?

For NR_FILE_PAGES, swapcache operations such as __swap_cache_add_folio(),
__swap_cache_del_folio(), and __swap_cache_alloc() update NR_FILE_PAGES
only in the node counter while accounting swapcache in the lruvec under
NR_SWAPCACHE:

mm/swap_state.c:__swap_cache_add_folio() {
	...
	node_stat_mod_folio(folio, NR_FILE_PAGES, nr_pages);
	lruvec_stat_mod_folio(folio, NR_SWAPCACHE, nr_pages);
	...
}

Reading node_page_state(pgdat, NR_FILE_PAGES) for the root lruvec
therefore includes all swapcache pages, which inflates the "file" counter
in the root cgroup's memory.numa_stat (double-counting swapcache alongside
"swapcached") and causes it to diverge from memory.stat even after a
flush.

Similarly, __folio_mod_stat() -> lruvec_stat_mod_folio() updates the node
NR_FILE_MAPPED counter without updating memcg rstat whenever an uncharged
folio (folio_memcg(folio) == NULL, such as a driver mapping or a
root-cgroup __GFP_ACCOUNT kernel folio) is mapped into userspace, and
nfs_folio_mark_unstable() and nfs_folio_clear_commit() update
NR_WRITEBACK only in the node counter for unstable NFS pages awaiting
server COMMIT.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110015321.611093-1-joannelkoong@gmail.com?part=2