On Thu, Jan 27, 2022 at 03:47:40PM +0100, Michal Hocko wrote:
>[...]
>> > +
>> > + /* 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);
free_area_init_memoryless_node() seems to be defined used out side
page_alloc.c? It just call free_area_init_node() directly. We want to use the
name to reflect the effect?
>> > + /*
>> > + * 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
>> > + */
Hmm... which following step would mark the node online? On x86, the node is
onlined in alloc_node_date(). This is not onlined here.
>> > + 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.
I agree with Mike.
>--
>Michal Hocko
>SUSE Labs
--
Wei Yang
Help you, Help me
