lkml.org 
[lkml]   [2018]   [Feb]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v3 4/4] mm/memory_hotplug: optimize memory hotplug
> 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

\
 
 \ /
  Last update: 2018-02-15 14:47    [W:0.098 / U:0.684 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site