Re: [PATCH v5 07/12] mm/mmzone: read each zoneref slot once when filtering by nodemask

["Vlastimil Babka (SUSE)" <vbabka@kernel.org>]

On 11/3/26 04:01, Zi Yan wrote:
> On Sat Oct 31, 2026 at 10:13 PM EDT, Gregory Price (Meta) wrote:
>> __next_zones_zonelist() tests a slot's zone for NULL, then reads the
>> slot again to look up the zone's node. Offlining a zone's last block
>> rebuilds the zonelists in place, so the second read can return the
>> terminator:
>>
>>   CPU 0                             CPU 1
>>   __next_zones_zonelist(nodemask)
>>     zonelist_zone(slot 1) = NORMAL
>>                                     offline_pages()
>>                                       build_all_zonelists()
>>                                         [MOVABLE, NORMAL, NULL]
>>                                         -> [NORMAL, NULL]
>>     zref_in_nodemask(slot 1)
>>       zone_to_nid(NULL)
>>
>> Current GCC and clang merge the two reads, but nothing requires it.
>> With mm/mmzone.o built with -fno-inline -fno-ipa-sra, holding a
>> nodemask allocation between the reads while the node's last movable
>> block goes offline oopses every time:
>>
>>   BUG: kernel NULL pointer dereference, address: 0000000000000060
>>   RIP: 0010:zone_to_nid+0x0/0x10
>>   Call Trace:
>>    zref_in_nodemask+0x9/0x20
>>    __next_zones_zonelist+0x23/0x70
>>    prepare_alloc_pages.constprop.0+0x16c/0x190
>>    __alloc_frozen_pages_noprof+0xaf/0x330
>>
>> Read the slot once and test that zone. A NULL zone still ends the walk.
> 
> It is better to spell out the rename from zref_in_nodemask() to
> zone_in_nodemask().

Added:

To achieve that, rename zref_in_nodemask() to zone_in_nodemask() and
pass a zone instead of zoneref pointer.

> 
>>
>> Fixes: 11cd8638c37f ("mm, page_alloc: remove stop_machine from build_all_zonelists")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Gregory Price (Meta) <gourry@gourry.net>
>> Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
>> ---
>>  mm/mmzone.c | 11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
> 
> Makes sense.
> 
> Reviewed-by: Zi Yan <ziy@nvidia.com>
> 
>