On Thu 27-01-22 15:37:23, Mike Rapoport wrote:
> On Thu, Jan 27, 2022 at 09:53:01AM +0100, Michal Hocko wrote:
[...]
> > + if (!node_online(nid)) {
> > + pr_warn("Node %d uninitialized by the platform. Please report with boot dmesg.\n", nid);
>
> Do we assume that platform code must allocate node data for all nodes in
> the system? Because if we don't this warning is misleading.
At least x86 does that (init_cpu_to_node). Now that you brought that up
I guess you are right that this could be more misleading than helpful.
What about
pr_info("Initializing node %d as memoryless\n", nid);
Is this better?
> > +
> > + /* Allocator not initialized yet */
> > + pgdat = arch_alloc_nodedata(nid);
> > + if (!pgdat) {
> > + pr_err("Cannot allocate %zuB for node %d.\n",
> > + sizeof(*pgdat), nid);
> > + continue;
> > + }
> > + arch_refresh_nodedata(nid, pgdat);
> > + free_area_init_memoryless_node(nid);
> > + /*
> > + * not marking this node online because we do not want to
> > + * confuse userspace by sysfs files/directories for node
> > + * without any memory attached to it (see topology_init)
> > + * The pgdat will get fully initialized when a memory is
> > + * hotpluged into it by hotadd_init_pgdat
> > + */
> > + continue;
>
> This can be made slightly more concise if we fall through after
> arch_refresh_nodedata(), e.g. something like
>
> ...
>
> arch_refresh_nodedata(nid, pgdat);
> }
>
> pgdat = NODE_DATA(nid);
> free_area_init_node(nid);
>
> /*
> * Do not mark memoryless node online because we do not want to
> * confuse userspace by sysfs files/directories for node
> * without any memory attached to it (see topology_init)
> * The pgdat will get fully initialized when a memory is
> * hotpluged into it by hotadd_init_pgdat
> */
> if (!pgdat->node_present_pages)
> continue;
>
> but I don't feel strongly about it.
I do not have strong preference either way. Unless this is considered
better by more people I would stick with what I have.
--
Michal Hocko
SUSE Labs
