Re: [PATCH] mm/memory-tiers: reset demotion targets of nodes without memory

[David Hildenbrand (Arm)]

On 10/19/26 22:22, Gregory Price wrote:
> On Mon, Oct 19, 2026 at 03:04:45PM -0400, Zi Yan wrote:
>> On Mon Oct 19, 2026 at 2:47 PM EDT, Gregory Price (Meta) wrote:
>>> When a node loses its last memory, offline_pages() clears N_MEMORY
>>> before the memory-tier callback rebuilds the demotion targets.
>>> disable_all_demotion_targets() resets only N_MEMORY nodes, so the node
>>> keeps its old node_demotion[] entry while it has no memory. Before
>>> 6c542ab75714, the reset covered every online node.
>>
>> establish_demotion_targets() uses for_each_node_state(node, N_MEMORY),
>> because the to-be-online node is already added into N_MEMORY?
>>
> 
> NODE_ADDED_FIRST_MEMORY happens after the memory is already added
> (though not necessarily online). 
> 
> NODE_REMOVED_LAST_MEMORY happens after the last memory block is removed
> (and therefore N_MEMORY is gone).
> 
> slightly different timing for each event.  There isn't a reasonable
> change we can make otherwise.
> 
>>>
>>> A reclaimer reads the stale entry only between the node regaining memory
>>> and its targets being rebuilt, and it filters the entry against the
>>> current allowed targets, so there is no user-visible effect. To see it,
>>> online and then offline a node's only DIMM: a debugger still finds the
>>> lower-tier node in node_demotion[nid].preferred.
>>>
>>> Reset the targets of every node.
>>>
>>> Fixes: 6c542ab75714 ("mm/demotion: build demotion targets based on explicit memory tiers")
>>> Signed-off-by: Gregory Price (Meta) <gourry@xxxxxxxxxx>
>>> ---
>>>  mm/memory-tiers.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c
>>> index 25e121851b586..7197f1a67a340 100644
>>> --- a/mm/memory-tiers.c
>>> +++ b/mm/memory-tiers.c
>>> @@ -378,7 +378,7 @@ static void disable_all_demotion_targets(void)
>>>  	struct memory_tier *memtier;
>>>  	int node;
>>>  
>>> -	for_each_node_state(node, N_MEMORY) {
>>> +	for_each_node(node) {
>>
>> I think the above code deserves a comment explaining why for_each_node()
>> is needed instead of for_each_node_state(N_MEMORY), because when a node
>> is added or removed during memory hotadd or hotremove matters here.
>>
> 
> Sure.  Will respin tomorrow and give others a chance to look at it.

A comment makes sense to me as well, it's not completely obvious.

Apart from that LGTM.

-- 
Cheers,

David