Re: [PATCH v5 20/36] mm/memcontrol: account private-node memory in per-node stats

[email protected]
Newsgroups dev.linux.lists.damon,dev.linux.lists.nvdimm,org.kernel.vger.kvm,org.kernel.vger.linux-cxl
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The patch updates `mem_cgroup_css_rstat_flush` to include `N_MEMORY_PRIVATE` nodes, but misses `flush_nmi_stats()` which still strictly iterates over `N_MEMORY`, leaking NMI slab stats on private nodes.
- [Medium] `hugetlb_cgroup_read_numa_stat()` in the hugetlb cgroup controller skips `N_MEMORY_PRIVATE` nodes, leading to hidden node memory and incorrect hierarchical totals.
--

--- Patch [20]: [PATCH v5 20/36] mm/memcontrol: account private-node memory in per-node stats ---
commit 923e253e1d279f965b728ad3baafd46c0f7044eb
Author: Gregory Price <[email protected]>

mm/memcontrol: account private-node memory in per-node stats

Private nodes folios are charged like any other - the node's per-cpu
lruvec counters are increment even for N_MEMORY_PRIVATE.

[ ... ]

> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index f0dde52dc9e0e..4d79e238bc57d 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4496,6 +4496,7 @@ static void mem_cgroup_css_rstat_flush(struct cgroup_subsys_state *css, int cpu)
>  	struct mem_cgroup *parent = parent_mem_cgroup(memcg);
>  	struct memcg_vmstats_percpu *statc;
>  	struct aggregate_control ac;
> +	nodemask_t reportable;
>  	int nid;
>  
>  	flush_nmi_stats(memcg, parent);

[Severity: Medium]
Will this still leak NMI slab stats on private nodes?

flush_nmi_stats() still strictly iterates over N_MEMORY instead of including
N_MEMORY_PRIVATE.

If a slab allocation occurs in NMI context on an N_MEMORY_PRIVATE node, the
node-local atomic counter is incremented, but it appears flush_nmi_stats()
will fail to drain it because it only iterates over N_MEMORY.

[ ... ]

[Severity: Medium]
Does mm/hugetlb_cgroup.c:hugetlb_cgroup_read_numa_stat() also need to be
updated to include N_MEMORY_PRIVATE?

It currently only covers N_MEMORY when calculating and printing per-node
hierarchical usage:

	for_each_node_state(nid, N_MEMORY) {
		usage = 0;
		...

If hugetlb memory is allocated on an N_MEMORY_PRIVATE node and charged to a
cgroup, reading the cgroup's NUMA stat file would skip accumulating or
displaying this node's usage.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=20
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.