All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Pekka Enberg <penberg@cs.helsinki.fi>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	gorcunov@openvz.org, kosaki.motohiro@jp.fujitsu.com,
	mel@csn.ul.ie, cl@linux-foundation.org, riel@redhat.com,
	linux-kernel@vger.kernel.org, rientjes@google.com
Subject: Re: [PATCH 1/2] mm: Introduce GFP_PANIC for early-boot allocations
Date: Sat, 9 May 2009 11:19:11 +0200	[thread overview]
Message-ID: <20090509091911.GA13784@elte.hu> (raw)
In-Reply-To: <4A054156.60501@cs.helsinki.fi>


* Pekka Enberg <penberg@cs.helsinki.fi> wrote:

> Hi Andrew,
>
> Andrew Morton wrote:
>> On Fri, 08 May 2009 18:10:28 +0300
>> Pekka Enberg <penberg@cs.helsinki.fi> wrote:
>>
>>> +#define GFP_PANIC	(__GFP_NOFAIL | __GFP_NORETRY)
>>
>> urgh, you have to be kidding me.  This significantly worsens complexity
>> and risk in core MM and it's just yuk.
>>
>> I think we can justify pulling such dopey party tricks to save 
>> pageframe space, or bits in page.flags and such.  But just to 
>> save a scrap of memory which would have been released during boot 
>> anwyay?  Don't think so.
>
> No, I wasn't kidding and I don't agree that it "significantly 
> worsens complexity". The point is not to save memory but to 
> clearly annotate those special call-sites that really don't need 
> to check for out-of-memory.

Frankly, i cannot believe that so many smart people dont see the 
simple, universal, un-arguable truism in the following statement:

 it is shorter, tidier, more maintainable, more reviewable to write:

	ptr = kmalloc(GFP_BOOT, size);

 than to write:

	ptr = kmalloc(GFP_KERNEL, size);
	BUG_ON(!ptr);

We have a lot of such patterns in platform code. Dozens and dozens 
of them.

There _might_ be some more nuanced second-level arguments: "yes, I 
agree in principle, but complexity elsewhere or other side-effects 
make this a net negative change."

Alas, those arguments would be wrong too:

 - we have a lot of such patterns and GFP_BOOT is unambigious 

 - post-bootup mis-use of GFP_BOOT could be warned about 
   unconditionally if used after free_initmem(), if we care enough.

 - Pekka's patch is dead simple. There's no "complexity" anywhere.

Agreeing to this and introducing this should have been a matter of 
30 seconds of thinking. Why are we still arguing about this? Dont we 
have enough bugs to worry about?

	Ingo

  reply	other threads:[~2009-05-09  9:20 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-08 15:10 [PATCH 1/2] mm: Introduce GFP_PANIC for early-boot allocations Pekka Enberg
2009-05-08 15:29 ` Christoph Lameter
2009-05-08 15:41   ` Pekka Enberg
2009-05-09  8:31     ` Ingo Molnar
2009-05-08 20:56 ` Andrew Morton
2009-05-09  8:39   ` Pekka Enberg
2009-05-09  9:19     ` Ingo Molnar [this message]
2009-05-09  9:19       ` Pekka Enberg
2009-05-09  9:31         ` Ingo Molnar
2009-05-09  9:32       ` Ingo Molnar
2009-05-10  3:34       ` Andrew Morton

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=20090509091911.GA13784@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux-foundation.org \
    --cc=gorcunov@openvz.org \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mel@csn.ul.ie \
    --cc=penberg@cs.helsinki.fi \
    --cc=riel@redhat.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.