Re: [RFC PATCH 2/4] mm/hotplug: walk_memroy_range on memory_block uit

[John Hubbard <jhubbard@nvidia.com>]

On 06/26/2017 04:40 PM, Wei Yang wrote:
> On Mon, Jun 26, 2017 at 12:32:40AM -0700, John Hubbard wrote:
>> On 06/24/2017 07:52 PM, Wei Yang wrote:
[...]
>>
>> Why is it safe to assume no holes in the memory range? (Maybe Michal's 
>> patch already covered this and I haven't got that far yet?)
>>
>> The documentation for this routine says that it walks through all
>> present memory sections in the range, so it seems like this patch
>> breaks that.
>>
> 
> Hmm... it is a little bit hard to describe.
> 
> First the documentation of the function is a little misleading. When you look
> at the code, it call the "func" only once for a memory_block, not for every
> present mem_section as it says. So have some memory in the memory_block would
> meet the requirement.
> 
> Second, after the check in patch 1, it is for sure the range is memory_block
> aligned, which means it must have some memory in that memory_block. It would
> be strange if someone claim to add a memory range but with no real memory.
> 
> This is why I remove the check here.

OK. In that case, it seems like we should update the function documentation
to match. Something like this, maybe? :

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index bdaafcf46f49..d36b2f4eaf39 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1872,14 +1872,14 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages)
 #endif /* CONFIG_MEMORY_HOTREMOVE */
 
 /**
- * walk_memory_range - walks through all mem sections in [start_pfn, end_pfn)
+ * walk_memory_range - walks through all mem blocks in [start_pfn, end_pfn)
  * @start_pfn: start pfn of the memory range
  * @end_pfn: end pfn of the memory range
  * @arg: argument passed to func
- * @func: callback for each memory section walked
+ * @func: callback for each memory block walked
  *
- * This function walks through all present mem sections in range
- * [start_pfn, end_pfn) and call func on each mem section.
+ * This function walks through all mem blocks in the range
+ * [start_pfn, end_pfn) and calls func on each mem block.
  *
  * Returns the return value of func.
  */


thanks,
john h

> 
> 
>>>  
>>>  		section = __nr_to_section(section_nr);
>>> -		/* same memblock? */
>>> -		if (mem)
>>> -			if ((section_nr >= mem->start_section_nr) &&
>>> -			    (section_nr <= mem->end_section_nr))
>>> -				continue;
>>
>> Yes, that deletion looks good.
>>
> 
> From this we can see, if there IS some memory, the function will be invoked
> and only invoked once.
> 
>> thanks,
>> john h
>>
>>>  
>>>  		mem = find_memory_block_hinted(section, mem);
>>>  		if (!mem)
>>>
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>