Re: [PATCH 1/4] mm/numa: introduce nearest_nodes_nodemask()

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The function fails silently if the input and output nodemasks alias, wiping the input.
- [Low] The non-NUMA fallback implementation incorrectly reports an empty mask for valid inputs.
--

--- Patch [1]: [PATCH 1/4] mm/numa: introduce nearest_nodes_nodemask() ---
commit 5ec50dc5c5bbc33fbae64a16296aa9f2dc086600
Author: Rakie Kim <[email protected]>

mm/numa: introduce nearest_nodes_nodemask()

This commit introduces nearest_nodes_nodemask(), a helper function that
finds all candidate nodes in a given nodemask located at the minimum
distance from a source node. It updates an output nodemask with the complete
set of nearest nodes, returning 0 on success.

> diff --git a/include/linux/numa.h b/include/linux/numa.h
> index e6baaf6051bcf..4f2a0c344122a 100644
> --- a/include/linux/numa.h
> +++ b/include/linux/numa.h
[ ... ]
> @@ -54,6 +56,15 @@ static inline int nearest_node_nodemask(int node, nodemask_t *mask)
>  	return NUMA_NO_NODE;
>  }
>  
> +static inline int nearest_nodes_nodemask(int node, const nodemask_t *mask,
> +					 nodemask_t *out)
> +{
> +	if (!out)
> +		return -EINVAL;
> +	nodes_clear(*out);

[Severity: Low]
On systems with CONFIG_NUMA disabled, there is only a single implicit node
(Node 0). If a caller passes an input mask containing Node 0, that node is
trivially the nearest.

Should this fallback stub preserve the valid node from the input mask
instead of unconditionally clearing the output mask and falsely reporting no
nearest nodes?

> +	return 0;
> +}
> +
>  static inline int memory_add_physaddr_to_nid(u64 start)
>  {
>  	return 0;
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 4e4421b22b59f..19417b0afc30b 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
[ ... ]
> +int nearest_nodes_nodemask(int node, const nodemask_t *mask, nodemask_t *out)
> +{
> +	int dist, n, min_dist = INT_MAX;
> +
> +	if (!out)
> +		return -EINVAL;
> +
> +	nodes_clear(*out);

[Severity: Medium]
What happens if a caller attempts to filter a nodemask in-place by passing
the same pointer for both the input and output masks?

In-place filtering is a common pattern in the kernel (for example, with
nodes_and()), but if nearest_nodes_nodemask(node, &my_mask, &my_mask) is
called, this initial nodes_clear(*out) will wipe out the input mask before
the loop even begins iterating.

> +
> +	for_each_node_mask(n, *mask) {
> +		dist = node_distance(node, n);
> +
> +		if (dist < min_dist) {
> +			min_dist = dist;
> +			nodes_clear(*out);

[Severity: Medium]
Similarly, if the input and output pointers alias, clearing the output mask
dynamically here destroys the remaining iteration state for the for_each
loop.

Could this be avoided by using a temporary nodemask on the stack,
and copying it to *out at the end of the function?

> +			node_set(n, *out);
> +		} else if (dist == min_dist) {
> +			node_set(n, *out);
> +		}
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(nearest_nodes_nodemask);
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1