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

On Mon 26-06-17 13:45:19, Vlastimil Babka wrote:
> On 06/23/2017 10:53 AM, Michal Hocko wrote:
[...]
> > - 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.

OK. What about the following.
"That means that !costly allocation requests are basically nofail but
there is no guarantee of thaat behavior so failures have to be checked
properly by callers (e.g. OOM killer victim is allowed to fail
currently).

> > 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>

Thanks!

> 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:

will fix

> (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?

the same wording as above?

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2017-06-26 12:14 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
2017-06-26 12:14     ` Michal Hocko [this message]
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=20170626121411.GK11534@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --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=neilb@suse.com \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox