Re: [PATCH v3] mm/vmscan: fix demotion targets checks in reclaim/demotion

From: Bing Jiao

Date: Fri Dec 26 2025 - 13:58:59 EST


On Wed, Dec 24, 2025 at 09:49:38AM +0800, Chen Ridong wrote:
> > +nodemask_t cpuset_node_get_allowed(struct cgroup *cgroup)
> > {
>
> Could we define it as:
>
> void cpuset_node_get_allowed(struct cgroup *cgroup, nodemask_t *node)
>
> to align with the naming style of node_get_allowed_targets?
>
> > -bool mem_cgroup_node_allowed(struct mem_cgroup *memcg, int nid)
> > +nodemask_t mem_cgroup_node_get_allowed(struct mem_cgroup *memcg)
>
> void mem_cgroup_node_get_allowed(struct mem_cgroup *memcg, nodemask_t *node)

Thanks for the suggestion. Pass a pointer is better.

Also, Gregory mentioned that the stack size may be an issue if
systems have many nodes.
Do you think it is better to use mem_cgroup_node_filter_allowed()
to keep the stack size smaller?

> > - demotion_nid = next_demotion_node(nid);
> > - if (demotion_nid == NUMA_NO_NODE)
> > + node_get_allowed_targets(pgdat, &allowed_mask);
> > + if (nodes_empty(allowed_mask))
> > + return false;

This is a fast-fail path. When the queried node is the farthest node,
allowed_mask will be empty. Thus, I would like to keep this check
before mem_cgroup_node_get_allowed().

> > +
> > + allowed_mems = mem_cgroup_node_get_allowed(memcg);
> > + nodes_and(allowed_mask, allowed_mask, allowed_mems);
> > + if (nodes_empty(allowed_mask))
> > return false;
> >
> node_get_allowed_targets(pgdat, &allowed_mask);
> mem_cgroup_node_get_allowed(memcg, allowed_mems);
> if (!nodes_intersects(allowed_mask, allowed_mems))
> return false;
>
> Would it look better?

Yes, nodes_intersects() is better than logic-and.
Will update in v3.

Best,
Bing