From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751730AbcBJNnC (ORCPT ); Wed, 10 Feb 2016 08:43:02 -0500 Received: from mx2.suse.de ([195.135.220.15]:50549 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751289AbcBJNnA (ORCPT ); Wed, 10 Feb 2016 08:43:00 -0500 Subject: Re: [PATCH v2 3/3] mm/compaction: speed up pageblock_pfn_to_page() when zone is contiguous To: Andrew Morton References: <1454566775-30973-1-git-send-email-iamjoonsoo.kim@lge.com> <1454566775-30973-3-git-send-email-iamjoonsoo.kim@lge.com> <20160204164929.a2f12b8a7edcdfa596abd850@linux-foundation.org> <56BA28C8.3060903@suse.cz> <20160209125301.c7e6067558c321cfb87602b5@linux-foundation.org> Cc: Joonsoo Kim , Aaron Lu , Mel Gorman , Rik van Riel , David Rientjes , LKML , Linux Memory Management List , Joonsoo Kim From: Vlastimil Babka Message-ID: <56BB3E61.50707@suse.cz> Date: Wed, 10 Feb 2016 14:42:57 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.0 MIME-Version: 1.0 In-Reply-To: <20160209125301.c7e6067558c321cfb87602b5@linux-foundation.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/09/2016 09:53 PM, Andrew Morton wrote: > On Tue, 9 Feb 2016 18:58:32 +0100 Vlastimil Babka 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f50.google.com (mail-wm0-f50.google.com [74.125.82.50]) by kanga.kvack.org (Postfix) with ESMTP id 3C8676B0009 for ; Wed, 10 Feb 2016 08:43:01 -0500 (EST) Received: by mail-wm0-f50.google.com with SMTP id c200so29143569wme.0 for ; Wed, 10 Feb 2016 05:43:01 -0800 (PST) Received: from mx2.suse.de (mx2.suse.de. [195.135.220.15]) by mx.google.com with ESMTPS id bg9si4656129wjb.182.2016.02.10.05.42.59 for (version=TLS1 cipher=AES128-SHA bits=128/128); Wed, 10 Feb 2016 05:43:00 -0800 (PST) Subject: Re: [PATCH v2 3/3] mm/compaction: speed up pageblock_pfn_to_page() when zone is contiguous References: <1454566775-30973-1-git-send-email-iamjoonsoo.kim@lge.com> <1454566775-30973-3-git-send-email-iamjoonsoo.kim@lge.com> <20160204164929.a2f12b8a7edcdfa596abd850@linux-foundation.org> <56BA28C8.3060903@suse.cz> <20160209125301.c7e6067558c321cfb87602b5@linux-foundation.org> From: Vlastimil Babka Message-ID: <56BB3E61.50707@suse.cz> Date: Wed, 10 Feb 2016 14:42:57 +0100 MIME-Version: 1.0 In-Reply-To: <20160209125301.c7e6067558c321cfb87602b5@linux-foundation.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Andrew Morton Cc: Joonsoo Kim , Aaron Lu , Mel Gorman , Rik van Riel , David Rientjes , LKML , Linux Memory Management List , Joonsoo Kim On 02/09/2016 09:53 PM, Andrew Morton wrote: > On Tue, 9 Feb 2016 18:58:32 +0100 Vlastimil Babka 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 -- 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: email@kvack.org