linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrea Arcangeli <aarcange@redhat.com>
To: Mel Gorman <mgorman@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Colin Cross <ccross@android.com>,
	Pekka Enberg <penberg@cs.helsinki.fi>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	David Rientjes <rientjes@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux-MM <linux-mm@kvack.org>
Subject: Re: [PATCH] mm: avoid livelock on !__GFP_FS allocations
Date: Mon, 14 Nov 2011 19:38:12 +0100	[thread overview]
Message-ID: <20111114183812.GC4414@redhat.com> (raw)
In-Reply-To: <20111114140421.GA27150@suse.de>

On Mon, Nov 14, 2011 at 02:04:21PM +0000, Mel Gorman wrote:
> In his fix, he avoided retrying the allocation if reclaim made no
> progress and __GFP_FS was not set. The problem is that this would
> result in GFP_NOIO allocations failing that previously succeeded
> which would be very unfortunate.

GFP_NOFS are made by filesystems/buffers to avoid locking up on fs/vfs
locking. Those also should be able to handle failure gracefully but
userland is more likely to get a -ENOMEM from these (for example
during direct-io) if those fs allocs fails. So clearly it sounds risky
to apply the modification quoted above and risk having any GFP_NOFS
fail. Said that I'm afraid we're not deadlock safe with current code
that cannot fail but there's no easy solution and no way to fix it in
the short term, and it's only a theoretical concern.

For !__GFP_FS allocations, __GFP_NOFAIL is the default for order <=
PAGE_ALLOC_COSTLY_ORDER and __GFP_NORETRY is the default for order >
PAGE_ALLOC_COSTLY_ORDER. This inconsistency is not so clean in my
view. Also for GFP_KERNEL/USER/__GFP_FS regular allocations the
__GFP_NOFAIL looks more like a __GFP_MAY_OOM.  But if we fix that and
we drop __GFP_NORETRY, and we set __GFP_NOFAIL within the
GFP_NOFS/NOIO #defines (to remove the magic PAGE_ALLOC_COSTLY_ORDER
check in should_alloc_retry) we may loop forever if somebody allocates
several mbytes of huge contiguous RAM with GFP_NOIO. So at least
there's a practical explanation for the current code.

Patch looks good to me (and safer) even if I don't like keeping
infinite loops from a purely theoretical standpoint.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2011-11-14 18:38 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-14 14:04 [PATCH] mm: avoid livelock on !__GFP_FS allocations Mel Gorman
2011-11-14 18:38 ` Andrea Arcangeli [this message]
2011-11-15 10:30   ` Mel Gorman
2011-11-14 23:03 ` Andrew Morton
2011-11-15 10:42   ` Mel Gorman
2011-11-15 15:43     ` Mel Gorman
2011-11-15 16:13 ` Minchan Kim
2011-11-15 17:36   ` Mel Gorman
2011-11-16  0:22     ` Minchan Kim
2011-11-16  0:28       ` Colin Cross
2011-11-16  0:45         ` Minchan Kim
2011-11-16  7:10           ` Pekka Enberg
2011-11-16 21:44             ` David Rientjes
2011-11-16 21:58               ` Rafael J. Wysocki
2011-11-16 22:07               ` Minchan Kim
2011-11-16 22:48                 ` David Rientjes
2011-11-15 21:40 ` David Rientjes
2011-11-16  9:52   ` Mel Gorman
2011-11-16 21:39     ` David Rientjes
  -- strict thread matches above, loose matches on Subject: below --
2011-10-25  6:39 Colin Cross
2011-10-25  7:40 ` Pekka Enberg
2011-10-25  7:51   ` Colin Cross
2011-10-25  8:08     ` Pekka Enberg
2011-10-25 22:12     ` David Rientjes
2011-10-25  9:09 ` Mel Gorman
2011-10-25  9:26   ` Colin Cross
2011-10-25 11:23     ` Mel Gorman
2011-10-25 17:08       ` Colin Cross
2011-11-01 12:28         ` Mel Gorman
2011-10-25 19:39       ` Pekka Enberg
2011-11-01 12:29         ` Mel Gorman
2011-10-25 19:29   ` Colin Cross
2011-10-25 22:18   ` David Rientjes
2011-10-26  1:46     ` Colin Cross
2011-10-26  5:47       ` David Rientjes
2011-10-26  6:12         ` David Rientjes
2011-10-26  6:16           ` Colin Cross
2011-10-26  6:24             ` David Rientjes
2011-10-26  6:26               ` Colin Cross
2011-10-26  6:33                 ` David Rientjes
2011-10-26  6:36                   ` Colin Cross
2011-10-26  6:51                     ` David Rientjes
2011-10-26  6:57                       ` Colin Cross
2011-10-26  7:10                         ` David Rientjes
2011-10-26  7:22                           ` Colin Cross
2011-11-01 12:36                             ` Mel Gorman
2011-10-25 22:10 ` David Rientjes

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=20111114183812.GC4414@redhat.com \
    --to=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=ccross@android.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=penberg@cs.helsinki.fi \
    --cc=rientjes@google.com \
    /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).