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

[John Hubbard <jhubbard@nvidia.com>]

On 06/24/2017 07:52 PM, Wei Yang wrote:
> hotplug memory range is memory_block aligned and walk_memroy_range guarded
> with check_hotplug_memory_range(). This is save to iterate on the
> memory_block base.> 
> This patch adjust the iteration unit and assume there is not hole in
> hotplug memory range.

Hi Wei,

In the patch subject, s/memroy/memory/ , and s/uit/unit/, and
s/save/safe.

Actually, I still have a tough time with it, so maybe the 
description above could instead be worded approximately
like this:

Given that a hotpluggable memory range is now block-aligned,
it is safe for walk_memory_range to iterate by blocks.

Change walk_memory_range() so that it iterates at block
boundaries, rather than section boundaries. Also, skip the check
for whether pages are present in the section, and assume 
that there are no holes in the range. (<Insert reason why 
that is safe, here>)


> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> ---
>  mm/memory_hotplug.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index f5d06afc8645..a79a83ec965f 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1858,17 +1858,11 @@ int walk_memory_range(unsigned long start_pfn, unsigned long end_pfn,
>  	unsigned long pfn, section_nr;
>  	int ret;
>  
> -	for (pfn = start_pfn; pfn < end_pfn; pfn += PAGES_PER_SECTION) {
> +	for (pfn = start_pfn; pfn < end_pfn;
> +		pfn += PAGES_PER_SECTION * sections_per_block) {

Here, and in one or two other spots in the patch, it would be nice
to repeat your approach from patch 0001, where you introduced a
pages_per_block variable. That definitely helps when reading the code.

>  		section_nr = pfn_to_section_nr(pfn);
> -		if (!present_section_nr(section_nr))
> -			continue;

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.

>  
>  		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.

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>