Re: [PATCH v4 8/8] mm/mempolicy: read the zoneref slot once in mempolicy_slab_node()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm/mempolicy: missing READ_ONCE() on zoneref load in mempolicy_slab_node()
--

commit 4beeef01124c3abac1f730eb599686961d03563c
Author: Gregory Price (Meta) <gourry@gourry.net>

mm/mempolicy: read the zoneref slot once in mempolicy_slab_node()

This commit updates mempolicy_slab_node() to store the zone pointer from
zonelist_zone(z) in a local variable before passing it to zone_to_nid(),
aiming to avoid reading the zoneref slot twice when zonelists are rebuilt in
place during memory offlining.

> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 0884c629670dd..cd63c3e2c029c 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2176,6 +2176,7 @@ unsigned int mempolicy_slab_node(void)
>  	case MPOL_PREFERRED_MANY:
>  	{
>  		struct zoneref *z;
> +		struct zone *zone;
>  
>  		/*
>  		 * Follow bind policy behavior and start allocation at the
> @@ -2186,7 +2187,8 @@ unsigned int mempolicy_slab_node(void)
>  		zonelist = &NODE_DATA(node)->node_zonelists[ZONELIST_FALLBACK];
>  		z = first_zones_zonelist(zonelist, highest_zoneidx,
>  							&policy->nodes);
> -		return zonelist_zone(z) ? zonelist_node_idx(z) : node;
> +		zone = zonelist_zone(z);
> +		return zone ? zone_to_nid(zone) : node;

[Severity: Medium]
Does storing the result of zonelist_zone(z) in a local variable guarantee a
single load of z->zone without READ_ONCE()?

In include/linux/mmzone.h, both zonelist_zone() and zone_to_nid() are static
inline helpers performing plain C loads:

static inline struct zone *zonelist_zone(struct zoneref *zoneref)
{
	return zoneref->zone;
}

When a task with an MPOL_BIND or MPOL_PREFERRED_MANY mempolicy calls
kmem_cache_alloc_noprof() -> mempolicy_slab_node() concurrently with
offline_pages() -> build_all_zonelists() rewriting the zonelist in place,
inlining zonelist_zone() and zone_to_nid() lowers this to a plain load of
z->zone. Without READ_ONCE(), this remains an unmarked data race, and the
compiler is still permitted to reload z->zone from memory and pass NULL to
zone_to_nid().

Earlier commits in this series ("mm/page_alloc: snapshot the preferred zone
against shrink rebuilds" and "mm/mmzone: read each zoneref slot once when
filtering by nodemask") used READ_ONCE(ac->preferred_zoneref->zone) and
READ_ONCE(z->zone) to enforce a single load. Should mempolicy_slab_node()
(or zonelist_zone() itself) use READ_ONCE(z->zone) here as well?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028211930.3504634-1-gourry@gourry.net?part=8