Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1462471

[PATCH 4.7 30/41] Revert "mm, mempool: only set __GFP_NOMEMALLOC if there are free elements"

From Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Newsgroups linux.kernel
Subject [PATCH 4.7 30/41] Revert "mm, mempool: only set __GFP_NOMEMALLOC if there are free elements"
Date 2016-08-14 23:00 +0200
Message-ID <s6aFI-4Et-41@gated-at.bofh.it> (permalink)
References <s6aw1-4AX-5@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


4.7-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Michal Hocko <mhocko@suse.com>

commit 4e390b2b2f34b8daaabf2df1df0cf8f798b87ddb upstream.

This reverts commit f9054c70d28b ("mm, mempool: only set __GFP_NOMEMALLOC
if there are free elements").

There has been a report about OOM killer invoked when swapping out to a
dm-crypt device.  The primary reason seems to be that the swapout out IO
managed to completely deplete memory reserves.  Ondrej was able to
bisect and explained the issue by pointing to f9054c70d28b ("mm,
mempool: only set __GFP_NOMEMALLOC if there are free elements").

The reason is that the swapout path is not throttled properly because
the md-raid layer needs to allocate from the generic_make_request path
which means it allocates from the PF_MEMALLOC context.  dm layer uses
mempool_alloc in order to guarantee a forward progress which used to
inhibit access to memory reserves when using page allocator.  This has
changed by f9054c70d28b ("mm, mempool: only set __GFP_NOMEMALLOC if
there are free elements") which has dropped the __GFP_NOMEMALLOC
protection when the memory pool is depleted.

If we are running out of memory and the only way forward to free memory
is to perform swapout we just keep consuming memory reserves rather than
throttling the mempool allocations and allowing the pending IO to
complete up to a moment when the memory is depleted completely and there
is no way forward but invoking the OOM killer.  This is less than
optimal.

The original intention of f9054c70d28b was to help with the OOM
situations where the oom victim depends on mempool allocation to make a
forward progress.  David has mentioned the following backtrace:

  schedule
  schedule_timeout
  io_schedule_timeout
  mempool_alloc
  __split_and_process_bio
  dm_request
  generic_make_request
  submit_bio
  mpage_readpages
  ext4_readpages
  __do_page_cache_readahead
  ra_submit
  filemap_fault
  handle_mm_fault
  __do_page_fault
  do_page_fault
  page_fault

We do not know more about why the mempool is depleted without being
replenished in time, though.  In any case the dm layer shouldn't depend
on any allocations outside of the dedicated pools so a forward progress
should be guaranteed.  If this is not the case then the dm should be
fixed rather than papering over the problem and postponing it to later
by accessing more memory reserves.

mempools are a mechanism to maintain dedicated memory reserves to
guaratee forward progress.  Allowing them an unbounded access to the
page allocator memory reserves is going against the whole purpose of
this mechanism.

Bisected by Ondrej Kozina.

[akpm@linux-foundation.org: coding-style fixes]
Link: http://lkml.kernel.org/r/20160721145309.GR26379@dhcp22.suse.cz
Signed-off-by: Michal Hocko <mhocko@suse.com>
Reported-by: Ondrej Kozina <okozina@redhat.com>
Reviewed-by: Johannes Weiner <hannes@cmpxchg.org>
Acked-by: NeilBrown <neilb@suse.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Mikulas Patocka <mpatocka@redhat.com>
Cc: Ondrej Kozina <okozina@redhat.com>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Mel Gorman <mgorman@suse.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 mm/mempool.c |   18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -306,7 +306,7 @@ EXPORT_SYMBOL(mempool_resize);
  * returns NULL. Note that due to preallocation, this function
  * *never* fails when called from process contexts. (it might
  * fail if called from an IRQ context.)
- * Note: neither __GFP_NOMEMALLOC nor __GFP_ZERO are supported.
+ * Note: using __GFP_ZERO is not supported.
  */
 void *mempool_alloc(mempool_t *pool, gfp_t gfp_mask)
 {
@@ -315,27 +315,16 @@ void *mempool_alloc(mempool_t *pool, gfp
 	wait_queue_t wait;
 	gfp_t gfp_temp;
 
-	/* If oom killed, memory reserves are essential to prevent livelock */
-	VM_WARN_ON_ONCE(gfp_mask & __GFP_NOMEMALLOC);
-	/* No element size to zero on allocation */
 	VM_WARN_ON_ONCE(gfp_mask & __GFP_ZERO);
-
 	might_sleep_if(gfp_mask & __GFP_DIRECT_RECLAIM);
 
+	gfp_mask |= __GFP_NOMEMALLOC;	/* don't allocate emergency reserves */
 	gfp_mask |= __GFP_NORETRY;	/* don't loop in __alloc_pages */
 	gfp_mask |= __GFP_NOWARN;	/* failures are OK */
 
 	gfp_temp = gfp_mask & ~(__GFP_DIRECT_RECLAIM|__GFP_IO);
 
 repeat_alloc:
-	if (likely(pool->curr_nr)) {
-		/*
-		 * Don't allocate from emergency reserves if there are
-		 * elements available.  This check is racy, but it will
-		 * be rechecked each loop.
-		 */
-		gfp_temp |= __GFP_NOMEMALLOC;
-	}
 
 	element = pool->alloc(gfp_temp, pool->pool_data);
 	if (likely(element != NULL))
@@ -359,12 +348,11 @@ repeat_alloc:
 	 * We use gfp mask w/o direct reclaim or IO for the first round.  If
 	 * alloc failed with that and @pool was empty, retry immediately.
 	 */
-	if ((gfp_temp & ~__GFP_NOMEMALLOC) != gfp_mask) {
+	if (gfp_temp != gfp_mask) {
 		spin_unlock_irqrestore(&pool->lock, flags);
 		gfp_temp = gfp_mask;
 		goto repeat_alloc;
 	}
-	gfp_temp = gfp_mask;
 
 	/* We must not sleep if !__GFP_DIRECT_RECLAIM */
 	if (!(gfp_mask & __GFP_DIRECT_RECLAIM)) {

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 4.7 00/41] 4.7.1-stable review Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 22:50 +0200
  [PATCH 4.7 15/41] KEYS: 64-bit MIPS needs to use compat_sys_keyctl for 32-bit userspace Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 22:50 +0200
  [PATCH 4.7 36/41] ext4: check for extents that wrap around Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 22:50 +0200
  [PATCH 4.7 13/41] tcp: consider recv buf for the initial window scale Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 22:50 +0200
  [PATCH 4.7 04/41] udp: use sk_filter_trim_cap for udp{,6}_queue_rcv_skb Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 22:50 +0200
  [PATCH 4.7 34/41] crypto: scatterwalk - Fix test in scatterwalk_done Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 22:50 +0200
  [PATCH 4.7 37/41] ext4: fix deadlock during page writeback Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 22:50 +0200
  [PATCH 4.7 10/41] net/sctp: terminate rhashtable walk correctly Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 22:50 +0200
  [PATCH 4.7 35/41] serial: mvebu-uart: free the IRQ in ->shutdown() Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 22:50 +0200
  [PATCH 4.7 27/41] fuse: fsync() did not return IO errors Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 23:00 +0200
  [PATCH 4.7 19/41] mm: memcontrol: fix swap counter leak on swapout from offline cgroup Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 23:00 +0200
  [PATCH 4.7 03/41] vfs: fix deadlock in file_remove_privs() on overlayfs Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 23:00 +0200
  [PATCH 4.7 29/41] fuse: fix wrong assignment of ->flags in fuse_send_init() Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 23:00 +0200
  [PATCH 4.7 21/41] x86/syscalls/64: Add compat_sys_keyctl for 32-bit userspace Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 23:00 +0200
  [PATCH 4.7 11/41] qed: Fix setting/clearing bit in completion bitmap Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 23:00 +0200
  [PATCH 4.7 30/41] Revert "mm, mempool: only set __GFP_NOMEMALLOC if there are free elements" Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 23:00 +0200
  [PATCH 4.7 20/41] mm: memcontrol: fix memcg id ref counter on swap charge move Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 23:00 +0200
  [PATCH 4.7 17/41] apparmor: fix ref count leak when profile sha1 hash is read Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 23:00 +0200
  [PATCH 4.7 25/41] x86/microcode: Fix suspend to RAM with builtin microcode Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-14 23:00 +0200
  Re: [PATCH 4.7 00/41] 4.7.1-stable review Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-15 10:00 +0200
    Re: [PATCH 4.7 00/41] 4.7.1-stable review Kevin Hilman <khilman@baylibre.com> - 2016-08-15 23:40 +0200
  Re: [PATCH 4.7 00/41] 4.7.1-stable review Guenter Roeck <linux@roeck-us.net> - 2016-08-15 15:10 +0200
    Re: [PATCH 4.7 00/41] 4.7.1-stable review Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-15 15:50 +0200
  Re: [PATCH 4.7 00/41] 4.7.1-stable review Shuah Khan <shuah.kh@samsung.com> - 2016-08-16 06:10 +0200
    Re: [PATCH 4.7 00/41] 4.7.1-stable review Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-08-16 12:50 +0200

csiph-web