All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Arkadiusz Miskiewicz <a.miskiewicz@gmail.com>,
	Ralf-Peter Rohbeck <Ralf-Peter.Rohbeck@quantum.com>,
	Olaf Hering <olaf@aepfle.de>,
	linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-mm@kvack.org, David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Mel Gorman <mgorman@techsingularity.net>,
	Rik van Riel <riel@redhat.com>
Subject: Re: [PATCH 0/4] reintroduce compaction feedback for OOM decisions
Date: Fri, 23 Sep 2016 10:26:28 +0200	[thread overview]
Message-ID: <20160923082627.GE4478@dhcp22.suse.cz> (raw)
In-Reply-To: <56f2c2ed-8a58-cf9c-dd00-c0d0e274607a@suse.cz>

On Thu 22-09-16 17:18:48, Vlastimil Babka wrote:
> On 09/21/2016 07:18 PM, Michal Hocko wrote:
> > On Tue 06-09-16 15:52:54, Vlastimil Babka wrote:
> > 
> > We still do not ignore fragindex in the full priority. This part has
> > always been quite unclear to me so I cannot really tell whether that
> > makes any difference or not but just to be on the safe side I would
> > preffer to have _all_ the shortcuts out of the way in the highest
> > priority. It is true that this will cause COMPACT_NOT_SUITABLE_ZONE
> > so keep retrying but still a complication to understand the workflow.
> > 
> > What do you think?
>  
> I was thinking that this shouldn't be a problem on non-costly orders and default
> extfrag_threshold. But better be safe. Moreover I think the issue is much more
> dangerous for compact_zonelist_suitable() as explained below.
> 
> ----8<----
> >From 0e6cb251aa6e3b1be7deff315c0238c4d478f22e Mon Sep 17 00:00:00 2001
> From: Vlastimil Babka <vbabka@suse.cz>
> Date: Thu, 22 Sep 2016 15:33:57 +0200
> Subject: [PATCH] mm, compaction: ignore fragindex on highest direct compaction
>  priority
> 
> Fragmentation index check in compaction_suitable() should be the last heuristic
> that we allow on the highest compaction priority. Since that's a potential
> premature OOM, disable it too. Even more problematic is its usage from
> compaction_zonelist_suitable() -> __compaction_suitable() where we check the
> order-0 watermark against free plus available-for-reclaim pages, but the
> fragindex considers only truly free pages. Thus we can get a result close to 0
> indicating failure do to lack of memory, and wrongly decide that compaction
> won't be suitable even after reclaim. The solution is to skip the fragindex
> check also in this context, regardless of priority.
> 
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
>  include/linux/compaction.h |  5 +++--
>  mm/compaction.c            | 44 +++++++++++++++++++++++---------------------
>  mm/internal.h              |  1 +
>  mm/vmscan.c                |  6 ++++--
>  4 files changed, 31 insertions(+), 25 deletions(-)

This is much more code churn than I expected. I was thiking about it
some more and I am really wondering whether it actually make any sense
to check the fragidx for !costly orders. Wouldn't it be much simpler to
just put it out of the way for those regardless of the compaction
priority. In other words does this check makes any measurable difference
for !costly orders?
-- 
Michal Hocko
SUSE Labs

WARNING: multiple messages have this Message-ID
From: Michal Hocko <mhocko@kernel.org>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Arkadiusz Miskiewicz <a.miskiewicz@gmail.com>,
	Ralf-Peter Rohbeck <Ralf-Peter.Rohbeck@quantum.com>,
	Olaf Hering <olaf@aepfle.de>,
	linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-mm@kvack.org, David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Mel Gorman <mgorman@techsingularity.net>,
	Rik van Riel <riel@redhat.com>
Subject: Re: [PATCH 0/4] reintroduce compaction feedback for OOM decisions
Date: Fri, 23 Sep 2016 10:26:28 +0200	[thread overview]
Message-ID: <20160923082627.GE4478@dhcp22.suse.cz> (raw)
In-Reply-To: <56f2c2ed-8a58-cf9c-dd00-c0d0e274607a@suse.cz>

On Thu 22-09-16 17:18:48, Vlastimil Babka wrote:
> On 09/21/2016 07:18 PM, Michal Hocko wrote:
> > On Tue 06-09-16 15:52:54, Vlastimil Babka wrote:
> > 
> > We still do not ignore fragindex in the full priority. This part has
> > always been quite unclear to me so I cannot really tell whether that
> > makes any difference or not but just to be on the safe side I would
> > preffer to have _all_ the shortcuts out of the way in the highest
> > priority. It is true that this will cause COMPACT_NOT_SUITABLE_ZONE
> > so keep retrying but still a complication to understand the workflow.
> > 
> > What do you think?
>  
> I was thinking that this shouldn't be a problem on non-costly orders and default
> extfrag_threshold. But better be safe. Moreover I think the issue is much more
> dangerous for compact_zonelist_suitable() as explained below.
> 
> ----8<----
> >From 0e6cb251aa6e3b1be7deff315c0238c4d478f22e Mon Sep 17 00:00:00 2001
> From: Vlastimil Babka <vbabka@suse.cz>
> Date: Thu, 22 Sep 2016 15:33:57 +0200
> Subject: [PATCH] mm, compaction: ignore fragindex on highest direct compaction
>  priority
> 
> Fragmentation index check in compaction_suitable() should be the last heuristic
> that we allow on the highest compaction priority. Since that's a potential
> premature OOM, disable it too. Even more problematic is its usage from
> compaction_zonelist_suitable() -> __compaction_suitable() where we check the
> order-0 watermark against free plus available-for-reclaim pages, but the
> fragindex considers only truly free pages. Thus we can get a result close to 0
> indicating failure do to lack of memory, and wrongly decide that compaction
> won't be suitable even after reclaim. The solution is to skip the fragindex
> check also in this context, regardless of priority.
> 
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
>  include/linux/compaction.h |  5 +++--
>  mm/compaction.c            | 44 +++++++++++++++++++++++---------------------
>  mm/internal.h              |  1 +
>  mm/vmscan.c                |  6 ++++--
>  4 files changed, 31 insertions(+), 25 deletions(-)

This is much more code churn than I expected. I was thiking about it
some more and I am really wondering whether it actually make any sense
to check the fragidx for !costly orders. Wouldn't it be much simpler to
just put it out of the way for those regardless of the compaction
priority. In other words does this check makes any measurable difference
for !costly orders?
-- 
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>

  reply	other threads:[~2016-09-23  8:26 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-06 13:52 Vlastimil Babka
2016-09-06 13:52 ` Vlastimil Babka
2016-09-06 13:52 ` [PATCH 1/4] Revert "mm, oom: prevent premature OOM killer invocation for high order request" Vlastimil Babka
2016-09-06 13:52   ` Vlastimil Babka
2016-09-21 17:04   ` Michal Hocko
2016-09-21 17:04     ` Michal Hocko
2016-09-06 13:52 ` [PATCH 2/4] mm, compaction: more reliably increase direct compaction priority Vlastimil Babka
2016-09-06 13:52   ` Vlastimil Babka
2016-09-21 17:13   ` Michal Hocko
2016-09-21 17:13     ` Michal Hocko
2016-09-22 12:51     ` Vlastimil Babka
2016-09-22 12:51       ` Vlastimil Babka
2016-09-22 14:08       ` Michal Hocko
2016-09-22 14:08         ` Michal Hocko
2016-09-22 14:52         ` Michal Hocko
2016-09-22 14:52           ` Michal Hocko
2016-09-22 14:59           ` Vlastimil Babka
2016-09-22 14:59             ` Vlastimil Babka
2016-09-22 15:06           ` Vlastimil Babka
2016-09-22 15:06             ` Vlastimil Babka
2016-09-23  4:04             ` Hillf Danton
2016-09-23  4:04               ` Hillf Danton
2016-09-23  6:55               ` Vlastimil Babka
2016-09-23  6:55                 ` Vlastimil Babka
2016-09-23  8:23                 ` Michal Hocko
2016-09-23  8:23                   ` Michal Hocko
2016-09-23 10:47                   ` Vlastimil Babka
2016-09-23 10:47                     ` Vlastimil Babka
2016-09-23 12:06                     ` Michal Hocko
2016-09-23 12:06                       ` Michal Hocko
2016-09-06 13:52 ` [PATCH 3/4] mm, compaction: restrict full priority to non-costly orders Vlastimil Babka
2016-09-06 13:52   ` Vlastimil Babka
2016-09-21 17:15   ` Michal Hocko
2016-09-21 17:15     ` Michal Hocko
2016-09-06 13:52 ` [PATCH 4/4] mm, compaction: make full priority ignore pageblock suitability Vlastimil Babka
2016-09-06 13:52   ` Vlastimil Babka
2016-09-15 18:51 ` [PATCH 0/4] reintroduce compaction feedback for OOM decisions Arkadiusz Miskiewicz
2016-09-15 18:51   ` Arkadiusz Miskiewicz
2016-09-21 17:18 ` Michal Hocko
2016-09-21 17:18   ` Michal Hocko
2016-09-22 15:18   ` Vlastimil Babka
2016-09-22 15:18     ` Vlastimil Babka
2016-09-23  8:26     ` Michal Hocko [this message]
2016-09-23  8:26       ` Michal Hocko
2016-09-23 10:55       ` Vlastimil Babka
2016-09-23 10:55         ` Vlastimil Babka
2016-09-23 12:09         ` Michal Hocko
2016-09-23 12:09           ` 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=20160923082627.GE4478@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=Ralf-Peter.Rohbeck@quantum.com \
    --cc=a.miskiewicz@gmail.com \
    --cc=akpm@linux-foundation.org \
    --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 \
    --subject='Re: [PATCH 0/4] reintroduce compaction feedback for OOM decisions' \
    /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: link

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.