Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1351745 > unrolled thread
| Started by | Michal Hocko <mhocko@kernel.org> |
|---|---|
| First post | 2016-03-07 17:10 +0100 |
| Last post | 2016-03-11 16:30 +0100 |
| Articles | 20 on this page of 38 — 6 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH] mm, oom: protect !costly allocations some more (was: Re: [PATCH 0/3] OOM detection rework v4) Michal Hocko <mhocko@kernel.org> - 2016-03-07 17:10 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more (was: Re: [PATCH 0/3] OOM detection rework v4) Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-03-08 04:50 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more (was: Re: [PATCH 0/3] OOM detection rework v4) Michal Hocko <mhocko@kernel.org> - 2016-03-08 10:10 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more (was: Re: [PATCH 0/3] OOM detection rework v4) Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-03-08 10:30 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more Vlastimil Babka <vbabka@suse.cz> - 2016-03-08 10:30 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-03-08 10:40 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more Michal Hocko <mhocko@kernel.org> - 2016-03-08 10:50 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more Vlastimil Babka <vbabka@suse.cz> - 2016-03-08 11:00 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more Michal Hocko <mhocko@kernel.org> - 2016-03-08 11:20 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more Vlastimil Babka <vbabka@suse.cz> - 2016-03-08 12:20 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more Michal Hocko <mhocko@kernel.org> - 2016-03-08 13:30 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more Vlastimil Babka <vbabka@suse.cz> - 2016-03-08 13:40 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more (was: Re: [PATCH 0/3] OOM detection rework v4) Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-03-08 11:00 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more (was: Re: [PATCH 0/3] OOM detection rework v4) Michal Hocko <mhocko@kernel.org> - 2016-03-08 15:00 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more (was: Re: [PATCH 0/3] OOM detection rework v4) Hugh Dickins <hughd@google.com> - 2016-03-08 11:40 +0100
[PATCH 1/3] mm, compaction: change COMPACT_ constants into enum Michal Hocko <mhocko@kernel.org> - 2016-03-08 14:50 +0100
Re: [PATCH 1/3] mm, compaction: change COMPACT_ constants into enum Vlastimil Babka <vbabka@suse.cz> - 2016-03-08 15:20 +0100
Re: [PATCH 1/3] mm, compaction: change COMPACT_ constants into enum "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2016-03-09 05:00 +0100
[PATCH 3/3] mm, oom: protect !costly allocations some more Michal Hocko <mhocko@kernel.org> - 2016-03-08 14:50 +0100
Re: [PATCH 3/3] mm, oom: protect !costly allocations some more Vlastimil Babka <vbabka@suse.cz> - 2016-03-08 15:40 +0100
Re: [PATCH 3/3] mm, oom: protect !costly allocations some more Michal Hocko <mhocko@kernel.org> - 2016-03-08 15:50 +0100
Re: [PATCH 3/3] mm, oom: protect !costly allocations some more Vlastimil Babka <vbabka@suse.cz> - 2016-03-08 16:10 +0100
Re: [PATCH 3/3] mm, oom: protect !costly allocations some more Michal Hocko <mhocko@kernel.org> - 2016-03-09 12:20 +0100
Re: [PATCH 3/3] mm, oom: protect !costly allocations some more Vlastimil Babka <vbabka@suse.cz> - 2016-03-09 15:10 +0100
Re: [PATCH 3/3] mm, oom: protect !costly allocations some more Hugh Dickins <hughd@google.com> - 2016-03-11 13:20 +0100
Re: [PATCH 3/3] mm, oom: protect !costly allocations some more Michal Hocko <mhocko@kernel.org> - 2016-03-11 14:10 +0100
Re: [PATCH 3/3] mm, oom: protect !costly allocations some more Hugh Dickins <hughd@google.com> - 2016-03-11 20:10 +0100
Re: [PATCH 3/3] mm, oom: protect !costly allocations some more Michal Hocko <mhocko@kernel.org> - 2016-03-14 17:30 +0100
[PATCH 0/2] oom rework: high order enahncements Michal Hocko <mhocko@kernel.org> - 2016-03-08 14:50 +0100
[PATCH 2/3] mm, compaction: cover all compaction mode in compact_zone Michal Hocko <mhocko@kernel.org> - 2016-03-08 14:50 +0100
Re: [PATCH 2/3] mm, compaction: cover all compaction mode in compact_zone Vlastimil Babka <vbabka@suse.cz> - 2016-03-08 15:30 +0100
Re: [PATCH 2/3] mm, compaction: cover all compaction mode in compact_zone "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2016-03-09 05:00 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more (was: Re: [PATCH 0/3] OOM detection rework v4) Joonsoo Kim <js1304@gmail.com> - 2016-03-08 16:20 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more (was: Re: [PATCH 0/3] OOM detection rework v4) Michal Hocko <mhocko@kernel.org> - 2016-03-08 17:10 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more (was: Re: [PATCH 0/3] OOM detection rework v4) Joonsoo Kim <js1304@gmail.com> - 2016-03-08 18:10 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more (was: Re: [PATCH 0/3] OOM detection rework v4) Michal Hocko <mhocko@kernel.org> - 2016-03-09 11:50 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more (was: Re: [PATCH 0/3] OOM detection rework v4) Joonsoo Kim <js1304@gmail.com> - 2016-03-11 16:00 +0100
Re: [PATCH] mm, oom: protect !costly allocations some more (was: Re: [PATCH 0/3] OOM detection rework v4) Michal Hocko <mhocko@kernel.org> - 2016-03-11 16:30 +0100
Page 1 of 2 [1] 2 Next page →
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-03-07 17:10 +0100 |
| Subject | [PATCH] mm, oom: protect !costly allocations some more (was: Re: [PATCH 0/3] OOM detection rework v4) |
| Message-ID | <ra5Tk-543-7@gated-at.bofh.it> |
On Mon 29-02-16 22:02:13, Michal Hocko wrote:
> Andrew,
> could you queue this one as well, please? This is more a band aid than a
> real solution which I will be working on as soon as I am able to
> reproduce the issue but the patch should help to some degree at least.
Joonsoo wasn't very happy about this approach so let me try a different
way. What do you think about the following? Hugh, Sergey does it help
for your load? I have tested it with the Hugh's load and there was no
major difference from the previous testing so at least nothing has blown
up as I am not able to reproduce the issue here.
Other changes in the compaction are still needed but I would like to not
depend on them right now.
---
From 0974f127e8eb7fe53e65f3a8b398db57effe9755 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Mon, 7 Mar 2016 15:30:37 +0100
Subject: [PATCH] mm, oom: protect !costly allocations some more
should_reclaim_retry will give up retries for higher order allocations
if none of the eligible zones has any requested or higher order pages
available even if we pass the watermak check for order-0. This is done
because there is no guarantee that the reclaimable and currently free
pages will form the required order.
This can, however, lead to situations were the high-order request (e.g.
order-2 required for the stack allocation during fork) will trigger
OOM too early - e.g. after the first reclaim/compaction round. Such a
system would have to be highly fragmented and there is no guarantee
further reclaim/compaction attempts would help but at least make sure
that the compaction was active before we go OOM and keep retrying even
if should_reclaim_retry tells us to oom if the last compaction round
was either inactive (deferred, skipped or bailed out early due to
contention) or it told us to continue.
Additionally define COMPACT_NONE which reflects cases where the
compaction is completely disabled.
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
include/linux/compaction.h | 2 ++
mm/page_alloc.c | 41 ++++++++++++++++++++++++-----------------
2 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/include/linux/compaction.h b/include/linux/compaction.h
index 4cd4ddf64cc7..a4cec4a03f7d 100644
--- a/include/linux/compaction.h
+++ b/include/linux/compaction.h
@@ -1,6 +1,8 @@
#ifndef _LINUX_COMPACTION_H
#define _LINUX_COMPACTION_H
+/* compaction disabled */
+#define COMPACT_NONE -1
/* Return values for compact_zone() and try_to_compact_pages() */
/* compaction didn't start as it was deferred due to past failures */
#define COMPACT_DEFERRED 0
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 269a04f20927..f89e3cbfdf90 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2819,28 +2819,22 @@ static struct page *
__alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
int alloc_flags, const struct alloc_context *ac,
enum migrate_mode mode, int *contended_compaction,
- bool *deferred_compaction)
+ unsigned long *compact_result)
{
- unsigned long compact_result;
struct page *page;
- if (!order)
+ if (!order) {
+ *compact_result = COMPACT_NONE;
return NULL;
+ }
current->flags |= PF_MEMALLOC;
- compact_result = try_to_compact_pages(gfp_mask, order, alloc_flags, ac,
+ *compact_result = try_to_compact_pages(gfp_mask, order, alloc_flags, ac,
mode, contended_compaction);
current->flags &= ~PF_MEMALLOC;
- switch (compact_result) {
- case COMPACT_DEFERRED:
- *deferred_compaction = true;
- /* fall-through */
- case COMPACT_SKIPPED:
+ if (*compact_result <= COMPACT_SKIPPED)
return NULL;
- default:
- break;
- }
/*
* At least in one zone compaction wasn't deferred or skipped, so let's
@@ -2875,8 +2869,9 @@ static inline struct page *
__alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
int alloc_flags, const struct alloc_context *ac,
enum migrate_mode mode, int *contended_compaction,
- bool *deferred_compaction)
+ unsigned long *compact_result)
{
+ *compact_result = COMPACT_NONE;
return NULL;
}
#endif /* CONFIG_COMPACTION */
@@ -3118,7 +3113,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
int alloc_flags;
unsigned long did_some_progress;
enum migrate_mode migration_mode = MIGRATE_ASYNC;
- bool deferred_compaction = false;
+ unsigned long compact_result;
int contended_compaction = COMPACT_CONTENDED_NONE;
int no_progress_loops = 0;
@@ -3227,7 +3222,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
page = __alloc_pages_direct_compact(gfp_mask, order, alloc_flags, ac,
migration_mode,
&contended_compaction,
- &deferred_compaction);
+ &compact_result);
if (page)
goto got_pg;
@@ -3240,7 +3235,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
* to heavily disrupt the system, so we fail the allocation
* instead of entering direct reclaim.
*/
- if (deferred_compaction)
+ if (compact_result == COMPACT_DEFERRED)
goto nopage;
/*
@@ -3294,6 +3289,18 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
did_some_progress > 0, no_progress_loops))
goto retry;
+ /*
+ * !costly allocations are really important and we have to make sure
+ * the compaction wasn't deferred or didn't bail out early due to locks
+ * contention before we go OOM.
+ */
+ if (order && order <= PAGE_ALLOC_COSTLY_ORDER) {
+ if (compact_result <= COMPACT_CONTINUE)
+ goto retry;
+ if (contended_compaction > COMPACT_CONTENDED_NONE)
+ goto retry;
+ }
+
/* Reclaim has failed us, start killing things */
page = __alloc_pages_may_oom(gfp_mask, order, ac, &did_some_progress);
if (page)
@@ -3314,7 +3321,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
page = __alloc_pages_direct_compact(gfp_mask, order, alloc_flags,
ac, migration_mode,
&contended_compaction,
- &deferred_compaction);
+ &compact_result);
if (page)
goto got_pg;
nopage:
--
2.7.0
--
Michal Hocko
SUSE Labs
[toc] | [next] | [standalone]
| From | Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> |
|---|---|
| Date | 2016-03-08 04:50 +0100 |
| Message-ID | <ragOK-3ML-7@gated-at.bofh.it> |
| In reply to | #1351745 |
Hello Michal,
On (03/07/16 17:08), Michal Hocko wrote:
> On Mon 29-02-16 22:02:13, Michal Hocko wrote:
> > Andrew,
> > could you queue this one as well, please? This is more a band aid than a
> > real solution which I will be working on as soon as I am able to
> > reproduce the issue but the patch should help to some degree at least.
>
> Joonsoo wasn't very happy about this approach so let me try a different
> way. What do you think about the following? Hugh, Sergey does it help
> for your load? I have tested it with the Hugh's load and there was no
> major difference from the previous testing so at least nothing has blown
> up as I am not able to reproduce the issue here.
(next-20160307 + "[PATCH] mm, oom: protect !costly allocations some more")
seems it's significantly less likely to oom-kill now, but I still can see
something like this
[ 501.942745] coretemp-sensor invoked oom-killer: gfp_mask=0x27000c0(GFP_KERNEL_ACCOUNT|__GFP_NOTRACK), order=2, oom_score_adj=0
[ 501.942796] CPU: 3 PID: 409 Comm: coretemp-sensor Not tainted 4.5.0-rc6-next-20160307-dbg-00015-g8a56edd-dirty #250
[ 501.942801] 0000000000000000 ffff88013114fb88 ffffffff812364e9 0000000000000000
[ 501.942804] ffff88013114fd28 ffff88013114fbf8 ffffffff8113b11c ffff88013114fba8
[ 501.942807] ffffffff810835c1 ffff88013114fbc8 0000000000000206 ffffffff81a46de0
[ 501.942808] Call Trace:
[ 501.942813] [<ffffffff812364e9>] dump_stack+0x67/0x90
[ 501.942817] [<ffffffff8113b11c>] dump_header.isra.5+0x54/0x359
[ 501.942820] [<ffffffff810835c1>] ? trace_hardirqs_on+0xd/0xf
[ 501.942823] [<ffffffff810f97c2>] oom_kill_process+0x89/0x503
[ 501.942825] [<ffffffff810f9ffe>] out_of_memory+0x372/0x38d
[ 501.942827] [<ffffffff810fe5ae>] __alloc_pages_nodemask+0x9b6/0xa92
[ 501.942830] [<ffffffff810fe882>] alloc_kmem_pages_node+0x1b/0x1d
[ 501.942833] [<ffffffff81041f86>] copy_process.part.9+0xfe/0x17f4
[ 501.942835] [<ffffffff810858f6>] ? lock_acquire+0x10f/0x1a3
[ 501.942837] [<ffffffff8104380f>] _do_fork+0xbd/0x5da
[ 501.942838] [<ffffffff81083598>] ? trace_hardirqs_on_caller+0x16c/0x188
[ 501.942842] [<ffffffff81001a79>] ? do_syscall_64+0x18/0xe6
[ 501.942844] [<ffffffff81043db2>] SyS_clone+0x19/0x1b
[ 501.942845] [<ffffffff81001abb>] do_syscall_64+0x5a/0xe6
[ 501.942848] [<ffffffff8151245a>] entry_SYSCALL64_slow_path+0x25/0x25
[ 501.942850] Mem-Info:
[ 501.942853] active_anon:151312 inactive_anon:54791 isolated_anon:0
active_file:31213 inactive_file:302048 isolated_file:0
unevictable:0 dirty:44 writeback:221 unstable:0
slab_reclaimable:43570 slab_unreclaimable:5651
mapped:16660 shmem:29495 pagetables:2542 bounce:0
free:10884 free_pcp:214 free_cma:0
[ 501.942859] DMA free:14896kB min:28kB low:40kB high:52kB active_anon:0kB inactive_anon:0kB active_file:96kB inactive_file:104kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:15984kB managed:15900kB mlocked:0kB dirty:0kB writeback:0kB mapped:124kB shmem:0kB slab_reclaimable:28kB slab_unreclaimable:108kB kernel_stack:16kB pagetables:0kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
[ 501.942862] lowmem_reserve[]: 0 3031 3855 3855
[ 501.942867] DMA32 free:23664kB min:6232kB low:9332kB high:12432kB active_anon:516228kB inactive_anon:129136kB active_file:96508kB inactive_file:954780kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:3194880kB managed:3107512kB mlocked:0kB dirty:136kB writeback:440kB mapped:51816kB shmem:91488kB slab_reclaimable:129856kB slab_unreclaimable:13876kB kernel_stack:2160kB pagetables:7888kB unstable:0kB bounce:0kB free_pcp:724kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:128 all_unreclaimable? no
[ 501.942870] lowmem_reserve[]: 0 0 824 824
[ 501.942876] Normal free:4784kB min:1696kB low:2540kB high:3384kB active_anon:89020kB inactive_anon:90028kB active_file:28248kB inactive_file:253308kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:917504kB managed:844512kB mlocked:0kB dirty:40kB writeback:444kB mapped:14700kB shmem:26492kB slab_reclaimable:44396kB slab_unreclaimable:8620kB kernel_stack:1328kB pagetables:2280kB unstable:0kB bounce:0kB free_pcp:244kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:60 all_unreclaimable? no
[ 501.942879] lowmem_reserve[]: 0 0 0 0
[ 501.942902] DMA: 6*4kB (UME) 3*8kB (M) 2*16kB (UM) 3*32kB (ME) 2*64kB (ME) 2*128kB (ME) 2*256kB (UE) 3*512kB (UME) 2*1024kB (ME) 1*2048kB (E) 2*4096kB (M) = 14896kB
[ 501.942912] DMA32: 564*4kB (UME) 2700*8kB (UM) 0*16kB 0*32kB 0*64kB 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 23856kB
[ 501.942921] Normal: 959*4kB (ME) 128*8kB (UM) 0*16kB 0*32kB 0*64kB 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 4860kB
[ 501.942922] Node 0 hugepages_total=0 hugepages_free=0 hugepages_surp=0 hugepages_size=2048kB
[ 501.942923] 362670 total pagecache pages
[ 501.942924] 0 pages in swap cache
[ 501.942926] Swap cache stats: add 150, delete 150, find 0/0
[ 501.942926] Free swap = 8388504kB
[ 501.942927] Total swap = 8388604kB
[ 501.942928] 1032092 pages RAM
[ 501.942928] 0 pages HighMem/MovableOnly
[ 501.942929] 40111 pages reserved
[ 501.942930] 0 pages hwpoisoned
[ 501.942930] [ pid ] uid tgid total_vm rss nr_ptes nr_pmds swapents oom_score_adj name
[ 501.942935] [ 162] 0 162 15823 1065 35 3 0 0 systemd-journal
[ 501.942991] [ 186] 0 186 8586 1054 19 3 0 -1000 systemd-udevd
[ 501.942993] [ 287] 0 287 3557 651 12 3 0 0 crond
[ 501.942995] [ 288] 81 288 8159 775 20 3 0 -900 dbus-daemon
[ 501.942997] [ 289] 0 289 3843 518 13 3 0 0 systemd-logind
[ 501.942999] [ 294] 0 294 22455 856 47 3 0 0 login
[ 501.943001] [ 302] 1000 302 8481 1029 20 3 0 0 systemd
[ 501.943003] [ 304] 1000 304 24212 438 47 3 0 0 (sd-pam)
[ 501.943005] [ 309] 1000 309 4431 1123 14 3 0 0 bash
[ 501.943007] [ 316] 1000 316 3712 764 13 3 0 0 startx
[ 501.943009] [ 338] 1000 338 3976 255 14 3 0 0 xinit
[ 501.943012] [ 339] 1000 339 44397 11311 90 3 0 0 Xorg
[ 501.943014] [ 341] 1000 341 39703 4045 78 3 0 0 openbox
[ 501.943016] [ 352] 1000 352 43465 2997 86 4 0 0 tint2
[ 501.943018] [ 355] 1000 355 33962 4351 57 3 0 0 urxvt
[ 501.943020] [ 356] 1000 356 4466 1155 13 3 0 0 bash
[ 501.943022] [ 359] 1000 359 4433 1116 13 3 0 0 bash
[ 501.943024] [ 364] 1000 364 49365 6236 62 3 0 0 urxvt
[ 501.943026] [ 365] 1000 365 4433 1093 15 3 0 0 bash
[ 501.943028] [ 368] 1000 368 5203 745 15 3 0 0 tmux
[ 501.943030] [ 370] 1000 370 6336 1374 17 3 0 0 tmux
[ 501.943046] [ 371] 1000 371 4433 1100 14 3 0 0 bash
[ 501.943049] [ 378] 1000 378 4433 1115 13 3 0 0 bash
[ 501.943051] [ 381] 1000 381 5203 763 16 3 0 0 tmux
[ 501.943053] [ 382] 1000 382 4433 1089 15 3 0 0 bash
[ 501.943055] [ 389] 1000 389 4433 1078 15 3 0 0 bash
[ 501.943057] [ 392] 1000 392 4433 1078 15 3 0 0 bash
[ 501.943058] [ 395] 1000 395 4433 1090 14 3 0 0 bash
[ 501.943060] [ 398] 1000 398 4433 1111 14 3 0 0 bash
[ 501.943062] [ 401] 1000 401 10126 1010 25 3 0 0 top
[ 501.943064] [ 403] 1000 403 4433 1129 14 3 0 0 bash
[ 501.943066] [ 409] 1000 409 3740 786 13 3 0 0 coretemp-sensor
[ 501.943069] [ 443] 1000 443 25873 3141 51 3 0 0 urxvt
[ 501.943071] [ 444] 1000 444 4433 1110 13 3 0 0 bash
[ 501.943073] [ 447] 1000 447 68144 55547 138 3 0 0 mutt
[ 501.943075] [ 450] 1000 450 29966 3825 51 3 0 0 urxvt
[ 501.943077] [ 451] 1000 451 4433 1117 14 3 0 0 bash
[ 501.943079] [ 456] 1000 456 29967 3793 53 3 0 0 urxvt
[ 501.943081] [ 457] 1000 457 4433 1085 14 3 0 0 bash
[ 501.943083] [ 462] 1000 462 29967 3845 51 4 0 0 urxvt
[ 501.943085] [ 463] 1000 463 4433 1093 14 3 0 0 bash
[ 501.943087] [ 468] 1000 468 29967 3793 50 3 0 0 urxvt
[ 501.943089] [ 469] 1000 469 4433 1086 15 3 0 0 bash
[ 501.943091] [ 493] 1000 493 52976 6416 69 3 0 0 urxvt
[ 501.943093] [ 494] 1000 494 4433 1106 14 3 0 0 bash
[ 501.943095] [ 499] 1000 499 29966 3792 54 3 0 0 urxvt
[ 501.943097] [ 500] 1000 500 4433 1078 14 3 0 0 bash
[ 501.943099] [ 525] 0 525 17802 1108 38 3 0 0 sudo
[ 501.943101] [ 528] 0 528 186583 768 207 4 0 0 journalctl
[ 501.943103] [ 550] 1000 550 42144 9259 66 4 0 0 urxvt
[ 501.943105] [ 551] 1000 551 4433 1067 14 4 0 0 bash
[ 501.943107] [ 557] 1000 557 11115 768 27 3 0 0 su
[ 501.943109] [ 579] 0 579 4462 1148 13 3 0 0 bash
[ 501.943111] [ 963] 1000 963 4433 1075 14 3 0 0 bash
[ 501.943113] [ 981] 1000 981 4433 1114 13 3 0 0 bash
[ 501.943115] [ 993] 1000 993 4432 1118 14 3 0 0 bash
[ 501.943117] [ 1062] 1000 1062 5203 734 15 3 0 0 tmux
[ 501.943119] [ 1063] 1000 1063 13805 10479 32 3 0 0 bash
[ 501.943121] [ 1145] 1000 1145 4466 1144 14 3 0 0 bash
[ 501.943123] [ 4331] 1000 4331 287422 64040 429 4 0 0 firefox
[ 501.943125] [ 4440] 1000 4440 8132 761 20 3 0 0 dbus-daemon
[ 501.943127] [ 4470] 1000 4470 83823 934 31 4 0 0 at-spi-bus-laun
[ 501.943129] [17875] 1000 17875 7549 1926 20 3 0 0 vim
[ 501.943131] [27066] 1000 27066 4432 1120 15 3 0 0 bash
[ 501.943133] [27073] 1000 27073 4432 1071 13 3 0 0 bash
[ 501.943135] [27079] 1000 27079 4432 1077 15 3 0 0 bash
[ 501.943137] [27085] 1000 27085 4432 1080 14 3 0 0 bash
[ 501.943139] [27091] 1000 27091 4432 1091 14 3 0 0 bash
[ 501.943141] [27097] 1000 27097 4432 1096 15 3 0 0 bash
[ 501.943143] [ 1235] 0 1235 3745 809 11 3 0 0 zram-test.sh
[ 501.943145] [ 2316] 1000 2316 1759 166 9 3 0 0 sleep
[ 501.943147] [ 2323] 0 2323 3302 1946 12 3 0 0 dd
[ 501.943148] Out of memory: Kill process 4331 (firefox) score 20 or sacrifice child
[ 501.943352] Killed process 4331 (firefox) total-vm:1149688kB, anon-rss:207844kB, file-rss:48172kB, shmem-rss:516kB
-ss
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-03-08 10:10 +0100 |
| Message-ID | <ralOp-7hm-5@gated-at.bofh.it> |
| In reply to | #1352601 |
On Tue 08-03-16 12:51:04, Sergey Senozhatsky wrote: > Hello Michal, > > On (03/07/16 17:08), Michal Hocko wrote: > > On Mon 29-02-16 22:02:13, Michal Hocko wrote: > > > Andrew, > > > could you queue this one as well, please? This is more a band aid than a > > > real solution which I will be working on as soon as I am able to > > > reproduce the issue but the patch should help to some degree at least. > > > > Joonsoo wasn't very happy about this approach so let me try a different > > way. What do you think about the following? Hugh, Sergey does it help > > for your load? I have tested it with the Hugh's load and there was no > > major difference from the previous testing so at least nothing has blown > > up as I am not able to reproduce the issue here. > > (next-20160307 + "[PATCH] mm, oom: protect !costly allocations some more") > > seems it's significantly less likely to oom-kill now, but I still can see > something like this Thanks for the testing. This is highly appreciated. If you are able to reproduce this then collecting compaction related tracepoints might be really helpful. > [ 501.942745] coretemp-sensor invoked oom-killer: gfp_mask=0x27000c0(GFP_KERNEL_ACCOUNT|__GFP_NOTRACK), order=2, oom_score_adj=0 [...] > [ 501.942853] active_anon:151312 inactive_anon:54791 isolated_anon:0 > active_file:31213 inactive_file:302048 isolated_file:0 > unevictable:0 dirty:44 writeback:221 unstable:0 > slab_reclaimable:43570 slab_unreclaimable:5651 > mapped:16660 shmem:29495 pagetables:2542 bounce:0 > free:10884 free_pcp:214 free_cma:0 [...] > [ 501.942867] DMA32 free:23664kB min:6232kB low:9332kB high:12432kB active_anon:516228kB inactive_anon:129136kB active_file:96508kB inactive_file:954780kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:3194880kB managed:3107512kB mlocked:0kB dirty:136kB writeback:440kB mapped:51816kB shmem:91488kB slab_reclaimable:129856kB slab_unreclaimable:13876kB kernel_stack:2160kB pagetables:7888kB unstable:0kB bounce:0kB free_pcp:724kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:128 all_unreclaimable? no > [ 501.942870] lowmem_reserve[]: 0 0 824 824 > [ 501.942876] Normal free:4784kB min:1696kB low:2540kB high:3384kB active_anon:89020kB inactive_anon:90028kB active_file:28248kB inactive_file:253308kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:917504kB managed:844512kB mlocked:0kB dirty:40kB writeback:444kB mapped:14700kB shmem:26492kB slab_reclaimable:44396kB slab_unreclaimable:8620kB kernel_stack:1328kB pagetables:2280kB unstable:0kB bounce:0kB free_pcp:244kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:60 all_unreclaimable? no Both DMA32 and Normal zones are over high watermarks so this OOM is due to the memory fragmentation. > [ 501.942912] DMA32: 564*4kB (UME) 2700*8kB (UM) 0*16kB 0*32kB 0*64kB 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 23856kB > [ 501.942921] Normal: 959*4kB (ME) 128*8kB (UM) 0*16kB 0*32kB 0*64kB 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 4860kB There are no order-2+ pages usable even after we know that the compaction was active and didn't back out early. I might be missing something of course and the patch might still be tweaked to be more conservative. Tracepoints should tell us more though. Thanks! -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> |
|---|---|
| Date | 2016-03-08 10:30 +0100 |
| Message-ID | <ram7N-7oJ-23@gated-at.bofh.it> |
| In reply to | #1352768 |
On (03/08/16 10:08), Michal Hocko wrote: > On Tue 08-03-16 12:51:04, Sergey Senozhatsky wrote: > > Hello Michal, > > > > On (03/07/16 17:08), Michal Hocko wrote: > > > On Mon 29-02-16 22:02:13, Michal Hocko wrote: > > > > Andrew, > > > > could you queue this one as well, please? This is more a band aid than a > > > > real solution which I will be working on as soon as I am able to > > > > reproduce the issue but the patch should help to some degree at least. > > > > > > Joonsoo wasn't very happy about this approach so let me try a different > > > way. What do you think about the following? Hugh, Sergey does it help > > > for your load? I have tested it with the Hugh's load and there was no > > > major difference from the previous testing so at least nothing has blown > > > up as I am not able to reproduce the issue here. > > > > (next-20160307 + "[PATCH] mm, oom: protect !costly allocations some more") > > > > seems it's significantly less likely to oom-kill now, but I still can see > > something like this > > Thanks for the testing. This is highly appreciated. If you are able to > reproduce this then collecting compaction related tracepoints might be > really helpful. > oh, wow... compaction is disabled, somehow. $ zcat /proc/config.gz | grep -i CONFIG_COMPACTION # CONFIG_COMPACTION is not set I should have checked that, sorry! will enable and re-test. -ss
[toc] | [prev] | [next] | [standalone]
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2016-03-08 10:30 +0100 |
| Subject | Re: [PATCH] mm, oom: protect !costly allocations some more |
| Message-ID | <ram7M-7oJ-17@gated-at.bofh.it> |
| In reply to | #1351745 |
On 03/07/2016 05:08 PM, Michal Hocko wrote:
> On Mon 29-02-16 22:02:13, Michal Hocko wrote:
>> Andrew,
>> could you queue this one as well, please? This is more a band aid than a
>> real solution which I will be working on as soon as I am able to
>> reproduce the issue but the patch should help to some degree at least.
>
> Joonsoo wasn't very happy about this approach so let me try a different
> way. What do you think about the following? Hugh, Sergey does it help
> for your load? I have tested it with the Hugh's load and there was no
> major difference from the previous testing so at least nothing has blown
> up as I am not able to reproduce the issue here.
>
> Other changes in the compaction are still needed but I would like to not
> depend on them right now.
> ---
> From 0974f127e8eb7fe53e65f3a8b398db57effe9755 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Mon, 7 Mar 2016 15:30:37 +0100
> Subject: [PATCH] mm, oom: protect !costly allocations some more
>
> should_reclaim_retry will give up retries for higher order allocations
> if none of the eligible zones has any requested or higher order pages
> available even if we pass the watermak check for order-0. This is done
> because there is no guarantee that the reclaimable and currently free
> pages will form the required order.
>
> This can, however, lead to situations were the high-order request (e.g.
> order-2 required for the stack allocation during fork) will trigger
> OOM too early - e.g. after the first reclaim/compaction round. Such a
> system would have to be highly fragmented and there is no guarantee
> further reclaim/compaction attempts would help but at least make sure
> that the compaction was active before we go OOM and keep retrying even
> if should_reclaim_retry tells us to oom if the last compaction round
> was either inactive (deferred, skipped or bailed out early due to
> contention) or it told us to continue.
>
> Additionally define COMPACT_NONE which reflects cases where the
> compaction is completely disabled.
>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
> include/linux/compaction.h | 2 ++
> mm/page_alloc.c | 41 ++++++++++++++++++++++++-----------------
> 2 files changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/compaction.h b/include/linux/compaction.h
> index 4cd4ddf64cc7..a4cec4a03f7d 100644
> --- a/include/linux/compaction.h
> +++ b/include/linux/compaction.h
> @@ -1,6 +1,8 @@
> #ifndef _LINUX_COMPACTION_H
> #define _LINUX_COMPACTION_H
>
> +/* compaction disabled */
> +#define COMPACT_NONE -1
> /* Return values for compact_zone() and try_to_compact_pages() */
> /* compaction didn't start as it was deferred due to past failures */
> #define COMPACT_DEFERRED 0
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 269a04f20927..f89e3cbfdf90 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2819,28 +2819,22 @@ static struct page *
> __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
> int alloc_flags, const struct alloc_context *ac,
> enum migrate_mode mode, int *contended_compaction,
> - bool *deferred_compaction)
> + unsigned long *compact_result)
> {
> - unsigned long compact_result;
> struct page *page;
>
> - if (!order)
> + if (!order) {
> + *compact_result = COMPACT_NONE;
> return NULL;
> + }
>
> current->flags |= PF_MEMALLOC;
> - compact_result = try_to_compact_pages(gfp_mask, order, alloc_flags, ac,
> + *compact_result = try_to_compact_pages(gfp_mask, order, alloc_flags, ac,
> mode, contended_compaction);
> current->flags &= ~PF_MEMALLOC;
>
> - switch (compact_result) {
> - case COMPACT_DEFERRED:
> - *deferred_compaction = true;
> - /* fall-through */
> - case COMPACT_SKIPPED:
> + if (*compact_result <= COMPACT_SKIPPED)
COMPACT_NONE is -1 and compact_result is unsigned long, so this won't
work as expected.
> return NULL;
> - default:
> - break;
> - }
>
> /*
> * At least in one zone compaction wasn't deferred or skipped, so let's
> @@ -2875,8 +2869,9 @@ static inline struct page *
> __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
> int alloc_flags, const struct alloc_context *ac,
> enum migrate_mode mode, int *contended_compaction,
> - bool *deferred_compaction)
> + unsigned long *compact_result)
> {
> + *compact_result = COMPACT_NONE;
> return NULL;
> }
> #endif /* CONFIG_COMPACTION */
> @@ -3118,7 +3113,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> int alloc_flags;
> unsigned long did_some_progress;
> enum migrate_mode migration_mode = MIGRATE_ASYNC;
> - bool deferred_compaction = false;
> + unsigned long compact_result;
> int contended_compaction = COMPACT_CONTENDED_NONE;
> int no_progress_loops = 0;
>
> @@ -3227,7 +3222,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> page = __alloc_pages_direct_compact(gfp_mask, order, alloc_flags, ac,
> migration_mode,
> &contended_compaction,
> - &deferred_compaction);
> + &compact_result);
> if (page)
> goto got_pg;
>
> @@ -3240,7 +3235,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> * to heavily disrupt the system, so we fail the allocation
> * instead of entering direct reclaim.
> */
> - if (deferred_compaction)
> + if (compact_result == COMPACT_DEFERRED)
> goto nopage;
>
> /*
> @@ -3294,6 +3289,18 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> did_some_progress > 0, no_progress_loops))
> goto retry;
>
> + /*
> + * !costly allocations are really important and we have to make sure
> + * the compaction wasn't deferred or didn't bail out early due to locks
> + * contention before we go OOM.
> + */
> + if (order && order <= PAGE_ALLOC_COSTLY_ORDER) {
> + if (compact_result <= COMPACT_CONTINUE)
Same here.
I was going to say that this didn't have effect on Sergey's test, but
turns out it did :)
> + goto retry;
> + if (contended_compaction > COMPACT_CONTENDED_NONE)
> + goto retry;
> + }
> +
> /* Reclaim has failed us, start killing things */
> page = __alloc_pages_may_oom(gfp_mask, order, ac, &did_some_progress);
> if (page)
> @@ -3314,7 +3321,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> page = __alloc_pages_direct_compact(gfp_mask, order, alloc_flags,
> ac, migration_mode,
> &contended_compaction,
> - &deferred_compaction);
> + &compact_result);
> if (page)
> goto got_pg;
> nopage:
>
[toc] | [prev] | [next] | [standalone]
| From | Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> |
|---|---|
| Date | 2016-03-08 10:40 +0100 |
| Subject | Re: [PATCH] mm, oom: protect !costly allocations some more |
| Message-ID | <ramht-7tc-23@gated-at.bofh.it> |
| In reply to | #1352788 |
On (03/08/16 10:24), Vlastimil Babka wrote:
[..]
> > @@ -3294,6 +3289,18 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> > did_some_progress > 0, no_progress_loops))
> > goto retry;
> >
> > + /*
> > + * !costly allocations are really important and we have to make sure
> > + * the compaction wasn't deferred or didn't bail out early due to locks
> > + * contention before we go OOM.
> > + */
> > + if (order && order <= PAGE_ALLOC_COSTLY_ORDER) {
> > + if (compact_result <= COMPACT_CONTINUE)
>
> Same here.
> I was going to say that this didn't have effect on Sergey's test, but
> turns out it did :)
I'm sorry, my test is not correct. I have disabled compaction last weeked on
purpose - to provoke more OOM-kills and OOM conditions for reworked printk()
patch set testing (http://marc.info/?l=linux-kernel&m=145734549308803); and I
forgot to re-enable it.
-ss
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-03-08 10:50 +0100 |
| Subject | Re: [PATCH] mm, oom: protect !costly allocations some more |
| Message-ID | <ramr9-7wy-23@gated-at.bofh.it> |
| In reply to | #1352788 |
On Tue 08-03-16 10:24:56, Vlastimil Babka wrote:
[...]
> > @@ -2819,28 +2819,22 @@ static struct page *
> > __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
> > int alloc_flags, const struct alloc_context *ac,
> > enum migrate_mode mode, int *contended_compaction,
> > - bool *deferred_compaction)
> > + unsigned long *compact_result)
> > {
> > - unsigned long compact_result;
> > struct page *page;
> >
> > - if (!order)
> > + if (!order) {
> > + *compact_result = COMPACT_NONE;
> > return NULL;
> > + }
> >
> > current->flags |= PF_MEMALLOC;
> > - compact_result = try_to_compact_pages(gfp_mask, order, alloc_flags, ac,
> > + *compact_result = try_to_compact_pages(gfp_mask, order, alloc_flags, ac,
> > mode, contended_compaction);
> > current->flags &= ~PF_MEMALLOC;
> >
> > - switch (compact_result) {
> > - case COMPACT_DEFERRED:
> > - *deferred_compaction = true;
> > - /* fall-through */
> > - case COMPACT_SKIPPED:
> > + if (*compact_result <= COMPACT_SKIPPED)
>
> COMPACT_NONE is -1 and compact_result is unsigned long, so this won't
> work as expected.
Well, COMPACT_NONE is documented as /* compaction disabled */ so we
should never get it from try_to_compact_pages.
[...]
> > @@ -3294,6 +3289,18 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> > did_some_progress > 0, no_progress_loops))
> > goto retry;
> >
> > + /*
> > + * !costly allocations are really important and we have to make sure
> > + * the compaction wasn't deferred or didn't bail out early due to locks
> > + * contention before we go OOM.
> > + */
> > + if (order && order <= PAGE_ALLOC_COSTLY_ORDER) {
> > + if (compact_result <= COMPACT_CONTINUE)
>
> Same here.
> I was going to say that this didn't have effect on Sergey's test, but
> turns out it did :)
This should work as expected because compact_result is unsigned long
and so this is the unsigned arithmetic. I can make
#define COMPACT_NONE -1UL
to make the intention more obvious if you prefer, though.
Thanks for the review.
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2016-03-08 11:00 +0100 |
| Subject | Re: [PATCH] mm, oom: protect !costly allocations some more |
| Message-ID | <ramAP-7A1-3@gated-at.bofh.it> |
| In reply to | #1352808 |
On 03/08/2016 10:46 AM, Michal Hocko wrote:
> On Tue 08-03-16 10:24:56, Vlastimil Babka wrote:
> [...]
>>> @@ -2819,28 +2819,22 @@ static struct page *
>>> __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
>>> int alloc_flags, const struct alloc_context *ac,
>>> enum migrate_mode mode, int *contended_compaction,
>>> - bool *deferred_compaction)
>>> + unsigned long *compact_result)
>>> {
>>> - unsigned long compact_result;
>>> struct page *page;
>>>
>>> - if (!order)
>>> + if (!order) {
>>> + *compact_result = COMPACT_NONE;
>>> return NULL;
>>> + }
>>>
>>> current->flags |= PF_MEMALLOC;
>>> - compact_result = try_to_compact_pages(gfp_mask, order, alloc_flags, ac,
>>> + *compact_result = try_to_compact_pages(gfp_mask, order, alloc_flags, ac,
>>> mode, contended_compaction);
>>> current->flags &= ~PF_MEMALLOC;
>>>
>>> - switch (compact_result) {
>>> - case COMPACT_DEFERRED:
>>> - *deferred_compaction = true;
>>> - /* fall-through */
>>> - case COMPACT_SKIPPED:
>>> + if (*compact_result <= COMPACT_SKIPPED)
>>
>> COMPACT_NONE is -1 and compact_result is unsigned long, so this won't
>> work as expected.
>
> Well, COMPACT_NONE is documented as /* compaction disabled */ so we
> should never get it from try_to_compact_pages.
Right.
>
> [...]
>>> @@ -3294,6 +3289,18 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>>> did_some_progress > 0, no_progress_loops))
>>> goto retry;
>>>
>>> + /*
>>> + * !costly allocations are really important and we have to make sure
>>> + * the compaction wasn't deferred or didn't bail out early due to locks
>>> + * contention before we go OOM.
>>> + */
>>> + if (order && order <= PAGE_ALLOC_COSTLY_ORDER) {
>>> + if (compact_result <= COMPACT_CONTINUE)
>>
>> Same here.
>> I was going to say that this didn't have effect on Sergey's test, but
>> turns out it did :)
>
> This should work as expected because compact_result is unsigned long
> and so this is the unsigned arithmetic. I can make
> #define COMPACT_NONE -1UL
>
> to make the intention more obvious if you prefer, though.
Well, what wasn't obvious to me is actually that here (unlike in the
test above) it was actually intended that COMPACT_NONE doesn't result in
a retry. But it makes sense, otherwise we would retry endlessly if
reclaim couldn't form a higher-order page, right.
> Thanks for the review.
>
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-03-08 11:20 +0100 |
| Subject | Re: [PATCH] mm, oom: protect !costly allocations some more |
| Message-ID | <ramUa-7VY-19@gated-at.bofh.it> |
| In reply to | #1352810 |
On Tue 08-03-16 10:52:15, Vlastimil Babka wrote:
> On 03/08/2016 10:46 AM, Michal Hocko wrote:
[...]
> >>> @@ -3294,6 +3289,18 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> >>> did_some_progress > 0, no_progress_loops))
> >>> goto retry;
> >>>
> >>> + /*
> >>> + * !costly allocations are really important and we have to make sure
> >>> + * the compaction wasn't deferred or didn't bail out early due to locks
> >>> + * contention before we go OOM.
> >>> + */
> >>> + if (order && order <= PAGE_ALLOC_COSTLY_ORDER) {
> >>> + if (compact_result <= COMPACT_CONTINUE)
> >>
> >> Same here.
> >> I was going to say that this didn't have effect on Sergey's test, but
> >> turns out it did :)
> >
> > This should work as expected because compact_result is unsigned long
> > and so this is the unsigned arithmetic. I can make
> > #define COMPACT_NONE -1UL
> >
> > to make the intention more obvious if you prefer, though.
>
> Well, what wasn't obvious to me is actually that here (unlike in the
> test above) it was actually intended that COMPACT_NONE doesn't result in
> a retry. But it makes sense, otherwise we would retry endlessly if
> reclaim couldn't form a higher-order page, right.
Yeah, that was the whole point. An alternative would be moving the test
into should_compact_retry(order, compact_result, contended_compaction)
which would be CONFIG_COMPACTION specific so we can get rid of the
COMPACT_NONE altogether. Something like the following. We would lose the
always initialized compact_result but this would matter only for
order==0 and we check for that. Even gcc doesn't complain.
A more important question is whether the criteria I have chosen are
reasonable and reasonably independent on the particular implementation
of the compaction. I still cannot convince myself about the convergence
here. Is it possible that the compaction would keep returning
compact_result <= COMPACT_CONTINUE while not making any progress at all?
Sure we can see a case where somebody is stealing the compacted blocks
but that is very same with the order-0 where parallel mem eaters will
piggy back on the reclaimer and there is no upper boundary as well well.
---
diff --git a/include/linux/compaction.h b/include/linux/compaction.h
index a4cec4a03f7d..4cd4ddf64cc7 100644
--- a/include/linux/compaction.h
+++ b/include/linux/compaction.h
@@ -1,8 +1,6 @@
#ifndef _LINUX_COMPACTION_H
#define _LINUX_COMPACTION_H
-/* compaction disabled */
-#define COMPACT_NONE -1
/* Return values for compact_zone() and try_to_compact_pages() */
/* compaction didn't start as it was deferred due to past failures */
#define COMPACT_DEFERRED 0
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f89e3cbfdf90..c5932a218fc6 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2823,10 +2823,8 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
{
struct page *page;
- if (!order) {
- *compact_result = COMPACT_NONE;
+ if (!order)
return NULL;
- }
current->flags |= PF_MEMALLOC;
*compact_result = try_to_compact_pages(gfp_mask, order, alloc_flags, ac,
@@ -2864,6 +2862,25 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
return NULL;
}
+
+static inline bool
+should_compact_retry(unsigned int order, unsigned long compact_result,
+ int contended_compaction)
+{
+ /*
+ * !costly allocations are really important and we have to make sure
+ * the compaction wasn't deferred or didn't bail out early due to locks
+ * contention before we go OOM.
+ */
+ if (order && order <= PAGE_ALLOC_COSTLY_ORDER) {
+ if (compact_result <= COMPACT_CONTINUE)
+ return true;
+ if (contended_compaction > COMPACT_CONTENDED_NONE)
+ return true;
+ }
+
+ return false;
+}
#else
static inline struct page *
__alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
@@ -2871,9 +2888,15 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
enum migrate_mode mode, int *contended_compaction,
unsigned long *compact_result)
{
- *compact_result = COMPACT_NONE;
return NULL;
}
+
+static inline bool
+should_compact_retry(unsigned int order, unsigned long compact_result,
+ int contended_compaction)
+{
+ return false;
+}
#endif /* CONFIG_COMPACTION */
/* Perform direct synchronous page reclaim */
@@ -3289,17 +3312,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
did_some_progress > 0, no_progress_loops))
goto retry;
- /*
- * !costly allocations are really important and we have to make sure
- * the compaction wasn't deferred or didn't bail out early due to locks
- * contention before we go OOM.
- */
- if (order && order <= PAGE_ALLOC_COSTLY_ORDER) {
- if (compact_result <= COMPACT_CONTINUE)
- goto retry;
- if (contended_compaction > COMPACT_CONTENDED_NONE)
- goto retry;
- }
+ if (should_compact_retry(order, compact_result, contended_compaction))
+ goto retry;
/* Reclaim has failed us, start killing things */
page = __alloc_pages_may_oom(gfp_mask, order, ac, &did_some_progress);
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2016-03-08 12:20 +0100 |
| Subject | Re: [PATCH] mm, oom: protect !costly allocations some more |
| Message-ID | <ranQe-5m-33@gated-at.bofh.it> |
| In reply to | #1352828 |
On 03/08/2016 11:10 AM, Michal Hocko wrote:
> On Tue 08-03-16 10:52:15, Vlastimil Babka wrote:
>> On 03/08/2016 10:46 AM, Michal Hocko wrote:
> [...]
>>>>> @@ -3294,6 +3289,18 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>>>>> did_some_progress > 0, no_progress_loops))
>>>>> goto retry;
>>>>>
>>>>> + /*
>>>>> + * !costly allocations are really important and we have to make sure
>>>>> + * the compaction wasn't deferred or didn't bail out early due to locks
>>>>> + * contention before we go OOM.
>>>>> + */
>>>>> + if (order && order <= PAGE_ALLOC_COSTLY_ORDER) {
>>>>> + if (compact_result <= COMPACT_CONTINUE)
>>>>
>>>> Same here.
>>>> I was going to say that this didn't have effect on Sergey's test, but
>>>> turns out it did :)
>>>
>>> This should work as expected because compact_result is unsigned long
>>> and so this is the unsigned arithmetic. I can make
>>> #define COMPACT_NONE -1UL
>>>
>>> to make the intention more obvious if you prefer, though.
>>
>> Well, what wasn't obvious to me is actually that here (unlike in the
>> test above) it was actually intended that COMPACT_NONE doesn't result in
>> a retry. But it makes sense, otherwise we would retry endlessly if
>> reclaim couldn't form a higher-order page, right.
>
> Yeah, that was the whole point. An alternative would be moving the test
> into should_compact_retry(order, compact_result, contended_compaction)
> which would be CONFIG_COMPACTION specific so we can get rid of the
> COMPACT_NONE altogether. Something like the following. We would lose the
> always initialized compact_result but this would matter only for
> order==0 and we check for that. Even gcc doesn't complain.
Yeah I like this version better, you can add my Acked-By.
Thanks.
> A more important question is whether the criteria I have chosen are
> reasonable and reasonably independent on the particular implementation
> of the compaction. I still cannot convince myself about the convergence
> here. Is it possible that the compaction would keep returning
> compact_result <= COMPACT_CONTINUE while not making any progress at all?
Theoretically, if reclaim/compaction suitability decisions and
allocation attempts didn't match the watermark checks, including the
alloc_flags and classzone_idx parameters. Possible scenarios:
- reclaim thinks compaction has enough to proceed, but compaction thinks
otherwise and returns COMPACT_SKIPPED
- compaction thinks it succeeded and returns COMPACT_PARTIAL, but
allocation attempt fails
- and perhaps some other combinations
> Sure we can see a case where somebody is stealing the compacted blocks
> but that is very same with the order-0 where parallel mem eaters will
> piggy back on the reclaimer and there is no upper boundary as well well.
Yep.
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-03-08 13:30 +0100 |
| Subject | Re: [PATCH] mm, oom: protect !costly allocations some more |
| Message-ID | <raoVZ-Mw-43@gated-at.bofh.it> |
| In reply to | #1352900 |
On Tue 08-03-16 12:12:20, Vlastimil Babka wrote:
> On 03/08/2016 11:10 AM, Michal Hocko wrote:
> > On Tue 08-03-16 10:52:15, Vlastimil Babka wrote:
> >> On 03/08/2016 10:46 AM, Michal Hocko wrote:
> > [...]
> >>>>> @@ -3294,6 +3289,18 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> >>>>> did_some_progress > 0, no_progress_loops))
> >>>>> goto retry;
> >>>>>
> >>>>> + /*
> >>>>> + * !costly allocations are really important and we have to make sure
> >>>>> + * the compaction wasn't deferred or didn't bail out early due to locks
> >>>>> + * contention before we go OOM.
> >>>>> + */
> >>>>> + if (order && order <= PAGE_ALLOC_COSTLY_ORDER) {
> >>>>> + if (compact_result <= COMPACT_CONTINUE)
> >>>>
> >>>> Same here.
> >>>> I was going to say that this didn't have effect on Sergey's test, but
> >>>> turns out it did :)
> >>>
> >>> This should work as expected because compact_result is unsigned long
> >>> and so this is the unsigned arithmetic. I can make
> >>> #define COMPACT_NONE -1UL
> >>>
> >>> to make the intention more obvious if you prefer, though.
> >>
> >> Well, what wasn't obvious to me is actually that here (unlike in the
> >> test above) it was actually intended that COMPACT_NONE doesn't result in
> >> a retry. But it makes sense, otherwise we would retry endlessly if
> >> reclaim couldn't form a higher-order page, right.
> >
> > Yeah, that was the whole point. An alternative would be moving the test
> > into should_compact_retry(order, compact_result, contended_compaction)
> > which would be CONFIG_COMPACTION specific so we can get rid of the
> > COMPACT_NONE altogether. Something like the following. We would lose the
> > always initialized compact_result but this would matter only for
> > order==0 and we check for that. Even gcc doesn't complain.
>
> Yeah I like this version better, you can add my Acked-By.
OK, patch updated and I will post it as a reply to the original email.
> Thanks.
>
> > A more important question is whether the criteria I have chosen are
> > reasonable and reasonably independent on the particular implementation
> > of the compaction. I still cannot convince myself about the convergence
> > here. Is it possible that the compaction would keep returning
> > compact_result <= COMPACT_CONTINUE while not making any progress at all?
>
> Theoretically, if reclaim/compaction suitability decisions and
> allocation attempts didn't match the watermark checks, including the
> alloc_flags and classzone_idx parameters. Possible scenarios:
>
> - reclaim thinks compaction has enough to proceed, but compaction thinks
> otherwise and returns COMPACT_SKIPPED
> - compaction thinks it succeeded and returns COMPACT_PARTIAL, but
> allocation attempt fails
> - and perhaps some other combinations
But that might happen right now as well so it wouldn't be a regression,
right?
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2016-03-08 13:40 +0100 |
| Subject | Re: [PATCH] mm, oom: protect !costly allocations some more |
| Message-ID | <rap5E-Qj-17@gated-at.bofh.it> |
| In reply to | #1352969 |
On 03/08/2016 01:22 PM, Michal Hocko wrote: >> Thanks. >> >>> A more important question is whether the criteria I have chosen are >>> reasonable and reasonably independent on the particular implementation >>> of the compaction. I still cannot convince myself about the convergence >>> here. Is it possible that the compaction would keep returning >>> compact_result <= COMPACT_CONTINUE while not making any progress at all? >> >> Theoretically, if reclaim/compaction suitability decisions and >> allocation attempts didn't match the watermark checks, including the >> alloc_flags and classzone_idx parameters. Possible scenarios: >> >> - reclaim thinks compaction has enough to proceed, but compaction thinks >> otherwise and returns COMPACT_SKIPPED >> - compaction thinks it succeeded and returns COMPACT_PARTIAL, but >> allocation attempt fails >> - and perhaps some other combinations > > But that might happen right now as well so it wouldn't be a regression, > right? Maybe, somehow, I didn't study closely how the retry decisions work. Your patch adds another way to retry so it's theoretically more dangerous. Just hinting at what to possibly check (the watermark checks) :)
[toc] | [prev] | [next] | [standalone]
| From | Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> |
|---|---|
| Date | 2016-03-08 11:00 +0100 |
| Message-ID | <ramAP-7A1-11@gated-at.bofh.it> |
| In reply to | #1351745 |
On (03/07/16 17:08), Michal Hocko wrote: > On Mon 29-02-16 22:02:13, Michal Hocko wrote: > > Andrew, > > could you queue this one as well, please? This is more a band aid than a > > real solution which I will be working on as soon as I am able to > > reproduce the issue but the patch should help to some degree at least. > > Joonsoo wasn't very happy about this approach so let me try a different > way. What do you think about the following? Hugh, Sergey does it help > for your load? I have tested it with the Hugh's load and there was no > major difference from the previous testing so at least nothing has blown > up as I am not able to reproduce the issue here. > > Other changes in the compaction are still needed but I would like to not > depend on them right now. works fine for me. $ cat /proc/vmstat | egrep -e "compact|swap" pgsteal_kswapd_dma 7 pgsteal_kswapd_dma32 6457075 pgsteal_kswapd_normal 1462767 pgsteal_kswapd_movable 0 pgscan_kswapd_dma 18 pgscan_kswapd_dma32 6544126 pgscan_kswapd_normal 1495604 pgscan_kswapd_movable 0 kswapd_inodesteal 29 kswapd_low_wmark_hit_quickly 1168 kswapd_high_wmark_hit_quickly 1627 compact_migrate_scanned 5762793 compact_free_scanned 54090239 compact_isolated 1303895 compact_stall 1542 compact_fail 1117 compact_success 425 compact_kcompatd_wake 0 no OOM-kills after 6 rounds of tests. Tested-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> thanks! -ss
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-03-08 15:00 +0100 |
| Message-ID | <raql5-1Bi-47@gated-at.bofh.it> |
| In reply to | #1352811 |
On Tue 08-03-16 18:58:24, Sergey Senozhatsky wrote: > On (03/07/16 17:08), Michal Hocko wrote: > > On Mon 29-02-16 22:02:13, Michal Hocko wrote: > > > Andrew, > > > could you queue this one as well, please? This is more a band aid than a > > > real solution which I will be working on as soon as I am able to > > > reproduce the issue but the patch should help to some degree at least. > > > > Joonsoo wasn't very happy about this approach so let me try a different > > way. What do you think about the following? Hugh, Sergey does it help > > for your load? I have tested it with the Hugh's load and there was no > > major difference from the previous testing so at least nothing has blown > > up as I am not able to reproduce the issue here. > > > > Other changes in the compaction are still needed but I would like to not > > depend on them right now. > > works fine for me. > > $ cat /proc/vmstat | egrep -e "compact|swap" > pgsteal_kswapd_dma 7 > pgsteal_kswapd_dma32 6457075 > pgsteal_kswapd_normal 1462767 > pgsteal_kswapd_movable 0 > pgscan_kswapd_dma 18 > pgscan_kswapd_dma32 6544126 > pgscan_kswapd_normal 1495604 > pgscan_kswapd_movable 0 > kswapd_inodesteal 29 > kswapd_low_wmark_hit_quickly 1168 > kswapd_high_wmark_hit_quickly 1627 > compact_migrate_scanned 5762793 > compact_free_scanned 54090239 > compact_isolated 1303895 > compact_stall 1542 > compact_fail 1117 > compact_success 425 > compact_kcompatd_wake 0 > > no OOM-kills after 6 rounds of tests. > > Tested-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com> Thanks for retesting! -- Michal Hocko SUSE Labs
[toc] | [prev] | [next] | [standalone]
| From | Hugh Dickins <hughd@google.com> |
|---|---|
| Date | 2016-03-08 11:40 +0100 |
| Subject | Re: [PATCH] mm, oom: protect !costly allocations some more (was: Re: [PATCH 0/3] OOM detection rework v4) |
| Message-ID | <randw-84x-31@gated-at.bofh.it> |
| In reply to | #1351745 |
[Multipart message — attachments visible in raw view] — view raw
On Mon, 7 Mar 2016, Michal Hocko wrote: > On Mon 29-02-16 22:02:13, Michal Hocko wrote: > > Andrew, > > could you queue this one as well, please? This is more a band aid than a > > real solution which I will be working on as soon as I am able to > > reproduce the issue but the patch should help to some degree at least. > > Joonsoo wasn't very happy about this approach so let me try a different > way. What do you think about the following? Hugh, Sergey does it help > for your load? I have tested it with the Hugh's load and there was no > major difference from the previous testing so at least nothing has blown > up as I am not able to reproduce the issue here. Did not help with my load at all, I'm afraid: quite the reverse, OOMed very much sooner (as usual on order=2), and with much more noise (multiple OOMs) than your previous patch. vmstats.xz attached; sorry, I don't have tracing built in, and must move on to the powerpc issue before going back to bed. I do hate replying without having something constructive to say, but have very little time to think about this, and no bright ideas so far. I do not understand why it's so easy for me to reproduce, yet impossible for you - unless it's that you are still doing all your testing in a VM? Is Sergey the only other to see this issue? Hugh
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-03-08 14:50 +0100 |
| Subject | [PATCH 1/3] mm, compaction: change COMPACT_ constants into enum |
| Message-ID | <raqbo-1xM-3@gated-at.bofh.it> |
| In reply to | #1351745 |
From: Michal Hocko <mhocko@suse.com>
compaction code is doing weird dances between
COMPACT_FOO -> int -> unsigned long
but there doesn't seem to be any reason for that. All functions which
return/use one of those constants are not expecting any other value
so it really makes sense to define an enum for them and make it clear
that no other values are expected.
This is a pure cleanup and shouldn't introduce any functional changes.
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
include/linux/compaction.h | 45 +++++++++++++++++++++++++++------------------
mm/compaction.c | 27 ++++++++++++++-------------
mm/page_alloc.c | 2 +-
3 files changed, 42 insertions(+), 32 deletions(-)
diff --git a/include/linux/compaction.h b/include/linux/compaction.h
index 4cd4ddf64cc7..b167801187e7 100644
--- a/include/linux/compaction.h
+++ b/include/linux/compaction.h
@@ -2,21 +2,29 @@
#define _LINUX_COMPACTION_H
/* Return values for compact_zone() and try_to_compact_pages() */
-/* compaction didn't start as it was deferred due to past failures */
-#define COMPACT_DEFERRED 0
-/* compaction didn't start as it was not possible or direct reclaim was more suitable */
-#define COMPACT_SKIPPED 1
-/* compaction should continue to another pageblock */
-#define COMPACT_CONTINUE 2
-/* direct compaction partially compacted a zone and there are suitable pages */
-#define COMPACT_PARTIAL 3
-/* The full zone was compacted */
-#define COMPACT_COMPLETE 4
-/* For more detailed tracepoint output */
-#define COMPACT_NO_SUITABLE_PAGE 5
-#define COMPACT_NOT_SUITABLE_ZONE 6
-#define COMPACT_CONTENDED 7
/* When adding new states, please adjust include/trace/events/compaction.h */
+enum compact_result {
+ /* compaction didn't start as it was deferred due to past failures */
+ COMPACT_DEFERRED,
+ /*
+ * compaction didn't start as it was not possible or direct reclaim
+ * was more suitable
+ */
+ COMPACT_SKIPPED,
+ /* compaction should continue to another pageblock */
+ COMPACT_CONTINUE,
+ /*
+ * direct compaction partially compacted a zone and there are suitable
+ * pages
+ */
+ COMPACT_PARTIAL,
+ /* The full zone was compacted */
+ COMPACT_COMPLETE,
+ /* For more detailed tracepoint output */
+ COMPACT_NO_SUITABLE_PAGE,
+ COMPACT_NOT_SUITABLE_ZONE,
+ COMPACT_CONTENDED,
+};
/* Used to signal whether compaction detected need_sched() or lock contention */
/* No contention detected */
@@ -38,12 +46,13 @@ extern int sysctl_extfrag_handler(struct ctl_table *table, int write,
extern int sysctl_compact_unevictable_allowed;
extern int fragmentation_index(struct zone *zone, unsigned int order);
-extern unsigned long try_to_compact_pages(gfp_t gfp_mask, unsigned int order,
+extern enum compact_result try_to_compact_pages(gfp_t gfp_mask,
+ unsigned int order,
int alloc_flags, const struct alloc_context *ac,
enum migrate_mode mode, int *contended);
extern void compact_pgdat(pg_data_t *pgdat, int order);
extern void reset_isolation_suitable(pg_data_t *pgdat);
-extern unsigned long compaction_suitable(struct zone *zone, int order,
+extern enum compact_result compaction_suitable(struct zone *zone, int order,
int alloc_flags, int classzone_idx);
extern void defer_compaction(struct zone *zone, int order);
@@ -53,7 +62,7 @@ extern void compaction_defer_reset(struct zone *zone, int order,
extern bool compaction_restarting(struct zone *zone, int order);
#else
-static inline unsigned long try_to_compact_pages(gfp_t gfp_mask,
+static inline enum compact_result try_to_compact_pages(gfp_t gfp_mask,
unsigned int order, int alloc_flags,
const struct alloc_context *ac,
enum migrate_mode mode, int *contended)
@@ -69,7 +78,7 @@ static inline void reset_isolation_suitable(pg_data_t *pgdat)
{
}
-static inline unsigned long compaction_suitable(struct zone *zone, int order,
+static inline enum compact_result compaction_suitable(struct zone *zone, int order,
int alloc_flags, int classzone_idx)
{
return COMPACT_SKIPPED;
diff --git a/mm/compaction.c b/mm/compaction.c
index 585de54dbe8c..0f61f12d82b6 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1195,7 +1195,7 @@ static inline bool is_via_compact_memory(int order)
return order == -1;
}
-static int __compact_finished(struct zone *zone, struct compact_control *cc,
+static enum compact_result __compact_finished(struct zone *zone, struct compact_control *cc,
const int migratetype)
{
unsigned int order;
@@ -1258,8 +1258,9 @@ static int __compact_finished(struct zone *zone, struct compact_control *cc,
return COMPACT_NO_SUITABLE_PAGE;
}
-static int compact_finished(struct zone *zone, struct compact_control *cc,
- const int migratetype)
+static enum compact_result compact_finished(struct zone *zone,
+ struct compact_control *cc,
+ const int migratetype)
{
int ret;
@@ -1278,7 +1279,7 @@ static int compact_finished(struct zone *zone, struct compact_control *cc,
* COMPACT_PARTIAL - If the allocation would succeed without compaction
* COMPACT_CONTINUE - If compaction should run now
*/
-static unsigned long __compaction_suitable(struct zone *zone, int order,
+static enum compact_result __compaction_suitable(struct zone *zone, int order,
int alloc_flags, int classzone_idx)
{
int fragindex;
@@ -1323,10 +1324,10 @@ static unsigned long __compaction_suitable(struct zone *zone, int order,
return COMPACT_CONTINUE;
}
-unsigned long compaction_suitable(struct zone *zone, int order,
+enum compact_result compaction_suitable(struct zone *zone, int order,
int alloc_flags, int classzone_idx)
{
- unsigned long ret;
+ enum compact_result ret;
ret = __compaction_suitable(zone, order, alloc_flags, classzone_idx);
trace_mm_compaction_suitable(zone, order, ret);
@@ -1336,9 +1337,9 @@ unsigned long compaction_suitable(struct zone *zone, int order,
return ret;
}
-static int compact_zone(struct zone *zone, struct compact_control *cc)
+static enum compact_result compact_zone(struct zone *zone, struct compact_control *cc)
{
- int ret;
+ enum compact_result ret;
unsigned long start_pfn = zone->zone_start_pfn;
unsigned long end_pfn = zone_end_pfn(zone);
const int migratetype = gfpflags_to_migratetype(cc->gfp_mask);
@@ -1483,11 +1484,11 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)
return ret;
}
-static unsigned long compact_zone_order(struct zone *zone, int order,
+static enum compact_result compact_zone_order(struct zone *zone, int order,
gfp_t gfp_mask, enum migrate_mode mode, int *contended,
int alloc_flags, int classzone_idx)
{
- unsigned long ret;
+ enum compact_result ret;
struct compact_control cc = {
.nr_freepages = 0,
.nr_migratepages = 0,
@@ -1524,7 +1525,7 @@ int sysctl_extfrag_threshold = 500;
*
* This is the main entry point for direct page compaction.
*/
-unsigned long try_to_compact_pages(gfp_t gfp_mask, unsigned int order,
+enum compact_result try_to_compact_pages(gfp_t gfp_mask, unsigned int order,
int alloc_flags, const struct alloc_context *ac,
enum migrate_mode mode, int *contended)
{
@@ -1532,7 +1533,7 @@ unsigned long try_to_compact_pages(gfp_t gfp_mask, unsigned int order,
int may_perform_io = gfp_mask & __GFP_IO;
struct zoneref *z;
struct zone *zone;
- int rc = COMPACT_DEFERRED;
+ enum compact_result rc = COMPACT_DEFERRED;
int all_zones_contended = COMPACT_CONTENDED_LOCK; /* init for &= op */
*contended = COMPACT_CONTENDED_NONE;
@@ -1546,7 +1547,7 @@ unsigned long try_to_compact_pages(gfp_t gfp_mask, unsigned int order,
/* Compact each zone in the list */
for_each_zone_zonelist_nodemask(zone, z, ac->zonelist, ac->high_zoneidx,
ac->nodemask) {
- int status;
+ enum compact_result status;
int zone_contended;
if (compaction_deferred(zone, order))
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 269a04f20927..4acc0aa1aee0 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2821,7 +2821,7 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
enum migrate_mode mode, int *contended_compaction,
bool *deferred_compaction)
{
- unsigned long compact_result;
+ enum compact_result compact_result;
struct page *page;
if (!order)
--
2.7.0
[toc] | [prev] | [next] | [standalone]
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2016-03-08 15:20 +0100 |
| Subject | Re: [PATCH 1/3] mm, compaction: change COMPACT_ constants into enum |
| Message-ID | <raqEq-1XQ-17@gated-at.bofh.it> |
| In reply to | #1353047 |
On 03/08/2016 02:42 PM, Michal Hocko wrote: > From: Michal Hocko <mhocko@suse.com> > > compaction code is doing weird dances between > COMPACT_FOO -> int -> unsigned long > > but there doesn't seem to be any reason for that. All functions which I vaguely recall trying this once and running into header dependency hell. But maybe it was something a bit different and involved storing a value in struct compact_control. > return/use one of those constants are not expecting any other value > so it really makes sense to define an enum for them and make it clear > that no other values are expected. > > This is a pure cleanup and shouldn't introduce any functional changes. > > Signed-off-by: Michal Hocko <mhocko@suse.com> Acked-by: Vlastimil Babka <vbabka@suse.cz> Thanks.
[toc] | [prev] | [next] | [standalone]
| From | "Hillf Danton" <hillf.zj@alibaba-inc.com> |
|---|---|
| Date | 2016-03-09 05:00 +0100 |
| Subject | Re: [PATCH 1/3] mm, compaction: change COMPACT_ constants into enum |
| Message-ID | <raDrY-2aP-3@gated-at.bofh.it> |
| In reply to | #1353047 |
> > From: Michal Hocko <mhocko@suse.com> > > compaction code is doing weird dances between > COMPACT_FOO -> int -> unsigned long > > but there doesn't seem to be any reason for that. All functions which > return/use one of those constants are not expecting any other value > so it really makes sense to define an enum for them and make it clear > that no other values are expected. > > This is a pure cleanup and shouldn't introduce any functional changes. > > Signed-off-by: Michal Hocko <mhocko@suse.com> > --- Acked-by: Hillf Danton <hillf.zj@alibaba-inc.com>
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2016-03-08 14:50 +0100 |
| Subject | [PATCH 3/3] mm, oom: protect !costly allocations some more |
| Message-ID | <raqbo-1xM-23@gated-at.bofh.it> |
| In reply to | #1351745 |
From: Michal Hocko <mhocko@suse.com>
should_reclaim_retry will give up retries for higher order allocations
if none of the eligible zones has any requested or higher order pages
available even if we pass the watermak check for order-0. This is done
because there is no guarantee that the reclaimable and currently free
pages will form the required order.
This can, however, lead to situations were the high-order request (e.g.
order-2 required for the stack allocation during fork) will trigger
OOM too early - e.g. after the first reclaim/compaction round. Such a
system would have to be highly fragmented and there is no guarantee
further reclaim/compaction attempts would help but at least make sure
that the compaction was active before we go OOM and keep retrying even
if should_reclaim_retry tells us to oom if the last compaction round
was either inactive (deferred, skipped or bailed out early due to
contention) or it told us to continue.
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
include/linux/compaction.h | 5 +++++
mm/page_alloc.c | 53 ++++++++++++++++++++++++++++++++--------------
2 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/include/linux/compaction.h b/include/linux/compaction.h
index b167801187e7..49e04326dcb8 100644
--- a/include/linux/compaction.h
+++ b/include/linux/compaction.h
@@ -14,6 +14,11 @@ enum compact_result {
/* compaction should continue to another pageblock */
COMPACT_CONTINUE,
/*
+ * whoever is calling compaction should retry because it was either
+ * not active or it tells us there is more work to be done.
+ */
+ COMPACT_SHOULD_RETRY = COMPACT_CONTINUE,
+ /*
* direct compaction partially compacted a zone and there are suitable
* pages
*/
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4acc0aa1aee0..041aeb1dc3b4 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2819,28 +2819,20 @@ static struct page *
__alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
int alloc_flags, const struct alloc_context *ac,
enum migrate_mode mode, int *contended_compaction,
- bool *deferred_compaction)
+ enum compact_result *compact_result)
{
- enum compact_result compact_result;
struct page *page;
if (!order)
return NULL;
current->flags |= PF_MEMALLOC;
- compact_result = try_to_compact_pages(gfp_mask, order, alloc_flags, ac,
+ *compact_result = try_to_compact_pages(gfp_mask, order, alloc_flags, ac,
mode, contended_compaction);
current->flags &= ~PF_MEMALLOC;
- switch (compact_result) {
- case COMPACT_DEFERRED:
- *deferred_compaction = true;
- /* fall-through */
- case COMPACT_SKIPPED:
+ if (*compact_result <= COMPACT_SKIPPED)
return NULL;
- default:
- break;
- }
/*
* At least in one zone compaction wasn't deferred or skipped, so let's
@@ -2870,15 +2862,41 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
return NULL;
}
+
+static inline bool
+should_compact_retry(unsigned int order, enum compact_result compact_result,
+ int contended_compaction)
+{
+ /*
+ * !costly allocations are really important and we have to make sure
+ * the compaction wasn't deferred or didn't bail out early due to locks
+ * contention before we go OOM.
+ */
+ if (order && order <= PAGE_ALLOC_COSTLY_ORDER) {
+ if (compact_result <= COMPACT_SHOULD_RETRY)
+ return true;
+ if (contended_compaction > COMPACT_CONTENDED_NONE)
+ return true;
+ }
+
+ return false;
+}
#else
static inline struct page *
__alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
int alloc_flags, const struct alloc_context *ac,
enum migrate_mode mode, int *contended_compaction,
- bool *deferred_compaction)
+ enum compact_result *compact_result)
{
return NULL;
}
+
+static inline bool
+should_compact_retry(unsigned int order, enum compact_result compact_result,
+ int contended_compaction)
+{
+ return false;
+}
#endif /* CONFIG_COMPACTION */
/* Perform direct synchronous page reclaim */
@@ -3118,7 +3136,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
int alloc_flags;
unsigned long did_some_progress;
enum migrate_mode migration_mode = MIGRATE_ASYNC;
- bool deferred_compaction = false;
+ enum compact_result compact_result;
int contended_compaction = COMPACT_CONTENDED_NONE;
int no_progress_loops = 0;
@@ -3227,7 +3245,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
page = __alloc_pages_direct_compact(gfp_mask, order, alloc_flags, ac,
migration_mode,
&contended_compaction,
- &deferred_compaction);
+ &compact_result);
if (page)
goto got_pg;
@@ -3240,7 +3258,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
* to heavily disrupt the system, so we fail the allocation
* instead of entering direct reclaim.
*/
- if (deferred_compaction)
+ if (compact_result == COMPACT_DEFERRED)
goto nopage;
/*
@@ -3294,6 +3312,9 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
did_some_progress > 0, no_progress_loops))
goto retry;
+ if (should_compact_retry(order, compact_result, contended_compaction))
+ goto retry;
+
/* Reclaim has failed us, start killing things */
page = __alloc_pages_may_oom(gfp_mask, order, ac, &did_some_progress);
if (page)
@@ -3314,7 +3335,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
page = __alloc_pages_direct_compact(gfp_mask, order, alloc_flags,
ac, migration_mode,
&contended_compaction,
- &deferred_compaction);
+ &compact_result);
if (page)
goto got_pg;
nopage:
--
2.7.0
[toc] | [prev] | [next] | [standalone]
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2016-03-08 15:40 +0100 |
| Subject | Re: [PATCH 3/3] mm, oom: protect !costly allocations some more |
| Message-ID | <raqXM-25J-19@gated-at.bofh.it> |
| In reply to | #1353052 |
On 03/08/2016 02:42 PM, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> should_reclaim_retry will give up retries for higher order allocations
> if none of the eligible zones has any requested or higher order pages
> available even if we pass the watermak check for order-0. This is done
> because there is no guarantee that the reclaimable and currently free
> pages will form the required order.
>
> This can, however, lead to situations were the high-order request (e.g.
> order-2 required for the stack allocation during fork) will trigger
> OOM too early - e.g. after the first reclaim/compaction round. Such a
> system would have to be highly fragmented and there is no guarantee
> further reclaim/compaction attempts would help but at least make sure
> that the compaction was active before we go OOM and keep retrying even
> if should_reclaim_retry tells us to oom if the last compaction round
> was either inactive (deferred, skipped or bailed out early due to
> contention) or it told us to continue.
>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
> include/linux/compaction.h | 5 +++++
> mm/page_alloc.c | 53 ++++++++++++++++++++++++++++++++--------------
> 2 files changed, 42 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/compaction.h b/include/linux/compaction.h
> index b167801187e7..49e04326dcb8 100644
> --- a/include/linux/compaction.h
> +++ b/include/linux/compaction.h
> @@ -14,6 +14,11 @@ enum compact_result {
> /* compaction should continue to another pageblock */
> COMPACT_CONTINUE,
> /*
> + * whoever is calling compaction should retry because it was either
> + * not active or it tells us there is more work to be done.
> + */
> + COMPACT_SHOULD_RETRY = COMPACT_CONTINUE,
Hmm, I'm not sure about this. AFAIK compact_zone() doesn't ever return
COMPACT_CONTINUE, and thus try_to_compact_pages() also doesn't. This
overloading of CONTINUE only applies to compaction_suitable(). But the
value that should_compact_retry() is testing comes only from
try_to_compact_pages(). So this is not wrong, but perhaps a bit misleading?
> + /*
> * direct compaction partially compacted a zone and there are suitable
> * pages
> */
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 4acc0aa1aee0..041aeb1dc3b4 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2819,28 +2819,20 @@ static struct page *
> __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
> int alloc_flags, const struct alloc_context *ac,
> enum migrate_mode mode, int *contended_compaction,
> - bool *deferred_compaction)
> + enum compact_result *compact_result)
> {
> - enum compact_result compact_result;
> struct page *page;
>
> if (!order)
> return NULL;
>
> current->flags |= PF_MEMALLOC;
> - compact_result = try_to_compact_pages(gfp_mask, order, alloc_flags, ac,
> + *compact_result = try_to_compact_pages(gfp_mask, order, alloc_flags, ac,
> mode, contended_compaction);
> current->flags &= ~PF_MEMALLOC;
>
> - switch (compact_result) {
> - case COMPACT_DEFERRED:
> - *deferred_compaction = true;
> - /* fall-through */
> - case COMPACT_SKIPPED:
> + if (*compact_result <= COMPACT_SKIPPED)
> return NULL;
> - default:
> - break;
> - }
>
> /*
> * At least in one zone compaction wasn't deferred or skipped, so let's
> @@ -2870,15 +2862,41 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
>
> return NULL;
> }
> +
> +static inline bool
> + (unsigned int order, enum compact_result compact_result,
> + int contended_compaction)
> +{
> + /*
> + * !costly allocations are really important and we have to make sure
> + * the compaction wasn't deferred or didn't bail out early due to locks
> + * contention before we go OOM.
> + */
> + if (order && order <= PAGE_ALLOC_COSTLY_ORDER) {
> + if (compact_result <= COMPACT_SHOULD_RETRY)
> + return true;
> + if (contended_compaction > COMPACT_CONTENDED_NONE)
> + return true;
> + }
> +
> + return false;
> +}
> #else
> static inline struct page *
> __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
> int alloc_flags, const struct alloc_context *ac,
> enum migrate_mode mode, int *contended_compaction,
> - bool *deferred_compaction)
> + enum compact_result *compact_result)
> {
> return NULL;
> }
> +
> +static inline bool
> +should_compact_retry(unsigned int order, enum compact_result compact_result,
> + int contended_compaction)
> +{
> + return false;
> +}
> #endif /* CONFIG_COMPACTION */
>
> /* Perform direct synchronous page reclaim */
> @@ -3118,7 +3136,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> int alloc_flags;
> unsigned long did_some_progress;
> enum migrate_mode migration_mode = MIGRATE_ASYNC;
> - bool deferred_compaction = false;
> + enum compact_result compact_result;
> int contended_compaction = COMPACT_CONTENDED_NONE;
> int no_progress_loops = 0;
>
> @@ -3227,7 +3245,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> page = __alloc_pages_direct_compact(gfp_mask, order, alloc_flags, ac,
> migration_mode,
> &contended_compaction,
> - &deferred_compaction);
> + &compact_result);
> if (page)
> goto got_pg;
>
> @@ -3240,7 +3258,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> * to heavily disrupt the system, so we fail the allocation
> * instead of entering direct reclaim.
> */
> - if (deferred_compaction)
> + if (compact_result == COMPACT_DEFERRED)
> goto nopage;
>
> /*
> @@ -3294,6 +3312,9 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> did_some_progress > 0, no_progress_loops))
> goto retry;
>
> + if (should_compact_retry(order, compact_result, contended_compaction))
> + goto retry;
> +
> /* Reclaim has failed us, start killing things */
> page = __alloc_pages_may_oom(gfp_mask, order, ac, &did_some_progress);
> if (page)
> @@ -3314,7 +3335,7 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> page = __alloc_pages_direct_compact(gfp_mask, order, alloc_flags,
> ac, migration_mode,
> &contended_compaction,
> - &deferred_compaction);
> + &compact_result);
> if (page)
> goto got_pg;
> nopage:
>
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web