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

[Bing Jiao <bingjiao@google.com>]

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