linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: David Rientjes <rientjes@google.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Richard Henderson <rth@twiddle.net>,
	Ivan Kokshaysky <ink@jurassic.park.msu.ru>,
	Matt Turner <mattst88@gmail.com>,
	Russell King <linux@arm.linux.org.uk>,
	Mikael Starvik <starvik@axis.com>,
	Jesper Nilsson <jesper.nilsson@axis.com>,
	David Howells <dhowells@redhat.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Roman Zippel <zippel@linux-m68k.org>,
	Michal Simek <monstr@monstr.eu>,
	Koichi Yasutake <yasutake.koichi@jp.panasonic.com>,
	Kyle McMartin <kyle@mcmartin.ca>, Helge Deller <deller@gmx.de>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Paul Mundt <lethal@linux-sh.org>,
	"David S. Miller" <davem@davemloft.net>,
	Jeff Dike <jdike@addtoit.com>, Ingo Molnar <mingo@redhat.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-arch@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [patch] arch: remove __GFP_REPEAT for order-0 allocations
Date: Tue, 28 Sep 2010 17:41:47 -0700	[thread overview]
Message-ID: <20100928174147.41e48aef.akpm@linux-foundation.org> (raw)
In-Reply-To: <alpine.DEB.2.00.1009281644110.21757@chino.kir.corp.google.com>

On Tue, 28 Sep 2010 16:52:01 -0700 (PDT) David Rientjes <rientjes@google.com> wrote:

> On Tue, 28 Sep 2010, Andrew Morton wrote:
> 
> > > > > So we can definitely remove __GFP_REPEAT for any order-0 allocation and 
> > > > > it's based on its implementation -- poorly defined as it may be -- and the 
> > > > > inherit design of any sane page allocator that retries such an allocation 
> > > > > if it's going to use reclaim in the first place.
> > > > 
> > > > Why was __GFP_REPEAT used in those callsites?  What were people trying
> > > > to achieve?
> > > > 
> > > 
> > > I can't predict what they were trying to achieve
> > 
> > Using my super powers it took me all of three minutes.
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/old-2.6-bkcvs.git
> > 
> > Do `git log > foo', and search foo for GFP_REPEAT.
> > 
> > A couple of interesting ones are:
> > 
> 
> Ok, so __GFP_REPEAT was used to replace the open-coding of retry loops by 
> putting that logic into the page allocator, and that logic was 
> subsequently changed to tie the bit to how many pages were reclaimed and 
> retry iff we haven't reclaimed the number of pages needed (in my patch, 
> that would be a single page).

It also shows that there was at least one version of the page allocator
(written by a very skilled and experienced Linux MM developer) which
would simply return NULL when there was no memory.  And here you are
assuring us that there will never ever be such a version again.

> It also shows that the page allocator has infinitely looped for 
> allocations under PAGE_ALLOC_COSTLY_ORDER since your patch from over seven 
> years ago.

Really don't give a shit what the allocator-of-the-moment happens to be
doing.

What I care about is that very smart, experienced and hard-working
Linux developers decided, a long time ago, that page-table allocations
are special, and need special treatment.  This is information!

What I also care about is lazy MM developers who just rip stuff out
without understanding it and without even bothering to make an
*attempt* to understand it.

> So, given the fact that the PAGE_ALLOC_COSTLY_ORDER logic has existed 
> since the same time, the semantics of __GFP_REPEAT have changed and are 
> often misrepresented, and we don't even invoke the __GFP_REPEAT logic for 
> any of the allocations in my patch since they are oom killable,

Probably this is because lazy ignorant MM developers broke earlier
intentions without even knowing that they were doing so.

> I think my patch should be merged.

Well before destroying the information, you tell me: why did MM
developers decide that page-table allocations needed special treatment?
 What experience led them to decide to implement that?  Has the problem
which they solved been fixed by other means?  Are those means
sufficiently permanently embedded (see "-aa" above) that we can afford
to destroy this piece of information?

If we break stuff and destroy this sort of historical knowledge then we
get to make the same mistakes every decade or so and the damn thing
*never* converges.

--
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:[~2010-09-29  0:42 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-28 10:45 David Rientjes
2010-09-28 21:36 ` Andrew Morton
2010-09-28 21:57   ` Russell King - ARM Linux
2010-09-28 22:47   ` David Rientjes
2010-09-28 22:53     ` Andrew Morton
2010-09-28 23:12       ` David Rientjes
2010-09-28 23:40         ` Andrew Morton
2010-09-28 23:52           ` David Rientjes
2010-09-29  0:41             ` Andrew Morton [this message]
2010-09-29  1: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=20100928174147.41e48aef.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=benh@kernel.crashing.org \
    --cc=davem@davemloft.net \
    --cc=deller@gmx.de \
    --cc=dhowells@redhat.com \
    --cc=geert@linux-m68k.org \
    --cc=heiko.carstens@de.ibm.com \
    --cc=hpa@zytor.com \
    --cc=ink@jurassic.park.msu.ru \
    --cc=jdike@addtoit.com \
    --cc=jesper.nilsson@axis.com \
    --cc=kyle@mcmartin.ca \
    --cc=lethal@linux-sh.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux@arm.linux.org.uk \
    --cc=mattst88@gmail.com \
    --cc=mingo@redhat.com \
    --cc=monstr@monstr.eu \
    --cc=paulus@samba.org \
    --cc=rientjes@google.com \
    --cc=rth@twiddle.net \
    --cc=schwidefsky@de.ibm.com \
    --cc=starvik@axis.com \
    --cc=tglx@linutronix.de \
    --cc=yasutake.koichi@jp.panasonic.com \
    --cc=zippel@linux-m68k.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