From: Michal Hocko <mhocko@kernel.org> To: Vlastimil Babka <vbabka@suse.cz> Cc: Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Linus Torvalds <torvalds@linux-foundation.org>, Arkadiusz Miskiewicz <a.miskiewicz@gmail.com>, Ralf-Peter Rohbeck <Ralf-Peter.Rohbeck@quantum.com>, Olaf Hering <olaf@aepfle.de>, Mel Gorman <mgorman@techsingularity.net>, Joonsoo Kim <iamjoonsoo.kim@lge.com>, David Rientjes <rientjes@google.com>, Rik van Riel <riel@redhat.com>, Hillf Danton <hillf.zj@alibaba-inc.com> Subject: Re: [PATCH 3/4] mm, compaction: ignore fragindex from compaction_zonelist_suitable() Date: Mon, 26 Sep 2016 22:15:05 +0200 [thread overview] Message-ID: <20160926201503.GB23827@dhcp22.suse.cz> (raw) In-Reply-To: <20160926162025.21555-4-vbabka@suse.cz> On Mon 26-09-16 18:20:24, Vlastimil Babka wrote: > The compaction_zonelist_suitable() function tries to determine if compaction > will be able to proceed after sufficient reclaim, i.e. whether there are > enough reclaimable pages to provide enough order-0 freepages for compaction. > > This addition of reclaimable pages to the free pages works well for the order-0 > watermark check, but in the fragmentation index check we only consider truly > free pages. Thus we can get fragindex value close to 0 which indicates failure > do to lack of memory, and wrongly decide that compaction won't be suitable even > after reclaim. > > Instead of trying to somehow adjust fragindex for reclaimable pages, let's just > skip it from compaction_zonelist_suitable(). Yes this makes a lot of sense to me. The fragidx should be mostly about a balance between reclaim and the compaction so it shouldn't be used for fail or retry decisions. > > Signed-off-by: Vlastimil Babka <vbabka@suse.cz> > Cc: Michal Hocko <mhocko@kernel.org> > Cc: Mel Gorman <mgorman@techsingularity.net> > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> > Cc: David Rientjes <rientjes@google.com> > Cc: Rik van Riel <riel@redhat.com> Acked-by: Michal Hocko <mhocko@suse.com> > --- > mm/compaction.c | 35 ++++++++++++++++++----------------- > 1 file changed, 18 insertions(+), 17 deletions(-) > > diff --git a/mm/compaction.c b/mm/compaction.c > index 86d4d0bbfc7c..5ff7f801c345 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -1379,7 +1379,6 @@ static enum compact_result __compaction_suitable(struct zone *zone, int order, > int classzone_idx, > unsigned long wmark_target) > { > - int fragindex; > unsigned long watermark; > > if (is_via_compact_memory(order)) > @@ -1415,6 +1414,18 @@ static enum compact_result __compaction_suitable(struct zone *zone, int order, > ALLOC_CMA, wmark_target)) > return COMPACT_SKIPPED; > > + return COMPACT_CONTINUE; > +} > + > +enum compact_result compaction_suitable(struct zone *zone, int order, > + unsigned int alloc_flags, > + int classzone_idx) > +{ > + enum compact_result ret; > + int fragindex; > + > + ret = __compaction_suitable(zone, order, alloc_flags, classzone_idx, > + zone_page_state(zone, NR_FREE_PAGES)); > /* > * fragmentation index determines if allocation failures are due to > * low memory or external fragmentation > @@ -1426,21 +1437,12 @@ static enum compact_result __compaction_suitable(struct zone *zone, int order, > * > * Only compact if a failure would be due to fragmentation. > */ > - fragindex = fragmentation_index(zone, order); > - if (fragindex >= 0 && fragindex <= sysctl_extfrag_threshold) > - return COMPACT_NOT_SUITABLE_ZONE; > - > - return COMPACT_CONTINUE; > -} > - > -enum compact_result compaction_suitable(struct zone *zone, int order, > - unsigned int alloc_flags, > - int classzone_idx) > -{ > - enum compact_result ret; > + if (ret == COMPACT_CONTINUE) { > + fragindex = fragmentation_index(zone, order); > + if (fragindex >= 0 && fragindex <= sysctl_extfrag_threshold) > + return COMPACT_NOT_SUITABLE_ZONE; > + } > > - ret = __compaction_suitable(zone, order, alloc_flags, classzone_idx, > - zone_page_state(zone, NR_FREE_PAGES)); > trace_mm_compaction_suitable(zone, order, ret); > if (ret == COMPACT_NOT_SUITABLE_ZONE) > ret = COMPACT_SKIPPED; > @@ -1473,8 +1475,7 @@ bool compaction_zonelist_suitable(struct alloc_context *ac, int order, > available += zone_page_state_snapshot(zone, NR_FREE_PAGES); > compact_result = __compaction_suitable(zone, order, alloc_flags, > ac_classzone_idx(ac), available); > - if (compact_result != COMPACT_SKIPPED && > - compact_result != COMPACT_NOT_SUITABLE_ZONE) > + if (compact_result != COMPACT_SKIPPED) > return true; > } > > -- > 2.10.0 -- Michal Hocko SUSE Labs
WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org> To: Vlastimil Babka <vbabka@suse.cz> Cc: Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Linus Torvalds <torvalds@linux-foundation.org>, Arkadiusz Miskiewicz <a.miskiewicz@gmail.com>, Ralf-Peter Rohbeck <Ralf-Peter.Rohbeck@quantum.com>, Olaf Hering <olaf@aepfle.de>, Mel Gorman <mgorman@techsingularity.net>, Joonsoo Kim <iamjoonsoo.kim@lge.com>, David Rientjes <rientjes@google.com>, Rik van Riel <riel@redhat.com>, Hillf Danton <hillf.zj@alibaba-inc.com> Subject: Re: [PATCH 3/4] mm, compaction: ignore fragindex from compaction_zonelist_suitable() Date: Mon, 26 Sep 2016 22:15:05 +0200 [thread overview] Message-ID: <20160926201503.GB23827@dhcp22.suse.cz> (raw) In-Reply-To: <20160926162025.21555-4-vbabka@suse.cz> On Mon 26-09-16 18:20:24, Vlastimil Babka wrote: > The compaction_zonelist_suitable() function tries to determine if compaction > will be able to proceed after sufficient reclaim, i.e. whether there are > enough reclaimable pages to provide enough order-0 freepages for compaction. > > This addition of reclaimable pages to the free pages works well for the order-0 > watermark check, but in the fragmentation index check we only consider truly > free pages. Thus we can get fragindex value close to 0 which indicates failure > do to lack of memory, and wrongly decide that compaction won't be suitable even > after reclaim. > > Instead of trying to somehow adjust fragindex for reclaimable pages, let's just > skip it from compaction_zonelist_suitable(). Yes this makes a lot of sense to me. The fragidx should be mostly about a balance between reclaim and the compaction so it shouldn't be used for fail or retry decisions. > > Signed-off-by: Vlastimil Babka <vbabka@suse.cz> > Cc: Michal Hocko <mhocko@kernel.org> > Cc: Mel Gorman <mgorman@techsingularity.net> > Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> > Cc: David Rientjes <rientjes@google.com> > Cc: Rik van Riel <riel@redhat.com> Acked-by: Michal Hocko <mhocko@suse.com> > --- > mm/compaction.c | 35 ++++++++++++++++++----------------- > 1 file changed, 18 insertions(+), 17 deletions(-) > > diff --git a/mm/compaction.c b/mm/compaction.c > index 86d4d0bbfc7c..5ff7f801c345 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -1379,7 +1379,6 @@ static enum compact_result __compaction_suitable(struct zone *zone, int order, > int classzone_idx, > unsigned long wmark_target) > { > - int fragindex; > unsigned long watermark; > > if (is_via_compact_memory(order)) > @@ -1415,6 +1414,18 @@ static enum compact_result __compaction_suitable(struct zone *zone, int order, > ALLOC_CMA, wmark_target)) > return COMPACT_SKIPPED; > > + return COMPACT_CONTINUE; > +} > + > +enum compact_result compaction_suitable(struct zone *zone, int order, > + unsigned int alloc_flags, > + int classzone_idx) > +{ > + enum compact_result ret; > + int fragindex; > + > + ret = __compaction_suitable(zone, order, alloc_flags, classzone_idx, > + zone_page_state(zone, NR_FREE_PAGES)); > /* > * fragmentation index determines if allocation failures are due to > * low memory or external fragmentation > @@ -1426,21 +1437,12 @@ static enum compact_result __compaction_suitable(struct zone *zone, int order, > * > * Only compact if a failure would be due to fragmentation. > */ > - fragindex = fragmentation_index(zone, order); > - if (fragindex >= 0 && fragindex <= sysctl_extfrag_threshold) > - return COMPACT_NOT_SUITABLE_ZONE; > - > - return COMPACT_CONTINUE; > -} > - > -enum compact_result compaction_suitable(struct zone *zone, int order, > - unsigned int alloc_flags, > - int classzone_idx) > -{ > - enum compact_result ret; > + if (ret == COMPACT_CONTINUE) { > + fragindex = fragmentation_index(zone, order); > + if (fragindex >= 0 && fragindex <= sysctl_extfrag_threshold) > + return COMPACT_NOT_SUITABLE_ZONE; > + } > > - ret = __compaction_suitable(zone, order, alloc_flags, classzone_idx, > - zone_page_state(zone, NR_FREE_PAGES)); > trace_mm_compaction_suitable(zone, order, ret); > if (ret == COMPACT_NOT_SUITABLE_ZONE) > ret = COMPACT_SKIPPED; > @@ -1473,8 +1475,7 @@ bool compaction_zonelist_suitable(struct alloc_context *ac, int order, > available += zone_page_state_snapshot(zone, NR_FREE_PAGES); > compact_result = __compaction_suitable(zone, order, alloc_flags, > ac_classzone_idx(ac), available); > - if (compact_result != COMPACT_SKIPPED && > - compact_result != COMPACT_NOT_SUITABLE_ZONE) > + if (compact_result != COMPACT_SKIPPED) > return true; > } > > -- > 2.10.0 -- 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:[~2016-09-26 20:15 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-09-26 16:20 [PATCH 0/4] followups to reintroduce compaction feedback for OOM decisions Vlastimil Babka 2016-09-26 16:20 ` Vlastimil Babka 2016-09-26 16:20 ` [PATCH 1/4] mm, compaction: more reliably increase direct compaction priority-fix Vlastimil Babka 2016-09-26 16:20 ` Vlastimil Babka 2016-09-27 3:25 ` Hillf Danton 2016-09-27 3:25 ` Hillf Danton 2016-09-26 16:20 ` [PATCH 2/4] mm, page_alloc: pull no_progress_loops update to should_reclaim_retry() Vlastimil Babka 2016-09-26 16:20 ` Vlastimil Babka 2016-09-26 16:20 ` [PATCH 3/4] mm, compaction: ignore fragindex from compaction_zonelist_suitable() Vlastimil Babka 2016-09-26 16:20 ` Vlastimil Babka 2016-09-26 20:15 ` Michal Hocko [this message] 2016-09-26 20:15 ` Michal Hocko 2016-09-29 9:57 ` Vlastimil Babka 2016-09-29 9:57 ` Vlastimil Babka 2016-09-26 16:20 ` [PATCH 4/4] mm, compaction: restrict fragindex to costly orders Vlastimil Babka 2016-09-26 16:20 ` Vlastimil Babka 2016-09-26 20:29 ` Michal Hocko 2016-09-26 20:29 ` Michal Hocko
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=20160926201503.GB23827@dhcp22.suse.cz \ --to=mhocko@kernel.org \ --cc=Ralf-Peter.Rohbeck@quantum.com \ --cc=a.miskiewicz@gmail.com \ --cc=akpm@linux-foundation.org \ --cc=hillf.zj@alibaba-inc.com \ --cc=iamjoonsoo.kim@lge.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mgorman@techsingularity.net \ --cc=olaf@aepfle.de \ --cc=riel@redhat.com \ --cc=rientjes@google.com \ --cc=torvalds@linux-foundation.org \ --cc=vbabka@suse.cz \ /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.