mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: NeilBrown <neilb@suse.com>
Cc: Pavel Machek <pavel@ucw.cz>,
	linux-mm@kvack.org, Mel Gorman <mgorman@suse.de>,
	Matthew Wilcox <willy@infradead.org>,
	Vlastimil Babka <vbabka@suse.cz>, Neil Brown <neilb@suse.de>,
	"Theodore Ts'o" <tytso@mit.edu>,
	Andrew Morton <akpm@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH] treewide: remove GFP_TEMPORARY allocation flag
Date: Mon, 28 Aug 2017 14:36:57 +0200	[thread overview]
Message-ID: <20170828123657.GK17097@dhcp22.suse.cz> (raw)
In-Reply-To: <87pobjhssq.fsf@notabene.neil.brown.name>

On Sat 26-08-17 14:11:33, NeilBrown wrote:
> On Fri, Aug 25 2017, Pavel Machek wrote:
> 
> > On Fri 2017-08-25 10:04:42, Michal Hocko wrote:
> >> On Fri 25-08-17 09:28:19, Pavel Machek wrote:
> >> > On Fri 2017-08-25 08:35:46, Michal Hocko wrote:
> >> > > On Wed 23-08-17 19:57:09, Pavel Machek wrote:
> >> [...]
> >> > > > Dunno. < 1msec probably is temporary, 1 hour probably is not. If it causes
> >> > > > problems, can you just #define GFP_TEMPORARY GFP_KERNEL ? Treewide replace,
> >> > > > and then starting again goes not look attractive to me.
> >> > > 
> >> > > I do not think we want a highlevel GFP_TEMPORARY without any meaning.
> >> > > This just supports spreading the flag usage without a clear semantic
> >> > > and it will lead to even bigger mess. Once we can actually define what
> >> > > the flag means we can also add its users based on that new semantic.
> >> > 
> >> > It has real meaning.
> >> 
> >> Which is?
> >
> > "This allocation is temporary. It lasts milliseconds, not hours."
> 
> It isn't sufficient to give a rule for when GFP_TEMPORARY will be used,
> you also need to explain (at least in general terms) how the information
> will be used.  Also you need to give guidelines on whether the flag
> should be set for allocation that will last seconds or minutes.
> 
> If we have a flag that doesn't have a well defined meaning that actually
> affects behavior, it will not be used consistently, and if we ever
> change exactly how it behaves we can expect things to break.  So it is
> better not to have a flag, than to have a poorly defined flag.

Absolutely agreed!

> My current thoughts is that the important criteria is not how long the
> allocation will be used for, but whether it is reclaimable.  Allocations
> that will only last 5 msecs are reclaimable by calling "usleep(5000)".
> Other allocations might be reclaimable in other ways.  Allocations that
> are not reclaimable may well be directed to a more restricted pool of
> memory, and might be more likely to fail.  If we grew a strong
> "reclaimable" concept, this 'temporary' concept that you want to hold on
> to would become a burden.

... and here again. The whole motivation for the flag was to gather
these objects together and reduce chances of internal fragmentation
due to long lived objects mixed with short term ones. Without an
explicit way to reclaim those objects or having a clear checkpoint to
wait for it is not really helping us to reach desired outcome (less
fragmented memory).

-- 
Michal Hocko
SUSE Labs

  reply	other threads:[~2017-08-28 12:37 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-28  9:19 Michal Hocko
2017-07-28  9:52 ` Mel Gorman
2017-07-28 10:27   ` Michal Hocko
2017-07-28 10:59     ` Mel Gorman
2017-07-28 13:15 ` Vlastimil Babka
2017-08-23 17:57 ` Pavel Machek
2017-08-25  6:35   ` Michal Hocko
2017-08-25  7:28     ` Pavel Machek
2017-08-25  8:04       ` Michal Hocko
2017-08-25 21:39         ` Pavel Machek
2017-08-26  4:11           ` NeilBrown
2017-08-28 12:36             ` Michal Hocko [this message]
2017-08-31  9:07               ` Pavel Machek
2017-08-31  9:29                 ` Mel Gorman
2017-08-28 12:35           ` Michal Hocko
2017-08-31  9:10             ` Pavel Machek

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=20170828123657.GK17097@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=neilb@suse.com \
    --cc=neilb@suse.de \
    --cc=pavel@ucw.cz \
    --cc=tytso@mit.edu \
    --cc=vbabka@suse.cz \
    --cc=willy@infradead.org \
    /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

all inboxes | Powered by JetHome®