On 07.12.21 17:36, Michal Hocko wrote:
> On Tue 07-12-21 17:27:29, Michal Hocko wrote:
> [...]
>> So your proposal is to drop set_node_online from the patch and add it as
>> a separate one which handles
>> - sysfs part (i.e. do not register a node which doesn't span a
>> physical address space)
>> - hotplug side of (drop the pgd allocation, register node lazily
>> when a first memblocks are registered)
>
> In other words, the first stage
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index c5952749ad40..f9024ba09c53 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6382,7 +6382,11 @@ static void __build_all_zonelists(void *data)
> if (self && !node_online(self->node_id)) {
> build_zonelists(self);
> } else {
> - for_each_online_node(nid) {
> + /*
> + * All possible nodes have pgdat preallocated
> + * free_area_init
> + */
> + for_each_node(nid) {
> pg_data_t *pgdat = NODE_DATA(nid);
>
> build_zonelists(pgdat);
> @@ -8032,8 +8036,24 @@ void __init free_area_init(unsigned long *max_zone_pfn)
> /* Initialise every node */
> mminit_verify_pageflags_layout();
> setup_nr_node_ids();
> - for_each_online_node(nid) {
> - pg_data_t *pgdat = NODE_DATA(nid);
> + for_each_node(nid) {
> + pg_data_t *pgdat;
> +
> + if (!node_online(nid)) {
> + pr_warn("Node %d uninitialized by the platform. Please report with boot dmesg.\n", nid);
> + pgdat = arch_alloc_nodedata(nid);
Is the buddy fully up an running at that point? I don't think so, so we
might have to allocate via memblock instead. But I might be wrong.
> + pgdat->per_cpu_nodestats = alloc_percpu(struct per_cpu_nodestat);
> + 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)
> + */
> + continue;
> + }
> +
> + pgdat = NODE_DATA(nid);
> free_area_init_node(nid);
>
> /* Any memory on that node */
>
Yes, and maybe in the same go, remove/rework hotadd_new_pgdat(), because
there is nothing to hotadd anymore. (we should double-check the
initialization performed in there, it might all not be necessary anymore)
--
Thanks,
David / dhildenb
