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

[Gregory Price <gourry@gourry.net>]

On Mon, Jan 05, 2026 at 05:01:52AM +0000, Bing Jiao wrote:
... snip ...
> +/**
> + * cpuset_nodes_allowed - return mems_allowed mask from a cgroup cpuset.
> + * @cgroup: pointer to struct cgroup.
> + * @mask: pointer to struct nodemask_t to be returned.
> + *
> + * Returns mems_allowed mask from a cgroup cpuset if it is cgroup v2 and
> + * has cpuset subsys. Otherwise, returns node_states[N_MEMORY].
> + *
> + * Returned @mask may be empty, and nodes in @mask are not guaranteed
> + * to be online.
> + **/
> +void cpuset_nodes_allowed(struct cgroup *cgroup, nodemask_t *mask)
> +void cpuset_nodes_allowed(struct cgroup *cgroup, nodemask_t *mask)
>  {
... snip ...
>  	/*
>  	 * Normally, accessing effective_mems would require the cpuset_mutex
> -	 * or callback_lock - but node_isset is atomic and the reference
> +	 * or callback_lock - but not doing so is acceptable and the reference


"node_isset is atomic" is an argument that not taking cpuset_mutex is
acceptable since it's a singular operation against a nodemask (one bit
it checked) - and therefore for a moment in time the node is either
allowed or not (and we make no absolute guarantee of corrected when this
race occurs, we just note that we're corrected).

nodes_copy is not atomic, and in fact this can result in returning an
empty nodemask if cs->effective_mems is being recalculated at the time
this copy occurs.

Rather than just saying "not doing so is acceptable" - can you please
change this comment to explain the implications of not acquiring the
mutex a little more clearly?

Example:
```
We do not acquire cpuset_mutex during this check because the correctness
of this information is stale immediately after the query anyway - this
saves lock contention in exchange for racing against mems_allowed rebinds.

As a result, @mask may be empty because cs->effective_mems can be rebound
during this call.  Callers must check the mask for validity on return.
```

The rest of the comments in the function explains a about this, but I
think with this update the comments need a little more rework.

~Gregory