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

[Bing Jiao <bingjiao@google.com>]

On Mon, Jan 05, 2026 at 10:54:05AM -0500, Gregory Price wrote:
> 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

Thanks for the suggestions. I will reword the comment in V6.

Best,
Bing