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

[Hao Li]

On Thu, Mar 19, 2026 at 10:56:09AM +0100, Vlastimil Babka (SUSE) wrote:
> 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?

This probably needs a bit more digging.

> 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.

Yes, this really is quite an old issue. It looks like we may need to dig
through the git history a bit more carefully.

I'd be happy to dig into it further.

> 
> > 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)))

Looks good to me.

I've gone through the full series, including the range-diff updates, and the
rest looks good to me.
Feel free to add my rb-tag to three updated patches. Thanks!

Reviewed-by: Hao Li <hao.li@xxxxxxxxx>

>                 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!
>