Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1331178

Re: [PATCH v2 3/3] mm/compaction: speed up pageblock_pfn_to_page() when zone is contiguous

From Vlastimil Babka <vbabka@suse.cz>
Newsgroups linux.kernel
Subject Re: [PATCH v2 3/3] mm/compaction: speed up pageblock_pfn_to_page() when zone is contiguous
Date 2016-02-10 14:50 +0100
Message-ID <r0DjA-2QB-15@gated-at.bofh.it> (permalink)
References (1 earlier) <qYlqO-4PS-17@gated-at.bofh.it> <qYCL1-18W-21@gated-at.bofh.it> <qYRgZ-2zl-1@gated-at.bofh.it> <r0kJZ-7oL-21@gated-at.bofh.it> <r0nyb-U6-17@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 02/09/2016 09:53 PM, Andrew Morton wrote:
> On Tue, 9 Feb 2016 18:58:32 +0100 Vlastimil Babka <vbabka@suse.cz> wrote:
> 
>> On 02/05/2016 05:11 PM, Joonsoo Kim wrote:
>>> Yeah, it seems wrong to me. :)
>>> Here goes fix.
>>
>> Doesn't apply for me, even after fixing the most obvious line wraps.
>> Seems like the version in mmotm is still your original patch and
>> Andrew's hotfix?
> 
> Yes, that patch was hopelessly mailer-mangled.  I painstakingly fixed
> it up and generated the incremental:

Thanks a lot. My review of the final patch also involved pain (due to
the cold, not the patch!).

You can take my Acked-by, but I also find the definitions of
set_zone_contiguous/clear_zone_contiguous() "in the header of the
consumer" (hotplug) somewhat unusual. It works, but e.g. mm/internal.h
would be more expected.

Then there's this:

> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -509,6 +509,8 @@ int __ref __add_pages(int nid, struct zone *zone, unsigned long phys_start_pfn,
>  	int start_sec, end_sec;
>  	struct vmem_altmap *altmap;
>  
> +	clear_zone_contiguous(zone);
> +
>  	/* during initialize mem_map, align hot-added range to section */
>  	start_sec = pfn_to_section_nr(phys_start_pfn);
>  	end_sec = pfn_to_section_nr(phys_start_pfn + nr_pages - 1);
> @@ -540,6 +542,8 @@ int __ref __add_pages(int nid, struct zone *zone, unsigned long phys_start_pfn,
>  	}
>  	vmemmap_populate_print_last();
>  
> +	set_zone_contiguous(zone);
> +
>  	return err;
>  }
>  EXPORT_SYMBOL_GPL(__add_pages);

Between the clear and set, __add_pages() might return with -EINVAL,
leaving the flag cleared potentially forever. Not critical, probably
rare, but it should be possible to avoid this by moving the clear below
the altmap check?

Thanks,
Vlastimil

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH v2 3/3] mm/compaction: speed up pageblock_pfn_to_page()  when zone is contiguous Vlastimil Babka <vbabka@suse.cz> - 2016-02-09 19:00 +0100
  Re: [PATCH v2 3/3] mm/compaction: speed up pageblock_pfn_to_page()  when zone is contiguous Andrew Morton <akpm@linux-foundation.org> - 2016-02-09 22:00 +0100
    Re: [PATCH v2 3/3] mm/compaction: speed up pageblock_pfn_to_page()  when zone is contiguous Vlastimil Babka <vbabka@suse.cz> - 2016-02-10 14:50 +0100
      Re: [PATCH v2 3/3] mm/compaction: speed up pageblock_pfn_to_page()  when zone is contiguous Andrew Morton <akpm@linux-foundation.org> - 2016-02-10 20:00 +0100
        Re: [PATCH v2 3/3] mm/compaction: speed up pageblock_pfn_to_page()  when zone is contiguous Joonsoo Kim <js1304@gmail.com> - 2016-02-11 03:00 +0100

csiph-web