linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Michal Hocko <mhocko@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	Mel Gorman <mgorman@suse.de>, NeilBrown <neilb@suse.com>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-mm@kvack.org, Michal Hocko <mhocko@suse.com>
Subject: Re: [PATCH 2/6] mm, tree wide: replace __GFP_REPEAT by __GFP_RETRY_MAYFAIL with more useful semantic
Date: Mon, 26 Jun 2017 13:45:19 +0200	[thread overview]
Message-ID: <db63b720-b7aa-1bd0-dde8-d324dfaa9c9b@suse.cz> (raw)
In-Reply-To: <20170623085345.11304-3-mhocko@kernel.org>

On 06/23/2017 10:53 AM, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
> 
> __GFP_REPEAT was designed to allow retry-but-eventually-fail semantic to
> the page allocator. This has been true but only for allocations requests
> larger than PAGE_ALLOC_COSTLY_ORDER. It has been always ignored for
> smaller sizes. This is a bit unfortunate because there is no way to
> express the same semantic for those requests and they are considered too
> important to fail so they might end up looping in the page allocator for
> ever, similarly to GFP_NOFAIL requests.
> 
> Now that the whole tree has been cleaned up and accidental or misled
> usage of __GFP_REPEAT flag has been removed for !costly requests we can
> give the original flag a better name and more importantly a more useful
> semantic. Let's rename it to __GFP_RETRY_MAYFAIL which tells the user that
> the allocator would try really hard but there is no promise of a
> success. This will work independent of the order and overrides the
> default allocator behavior. Page allocator users have several levels of
> guarantee vs. cost options (take GFP_KERNEL as an example)
> - GFP_KERNEL & ~__GFP_RECLAIM - optimistic allocation without _any_
>   attempt to free memory at all. The most light weight mode which even
>   doesn't kick the background reclaim. Should be used carefully because
>   it might deplete the memory and the next user might hit the more
>   aggressive reclaim
> - GFP_KERNEL & ~__GFP_DIRECT_RECLAIM (or GFP_NOWAIT)- optimistic
>   allocation without any attempt to free memory from the current context
>   but can wake kswapd to reclaim memory if the zone is below the low
>   watermark. Can be used from either atomic contexts or when the request
>   is a performance optimization and there is another fallback for a slow
>   path.
> - (GFP_KERNEL|__GFP_HIGH) & ~__GFP_DIRECT_RECLAIM (aka GFP_ATOMIC) - non
>   sleeping allocation with an expensive fallback so it can access some
>   portion of memory reserves. Usually used from interrupt/bh context with
>   an expensive slow path fallback.
> - GFP_KERNEL - both background and direct reclaim are allowed and the
>   _default_ page allocator behavior is used. That means that !costly
>   allocation requests are basically nofail (unless the requesting task
>   is killed by the OOM killer)

Should we explicitly point out that failure must be handled? After lots
of talking about "too small to fail", people might get the wrong impression.

> and costly will fail early rather than
>   cause disruptive reclaim.
> - GFP_KERNEL | __GFP_NORETRY - overrides the default allocator behavior and
>   all allocation requests fail early rather than cause disruptive
>   reclaim (one round of reclaim in this implementation). The OOM killer
>   is not invoked.
> - GFP_KERNEL | __GFP_RETRY_MAYFAIL - overrides the default allocator behavior
>   and all allocation requests try really hard. The request will fail if the
>   reclaim cannot make any progress. The OOM killer won't be triggered.
> - GFP_KERNEL | __GFP_NOFAIL - overrides the default allocator behavior
>   and all allocation requests will loop endlessly until they
>   succeed. This might be really dangerous especially for larger orders.
> 
> Existing users of __GFP_REPEAT are changed to __GFP_RETRY_MAYFAIL because
> they already had their semantic. No new users are added.
> __alloc_pages_slowpath is changed to bail out for __GFP_RETRY_MAYFAIL if
> there is no progress and we have already passed the OOM point. This
> means that all the reclaim opportunities have been exhausted except the
> most disruptive one (the OOM killer) and a user defined fallback
> behavior is more sensible than keep retrying in the page allocator.
> 
> Changes since RFC
> - udpate documentation wording as per Neil Brown
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

Some more minor comments below:

...

> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 4c6656f1fee7..6be1f836b69e 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -25,7 +25,7 @@ struct vm_area_struct;
>  #define ___GFP_FS		0x80u
>  #define ___GFP_COLD		0x100u
>  #define ___GFP_NOWARN		0x200u
> -#define ___GFP_REPEAT		0x400u
> +#define ___GFP_RETRY_MAYFAIL		0x400u

Seems like one tab too many, the end result is off:
(sigh, tabs are not only error prone, but also we make less money due to
them, I heard)

#define ___GFP_NOWARN           0x200u
#define ___GFP_RETRY_MAYFAIL            0x400u
#define ___GFP_NOFAIL           0x800u


>  #define ___GFP_NOFAIL		0x800u
>  #define ___GFP_NORETRY		0x1000u
>  #define ___GFP_MEMALLOC		0x2000u
> @@ -136,26 +136,55 @@ struct vm_area_struct;
>   *
>   * __GFP_RECLAIM is shorthand to allow/forbid both direct and kswapd reclaim.
>   *
> - * __GFP_REPEAT: Try hard to allocate the memory, but the allocation attempt
> - *   _might_ fail.  This depends upon the particular VM implementation.
> + * The default allocator behavior depends on the request size. We have a concept
> + * of so called costly allocations (with order > PAGE_ALLOC_COSTLY_ORDER).
> + * !costly allocations are too essential to fail so they are implicitly
> + * non-failing (with some exceptions like OOM victims might fail) by default while

Again, emphasize need for error handling?

  reply	other threads:[~2017-06-26 11:45 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-23  8:53 [PATCH 0/6] mm: give __GFP_REPEAT a better semantic Michal Hocko
2017-06-23  8:53 ` [PATCH 1/6] MIPS: do not use __GFP_REPEAT for order-0 request Michal Hocko
2017-06-23 10:27   ` Ralf Baechle
2017-06-26  9:36   ` Vlastimil Babka
2017-06-23  8:53 ` [PATCH 2/6] mm, tree wide: replace __GFP_REPEAT by __GFP_RETRY_MAYFAIL with more useful semantic Michal Hocko
2017-06-26 11:45   ` Vlastimil Babka [this message]
2017-06-26 12:14     ` Michal Hocko
2017-06-26 12:17       ` Vlastimil Babka
2017-06-26 12:38         ` Michal Hocko
2017-06-26 12:42           ` Michal Hocko
2017-06-26 11:53   ` Vlastimil Babka
2017-06-23  8:53 ` [PATCH 3/6] xfs: map KM_MAYFAIL to __GFP_RETRY_MAYFAIL Michal Hocko
2017-06-27  8:49   ` Michal Hocko
2017-06-27 13:47     ` Christoph Hellwig
2017-06-27 14:06       ` Michal Hocko
2017-06-28  4:12         ` Darrick J. Wong
2017-06-23  8:53 ` [PATCH 4/6] mm: kvmalloc support __GFP_RETRY_MAYFAIL for all sizes Michal Hocko
2017-06-26 12:00   ` Vlastimil Babka
2017-06-26 12:18     ` Michal Hocko
2017-06-23  8:53 ` [PATCH 5/6] drm/i915: use __GFP_RETRY_MAYFAIL Michal Hocko
2017-06-23  8:53 ` [PATCH 6/6] mm, migration: do not trigger OOM killer when migrating memory Michal Hocko
2017-06-23 20:43   ` Andrew Morton
2017-06-26  5:28     ` Michal Hocko
2017-06-26 12:13   ` Vlastimil Babka

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=db63b720-b7aa-1bd0-dde8-d324dfaa9c9b@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=mhocko@kernel.org \
    --cc=mhocko@suse.com \
    --cc=neilb@suse.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).