From: Michal Hocko <mhocko@kernel.org> To: Andrew Morton <akpm@linux-foundation.org> Cc: linux-mm@kvack.org, Michael Ellerman <mpe@ellerman.id.au>, Vlastimil Babka <vbabka@suse.cz>, Joonsoo Kim <iamjoonsoo.kim@lge.com>, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>, Reza Arbab <arbab@linux.vnet.ibm.com>, Yasuaki Ishimatsu <yasu.isimatu@gmail.com>, qiuxishi@huawei.com, Igor Mammedov <imammedo@redhat.com>, Vitaly Kuznetsov <vkuznets@redhat.com>, LKML <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 1/2] mm: drop migrate type checks from has_unmovable_pages Date: Thu, 26 Oct 2017 15:59:32 +0200 [thread overview] Message-ID: <20171026135932.p5st4z7t6akmmxkf@dhcp22.suse.cz> (raw) In-Reply-To: <20171019122118.y6cndierwl2vnguj@dhcp22.suse.cz> On Thu 19-10-17 14:21:18, Michal Hocko wrote: [...] > From 8cbd811d741f5dd93d1b21bb3ef94482a4d0bd32 Mon Sep 17 00:00:00 2001 > From: Michal Hocko <mhocko@suse.com> > Date: Thu, 19 Oct 2017 14:14:02 +0200 > Subject: [PATCH] mm: distinguish CMA and MOVABLE isolation in > has_unmovable_pages > > Joonsoo has noticed that "mm: drop migrate type checks from > has_unmovable_pages" would break CMA allocator because it relies on > has_unmovable_pages returning false even for CMA pageblocks which in > fact don't have to be movable: > alloc_contig_range > start_isolate_page_range > set_migratetype_isolate > has_unmovable_pages > > This is a result of the code sharing between CMA and memory hotplug > while each one has a different idea of what has_unmovable_pages should > return. This is unfortunate but fixing it properly would require a lot > of code duplication. > > Fix the issue by introducing the requested migrate type argument > and special case MIGRATE_CMA case where CMA page blocks are handled > properly. This will work for memory hotplug because it requires > MIGRATE_MOVABLE. > > Reported-by: Joonsoo Kim <iamjoonsoo.kim@lge.com> > Signed-off-by: Michal Hocko <mhocko@suse.com> > --- Andrew, could you add this one to the bundle as well? After mm-drop-migrate-type-checks-from-has_unmovable_pages.patch, please. Joonsoo would like to see a larger change in this area [1] but I think we need to think those much more through [2] and Joonsoo agreed to take the simpler patch first [3]. Thanks! [1] http://lkml.kernel.org/r/20171024044423.GA31424@js1304-P5Q-DELUXE [2] http://lkml.kernel.org/r/20171024122526.3kmabkcbmj4johli@dhcp22.suse.cz [3] http://lkml.kernel.org/r/20171026024707.GA11791@js1304-P5Q-DELUXE > include/linux/page-isolation.h | 2 +- > mm/page_alloc.c | 12 +++++++++++- > mm/page_isolation.c | 10 +++++----- > 3 files changed, 17 insertions(+), 7 deletions(-) > > diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h > index d4cd2014fa6f..fa9db0c7b54e 100644 > --- a/include/linux/page-isolation.h > +++ b/include/linux/page-isolation.h > @@ -30,7 +30,7 @@ static inline bool is_migrate_isolate(int migratetype) > #endif > > bool has_unmovable_pages(struct zone *zone, struct page *page, int count, > - bool skip_hwpoisoned_pages); > + int migratetype, bool skip_hwpoisoned_pages); > void set_pageblock_migratetype(struct page *page, int migratetype); > int move_freepages_block(struct zone *zone, struct page *page, > int migratetype, int *num_movable); > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 5b4d85ae445c..259aeb22462f 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -7344,6 +7344,7 @@ void *__init alloc_large_system_hash(const char *tablename, > * race condition. So you can't expect this function should be exact. > */ > bool has_unmovable_pages(struct zone *zone, struct page *page, int count, > + int migratetype, > bool skip_hwpoisoned_pages) > { > unsigned long pfn, iter, found; > @@ -7356,6 +7357,15 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count, > if (zone_idx(zone) == ZONE_MOVABLE) > return false; > > + /* > + * CMA allocations (alloc_contig_range) really need to mark isolate > + * CMA pageblocks even when they are not movable in fact so consider > + * them movable here. > + */ > + if (is_migrate_cma(migratetype) && > + is_migrate_cma(get_pageblock_migratetype(page))) > + return false; > + > pfn = page_to_pfn(page); > for (found = 0, iter = 0; iter < pageblock_nr_pages; iter++) { > unsigned long check = pfn + iter; > @@ -7441,7 +7451,7 @@ bool is_pageblock_removable_nolock(struct page *page) > if (!zone_spans_pfn(zone, pfn)) > return false; > > - return !has_unmovable_pages(zone, page, 0, true); > + return !has_unmovable_pages(zone, page, 0, MIGRATE_MOVABLE, true); > } > > #if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || defined(CONFIG_CMA) > diff --git a/mm/page_isolation.c b/mm/page_isolation.c > index 757410d9f758..8616f5332c77 100644 > --- a/mm/page_isolation.c > +++ b/mm/page_isolation.c > @@ -14,7 +14,7 @@ > #define CREATE_TRACE_POINTS > #include <trace/events/page_isolation.h> > > -static int set_migratetype_isolate(struct page *page, > +static int set_migratetype_isolate(struct page *page, int migratetype, > bool skip_hwpoisoned_pages) > { > struct zone *zone; > @@ -51,7 +51,7 @@ static int set_migratetype_isolate(struct page *page, > * FIXME: Now, memory hotplug doesn't call shrink_slab() by itself. > * We just check MOVABLE pages. > */ > - if (!has_unmovable_pages(zone, page, arg.pages_found, > + if (!has_unmovable_pages(zone, page, arg.pages_found, migratetype, > skip_hwpoisoned_pages)) > ret = 0; > > @@ -63,14 +63,14 @@ static int set_migratetype_isolate(struct page *page, > out: > if (!ret) { > unsigned long nr_pages; > - int migratetype = get_pageblock_migratetype(page); > + int mt = get_pageblock_migratetype(page); > > set_pageblock_migratetype(page, MIGRATE_ISOLATE); > zone->nr_isolate_pageblock++; > nr_pages = move_freepages_block(zone, page, MIGRATE_ISOLATE, > NULL); > > - __mod_zone_freepage_state(zone, -nr_pages, migratetype); > + __mod_zone_freepage_state(zone, -nr_pages, mt); > } > > spin_unlock_irqrestore(&zone->lock, flags); > @@ -182,7 +182,7 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn, > pfn += pageblock_nr_pages) { > page = __first_valid_page(pfn, pageblock_nr_pages); > if (page && > - set_migratetype_isolate(page, skip_hwpoisoned_pages)) { > + set_migratetype_isolate(page, migratetype, skip_hwpoisoned_pages)) { > undo_pfn = pfn; > goto undo; > } > -- > 2.14.2 > > -- > Michal Hocko > SUSE Labs -- Michal Hocko SUSE Labs
WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org> To: Andrew Morton <akpm@linux-foundation.org> Cc: linux-mm@kvack.org, Michael Ellerman <mpe@ellerman.id.au>, Vlastimil Babka <vbabka@suse.cz>, Joonsoo Kim <iamjoonsoo.kim@lge.com>, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>, Reza Arbab <arbab@linux.vnet.ibm.com>, Yasuaki Ishimatsu <yasu.isimatu@gmail.com>, qiuxishi@huawei.com, Igor Mammedov <imammedo@redhat.com>, Vitaly Kuznetsov <vkuznets@redhat.com>, LKML <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 1/2] mm: drop migrate type checks from has_unmovable_pages Date: Thu, 26 Oct 2017 15:59:32 +0200 [thread overview] Message-ID: <20171026135932.p5st4z7t6akmmxkf@dhcp22.suse.cz> (raw) In-Reply-To: <20171019122118.y6cndierwl2vnguj@dhcp22.suse.cz> On Thu 19-10-17 14:21:18, Michal Hocko wrote: [...] > From 8cbd811d741f5dd93d1b21bb3ef94482a4d0bd32 Mon Sep 17 00:00:00 2001 > From: Michal Hocko <mhocko@suse.com> > Date: Thu, 19 Oct 2017 14:14:02 +0200 > Subject: [PATCH] mm: distinguish CMA and MOVABLE isolation in > has_unmovable_pages > > Joonsoo has noticed that "mm: drop migrate type checks from > has_unmovable_pages" would break CMA allocator because it relies on > has_unmovable_pages returning false even for CMA pageblocks which in > fact don't have to be movable: > alloc_contig_range > start_isolate_page_range > set_migratetype_isolate > has_unmovable_pages > > This is a result of the code sharing between CMA and memory hotplug > while each one has a different idea of what has_unmovable_pages should > return. This is unfortunate but fixing it properly would require a lot > of code duplication. > > Fix the issue by introducing the requested migrate type argument > and special case MIGRATE_CMA case where CMA page blocks are handled > properly. This will work for memory hotplug because it requires > MIGRATE_MOVABLE. > > Reported-by: Joonsoo Kim <iamjoonsoo.kim@lge.com> > Signed-off-by: Michal Hocko <mhocko@suse.com> > --- Andrew, could you add this one to the bundle as well? After mm-drop-migrate-type-checks-from-has_unmovable_pages.patch, please. Joonsoo would like to see a larger change in this area [1] but I think we need to think those much more through [2] and Joonsoo agreed to take the simpler patch first [3]. Thanks! [1] http://lkml.kernel.org/r/20171024044423.GA31424@js1304-P5Q-DELUXE [2] http://lkml.kernel.org/r/20171024122526.3kmabkcbmj4johli@dhcp22.suse.cz [3] http://lkml.kernel.org/r/20171026024707.GA11791@js1304-P5Q-DELUXE > include/linux/page-isolation.h | 2 +- > mm/page_alloc.c | 12 +++++++++++- > mm/page_isolation.c | 10 +++++----- > 3 files changed, 17 insertions(+), 7 deletions(-) > > diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h > index d4cd2014fa6f..fa9db0c7b54e 100644 > --- a/include/linux/page-isolation.h > +++ b/include/linux/page-isolation.h > @@ -30,7 +30,7 @@ static inline bool is_migrate_isolate(int migratetype) > #endif > > bool has_unmovable_pages(struct zone *zone, struct page *page, int count, > - bool skip_hwpoisoned_pages); > + int migratetype, bool skip_hwpoisoned_pages); > void set_pageblock_migratetype(struct page *page, int migratetype); > int move_freepages_block(struct zone *zone, struct page *page, > int migratetype, int *num_movable); > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 5b4d85ae445c..259aeb22462f 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -7344,6 +7344,7 @@ void *__init alloc_large_system_hash(const char *tablename, > * race condition. So you can't expect this function should be exact. > */ > bool has_unmovable_pages(struct zone *zone, struct page *page, int count, > + int migratetype, > bool skip_hwpoisoned_pages) > { > unsigned long pfn, iter, found; > @@ -7356,6 +7357,15 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count, > if (zone_idx(zone) == ZONE_MOVABLE) > return false; > > + /* > + * CMA allocations (alloc_contig_range) really need to mark isolate > + * CMA pageblocks even when they are not movable in fact so consider > + * them movable here. > + */ > + if (is_migrate_cma(migratetype) && > + is_migrate_cma(get_pageblock_migratetype(page))) > + return false; > + > pfn = page_to_pfn(page); > for (found = 0, iter = 0; iter < pageblock_nr_pages; iter++) { > unsigned long check = pfn + iter; > @@ -7441,7 +7451,7 @@ bool is_pageblock_removable_nolock(struct page *page) > if (!zone_spans_pfn(zone, pfn)) > return false; > > - return !has_unmovable_pages(zone, page, 0, true); > + return !has_unmovable_pages(zone, page, 0, MIGRATE_MOVABLE, true); > } > > #if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || defined(CONFIG_CMA) > diff --git a/mm/page_isolation.c b/mm/page_isolation.c > index 757410d9f758..8616f5332c77 100644 > --- a/mm/page_isolation.c > +++ b/mm/page_isolation.c > @@ -14,7 +14,7 @@ > #define CREATE_TRACE_POINTS > #include <trace/events/page_isolation.h> > > -static int set_migratetype_isolate(struct page *page, > +static int set_migratetype_isolate(struct page *page, int migratetype, > bool skip_hwpoisoned_pages) > { > struct zone *zone; > @@ -51,7 +51,7 @@ static int set_migratetype_isolate(struct page *page, > * FIXME: Now, memory hotplug doesn't call shrink_slab() by itself. > * We just check MOVABLE pages. > */ > - if (!has_unmovable_pages(zone, page, arg.pages_found, > + if (!has_unmovable_pages(zone, page, arg.pages_found, migratetype, > skip_hwpoisoned_pages)) > ret = 0; > > @@ -63,14 +63,14 @@ static int set_migratetype_isolate(struct page *page, > out: > if (!ret) { > unsigned long nr_pages; > - int migratetype = get_pageblock_migratetype(page); > + int mt = get_pageblock_migratetype(page); > > set_pageblock_migratetype(page, MIGRATE_ISOLATE); > zone->nr_isolate_pageblock++; > nr_pages = move_freepages_block(zone, page, MIGRATE_ISOLATE, > NULL); > > - __mod_zone_freepage_state(zone, -nr_pages, migratetype); > + __mod_zone_freepage_state(zone, -nr_pages, mt); > } > > spin_unlock_irqrestore(&zone->lock, flags); > @@ -182,7 +182,7 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn, > pfn += pageblock_nr_pages) { > page = __first_valid_page(pfn, pageblock_nr_pages); > if (page && > - set_migratetype_isolate(page, skip_hwpoisoned_pages)) { > + set_migratetype_isolate(page, migratetype, skip_hwpoisoned_pages)) { > undo_pfn = pfn; > goto undo; > } > -- > 2.14.2 > > -- > Michal Hocko > SUSE Labs -- Michal Hocko SUSE Labs -- 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>
next prev parent reply other threads:[~2017-10-26 13:59 UTC|newest] Thread overview: 102+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-09-18 7:08 [PATCH v2 0/2] mm, memory_hotplug: redefine memory offline retry logic Michal Hocko 2017-09-18 7:08 ` Michal Hocko 2017-09-18 7:08 ` [PATCH 1/2] mm, memory_hotplug: do not fail offlining too early Michal Hocko 2017-09-18 7:08 ` Michal Hocko 2017-10-10 12:05 ` Michael Ellerman 2017-10-10 12:05 ` Michael Ellerman 2017-10-10 12:27 ` Michal Hocko 2017-10-10 12:27 ` Michal Hocko 2017-10-11 2:37 ` Michael Ellerman 2017-10-11 2:37 ` Michael Ellerman 2017-10-11 5:19 ` Michael Ellerman 2017-10-11 5:19 ` Michael Ellerman 2017-10-11 14:05 ` Anshuman Khandual 2017-10-11 14:05 ` Anshuman Khandual 2017-10-11 14:16 ` Michal Hocko 2017-10-11 14:16 ` Michal Hocko 2017-10-11 6:51 ` Michal Hocko 2017-10-11 6:51 ` Michal Hocko 2017-10-11 8:04 ` Vlastimil Babka 2017-10-11 8:04 ` Vlastimil Babka 2017-10-11 8:13 ` Michal Hocko 2017-10-11 8:13 ` Michal Hocko 2017-10-11 11:17 ` Vlastimil Babka 2017-10-11 11:17 ` Vlastimil Babka 2017-10-11 11:24 ` Michal Hocko 2017-10-11 11:24 ` Michal Hocko 2017-10-13 11:42 ` Michael Ellerman 2017-10-13 11:42 ` Michael Ellerman 2017-10-13 11:58 ` Michal Hocko 2017-10-13 11:58 ` Michal Hocko 2017-10-13 12:00 ` [PATCH 1/2] mm: drop migrate type checks from has_unmovable_pages Michal Hocko 2017-10-13 12:00 ` Michal Hocko 2017-10-13 12:00 ` [PATCH 2/2] mm, page_alloc: fail has_unmovable_pages when seeing reserved pages Michal Hocko 2017-10-13 12:00 ` Michal Hocko 2017-10-13 12:04 ` Vlastimil Babka 2017-10-13 12:04 ` Vlastimil Babka 2017-10-13 12:07 ` Michal Hocko 2017-10-13 12:07 ` Michal Hocko 2017-10-17 13:03 ` Vlastimil Babka 2017-10-17 13:03 ` Vlastimil Babka 2017-10-17 11:41 ` [PATCH 1/2] mm: drop migrate type checks from has_unmovable_pages Michael Ellerman 2017-10-17 11:41 ` Michael Ellerman 2017-10-17 12:03 ` Michal Hocko 2017-10-17 12:03 ` Michal Hocko 2017-10-17 13:02 ` Vlastimil Babka 2017-10-17 13:02 ` Vlastimil Babka 2017-10-19 2:51 ` Joonsoo Kim 2017-10-19 2:51 ` Joonsoo Kim 2017-10-19 7:15 ` Michal Hocko 2017-10-19 7:15 ` Michal Hocko 2017-10-19 7:33 ` Joonsoo Kim 2017-10-19 7:33 ` Joonsoo Kim 2017-10-19 8:20 ` Michal Hocko 2017-10-19 8:20 ` Michal Hocko 2017-10-19 12:21 ` Michal Hocko 2017-10-19 12:21 ` Michal Hocko 2017-10-20 2:13 ` Joonsoo Kim 2017-10-20 2:13 ` Joonsoo Kim 2017-10-20 5:59 ` Michal Hocko 2017-10-20 5:59 ` Michal Hocko 2017-10-20 6:50 ` Joonsoo Kim 2017-10-20 6:50 ` Joonsoo Kim 2017-10-20 7:02 ` Michal Hocko 2017-10-20 7:02 ` Michal Hocko 2017-10-23 5:23 ` Joonsoo Kim 2017-10-23 5:23 ` Joonsoo Kim 2017-10-23 8:10 ` Michal Hocko 2017-10-23 8:10 ` Michal Hocko 2017-10-24 4:44 ` Joonsoo Kim 2017-10-24 4:44 ` Joonsoo Kim 2017-10-24 7:44 ` Michal Hocko 2017-10-24 7:44 ` Michal Hocko 2017-10-24 8:12 ` Vlastimil Babka 2017-10-24 8:12 ` Vlastimil Babka 2017-10-24 12:25 ` Michal Hocko 2017-10-24 12:25 ` Michal Hocko 2017-10-26 2:47 ` Joonsoo Kim 2017-10-26 2:47 ` Joonsoo Kim 2017-10-26 7:41 ` Michal Hocko 2017-10-26 7:41 ` Michal Hocko 2017-10-20 7:22 ` Xishi Qiu 2017-10-20 7:22 ` Xishi Qiu 2017-10-20 8:17 ` Michal Hocko 2017-10-20 8:17 ` Michal Hocko 2017-10-23 5:26 ` Joonsoo Kim 2017-10-23 5:26 ` Joonsoo Kim 2017-10-26 13:04 ` Vlastimil Babka 2017-10-26 13:04 ` Vlastimil Babka 2017-10-26 13:59 ` Michal Hocko [this message] 2017-10-26 13:59 ` Michal Hocko 2017-09-18 7:08 ` [PATCH 2/2] mm, memory_hotplug: remove timeout from __offline_memory Michal Hocko 2017-09-18 7:08 ` Michal Hocko [not found] <AM3PR04MB14892A9D6D2FBCE21B8C1F0FF12B0@AM3PR04MB1489.eurprd04.prod.outlook.com> 2017-11-13 7:33 ` [PATCH 1/2] mm: drop migrate type checks from has_unmovable_pages Ran Wang 2017-11-13 7:33 ` Ran Wang 2017-11-13 11:02 ` Michal Hocko 2017-11-13 11:02 ` Michal Hocko 2017-11-14 6:10 ` Ran Wang 2017-11-14 6:10 ` Ran Wang 2017-11-14 7:06 ` Michal Hocko 2017-11-14 7:06 ` Michal Hocko 2017-11-14 7:45 ` Ran Wang 2017-11-14 7:45 ` Ran Wang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20171026135932.p5st4z7t6akmmxkf@dhcp22.suse.cz \ --to=mhocko@kernel.org \ --cc=akpm@linux-foundation.org \ --cc=arbab@linux.vnet.ibm.com \ --cc=iamjoonsoo.kim@lge.com \ --cc=imammedo@redhat.com \ --cc=kamezawa.hiroyu@jp.fujitsu.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mpe@ellerman.id.au \ --cc=qiuxishi@huawei.com \ --cc=vbabka@suse.cz \ --cc=vkuznets@redhat.com \ --cc=yasu.isimatu@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.