Re: [PATCH v3 1/3] driver/base: Optimize memory block registration to reduce boot time

[David Hildenbrand]

On 05.05.25 09:57, Oscar Salvador wrote:
> On Mon, May 05, 2025 at 09:38:43AM +0200, David Hildenbrand wrote:
> > On 05.05.25 09:28, Oscar Salvador wrote:
> > > On Mon, May 05, 2025 at 09:16:48AM +0200, David Hildenbrand wrote:
> > > > memory hotplug code never calls register_one_node(), unless I am missing
> > > > something.
> > > >
> > > > During add_memory_resource(), we call __try_online_node(nid, false), meaning
> > > > we skip register_one_node().
> > > >
> > > > The only caller of __try_online_node(nid, true) is try_online_node(), called
> > > > from CPU hotplug code, and I *guess* that is not required.
> > >
> > > Well, I guess this is because we need to link the cpus to the node.
> > > register_one_node() has two jobs: 1) register cpus belonging to the node
> > > and 2) register memory-blocks belonging to the node (if any).
> >
> > Ah, via __register_one_node() ...
> >
> > I would assume that an offline node
> >
> > (1) has no memory
> > (2) has no CPUs
>
> That is right.
>
> > When we *hotplug* either memory or CPUs, and we first online the node, there
> > is nothing to register. Because if there would be something, the node would
> > already be online.
>
> I think I do not understand this, but let us imagine the following
> scenario:
>
> - You craft a VM with qemu that has a numa node which is memoryless and cpuless.
>    This node will be allocated in free_area_init()->alloc_offline_node_data() but
>    it will not be marked online because it does not have any resources.
>
>    Then if you create a cpu device and hotplug in there, this will
>    trigger try_online_node() from the cpu callback and go all the way to
>    __register_one_node() to link the hotplugged cpu to the corresponding node.

Assume you hotplug the second CPU. The node is already 
registered/online, so who does the register_cpu_under_node() call?

It's register_cpu() I guess? But no idea in which order that is called 
with node onlining.

The code has to be cleaned up such that onlining a node does not 
traverse any cpus / memory.

Whoever adds a CPU / memory *after onlining the node* must register the 
device manually under the *now online* node.

Maybe we have to adjust CPU hotplug code if the existing
register_cpu_under_node() is misplaced.

register_cpu_under_node() has this weird !node_online check in there ...

... it's a mess

>    Now, I do not see an issue with that.
>    The only think that makes me go "meh", is that register_one_node()
>    calls register_memory_blocks_under_node() blindly when there might not
>    be any memory block to register (I am sure we bail out if we see that
>    somewhere, but still feels kinda of "wrong"?

Yes


--
Cheers,

David / dhildenb