All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Matt Mackall <mpm@selenic.com>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"Kirill A. Shutemov" <kirill@shutemov.name>,
	Christoph Lameter <cl@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-crypto@vger.kernel.org, Geert.Uytterhoeven@sonycom.com
Subject: Re: [PATCH] Export symbol ksize()
Date: Sun, 15 Feb 2009 17:00:52 -0800	[thread overview]
Message-ID: <20090215170052.44ee8fd5.akpm@linux-foundation.org> (raw)
In-Reply-To: <1234741781.5669.204.camel@calx>

On Sun, 15 Feb 2009 17:49:41 -0600 Matt Mackall <mpm@selenic.com> wrote:

> On Sun, 2009-02-15 at 13:55 -0800, Andrew Morton wrote:
> > On Sun, 15 Feb 2009 15:43:14 -0600 Matt Mackall <mpm@selenic.com> wrote:
> > 
> > > On Sun, 2009-02-15 at 13:36 -0800, Andrew Morton wrote:
> > > > On Thu, 12 Feb 2009 17:55:04 +0200 Pekka Enberg <penberg@cs.helsinki.fi> wrote:
> > > > 
> > > > > On Thu, Feb 12, 2009 at 12:45:21PM +0200, Pekka Enberg wrote:
> > > > > > > 
> > > > > > > Because the API was being widely abused in the nommu code, for example.
> > > > > > > I'd rather not add it back for this special case which can be handled
> > > > > > > otherwise.
> > > > > 
> > > > > On Thu, 2009-02-12 at 18:50 +0800, Herbert Xu wrote:
> > > > > > I'm sorry but that's like banning the use of heaters just because
> > > > > > they can abused and cause fires.
> > > > > > 
> > > > > > I think I've said this to you before but in networking we very much
> > > > > > want to use ksize because the standard case of a 1500-byte packet
> > > > > > has loads of extra room given by kmalloc which all goes to waste
> > > > > > right now.
> > > > > > 
> > > > > > If we could use ksize then we can stuff loads of metadata in that
> > > > > > space.
> > > > > 
> > > > > OK, fair enough, I applied Kirill's patch. Thanks.
> > > > > 
> > > > 
> > > > Could we please have more details regarding this:
> > > > 
> > > > > The ksize() function is not exported to modules because it has non-standard
> > > > > behavour across different slab allocators. 
> > > > 
> > > > How does the behaviour differ?  It this documented?  Can we fix it?
> > > 
> > > SLAB and SLUB support calling ksize() on objects returned by
> > > kmem_cache_alloc.
> > > 
> > > SLOB only supports it on objects from kmalloc. This is because it does
> > > not store any size or type information in kmem_cache_alloc'ed objects.
> > > Instead, it infers them from the cache argument.
> > 
> > OK.  This is really bad, isn't it?
> 
> No. There are very few ksize callers and very few of those are making
> this particular category error.
> 
> And it -is- a category error. The fact that kmalloc is implemented on
> top of kmem_cache_alloc is an implementation detail that callers should
> not assume. They shouldn't call kfree() on kmem_cache_alloc objects
> (even though it might just happen to work), nor should they call
> ksize().

But they could call a new kmem_cache_size(cachep, obj)?

> > > Ideally SLAB and SLUB would complain about using ksize inappropriately
> > > when debugging was enabled.
> > > 
> > 
> > OK, thanks.
> > 
> > Ideally we would support ksize() for both kmalloc() and
> > kmem_cache_alloc() memory across all implementations.
> 
> There's never a good reason to call ksize on a kmem_cache_alloced
> object. You -must- statically know what type of object you have already
> to be able to free it later with kmem_cache_free, ergo, you can
> statically know how big it is too.

But kmem_cache_size() would tell you how much extra secret memory there
is available after the object?

How that gets along with redzoning is a bit of a mystery though.

The whole concept is quite hacky and nasty, isn't it?.  Does
networking/crypto actually show any gain from pulling this stunt?

> Another alternative to the above is to throw sparse at it, and have it
> track what allocators a pointer might have come through. 
> 
> But as far as I'm aware, there's only been one actual bug in this area:
> nommu was calling ksize on pointers of all kinds, including stuff
> allocated at compile time.
> 
> > Gee this sucks.  Biggest mistake I ever made.  Are we working hard
> > enough to remove some of these sl?b implementations?  Would it help if
> > I randomly deleted a couple?
> 
> Again, I think there's a strong argument for having two. We can't
> reasonably expect one allocator to work well on supercomputers and
> phones.

We can't reasonably expect an OS to work well on supercomputers and
phones ;)  It's a matter of how much person-power gets tossed at it.

> One will likely value performance significantly higher than
> memory usage and vice-versa.
> 
> I think most of the pain here is actually peripheral. SLUB in particular
> has churned a lot of interfaces. But we would have had that had we
> instead decided to throw a lot of effort into making SLAB better.

hm.

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

WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org>
To: Matt Mackall <mpm@selenic.com>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"Kirill A. Shutemov" <kirill@shutemov.name>,
	Christoph Lameter <cl@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-crypto@vger.kernel.org, Geert.Uytterhoeven@sonycom.com
Subject: Re: [PATCH] Export symbol ksize()
Date: Sun, 15 Feb 2009 17:00:52 -0800	[thread overview]
Message-ID: <20090215170052.44ee8fd5.akpm@linux-foundation.org> (raw)
In-Reply-To: <1234741781.5669.204.camel@calx>

On Sun, 15 Feb 2009 17:49:41 -0600 Matt Mackall <mpm@selenic.com> wrote:

> On Sun, 2009-02-15 at 13:55 -0800, Andrew Morton wrote:
> > On Sun, 15 Feb 2009 15:43:14 -0600 Matt Mackall <mpm@selenic.com> wrote:
> > 
> > > On Sun, 2009-02-15 at 13:36 -0800, Andrew Morton wrote:
> > > > On Thu, 12 Feb 2009 17:55:04 +0200 Pekka Enberg <penberg@cs.helsinki.fi> wrote:
> > > > 
> > > > > On Thu, Feb 12, 2009 at 12:45:21PM +0200, Pekka Enberg wrote:
> > > > > > > 
> > > > > > > Because the API was being widely abused in the nommu code, for example.
> > > > > > > I'd rather not add it back for this special case which can be handled
> > > > > > > otherwise.
> > > > > 
> > > > > On Thu, 2009-02-12 at 18:50 +0800, Herbert Xu wrote:
> > > > > > I'm sorry but that's like banning the use of heaters just because
> > > > > > they can abused and cause fires.
> > > > > > 
> > > > > > I think I've said this to you before but in networking we very much
> > > > > > want to use ksize because the standard case of a 1500-byte packet
> > > > > > has loads of extra room given by kmalloc which all goes to waste
> > > > > > right now.
> > > > > > 
> > > > > > If we could use ksize then we can stuff loads of metadata in that
> > > > > > space.
> > > > > 
> > > > > OK, fair enough, I applied Kirill's patch. Thanks.
> > > > > 
> > > > 
> > > > Could we please have more details regarding this:
> > > > 
> > > > > The ksize() function is not exported to modules because it has non-standard
> > > > > behavour across different slab allocators. 
> > > > 
> > > > How does the behaviour differ?  It this documented?  Can we fix it?
> > > 
> > > SLAB and SLUB support calling ksize() on objects returned by
> > > kmem_cache_alloc.
> > > 
> > > SLOB only supports it on objects from kmalloc. This is because it does
> > > not store any size or type information in kmem_cache_alloc'ed objects.
> > > Instead, it infers them from the cache argument.
> > 
> > OK.  This is really bad, isn't it?
> 
> No. There are very few ksize callers and very few of those are making
> this particular category error.
> 
> And it -is- a category error. The fact that kmalloc is implemented on
> top of kmem_cache_alloc is an implementation detail that callers should
> not assume. They shouldn't call kfree() on kmem_cache_alloc objects
> (even though it might just happen to work), nor should they call
> ksize().

But they could call a new kmem_cache_size(cachep, obj)?

> > > Ideally SLAB and SLUB would complain about using ksize inappropriately
> > > when debugging was enabled.
> > > 
> > 
> > OK, thanks.
> > 
> > Ideally we would support ksize() for both kmalloc() and
> > kmem_cache_alloc() memory across all implementations.
> 
> There's never a good reason to call ksize on a kmem_cache_alloced
> object. You -must- statically know what type of object you have already
> to be able to free it later with kmem_cache_free, ergo, you can
> statically know how big it is too.

But kmem_cache_size() would tell you how much extra secret memory there
is available after the object?

How that gets along with redzoning is a bit of a mystery though.

The whole concept is quite hacky and nasty, isn't it?.  Does
networking/crypto actually show any gain from pulling this stunt?

> Another alternative to the above is to throw sparse at it, and have it
> track what allocators a pointer might have come through. 
> 
> But as far as I'm aware, there's only been one actual bug in this area:
> nommu was calling ksize on pointers of all kinds, including stuff
> allocated at compile time.
> 
> > Gee this sucks.  Biggest mistake I ever made.  Are we working hard
> > enough to remove some of these sl?b implementations?  Would it help if
> > I randomly deleted a couple?
> 
> Again, I think there's a strong argument for having two. We can't
> reasonably expect one allocator to work well on supercomputers and
> phones.

We can't reasonably expect an OS to work well on supercomputers and
phones ;)  It's a matter of how much person-power gets tossed at it.

> One will likely value performance significantly higher than
> memory usage and vice-versa.
> 
> I think most of the pain here is actually peripheral. SLUB in particular
> has churned a lot of interfaces. But we would have had that had we
> instead decided to throw a lot of effort into making SLAB better.

hm.

  reply	other threads:[~2009-02-16  1:00 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-10 13:21 [PATCH] Export symbol ksize() Kirill A. Shutemov
2009-02-10 13:21 ` Kirill A. Shutemov
2009-02-10 13:35 ` Pekka Enberg
2009-02-10 13:35   ` Pekka Enberg
2009-02-10 13:46   ` Kirill A. Shutemov
2009-02-10 14:06     ` Pekka J Enberg
2009-02-10 14:06       ` Pekka J Enberg
2009-02-10 14:06       ` Pekka J Enberg
2009-02-12 10:43       ` Herbert Xu
2009-02-12 10:43         ` Herbert Xu
2009-02-12 10:45         ` Pekka Enberg
2009-02-12 10:45           ` Pekka Enberg
2009-02-12 10:50           ` Herbert Xu
2009-02-12 10:50             ` Herbert Xu
2009-02-12 13:10             ` Nick Piggin
2009-02-12 13:10               ` Nick Piggin
2009-02-12 23:09               ` Herbert Xu
2009-02-12 23:09                 ` Herbert Xu
2009-02-12 23:37                 ` Matt Mackall
2009-02-12 23:37                   ` Matt Mackall
2009-02-13 13:20                   ` Nick Piggin
2009-02-13 13:20                     ` Nick Piggin
2009-02-13 16:57                     ` Kyle Moffett
2009-02-13 16:57                       ` Kyle Moffett
2009-02-12 15:55             ` Pekka Enberg
2009-02-12 15:55               ` Pekka Enberg
2009-02-12 23:09               ` Herbert Xu
2009-02-12 23:09                 ` Herbert Xu
2009-02-15 21:36               ` Andrew Morton
2009-02-15 21:36                 ` Andrew Morton
2009-02-15 21:43                 ` Matt Mackall
2009-02-15 21:43                   ` Matt Mackall
2009-02-15 21:55                   ` Andrew Morton
2009-02-15 21:55                     ` Andrew Morton
2009-02-15 23:49                     ` Matt Mackall
2009-02-15 23:49                       ` Matt Mackall
2009-02-16  1:00                       ` Andrew Morton [this message]
2009-02-16  1:00                         ` Andrew Morton
2009-02-16  1:21                         ` Herbert Xu
2009-02-16  1:21                           ` Herbert Xu
2009-02-16  1:28                           ` Matt Mackall
2009-02-16  1:28                             ` Matt Mackall
2009-02-16  1:52                             ` Herbert Xu
2009-02-16  1:52                               ` Herbert Xu
2009-02-16  1:54                               ` Matt Mackall
2009-02-16  1:54                                 ` Matt Mackall
2009-02-16  1:57                                 ` Herbert Xu
2009-02-16  1:57                                   ` Herbert Xu
2009-02-16  1:38                         ` Matt Mackall
2009-02-16  1:38                           ` Matt Mackall
2009-02-17  8:43                           ` Geert Uytterhoeven
2009-02-17  8:43                             ` Geert Uytterhoeven
2009-02-17  8:43                             ` Geert Uytterhoeven
2009-02-17 16:17                       ` Christoph Lameter
2009-02-17 16:17                         ` Christoph Lameter
2009-02-17 17:03                         ` Pekka Enberg
2009-02-17 17:03                           ` Pekka Enberg
2009-02-17 17:06                           ` Christoph Lameter
2009-02-17 17:06                             ` Christoph Lameter
2009-02-16 13:56       ` Johannes Weiner
2009-02-16 13:56         ` Johannes Weiner
2009-02-16 14:09         ` Pekka Enberg
2009-02-16 14:09           ` Pekka Enberg
2009-02-16 16:32         ` Joe Perches
2009-02-16 16:32           ` Joe Perches
2009-02-16 17:29           ` Pekka Enberg
2009-02-16 17:29             ` Pekka Enberg

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=20090215170052.44ee8fd5.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=Geert.Uytterhoeven@sonycom.com \
    --cc=cl@linux-foundation.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=kirill@shutemov.name \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mpm@selenic.com \
    --cc=penberg@cs.helsinki.fi \
    /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.