From: Michal Hocko <mhocko@suse.cz>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Ralf-Peter Rohbeck <Ralf-Peter.Rohbeck@quantum.com>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
Joonsoo Kim <iamjoonsoo.kim@lge.com>
Subject: Re: OOM killer changes
Date: Fri, 19 Aug 2016 10:26:40 +0200 [thread overview]
Message-ID: <20160819082639.GE32619@dhcp22.suse.cz> (raw)
In-Reply-To: <d443b884-87e7-1c93-8684-3a3a35759fb1@suse.cz>
On Fri 19-08-16 09:47:59, Vlastimil Babka wrote:
> On 08/19/2016 09:33 AM, Michal Hocko wrote:
> > On Fri 19-08-16 08:27:34, Vlastimil Babka wrote:
> >> On 08/19/2016 04:42 AM, Ralf-Peter Rohbeck wrote:
> >>> On 18.08.2016 13:12, Vlastimil Babka wrote:
> >>>> On 18.8.2016 22:01, Ralf-Peter Rohbeck wrote:
> >>>>> On 17.08.2016 23:57, Vlastimil Babka wrote:
> >>>>>> Vlastimil
> >>>>> Yes, that change was in my test with linux-next-20160817. Here's the diff:
> >>>>>
> >>>>> diff --git a/mm/compaction.c b/mm/compaction.c
> >>>>> index f94ae67..60a9ca2 100644
> >>>>> --- a/mm/compaction.c
> >>>>> +++ b/mm/compaction.c
> >>>>> @@ -1083,8 +1083,10 @@ static void isolate_freepages(struct
> >>>>> compact_control *cc)
> >>>>> continue;
> >>>>>
> >>>>> /* Check the block is suitable for migration */
> >>>>> +/*
> >>>>> if (!suitable_migration_target(page))
> >>>>> continue;
> >>>>> +*/
> >>>> OK, could you please also try if uncommenting the above still works without OOM?
> >>>> Or just plain linux-next-20160817, I guess we don't need the printk's to test
> >>>> this difference.
> >>>>
> >>>> Thanks a lot!
> >>>> Vlastimil
> >>>>
> >>> With the two lines back in I had OOMs again. See the attached logs.
> >>
> >> Thanks for the confirmation.
> >>
> >> We however shouldn't disable the heuristic completely, so here's a compromise
> >> patch hooking into the new compaction priorities. Can you please test on top of
> >> linux-next?
> >>
> >> -----8<-----
> >> >From 0927cc2a4c6a3247111168eace9012c23d06f9db Mon Sep 17 00:00:00 2001
> >> From: Vlastimil Babka <vbabka@suse.cz>
> >> Date: Thu, 18 Aug 2016 16:01:14 +0200
> >> Subject: [PATCH] mm, compaction: make full priority ignore pageblock
> >> suitability
> >>
> >> Ralf-Peter Rohbeck has reported premature OOMs for order-2 allocations (stack)
> >> due to OOM rework in 4.7. In his scenario (parallel kernel build and dd writing
> >> to two drives) many pageblocks get marked as Unmovable and compaction free
> >> scanner struggles to isolate free pages. Joonsoo Kim pointed out that the free
> >> scanner skips pageblocks that are not movable to prevent filling them and
> >> forcing non-movable allocations to fallback to other pageblocks. Such heuristic
> >> makes sense to help prevent long-term fragmentation, but premature OOMs are
> >> relatively more urgent problem. As a compromise, this patch disables the
> >> heuristic only for the ultimate compaction priority.
> >>
> >> Reported-by: Ralf-Peter Rohbeck <Ralf-Peter.Rohbeck@quantum.com>
> >> Suggested-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> >> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> >
> > Thanks to both of you! I do agree that we should drop all these
> > heuristics when we struggle and there is an OOM risk. I have just a
> > small nit here. I would prefer
> > s@COMPACT_PRIO_SYNC_FULL@MIN_COMPACT_PRIORITY@ when disabling them
> > because this would be easier to follow and it would be easier for future
> > changes.
>
> OK, but then we should start with a change to
> mm-compaction-add-the-ultimate-direct-compaction-priority.patch
> (fix at the end of this e-mail) to make things consistent.
> Then I will apply that to the new patch if it's successfully tested.
This can go as a separate clean up patch. No need to alter previous
patches sitting in the mmotm.
> > Which brings me to another thing I was suggesting earlier. I
> > believe we should go to this MIN_COMPACT_PRIORITY only for !costly
> > requests because costly orders shouldn't get all those exceptions and
> > risk long term fragmentation issues. We do not have that many costly
> > requests (except for hugetlb) so it doesn't matter all that much right
> > now but long term we want to differentiate those I believe.
>
> I'll send such change afterwards as well.
Thanks!
> > That being said, let's wait for the feedback on this patch + linux-next.
> > If it works out I will send a stable 4.7 patch which drops compaction
> > feedback from should_compact_retry (turn it to the !COMPACTION version)
> > so that 4.7 users do not suffer from the premature OOM and will ask
> > Andrew to sneak the compaction patches to 4.8 as they fix a real issue
> > and the risk is not really high.
>
> Agreed.
>
> > Acked-by: Michal Hocko <mhocko@suse.com>
>
> Thanks!
>
> -----8<-----
> >From c4da7022e85e52f5463055cdc474656652e7a504 Mon Sep 17 00:00:00 2001
> From: Vlastimil Babka <vbabka@suse.cz>
> Date: Fri, 19 Aug 2016 09:40:31 +0200
> Subject: [PATCH] mm, compaction: add the ultimate direct compaction
> priority-fix
>
> Use the MIN_COMPACT_PRIORITY alias instead of COMPACT_PRIO_SYNC_FULL to
> disable heuristics "because this would be easier to follow and it would be
> easier for future changes", per Michal.
>
> Suggested-by: Michal Hocko <mhocko@suse.cz>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> Fixes: mmotm mm-compaction-add-the-ultimate-direct-compaction-priority.patch
I guess Fixes is a bit misleading. This is not a bug it is a cleanup
patch.
Acked-by: Michal Hocko <mhocko@suse.com>
Thanks!
> ---
> mm/compaction.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index ae4f40afcca1..3e35fce2cace 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -1644,8 +1644,8 @@ static enum compact_result compact_zone_order(struct zone *zone, int order,
> .alloc_flags = alloc_flags,
> .classzone_idx = classzone_idx,
> .direct_compaction = true,
> - .whole_zone = (prio == COMPACT_PRIO_SYNC_FULL),
> - .ignore_skip_hint = (prio == COMPACT_PRIO_SYNC_FULL)
> + .whole_zone = (prio == MIN_COMPACT_PRIORITY),
> + .ignore_skip_hint = (prio == MIN_COMPACT_PRIORITY)
> };
> INIT_LIST_HEAD(&cc.freepages);
> INIT_LIST_HEAD(&cc.migratepages);
> @@ -1691,7 +1691,7 @@ enum compact_result try_to_compact_pages(gfp_t gfp_mask, unsigned int order,
> ac->nodemask) {
> enum compact_result status;
>
> - if (prio > COMPACT_PRIO_SYNC_FULL
> + if (prio > MIN_COMPACT_PRIORITY
> && compaction_deferred(zone, order)) {
> rc = max_t(enum compact_result, COMPACT_DEFERRED, rc);
> continue;
> --
> 2.9.2
>
>
--
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-08-19 8:26 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <d8f3adcc-3607-1ef6-9ec5-82b2e125eef2@quantum.com>
2016-08-01 6:16 ` OOM killer changes Michal Hocko
[not found] ` <b1a39756-a0b5-1900-6575-d6e1f502cb26@Quantum.com>
[not found] ` <20160801182358.GB31957@dhcp22.suse.cz>
[not found] ` <30dbabc4-585c-55a5-9f3a-4e243c28356a@Quantum.com>
2016-08-01 19:26 ` Michal Hocko
2016-08-01 19:35 ` Ralf-Peter Rohbeck
2016-08-01 19:43 ` Michal Hocko
2016-08-01 19:52 ` Ralf-Peter Rohbeck
2016-08-01 20:09 ` Michal Hocko
2016-08-01 20:16 ` Ralf-Peter Rohbeck
2016-08-01 20:26 ` Michal Hocko
2016-08-01 21:14 ` Ralf-Peter Rohbeck
2016-08-01 21:27 ` Ralf-Peter Rohbeck
2016-08-02 7:10 ` Michal Hocko
2016-08-02 19:25 ` Ralf-Peter Rohbeck
2016-08-15 4:48 ` Ralf-Peter Rohbeck
2016-08-15 9:16 ` Vlastimil Babka
2016-08-15 15:01 ` Michal Hocko
2016-08-15 18:42 ` Ralf-Peter Rohbeck
2016-08-16 7:32 ` Michal Hocko
2016-08-16 7:43 ` Michal Hocko
2016-08-17 9:14 ` Ralf-Peter Rohbeck
2016-08-17 9:23 ` Vlastimil Babka
2016-08-17 9:28 ` Ralf-Peter Rohbeck
2016-08-17 9:33 ` Michal Hocko
2016-08-17 23:37 ` Ralf-Peter Rohbeck
2016-08-18 6:57 ` Vlastimil Babka
2016-08-18 20:01 ` Ralf-Peter Rohbeck
2016-08-18 20:12 ` Vlastimil Babka
2016-08-19 2:42 ` Ralf-Peter Rohbeck
2016-08-19 6:27 ` Vlastimil Babka
2016-08-19 7:33 ` Michal Hocko
2016-08-19 7:47 ` Vlastimil Babka
2016-08-19 8:26 ` Michal Hocko [this message]
2016-08-24 18:13 ` Ralf-Peter Rohbeck
2016-08-25 7:22 ` Michal Hocko
2016-08-25 20:35 ` Ralf-Peter Rohbeck
2016-08-26 8:35 ` Michal Hocko
2016-09-06 11:09 ` Vlastimil Babka
2016-08-23 5:02 ` Joonsoo Kim
2016-08-23 7:45 ` Michal Hocko
2016-08-17 0:26 ` Ralf-Peter Rohbeck
2016-08-17 7:43 ` Vlastimil Babka
2016-08-16 3:12 ` Joonsoo Kim
2016-08-16 7:44 ` Vlastimil Babka
2016-08-17 4:48 ` Ralf-Peter Rohbeck
2016-08-17 7:56 ` Vlastimil Babka
2016-08-17 8:16 ` Joonsoo Kim
2016-08-17 9:21 ` Ralf-Peter Rohbeck
2016-08-17 9:11 ` Ralf-Peter Rohbeck
2016-08-17 9:20 ` Vlastimil Babka
2016-08-02 7:11 ` Vlastimil Babka
2016-08-02 9:02 ` 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=20160819082639.GE32619@dhcp22.suse.cz \
--to=mhocko@suse.cz \
--cc=Ralf-Peter.Rohbeck@quantum.com \
--cc=akpm@linux-foundation.org \
--cc=iamjoonsoo.kim@lge.com \
--cc=linux-mm@kvack.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).