mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Pekka Enberg <penberg@cs.helsinki.fi>
Cc: Christoph Lameter <clameter@sgi.com>,
	"Rafael J. Wysocki" <rjw@sisk.pl>,
	Pawel Staszewski <pstaszewski@artcom.pl>,
	LKML <linux-kernel@vger.kernel.org>,
	Adrian Bunk <bunk@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Natalie Protasevich <protasnb@gmail.com>
Subject: Re: 2.6.25-rc7-git2: Reported regressions from 2.6.24
Date: Fri, 28 Mar 2008 10:27:44 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.1.00.0803281017500.14670@woody.linux-foundation.org> (raw)
In-Reply-To: <84144f020803281015s60370d9dpec0913c33004b6c2@mail.gmail.com>



On Fri, 28 Mar 2008, Pekka Enberg wrote:
> 
> On Fri, Mar 28, 2008 at 6:00 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >  And the fact is, passing in GFP_ZERO from the SLUB code is a bug
> >  regardless, because it unnecessarily does the dual memset().
> 
> We clear GFP_ZERO in new_slab() so the normal kmalloc()/kzalloc() path
> should be fine but don't do it for kmalloc_large() nor
> kmalloc_large_node(). Is that the bug here?

Dammit, NO.

The bug was that the commit I made (which was correct and robust) was then 
partially reverted by Christoph for no good reason. At that point, 
kmalloc_large() didn't even exist, so at that point the change was 
"technically correct" (since the only user of gfpflags really did end up 
clearing it somewhere deep in its callchain).

So when that original 3811dbf67162bd08412f1b0e02e554f353e93bdb happened, 
it wasn't an outright bug - but that doesn't make it right. That commit 
was always just a bug waiting to happen, because it just set things up for 
later problems by retaining that bit when it really shouldn't have been 
retained, and forcing all future callers to be careful. Which they 
obviously were not!

Yes, you can clear GFP_ZERO in multiple illogical places, and it will fix 
the bug. Or you can clear it in *one* place, that is in on the direct 
callchain from the person who actually does the memset(0), and even add a 
comment that says exactly what is going on.

So the fact is, commit 3811dbf67162bd08412f1b0e02e554f353e93bdb is and was 
total and utter crap. I've reverted it in my tree. It's crap not because 
it was buggy when it was put in, but because it was *fragile* when it was 
put in. And that fragility ended up causing a bug later.

I'm getting really tired of slub. It was supposed to be simpler code than 
slab, and yes, it's simpler, but it has been buggy as hell, and part of it 
has been that people just haven't been careful enough, and haven't written 
code to be defensive and easy-to-follow.

So the *last* thing we want to do is to clear GFP_ZERO in multiple subtle 
places based on new random code being added. We want to clear it at the 
top level, so that no other code never ever even has to _think_ about it!

			Linus

  reply	other threads:[~2008-03-28 17:29 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-27 22:53 Rafael J. Wysocki
2008-03-28  0:18 ` Carlos R. Mafra
2008-03-28  0:23   ` Rafael J. Wysocki
2008-03-28  2:30 ` Linus Torvalds
2008-03-28  3:24   ` Christoph Lameter
2008-03-28  4:00     ` Linus Torvalds
2008-03-28 10:48       ` Paweł Staszewski
2008-03-28 17:46         ` Andrew Morton
2008-03-28 21:57           ` Rafael J. Wysocki
2008-03-28 17:15       ` Pekka Enberg
2008-03-28 17:27         ` Linus Torvalds [this message]
2008-03-28 18:08           ` Pekka Enberg
2008-03-28 18:20             ` Linus Torvalds
2008-03-28 18:38               ` Christoph Lameter
2008-03-28 18:47               ` Andrew Morton
2008-03-28 18:53                 ` Christoph Lameter
2008-03-28 19:37                 ` Linus Torvalds
2008-03-28 19:59                   ` Linus Torvalds
2008-03-28 19:59               ` Pekka Enberg
2008-03-28 20:24                 ` Linus Torvalds
2008-03-28 18:37           ` Christoph Lameter
2008-03-28 19:32             ` Linus Torvalds
2008-03-28 18:33       ` Christoph Lameter
2008-03-28 19:25         ` Linus Torvalds
2008-03-29 20:42           ` Christoph Lameter
2008-03-29 21:29             ` Linus Torvalds
2008-03-29 23:52               ` Pekka Enberg
2008-03-31 18:56                 ` Christoph Lameter
2008-03-31 18:45               ` Christoph Lameter
2008-03-28  3:31   ` Yinghai Lu
2008-03-31 10:14     ` Kamalesh Babulal
2008-03-31 12:10       ` Rafael J. Wysocki
2008-03-28 11:29   ` Haavard Skinnemoen
2008-03-28 16:11     ` Rafael J. Wysocki
2008-03-28 16:10   ` Rafael J. Wysocki
2008-03-28 16:47     ` Linus Torvalds
2008-03-28 17:36       ` Adrian Bunk
2008-03-28 20:33         ` Ingo Molnar
2008-03-28 22:28       ` Rafael J. Wysocki
2008-03-31 13:34         ` Ingo Molnar
2008-03-28 10:24 ` Thomas Gleixner
2008-03-28 10:58   ` Thomas Gleixner
2008-03-28 11:00     ` Peter Zijlstra
2008-03-28 11:13       ` Adrian Bunk
2008-03-28 11:16         ` Thomas Gleixner
2008-03-28 11:31           ` Adrian Bunk
2008-03-28 16:17             ` Rafael J. Wysocki
2008-03-28 17:06               ` Adrian Bunk
2008-03-28 20:42               ` Ingo Molnar
2008-03-28 22:33                 ` Rafael J. Wysocki
2008-03-28 11:44         ` Peter Zijlstra
2008-03-28 16:12   ` Rafael J. Wysocki
2008-03-28 16:18     ` Thomas Gleixner
2008-03-28 18:57 ` Mark Lord
2008-03-28 22:37   ` Rafael J. Wysocki

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=alpine.LFD.1.00.0803281017500.14670@woody.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=bunk@kernel.org \
    --cc=clameter@sgi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=penberg@cs.helsinki.fi \
    --cc=protasnb@gmail.com \
    --cc=pstaszewski@artcom.pl \
    --cc=rjw@sisk.pl \
    /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®