Re: [PATCH 2/3] slab: create barns for online memoryless nodes

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

On 3/19/26 08:01, Hao Li wrote:
> On Wed, Mar 18, 2026 at 01:11:58PM +0100, Vlastimil Babka (SUSE) wrote:
>> On 3/18/26 10:27, Hao Li wrote:
>> > On Wed, Mar 11, 2026 at 09:25:56AM +0100, Vlastimil Babka (SUSE) wrote:
>> > 
>> > I had a somewhat related question here.
>> > 
>> > During memory hotplug, we call node_set() on slab_nodes when memory is brought
>> > online, but we do not seem to call node_clear() when memory is taken offline. I
>> > was wondering what the reasoning behind this is.
>> 
>> Probably nobody took the task the implement the necessary teardown.
>> 
>> > That also made me wonder about a related case. If I am understanding this
>> > correctly, even if all memory of a node has been offlined, slab_nodes would
>> > still make it appear that the node has memory, even though in reality it no
>> > longer does. If so, then in patch 3, the condition
>> > "if (unlikely(!node_isset(numa_node, slab_nodes)))" in can_free_to_pcs() seems
>> > would cause the object free path to skip sheaves.
>> 
>> Maybe the condition should be looking at N_MEMORY then?
> 
> Yes, that's what I was thinking too.
> I feel that, at least for the current patchset, this is probably a reasonable
> approach.

Ack.

>> 
>> Also ideally we should be using N_NORMAL_MEMORY everywhere for slab_nodes.
>> Oh we actually did, but give that up in commit 1bf47d4195e45.
> 
> Thanks, I hadn't realized that node_clear had actually existed before.
> 
>> 
>> Note in practice full memory offline of a node can only be achieved if it
>> was all ZONE_MOVABLE and thus no slab allocations ever happened on it. But
>> if it has only movable memory, it's practically memoryless for slab
>> purposes.
> 
> That's a good point! I just realized that too.
> 
>> Maybe the condition should be looking at N_NORMAL_MEMORY then.
>> That would cover the case when it became offline and also the case when it's
>> online but with only movable memory?
> 
> Exactly, conceptually, N_NORMAL_MEMORY seems more precise than N_MEMORY. I took
> a quick look through the code, though, and it seems that N_NORMAL_MEMORY hasn't
> been fully handled in the hotplug code.

Huh you're right, the hotplug code doesn't seem to set it. How much code
that we have is broken by that?
It seems hotplug doesn't handle it since 2007 in commit 37b07e4163f7
("memoryless nodes: fixup uses of node_online_map in generic code"),
although the initial support in 7ea1530ab3fd ("Memoryless nodes: introduce
mask of nodes with memory") did set it from hotplug.

> Given that, I think it makes sense to use N_MEMORY for now, and then switch to
> N_NORMAL_MEMORY later once the handling there is improved.

So I'll do this:

diff --git a/mm/slub.c b/mm/slub.c
index 01ab90bb4622..fb2c5c57bc4e 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -6029,7 +6029,7 @@ static __always_inline bool can_free_to_pcs(struct
slab *slab)
         * point to the closest node as we would on a proper memoryless node
         * setup.
         */
-       if (unlikely(!node_isset(numa_node, slab_nodes)))
+       if (unlikely(!node_state(numa_node, N_MEMORY)))
                goto check_pfmemalloc;
 #endif


>> 
>> I don't know if with CONFIG_HAVE_MEMORYLESS_NODES it's possible that
>> numa_mem_id() (the closest node with memory) would be ZONE_MOVABLE only.
>> Maybe let's hope not, and not adjust that part?
>> 
> 
> I think that, in the CONFIG_HAVE_MEMORYLESS_NODES=y case, numa_mem_id() ends up
> calling local_memory_node(), and the NUMA node it returns should be one that
> can allocate slab memory. So the slab_node == numa_node check seems reasonable
> to me.
> 
> So it seems that the issue being discussed here may only be specific to the
> CONFIG_HAVE_MEMORYLESS_NODES=n case.

Great. Thanks!