> This should be a separate patch IMHO. It is an optimization on its
> own. The original code tries to be sparse neutral but we do depend on
> sparse anyway.
Yes, Mingo already asked me to split this patch. I've done just that
and will send it out soon.
>
> [...]
>> /* register memory section under specified node if it spans that node */
>> -int register_mem_sect_under_node(struct memory_block *mem_blk, int nid)
>> +int register_mem_sect_under_node(struct memory_block *mem_blk, int nid,
>> + bool check_nid)
>
> This check_nid begs for a documentation. When do we need to set it? I
> can see that register_new_memory path doesn't check node id. It is quite
> reasonable to expect that a new memblock doesn't span multiple numa
> nodes which can be the case for register_one_node but a word or two are
> really due.
OK, I will add a comment, and BTW, this is also going to be a separate
patch for ease of review.
>
>> {
>> int ret;
>> unsigned long pfn, sect_start_pfn, sect_end_pfn;
>> @@ -423,11 +424,13 @@ int register_mem_sect_under_node(struct memory_block *mem_blk, int nid)
>> continue;
>> }
>>
>> - page_nid = get_nid_for_pfn(pfn);
>> - if (page_nid < 0)
>> - continue;
>> - if (page_nid != nid)
>> - continue;
>> + if (check_nid) {
>> + page_nid = get_nid_for_pfn(pfn);
>> + if (page_nid < 0)
>> + continue;
>> + if (page_nid != nid)
>> + continue;
>> + }
>> ret = sysfs_create_link_nowarn(&node_devices[nid]->dev.kobj,
>> &mem_blk->dev.kobj,
>> kobject_name(&mem_blk->dev.kobj));
>> @@ -502,7 +505,7 @@ int link_mem_sections(int nid, unsigned long start_pfn, unsigned long nr_pages)
>>
>> mem_blk = find_memory_block_hinted(mem_sect, mem_blk);
>>
>> - ret = register_mem_sect_under_node(mem_blk, nid);
>> + ret = register_mem_sect_under_node(mem_blk, nid, true);
>> if (!err)
>> err = ret;
>>
>
> I would be tempted to split this into a separate patch as well. The
> review will be much easier.
Yes, but that would be the last patch in the series.
> This is quite ugly. You allocate 256MB for small numa systems and 512MB
> for larger NUMAs unconditionally for MEMORY_HOTPLUG. I see you need it
> to safely replace page_to_nid by get_section_nid but this is just too
> high of the price. Please note that this shouldn't be really needed. At
> least not for onlining. We already _do_ know the node association with
> the pfn range. So we should be able to get the nid from memblock.
OK, I will think for a different place to store nid temporarily, or
how to get it.
Thank you,
Pavel
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
