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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aUs_pLTcsVK8zf0g@gourry-fedora-PF4VCD3F>
Date: Tue, 23 Dec 2025 20:19:32 -0500
From: Gregory Price <gourry@...rry.net>
To: Bing Jiao <bingjiao@...gle.com>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org, longman@...hat.com, hannes@...xchg.org,
	mhocko@...nel.org, roman.gushchin@...ux.dev, shakeel.butt@...ux.dev,
	muchun.song@...ux.dev, tj@...nel.org, mkoutny@...e.com,
	david@...nel.org, zhengqi.arch@...edance.com,
	lorenzo.stoakes@...cle.com, axelrasmussen@...gle.com,
	chenridong@...weicloud.com, yuanchu@...gle.com, weixugc@...gle.com,
	cgroups@...r.kernel.org
Subject: Re: [PATCH v3] mm/vmscan: fix demotion targets checks in
 reclaim/demotion

On Tue, Dec 23, 2025 at 09:19:59PM +0000, Bing Jiao wrote:
> -static inline bool cpuset_node_allowed(struct cgroup *cgroup, int nid)
> +static inline nodemask_t cpuset_node_get_allowed(struct cgroup *cgroup)
>  {
> -	return true;
> +	return node_possible_map;
...
> -static inline bool mem_cgroup_node_allowed(struct mem_cgroup *memcg, int nid)
> +static inline nodemask_t mem_cgroup_node_get_allowed(struct mem_cgroup *memcg)
>  {
> -	return true;
> +	return node_possible_map;
>  }
...
> -bool cpuset_node_allowed(struct cgroup *cgroup, int nid)
> +nodemask_t cpuset_node_get_allowed(struct cgroup *cgroup)
>  {
> +	nodemask_t nodes = node_possible_map;
...
> -bool mem_cgroup_node_allowed(struct mem_cgroup *memcg, int nid)
> +nodemask_t mem_cgroup_node_get_allowed(struct mem_cgroup *memcg)
>  {
> -	return memcg ? cpuset_node_allowed(memcg->css.cgroup, nid) : true;
> +	if (memcg)
> +		return cpuset_node_get_allowed(memcg->css.cgroup);
> +	return node_possible_map;
>  }


node_possible_map or node_states[N_MEMORY]?

The latter seems more appropriate to me since node_possible_map will
include offline nodes.

> -	allowed = node_isset(nid, cs->effective_mems);
> +	nodes_copy(nodes, cs->effective_mems);
>  	css_put(css);
> -	return allowed;
> +	return nodes;
>  }

I saw in vmscan you check for returning an empty nodemask, may want to
at least add a comment to the function definition that says this needs
to be checked.

> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index a4b308a2f9ad..711a04baf258 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -345,18 +345,24 @@ static bool can_demote(int nid, struct scan_control *sc,
>  		       struct mem_cgroup *memcg)
>  {
>  	int demotion_nid;
> +	struct pglist_data *pgdat = NODE_DATA(nid);
> +	nodemask_t allowed_mask, allowed_mems;

Only other concern i have is the number of nodemasks being added to the
stack.  Should be <512 bytes, but I've run into situations where builds
have screamed at me for adding one nodemask to the stack, let alone 3+.

Have you run this through klp?

~Gregory