2007-06-02 01:37:54

by Christoph Lameter

[permalink] [raw]
Subject: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

Instead of returning the smallest available object return ZERO_SIZE_PTR.

A ZERO_SIZE_PTR can be legitimately used as an object pointer as long
as it is not deferenced. The dereference of ZERO_SIZE_PTR causes a
distinctive fault. kfree will handle a ZERO_SIZE_PTR in the same way as
NULL.

This enables functions to transparently use zero sized object. F.e.
if n = number of objects then the following code snippet will work
wether n = 0 or larger.

objects = kmalloc(n * sizeof(object), GFP_KERNEL);

for (i = 0; i < n; i++)
objects[i].x = y;

kfree(objects);

In addition to the warning for kmalloc(0) that is already there this patch
will cause a failure if there is an attempt to access objects in the zero
sized array.

Signed-off-by: Christoph Lameter <[email protected]>

---
include/linux/slub_def.h | 29 ++++++++++++++++++++++-------
mm/slub.c | 10 +++++-----
2 files changed, 27 insertions(+), 12 deletions(-)

Index: slub/include/linux/slub_def.h
===================================================================
--- slub.orig/include/linux/slub_def.h 2007-06-01 18:08:32.000000000 -0700
+++ slub/include/linux/slub_def.h 2007-06-01 18:22:56.000000000 -0700
@@ -74,14 +74,17 @@ extern struct kmem_cache kmalloc_caches[
*/
static inline int kmalloc_index(size_t size)
{
+
/*
- * We should return 0 if size == 0 (which would result in the
- * kmalloc caller to get NULL) but we use the smallest object
- * here for legacy reasons. Just issue a warning so that
- * we can discover locations where we do 0 sized allocations.
+ * The behavior for zero sized allocs changes. We no longer
+ * allocate memory but return ZERO_SIZE_PTR.
+ * WARN so that people can review and fix their code.
*/
WARN_ON_ONCE(size == 0);

+ if (!size)
+ return 0;
+
if (size > KMALLOC_MAX_SIZE)
return -1;

@@ -127,13 +130,25 @@ static inline struct kmem_cache *kmalloc
#define SLUB_DMA 0
#endif

+
+/*
+ * ZERO_SIZE_PTR will be returned for zero sized kmalloc requests.
+ *
+ * Dereferencing ZERO_SIZE_PTR will lead to a distinct access fault.
+ *
+ * ZERO_SIZE_PTR can be passed to kfree() in the same way that NULL can.
+ * Both make kfree() a no-op.
+ */
+#define ZERO_SIZE_PTR ((void *)16)
+
+
static inline void *kmalloc(size_t size, gfp_t flags)
{
if (__builtin_constant_p(size) && !(flags & SLUB_DMA)) {
struct kmem_cache *s = kmalloc_slab(size);

if (!s)
- return NULL;
+ return ZERO_SIZE_PTR;

return kmem_cache_alloc(s, flags);
} else
@@ -146,7 +161,7 @@ static inline void *kzalloc(size_t size,
struct kmem_cache *s = kmalloc_slab(size);

if (!s)
- return NULL;
+ return ZERO_SIZE_PTR;

return kmem_cache_zalloc(s, flags);
} else
@@ -162,7 +177,7 @@ static inline void *kmalloc_node(size_t
struct kmem_cache *s = kmalloc_slab(size);

if (!s)
- return NULL;
+ return ZERO_SIZE_PTR;

return kmem_cache_alloc_node(s, flags, node);
} else
Index: slub/mm/slub.c
===================================================================
--- slub.orig/mm/slub.c 2007-06-01 18:08:32.000000000 -0700
+++ slub/mm/slub.c 2007-06-01 18:22:56.000000000 -0700
@@ -2286,7 +2286,7 @@ void *__kmalloc(size_t size, gfp_t flags

if (s)
return slab_alloc(s, flags, -1, __builtin_return_address(0));
- return NULL;
+ return ZERO_SIZE_PTR;
}
EXPORT_SYMBOL(__kmalloc);

@@ -2297,7 +2297,7 @@ void *__kmalloc_node(size_t size, gfp_t

if (s)
return slab_alloc(s, flags, node, __builtin_return_address(0));
- return NULL;
+ return ZERO_SIZE_PTR;
}
EXPORT_SYMBOL(__kmalloc_node);
#endif
@@ -2338,7 +2338,7 @@ void kfree(const void *x)
struct kmem_cache *s;
struct page *page;

- if (!x)
+ if (x <= ZERO_SIZE_PTR)
return;

page = virt_to_head_page(x);
@@ -2707,7 +2707,7 @@ void *__kmalloc_track_caller(size_t size
struct kmem_cache *s = get_slab(size, gfpflags);

if (!s)
- return NULL;
+ return ZERO_SIZE_PTR;

return slab_alloc(s, gfpflags, -1, caller);
}
@@ -2718,7 +2718,7 @@ void *__kmalloc_node_track_caller(size_t
struct kmem_cache *s = get_slab(size, gfpflags);

if (!s)
- return NULL;
+ return ZERO_SIZE_PTR;

return slab_alloc(s, gfpflags, node, caller);
}


2007-06-02 02:10:38

by John Anthony Kazos Jr.

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

> + * The behavior for zero sized allocs changes. We no longer
> + * allocate memory but return ZERO_SIZE_PTR.
> + * WARN so that people can review and fix their code.

I don't see why people have so much opposition to zero-size memory
allocations. There's all sorts of situations where you want a resizeable
array that may have zero objects, especially in these days of
hotpluggability.

Not only is it simpler (and therefore less likely to be buggy) to write
code to simply resize to current number of objects, but not having to make
additional code for checking the special case of count==0 leading to
different function calls (instead of always reallocating, you might have
to allocate instead, and instead of reallocating to zero you might have to
free instead) will very slightly increase the object-text size.

The standard-C behavior of valid zero-size allocation has a very good
reason.

2007-06-02 02:16:01

by Christoph Lameter

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Fri, 1 Jun 2007, John Anthony Kazos Jr. wrote:

> > + * The behavior for zero sized allocs changes. We no longer
> > + * allocate memory but return ZERO_SIZE_PTR.
> > + * WARN so that people can review and fix their code.
>
> I don't see why people have so much opposition to zero-size memory
> allocations. There's all sorts of situations where you want a resizeable
> array that may have zero objects, especially in these days of
> hotpluggability.

In case you have not read the description to the end: This patch does
exactly what you want and legitimizes zero size object use. The warning
will be remove before 2.6.22 is released.

2007-06-02 02:21:50

by John Anthony Kazos Jr.

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

> > > + * The behavior for zero sized allocs changes. We no longer
> > > + * allocate memory but return ZERO_SIZE_PTR.
> > > + * WARN so that people can review and fix their code.
> >
> > I don't see why people have so much opposition to zero-size memory
> > allocations. There's all sorts of situations where you want a resizeable
> > array that may have zero objects, especially in these days of
> > hotpluggability.
>
> In case you have not read the description to the end: This patch does
> exactly what you want and legitimizes zero size object use. The warning
> will be remove before 2.6.22 is released.

Ah, sorry then. I just saw this, and remembered a rather contrary
discussion about it quite a time ago. Excellent then.

2007-06-02 02:55:16

by Linus Torvalds

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)



On Fri, 1 Jun 2007, Christoph Lameter wrote:
>
> - if (!x)
> + if (x <= ZERO_SIZE_PTR)
> return;

Btw, this is _not_ safe.

A number of gcc versions have done signed arithmetic on pointers. It's
insane and stupid, but it happens, and it so happens to work on
architectures where the point where the sign changes over is not a valid
pointer area.

On x86, doing signed arithmetic on pointers is a clear and unambiguous
_bug_ (because a C object really _can_ start in "positive" space and end
in "negative" pointer space), but I think some gcc versions did it there
too.

On some other architectures, like x86-64, the virtual memory around the
magic switch-over point is not mappable, so a C object cannot validly
straddle the area where positive overflows into negative, and as such a
compiler _could_ consider pointers to be signed (although I really don't
see the point).

So when I suggested the uglier

if ((unsigned long)x <= 16)
return;

I really did mean to use that ugly cast.. Yours is prettier, but sadly,
yours is simply not safe: a signed comparison might end up making _all_
kernel pointers trigger that test.

Linus

2007-06-02 03:06:35

by Christoph Lameter

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Fri, 1 Jun 2007, Linus Torvalds wrote:

> So when I suggested the uglier
>
> if ((unsigned long)x <= 16)
> return;
>
> I really did mean to use that ugly cast.. Yours is prettier, but sadly,
> yours is simply not safe: a signed comparison might end up making _all_
> kernel pointers trigger that test.

Maybe we can have a compromise? Lets at least keep the ZERO_SIZE_PTR
reference in there.


SLUB: Make sure that the comparision with ZERO_SIZE_PTR is unsigned

Signed-off-by: Christoph Lameter <[email protected]>


---
mm/slub.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

Index: slub/mm/slub.c
===================================================================
--- slub.orig/mm/slub.c 2007-06-01 20:00:56.000000000 -0700
+++ slub/mm/slub.c 2007-06-01 20:04:26.000000000 -0700
@@ -2338,7 +2338,13 @@ void kfree(const void *x)
struct kmem_cache *s;
struct page *page;

- if (x <= ZERO_SIZE_PTR)
+ /*
+ * This has to be an unsigned comparison. According to Linus
+ * some gcc version tread a pointer as a signed entity. Then
+ * this comparison would be true for all "negative" pointers
+ * (which would cover the whole upper half of the address space).
+ */
+ if ((unsigned long)x <= (unsigned long)ZERO_SIZE_PTR)
return;

page = virt_to_head_page(x);

2007-06-02 03:42:06

by Andrew Morton

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

> On Fri, 1 Jun 2007 18:37:46 -0700 (PDT) Christoph Lameter <[email protected]> wrote:
>
> +#define ZERO_SIZE_PTR ((void *)16)

Jeremy's point was a good one. The kernel _does_ use address-comparison
to determine object-inequality in an unknown but non-zero number of places.

It is of course unlikely that this will occur in conjunction with zero-sized
objects, but who knows?

2007-06-02 04:01:19

by Christoph Lameter

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Fri, 1 Jun 2007, Andrew Morton wrote:

> > On Fri, 1 Jun 2007 18:37:46 -0700 (PDT) Christoph Lameter <[email protected]> wrote:
> >
> > +#define ZERO_SIZE_PTR ((void *)16)
>
> Jeremy's point was a good one. The kernel _does_ use address-comparison
> to determine object-inequality in an unknown but non-zero number of places.
>
> It is of course unlikely that this will occur in conjunction with zero-sized
> objects, but who knows?

The zero sized objects are always the same and have the same content of
nothingness. So the kernel would find that they are the same which they
indeed are. Why could this be a problem?

2007-06-02 04:31:43

by Andrew Morton

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Fri, 1 Jun 2007 21:01:09 -0700 (PDT) Christoph Lameter <[email protected]> wrote:

> On Fri, 1 Jun 2007, Andrew Morton wrote:
>
> > > On Fri, 1 Jun 2007 18:37:46 -0700 (PDT) Christoph Lameter <[email protected]> wrote:
> > >
> > > +#define ZERO_SIZE_PTR ((void *)16)
> >
> > Jeremy's point was a good one. The kernel _does_ use address-comparison
> > to determine object-inequality in an unknown but non-zero number of places.
> >
> > It is of course unlikely that this will occur in conjunction with zero-sized
> > objects, but who knows?
>
> The zero sized objects are always the same and have the same content of
> nothingness. So the kernel would find that they are the same which they
> indeed are. Why could this be a problem?

They are different instances which happen to have the same length (zero).

But the code will incorrectly decide that they are the same instance. It
might cause refcounting or accounting errors, for example. I don't know - the
kernel's a big place.

I agree the risk is low, but if something _does_ blow up, it will do so subtly.

2007-06-02 04:45:25

by Christoph Lameter

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Fri, 1 Jun 2007, Andrew Morton wrote:

> They are different instances which happen to have the same length (zero).

I guess one could use the slab allocators as a type of reservation
ticket generator with zero sized objects. Hmmm.... But is that really a
useful thing to do?

> But the code will incorrectly decide that they are the same instance. It
> might cause refcounting or accounting errors, for example. I don't know - the
> kernel's a big place.

That would have to occur with objects that are repeatedly allocated and
then linked toghether etc. Linking typicallty requires a listhead so its
typically difficult to do zero length objects.

> I agree the risk is low, but if something _does_ blow up, it will do so subtly.

The cases that we have seen so far are due to array allocations of N
elements where N == 0 leads to the creation of a zero sized object.
The objects of the array are not zero sized it is just that zero of
them are allocated.

2007-06-02 04:54:47

by Andrew Morton

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Fri, 1 Jun 2007 21:45:15 -0700 (PDT) Christoph Lameter <[email protected]> wrote:

> On Fri, 1 Jun 2007, Andrew Morton wrote:
>
> > They are different instances which happen to have the same length (zero).
>
> I guess one could use the slab allocators as a type of reservation
> ticket generator with zero sized objects. Hmmm.... But is that really a
> useful thing to do?
>
> > But the code will incorrectly decide that they are the same instance. It
> > might cause refcounting or accounting errors, for example. I don't know - the
> > kernel's a big place.
>
> That would have to occur with objects that are repeatedly allocated and
> then linked toghether etc. Linking typicallty requires a listhead so its
> typically difficult to do zero length objects.

Well I can't immediately think of a scenario in which it's likely to occur,
but we're in the position of trying to prove a negative.

Poke Bill Irwin - he'll think of something ;)

> > I agree the risk is low, but if something _does_ blow up, it will do so subtly.
>
> The cases that we have seen so far are due to array allocations of N
> elements where N == 0 leads to the creation of a zero sized object.
> The objects of the array are not zero sized it is just that zero of
> them are allocated.

We lose leak-detection and double-free detection this way, too. Not a big
deal.

2007-06-03 16:15:51

by Matt Mackall

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Fri, Jun 01, 2007 at 09:45:15PM -0700, Christoph Lameter wrote:
> On Fri, 1 Jun 2007, Andrew Morton wrote:
>
> > They are different instances which happen to have the same length (zero).
>
> I guess one could use the slab allocators as a type of reservation
> ticket generator with zero sized objects. Hmmm.... But is that really a
> useful thing to do?

Yes. Old kmalloc(0) gives you a unique pointer that you can use as a
cookie. Which you can then use for all the things you'd use a cookie
for.

But I'd be pretty surprised if anyone was actually doing that. Because
it's easy enough to simply alloc some actual space and stuff some
actual data in it that corresponds to the cookie and use the pointer
to the actual space as the cookie.

On architectures where we have more address space, we could have a
dedicated lala-land for these things.

> > I agree the risk is low, but if something _does_ blow up, it will do so subtly.

Arguable the proposed badptr behavior is correct. It's basically "how many
angels can dance on the head of a pin"? All the returned pointers are
at least 0 bytes away from the previous one.

--
Mathematics is the supreme nostalgia of our time.

2007-06-03 16:17:40

by Matt Mackall

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Fri, Jun 01, 2007 at 09:54:27PM -0700, Andrew Morton wrote:
> We lose leak-detection and double-free detection this way, too. Not a big
> deal.

We could keep a global counter and warn if count < 0 or > N.

An awful lot of fuss over nothing though.

--
Mathematics is the supreme nostalgia of our time.

2007-06-04 14:42:59

by Alan

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

> > > I agree the risk is low, but if something _does_ blow up, it will do so subtly.
>
> Arguable the proposed badptr behavior is correct. It's basically "how many
> angels can dance on the head of a pin"? All the returned pointers are
> at least 0 bytes away from the previous one.

C++ very carefully keeps objects of zero size at differing addresses to
avoid exactly this kind of pointer confusion. Given the trivial fix is
simply

size += !size;

at the start of malloc what is there worth arguing about ?

2007-06-04 15:08:34

by Pekka Enberg

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Fri, 1 Jun 2007, Andrew Morton wrote:
> > They are different instances which happen to have the same length (zero).

On 6/2/07, Christoph Lameter <[email protected]> wrote:
> I guess one could use the slab allocators as a type of reservation
> ticket generator with zero sized objects. Hmmm.... But is that really a
> useful thing to do?

Can someone remind me why we can't do what mm/slab.c does for
zero-size allocations in slub too?

2007-06-04 16:16:47

by Christoph Lameter

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Mon, 4 Jun 2007, Pekka Enberg wrote:

> On Fri, 1 Jun 2007, Andrew Morton wrote:
> > > They are different instances which happen to have the same length (zero).
>
> On 6/2/07, Christoph Lameter <[email protected]> wrote:
> > I guess one could use the slab allocators as a type of reservation
> > ticket generator with zero sized objects. Hmmm.... But is that really a
> > useful thing to do?
>
> Can someone remind me why we can't do what mm/slab.c does for
> zero-size allocations in slub too?

We are doing that right now. The problem is that people keep storing stuff
in memory allocated with kmalloc(0).

2007-06-04 16:22:17

by Pekka Enberg

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On 6/4/07, Christoph Lameter <[email protected]> wrote:
> We are doing that right now. The problem is that people keep storing stuff
> in memory allocated with kmalloc(0).

Ok, makes sense. I guess I might as well throw my suggestion in the
mix. Lets create a new kmalloc cache for zero-length objects where
object size is zero but there are regular red-zones on both sides.

2007-06-04 16:27:16

by Linus Torvalds

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)



On Mon, 4 Jun 2007, Pekka Enberg wrote:
>
> Ok, makes sense. I guess I might as well throw my suggestion in the
> mix. Lets create a new kmalloc cache for zero-length objects where
> object size is zero but there are regular red-zones on both sides.

Well, the red-zones won't catch readers, and more importantly, even for
writers they are *really* inconvenient, because it will just tell you
something bad happened, it won't tell you *where* it happened.

Since comparing the addresses of two zero-sized allocations is insane and
not done _anyway_, it's just much better to return an invalid address.

The thing is, why *should* we care about comparing addresses? We'll give
the right result (you got many perfectly separate allocations, they're
just zero bytes apart, exactly like you asked for!). The fact that C++ has
some semantics for it is not a good argument - C++ is a broken language,
and it's not the language we use for the kernel anyway.

Linus

2007-06-04 16:33:01

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

Linus Torvalds wrote:
> The thing is, why *should* we care about comparing addresses? We'll give
> the right result (you got many perfectly separate allocations, they're
> just zero bytes apart, exactly like you asked for!). The fact that C++ has
> some semantics for it is not a good argument - C++ is a broken language,
> and it's not the language we use for the kernel anyway.

C too, but I really honestly can't think of a scenario - realistic or
contrived - in which you'd end up doing a zero-sized allocation and care
that its address has been aliased. But we'll find out when we do it ;)

J

2007-06-04 16:37:17

by Pekka Enberg

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On 6/4/07, Linus Torvalds <[email protected]> wrote:
> Well, the red-zones won't catch readers, and more importantly, even for
> writers they are *really* inconvenient, because it will just tell you
> something bad happened, it won't tell you *where* it happened.

True.

On 6/4/07, Linus Torvalds <[email protected]> wrote:
> Since comparing the addresses of two zero-sized allocations is insane and
> not done _anyway_, it's just much better to return an invalid address.

Then we might as well return your regular NULL pointer for zero-length
allocations as you can't do anything sane with ZERO_SIZE_PTR either.

2007-06-04 16:38:42

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

Pekka Enberg wrote:
> Then we might as well return your regular NULL pointer for zero-length
> allocations as you can't do anything sane with ZERO_SIZE_PTR either.

Round and round. You can't distinguish NULL from allocation failure,
which kmalloc(0) decidedly *isn't*.

J

2007-06-04 16:43:20

by Roland Dreier

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

> Then we might as well return your regular NULL pointer for zero-length
> allocations as you can't do anything sane with ZERO_SIZE_PTR either.

No, because as was mentioned earlier in the thread, we want code to be
able to handle 0-sized allocations without special cases. The goal is
that code like

buf = kmalloc(nobj * obj_size);
if (buf == NULL)
return -ENOMEM;

should work fine if nobj happens to be 0. But we do want to get an
oops if the code actually tries to read or write *buf.

- R.

2007-06-04 16:48:54

by Linus Torvalds

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)



On Mon, 4 Jun 2007, Pekka Enberg wrote:
>
> Then we might as well return your regular NULL pointer for zero-length
> allocations as you can't do anything sane with ZERO_SIZE_PTR either.


No. NULL really _is_ special.

We use NULL in tons of places for saying "we haven't allocated anything at
all".

That's *not* the same as saying "we have initialized this pointer, it just
happens to point to a zero-sized object".

Two *totally* different things. I don't understand why people mix them up.

We literally have code that tests pointers for NULL to determine if the
subsystem has been initialized.

In other words: YOU MUST NOT RETURN NULL FOR A "SUCCESSFUL POINTER
ALLOCATION", regardless of the size.

Yeah, yeah, I realize that the C library traditionally allows it, and user
space is used to it, but it's still wrong and stupid. We can do so much
better.

When malloc() returns NULL, it means that it failed.

When a pointer is NULL, it means that it doesn't exist.

And ZERO_SIZE_PTR is _neither_ of those cases!

Linus

2007-06-04 17:02:19

by Pekka Enberg

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On 6/4/07, Roland Dreier <[email protected]> wrote:
> No, because as was mentioned earlier in the thread, we want code to be
> able to handle 0-sized allocations without special cases. The goal is
> that code like
>
> buf = kmalloc(nobj * obj_size);
> if (buf == NULL)
> return -ENOMEM;
>
> should work fine if nobj happens to be 0. But we do want to get an
> oops if the code actually tries to read or write *buf.

Aah, missed that part of the discussion. Sorry for the noise ;-)

2007-06-04 17:15:48

by Alan

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

> The thing is, why *should* we care about comparing addresses? We'll give

Because people use it to tell objects apart. All over the kernel we do
things like

if (inode1 == inode2)

to figure out if they are the same inode. Now inodes are not zero sized
so we are safe there

> the right result (you got many perfectly separate allocations, they're
> just zero bytes apart, exactly like you asked for!). The fact that C++ has
> some semantics for it is not a good argument

Its the usual semantic in languages that use a pointer as an object
handle. C is actually the oddity here, the other languages use it to
avoid aliasing of object handles and the resulting confusion.

Clearly we do need an option to make the kernel explode when misusing
such pointers for debugging whatever the default for other behaviour is.
And on a 64bit box we probably have enough address space to hand out a
few million unique faulting pointers.

2007-06-04 17:51:16

by Linus Torvalds

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)



On Mon, 4 Jun 2007, Alan Cox wrote:

> > The thing is, why *should* we care about comparing addresses? We'll give
>
> Because people use it to tell objects apart. All over the kernel we do
> things like
>
> if (inode1 == inode2)

But that only makes sense if your objects have meaning, which is not
possible with a zero-sized object.

Let's take an example: one of the few reasons to check for equality (or
inequality) is for locking order.

So on UP, the lock goes away, and let's say that you (insanely) only have
a spinlock in the structure in question, so you have a zero-sized
structure that you want to test ordering on because you want to avoid
ABBA deadlocks.

So you write your code as

double_lock()
{
if (ptr1 == ptr2) {
lock(ptr1->lock);
return;
}
if (ptr1 < ptr2) {
lock(ptr1->lock);
lock(ptr2->lock);
return;
}
lock(ptr2->lock);
lock(ptr1->lock);
}

and the interesting thing is that this actually *works* even for the case
where the lock has gone away: even though ptr1/ptr2 are always equal.

In other words, the only _valid_ reasons to compare pointers like this end
up degenerating into working cases even for a zero-sized pointer.

The exception is if you use the memory allocator as a "ID allocator", but
quite frankly, if you use a size of zero, it's your own damn problem.
Insane code is not an argument for insane behaviour.

If people can't be bothered to create a "random ID generator" themselves,
they had damn well better use "kmalloc(1)" rather than "kmalloc(0)" to get
a unique cookie. Asking the allocator to do something idiotic because some
idiot thinks a memory allocator is a cookie allocator is just crazy.

I can understand that things like user-level libraries have to take crazy
people into account, but the kernel internal libraries definitely do not.

(Right now we warn once for zero-sized allocations anyway, and all the
cases we've found so far are either bugs that would have been found with
ZERO_ALLOC_PTR or would have been perfectly fine with it, so I don't think
anybody really _is_ that insane in the kernel)

Linus

2007-06-04 17:58:53

by William Lee Irwin III

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Fri, 1 Jun 2007 21:45:15 -0700 (PDT) Christoph Lameter <[email protected]> wrote:
>> That would have to occur with objects that are repeatedly allocated and
>> then linked toghether etc. Linking typicallty requires a listhead so its
>> typically difficult to do zero length objects.

On Fri, Jun 01, 2007 at 09:54:27PM -0700, Andrew Morton wrote:
> Well I can't immediately think of a scenario in which it's likely to occur,
> but we're in the position of trying to prove a negative.
> Poke Bill Irwin - he'll think of something ;)

I've yet to see anyone get quite that creative, but I've not gone fishing
for instances of this. I can think of plenty of places where one could do
something like this in practice, but don't care to give anyone any ideas.


-- wli

2007-06-04 18:04:58

by William Lee Irwin III

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Mon, Jun 04, 2007 at 10:50:41AM -0700, Linus Torvalds wrote:
> The exception is if you use the memory allocator as a "ID allocator", but
> quite frankly, if you use a size of zero, it's your own damn problem.
> Insane code is not an argument for insane behaviour.
> If people can't be bothered to create a "random ID generator" themselves,
> they had damn well better use "kmalloc(1)" rather than "kmalloc(0)" to get
> a unique cookie. Asking the allocator to do something idiotic because some
> idiot thinks a memory allocator is a cookie allocator is just crazy.

It's not such a great idea in general. Maybe it's a dumb device to cut
down on lines of code for merging or some such.


On Mon, Jun 04, 2007 at 10:50:41AM -0700, Linus Torvalds wrote:
> I can understand that things like user-level libraries have to take crazy
> people into account, but the kernel internal libraries definitely do not.
> (Right now we warn once for zero-sized allocations anyway, and all the
> cases we've found so far are either bugs that would have been found with
> ZERO_ALLOC_PTR or would have been perfectly fine with it, so I don't think
> anybody really _is_ that insane in the kernel)

There are always drivers for that, but I doubt any were sufficiently
creative to pick up on this. At least I've not see any.


-- wli

2007-06-04 18:23:35

by Cyrill Gorcunov

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Pekka Enberg - Mon, Jun 04, 2007 at 07:37:01PM +0300]
| On 6/4/07, Linus Torvalds <[email protected]> wrote:
| >Well, the red-zones won't catch readers, and more importantly, even for
| >writers they are *really* inconvenient, because it will just tell you
| >something bad happened, it won't tell you *where* it happened.
|
| True.
|
| On 6/4/07, Linus Torvalds <[email protected]> wrote:
| >Since comparing the addresses of two zero-sized allocations is insane and
| >not done _anyway_, it's just much better to return an invalid address.
|
| Then we might as well return your regular NULL pointer for zero-length
| allocations as you can't do anything sane with ZERO_SIZE_PTR either.

Hi Pekka,

you know, I'm absolutely agree with you. Hey, kernel hackers don't blame
me ;). But lets just take an example from a real life: I'm asking for
someone to give me 50 cents he would probably give me this. But if I'm
asking someone to give me 0 cents - he''ll give me nothing!!! Nobody
giving me a spec. papper on which "zero cents" is written. :)
So the code

p = kmalloc(0);
if (!p) {
return -ENOMEM;
}

is absolutely right - we physically have _no_ zero-sized memory. And as result
we have no reason to keep spec. slab to track zero-sized allocs.

Please, don't get me wrong, I'm not a kernel specialist...
And sorry for meddling into coversation.

Cyrill

2007-06-04 18:47:41

by Linus Torvalds

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)



On Fri, 1 Jun 2007, Christoph Lameter wrote:
>
> Instead of returning the smallest available object return ZERO_SIZE_PTR.

Ok, I just noticed that this still has a bug: not just kfree(), but
krealloc() needs to treat ZERO_SIZE_PTR properly.

Your patch introduces two bugs in mm/slub.c:krealloc():

- The

if (unlikely(!p))
return kmalloc(new_size, flags);

test needs to be for NULL or ZERO_SIZE_PTR. Otherwise it will oops in
ksize(p), I think.

- And the

if (unlikely(!new_size)) {
kfree(p);
return NULL;
}

thing should logically return ZERO_SIZE_PTR instead of NULL.

So basically

krealloc(kmalloc(0), n, flags);

must work, and

krealloc(old, 0, flags)

should return a zero-sized allocation.

I'd forgotten about krealloc(), because that whole concept is fairly new.

Linus

2007-06-04 19:01:58

by Christoph Lameter

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Mon, 4 Jun 2007, Linus Torvalds wrote:

> Ok, I just noticed that this still has a bug: not just kfree(), but
> krealloc() needs to treat ZERO_SIZE_PTR properly.


SLUB: Return ZERO_SIZE_PTR for kmalloc(0) V2

Instead of returning the smallest available object return ZERO_SIZE_PTR.

A ZERO_SIZE_PTR can be legitimately used as an object pointer as long
as it is not deferenced. The dereference of ZERO_SIZE_PTR causes a
distinctive fault.
kfree can handle a ZERO_SIZE_PTR in the same way as NULL.

This enables functions to use zero sized object. e.g. n = number of objects.


objects = kmalloc(n * sizeof(object));

for (i = 0; i < n; i++)
objects[i].x = y;

kfree(objects);

In addition to the warning that is already there for kmalloc(0) this patch
will cause the code to fail if there is an attempt to access objects in the
zero sized array.

V1->V2:
- Add support for ZERO_SIZE_PTR to krealloc
- Add support for ZERO_SIZE_PTR to ksize
- Fix spelling
- Insure we do an unsigned comparison in kfree.

Signed-off-by: Christoph Lameter <[email protected]>

---
include/linux/slub_def.h | 29 ++++++++++++++++++++++-------
mm/slub.c | 26 ++++++++++++++++++--------
2 files changed, 40 insertions(+), 15 deletions(-)

Index: slub/include/linux/slub_def.h
===================================================================
--- slub.orig/include/linux/slub_def.h 2007-06-01 18:08:32.000000000 -0700
+++ slub/include/linux/slub_def.h 2007-06-04 11:49:59.000000000 -0700
@@ -74,14 +74,17 @@ extern struct kmem_cache kmalloc_caches[
*/
static inline int kmalloc_index(size_t size)
{
+
/*
- * We should return 0 if size == 0 (which would result in the
- * kmalloc caller to get NULL) but we use the smallest object
- * here for legacy reasons. Just issue a warning so that
- * we can discover locations where we do 0 sized allocations.
+ * The behavior for zero sized allocs changes. We no longer
+ * allocate memory but return ZERO_SIZE_PTR.
+ * WARN so that people can review and fix their code.
*/
WARN_ON_ONCE(size == 0);

+ if (!size)
+ return 0;
+
if (size > KMALLOC_MAX_SIZE)
return -1;

@@ -127,13 +130,25 @@ static inline struct kmem_cache *kmalloc
#define SLUB_DMA 0
#endif

+
+/*
+ * ZERO_SIZE_PTR will be returned for zero sized kmalloc requests.
+ *
+ * Dereferencing ZERO_SIZE_PTR will lead to a distinct access fault.
+ *
+ * ZERO_SIZE_PTR can be passed to kfree though in the same way that NULL can.
+ * Both make kfree a no-op.
+ */
+#define ZERO_SIZE_PTR ((void *)16)
+
+
static inline void *kmalloc(size_t size, gfp_t flags)
{
if (__builtin_constant_p(size) && !(flags & SLUB_DMA)) {
struct kmem_cache *s = kmalloc_slab(size);

if (!s)
- return NULL;
+ return ZERO_SIZE_PTR;

return kmem_cache_alloc(s, flags);
} else
@@ -146,7 +161,7 @@ static inline void *kzalloc(size_t size,
struct kmem_cache *s = kmalloc_slab(size);

if (!s)
- return NULL;
+ return ZERO_SIZE_PTR;

return kmem_cache_zalloc(s, flags);
} else
@@ -162,7 +177,7 @@ static inline void *kmalloc_node(size_t
struct kmem_cache *s = kmalloc_slab(size);

if (!s)
- return NULL;
+ return ZERO_SIZE_PTR;

return kmem_cache_alloc_node(s, flags, node);
} else
Index: slub/mm/slub.c
===================================================================
--- slub.orig/mm/slub.c 2007-06-01 18:08:32.000000000 -0700
+++ slub/mm/slub.c 2007-06-04 11:59:41.000000000 -0700
@@ -2286,7 +2286,7 @@ void *__kmalloc(size_t size, gfp_t flags

if (s)
return slab_alloc(s, flags, -1, __builtin_return_address(0));
- return NULL;
+ return ZERO_SIZE_PTR;
}
EXPORT_SYMBOL(__kmalloc);

@@ -2297,16 +2297,20 @@ void *__kmalloc_node(size_t size, gfp_t

if (s)
return slab_alloc(s, flags, node, __builtin_return_address(0));
- return NULL;
+ return ZERO_SIZE_PTR;
}
EXPORT_SYMBOL(__kmalloc_node);
#endif

size_t ksize(const void *object)
{
- struct page *page = get_object_page(object);
+ struct page *page;
struct kmem_cache *s;

+ if (object == ZERO_SIZE_PTR)
+ return 0;
+
+ page = get_object_page(object);
BUG_ON(!page);
s = page->slab;
BUG_ON(!s);
@@ -2338,7 +2342,13 @@ void kfree(const void *x)
struct kmem_cache *s;
struct page *page;

- if (!x)
+ /*
+ * This has to be an unsigned comparison. According to Linus
+ * some gcc version treat a pointer as a signed entity. Then
+ * this comparison would be true for all "negative" pointers
+ * (which would cover the whole upper half of the address space).
+ */
+ if ((unsigned long)x <= (unsigned long)ZERO_SIZE_PTR)
return;

page = virt_to_head_page(x);
@@ -2443,12 +2453,12 @@ void *krealloc(const void *p, size_t new
void *ret;
size_t ks;

- if (unlikely(!p))
+ if (unlikely(!p || p == ZERO_SIZE_PTR))
return kmalloc(new_size, flags);

if (unlikely(!new_size)) {
kfree(p);
- return NULL;
+ return ZERO_SIZE_PTR;
}

ks = ksize(p);
@@ -2707,7 +2717,7 @@ void *__kmalloc_track_caller(size_t size
struct kmem_cache *s = get_slab(size, gfpflags);

if (!s)
- return NULL;
+ return ZERO_SIZE_PTR;

return slab_alloc(s, gfpflags, -1, caller);
}
@@ -2718,7 +2728,7 @@ void *__kmalloc_node_track_caller(size_t
struct kmem_cache *s = get_slab(size, gfpflags);

if (!s)
- return NULL;
+ return ZERO_SIZE_PTR;

return slab_alloc(s, gfpflags, node, caller);
}

2007-06-04 19:13:39

by Pekka Enberg

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

Hi Christoph,

On 6/4/07, Christoph Lameter <[email protected]> wrote:
> /*
> - * We should return 0 if size == 0 (which would result in the
> - * kmalloc caller to get NULL) but we use the smallest object
> - * here for legacy reasons. Just issue a warning so that
> - * we can discover locations where we do 0 sized allocations.
> + * The behavior for zero sized allocs changes. We no longer
> + * allocate memory but return ZERO_SIZE_PTR.
> + * WARN so that people can review and fix their code.
> */
> WARN_ON_ONCE(size == 0);

I thought the whole point of this patch was to get rid of the WARN_ON
as you will get a nice oops if you dereference the pointer?

2007-06-04 19:15:43

by Christoph Lameter

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Mon, 4 Jun 2007, Pekka Enberg wrote:

> I thought the whole point of this patch was to get rid of the WARN_ON
> as you will get a nice oops if you dereference the pointer?

That is another patchset. See
http://marc.info/?l=linux-kernel&w=2&r=1&s=DEVELKERNEL&q=b

2007-06-04 22:30:57

by Pekka Enberg

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

Christoph Lameter wrote:
> That is another patchset. See
> http://marc.info/?l=linux-kernel&w=2&r=1&s=DEVELKERNEL&q=b

Oh my, I am totally confused now.

First you fix kmalloc(0) to be legal and safe. And then you want to
DEVEL_WARN_ON_ONCE when size is zero so people can fix their code?

I don't get it.

I thought we wanted to support kmalloc(0) so that as long as you don't
dereference the pointer, it's all legal and good. Right? So we obviously
should shut up the WARN_ON because if you do oops, you can clearly see
that it happened at ZERO_SIZE_PTR and have a nice stack trace anyway...

Btw, if I am again missing something totally obvious, could you please
be so kind to send me a batch of the same pills that the smart people
take. I am all out.

Pekka

2007-06-04 22:39:19

by Christoph Lameter

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Mon, 4 Jun 2007, Pekka Enberg wrote:

> Christoph Lameter wrote:
> > That is another patchset. See
> > http://marc.info/?l=linux-kernel&w=2&r=1&s=DEVELKERNEL&q=b
>
> Oh my, I am totally confused now.
>
> First you fix kmalloc(0) to be legal and safe. And then you want to
> DEVEL_WARN_ON_ONCE when size is zero so people can fix their code?
>
> I don't get it.

Sorry complex situation. Andrew wants the warnings, Linus wants the
ZERO_SIZE_PTR. Not sure where we are going here.

> I thought we wanted to support kmalloc(0) so that as long as you don't
> dereference the pointer, it's all legal and good. Right? So we obviously
> should shut up the WARN_ON because if you do oops, you can clearly see that it
> happened at ZERO_SIZE_PTR and have a nice stack trace anyway...

Well so far we agreed to keep the warnings in at least till release data
and I have not heard differently yet.

> Btw, if I am again missing something totally obvious, could you please be so
> kind to send me a batch of the same pills that the smart people take. I am all
> out.

Heheheh.... Mind boogling isnt it? Patch duplicity....

Seriously: Andrew/Linus could you make up your mind which way we are
going here?

I'd say lets drop the DEVELKERNEL stuff and the warnings and go with
ZERO_SIZE_PTR. The DEVELKERNEL patch has the danger that subtle changes
occur at release time that we have not anticipated.

2007-06-04 22:54:17

by Andrew Morton

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Mon, 4 Jun 2007 15:39:08 -0700 (PDT)
Christoph Lameter <[email protected]> wrote:

> > Btw, if I am again missing something totally obvious, could you please be so
> > kind to send me a batch of the same pills that the smart people take. I am all
> > out.
>
> Heheheh.... Mind boogling isnt it? Patch duplicity....
>
> Seriously: Andrew/Linus could you make up your mind which way we are
> going here?

I've no strong opinions - I'm just randomly waffling.

> I'd say lets drop the DEVELKERNEL stuff and the warnings and go with
> ZERO_SIZE_PTR. The DEVELKERNEL patch has the danger that subtle changes
> occur at release time that we have not anticipated.

OK.

2007-06-04 23:10:04

by Christoph Lameter

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Mon, 4 Jun 2007, Andrew Morton wrote:

> > I'd say lets drop the DEVELKERNEL stuff and the warnings and go with
> > ZERO_SIZE_PTR. The DEVELKERNEL patch has the danger that subtle changes
> > occur at release time that we have not anticipated.
>
> OK.

Here a version of the patch that drops the WARN_ONs


SLUB: Return ZERO_SIZE_PTR for kmalloc(0) V3

Instead of returning the smallest available object return ZERO_SIZE_PTR.

A ZERO_SIZE_PTR can be legitimately used as an object pointer as long
as it is not deferenced. The dereference of ZERO_SIZE_PTR causes a
distinctive fault.
kfree can handle a ZERO_SIZE_PTR in the same way as NULL.

This enables functions to use zero sized object. e.g. n = number of objects.


objects = kmalloc(n * sizeof(object));

for (i = 0; i < n; i++)
objects[i].x = y;

kfree(objects);

V1->V2:
- Add support for ZERO_SIZE_PTR to krealloc
- Add support for ZERO_SIZE_PTR to ksize

V2->V3
- Drop WARN_ON for zero sized allocs

Signed-off-by: Christoph Lameter <[email protected]>

---
include/linux/slub_def.h | 27 +++++++++++++++++----------
mm/slub.c | 26 ++++++++++++++++++--------
2 files changed, 35 insertions(+), 18 deletions(-)

Index: slub/include/linux/slub_def.h
===================================================================
--- slub.orig/include/linux/slub_def.h 2007-06-01 18:08:32.000000000 -0700
+++ slub/include/linux/slub_def.h 2007-06-04 16:06:08.000000000 -0700
@@ -74,13 +74,8 @@ extern struct kmem_cache kmalloc_caches[
*/
static inline int kmalloc_index(size_t size)
{
- /*
- * We should return 0 if size == 0 (which would result in the
- * kmalloc caller to get NULL) but we use the smallest object
- * here for legacy reasons. Just issue a warning so that
- * we can discover locations where we do 0 sized allocations.
- */
- WARN_ON_ONCE(size == 0);
+ if (!size)
+ return 0;

if (size > KMALLOC_MAX_SIZE)
return -1;
@@ -127,13 +122,25 @@ static inline struct kmem_cache *kmalloc
#define SLUB_DMA 0
#endif

+
+/*
+ * ZERO_SIZE_PTR will be returned for zero sized kmalloc requests.
+ *
+ * Dereferencing ZERO_SIZE_PTR will lead to a distinct access fault.
+ *
+ * ZERO_SIZE_PTR can be passed to kfree though in the same way that NULL can.
+ * Both make kfree a no-op.
+ */
+#define ZERO_SIZE_PTR ((void *)16)
+
+
static inline void *kmalloc(size_t size, gfp_t flags)
{
if (__builtin_constant_p(size) && !(flags & SLUB_DMA)) {
struct kmem_cache *s = kmalloc_slab(size);

if (!s)
- return NULL;
+ return ZERO_SIZE_PTR;

return kmem_cache_alloc(s, flags);
} else
@@ -146,7 +153,7 @@ static inline void *kzalloc(size_t size,
struct kmem_cache *s = kmalloc_slab(size);

if (!s)
- return NULL;
+ return ZERO_SIZE_PTR;

return kmem_cache_zalloc(s, flags);
} else
@@ -162,7 +169,7 @@ static inline void *kmalloc_node(size_t
struct kmem_cache *s = kmalloc_slab(size);

if (!s)
- return NULL;
+ return ZERO_SIZE_PTR;

return kmem_cache_alloc_node(s, flags, node);
} else
Index: slub/mm/slub.c
===================================================================
--- slub.orig/mm/slub.c 2007-06-01 18:08:32.000000000 -0700
+++ slub/mm/slub.c 2007-06-04 16:05:27.000000000 -0700
@@ -2286,7 +2286,7 @@ void *__kmalloc(size_t size, gfp_t flags

if (s)
return slab_alloc(s, flags, -1, __builtin_return_address(0));
- return NULL;
+ return ZERO_SIZE_PTR;
}
EXPORT_SYMBOL(__kmalloc);

@@ -2297,16 +2297,20 @@ void *__kmalloc_node(size_t size, gfp_t

if (s)
return slab_alloc(s, flags, node, __builtin_return_address(0));
- return NULL;
+ return ZERO_SIZE_PTR;
}
EXPORT_SYMBOL(__kmalloc_node);
#endif

size_t ksize(const void *object)
{
- struct page *page = get_object_page(object);
+ struct page *page;
struct kmem_cache *s;

+ if (object == ZERO_SIZE_PTR)
+ return 0;
+
+ page = get_object_page(object);
BUG_ON(!page);
s = page->slab;
BUG_ON(!s);
@@ -2338,7 +2342,13 @@ void kfree(const void *x)
struct kmem_cache *s;
struct page *page;

- if (!x)
+ /*
+ * This has to be an unsigned comparison. According to Linus
+ * some gcc version treat a pointer as a signed entity. Then
+ * this comparison would be true for all "negative" pointers
+ * (which would cover the whole upper half of the address space).
+ */
+ if ((unsigned long)x <= (unsigned long)ZERO_SIZE_PTR)
return;

page = virt_to_head_page(x);
@@ -2443,12 +2453,12 @@ void *krealloc(const void *p, size_t new
void *ret;
size_t ks;

- if (unlikely(!p))
+ if (unlikely(!p || p == ZERO_SIZE_PTR))
return kmalloc(new_size, flags);

if (unlikely(!new_size)) {
kfree(p);
- return NULL;
+ return ZERO_SIZE_PTR;
}

ks = ksize(p);
@@ -2707,7 +2717,7 @@ void *__kmalloc_track_caller(size_t size
struct kmem_cache *s = get_slab(size, gfpflags);

if (!s)
- return NULL;
+ return ZERO_SIZE_PTR;

return slab_alloc(s, gfpflags, -1, caller);
}
@@ -2718,7 +2728,7 @@ void *__kmalloc_node_track_caller(size_t
struct kmem_cache *s = get_slab(size, gfpflags);

if (!s)
- return NULL;
+ return ZERO_SIZE_PTR;

return slab_alloc(s, gfpflags, node, caller);
}

2007-06-05 05:27:36

by Pekka Enberg

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

Christoph Lameter wrote:
> SLUB: Return ZERO_SIZE_PTR for kmalloc(0) V3
>
> Instead of returning the smallest available object return ZERO_SIZE_PTR.
>
> A ZERO_SIZE_PTR can be legitimately used as an object pointer as long
> as it is not deferenced. The dereference of ZERO_SIZE_PTR causes a
> distinctive fault.
> kfree can handle a ZERO_SIZE_PTR in the same way as NULL.

FWIW, I am happy :-). We should add a comment to kmalloc() that we
return non-unique pointers for zero-length allocations though.

Acked-by: Pekka Enberg <[email protected]>

2007-06-05 08:53:59

by Rene Herman

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On 06/05/2007 01:09 AM, Christoph Lameter wrote:

> Here a version of the patch that drops the WARN_ONs

And now all that's done, how about yet another random person stepping in and
suggesting NIL or maybe NIL_PTR instead of ZERO_SIZE_PTR?

I understand the idea is that code need not necesarily care about zero sized
allocation meaning it won't (generally) need to spell it out but it's still a
dreadful name... :-(

Rene.

2007-06-05 12:07:49

by John Anthony Kazos Jr.

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

> > Here a version of the patch that drops the WARN_ONs
>
> And now all that's done, how about yet another random person stepping in and
> suggesting NIL or maybe NIL_PTR instead of ZERO_SIZE_PTR?
>
> I understand the idea is that code need not necesarily care about zero sized
> allocation meaning it won't (generally) need to spell it out but it's still a
> dreadful name... :-(

The name says exactly what it is. It's not at all dreadful. If we're going
to return a special value in the zero-size case (and in only that case) as
a valid pointer instead of actually allocating one byte and treating it as
zero, what we have is...a zero-size pointer. ZERO_SIZE_PTR is a pretty
damn good name.

2007-06-05 12:57:23

by Rene Herman

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On 06/05/2007 02:07 PM, John Anthony Kazos Jr. wrote:

> The name says exactly what it is. It's not at all dreadful. If we're going
> to return a special value in the zero-size case (and in only that case) as
> a valid pointer instead of actually allocating one byte and treating it as
> zero, what we have is...a zero-size pointer.

No, what we have is a sizeof(pointer) sized pointer pointing to an object of
size zero. ZERO_SIZE_PTR is butt-ugly. With a really ugly butt.

Rene.

2007-06-05 13:58:24

by John Anthony Kazos Jr.

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

> > The name says exactly what it is. It's not at all dreadful. If we're going
> > to return a special value in the zero-size case (and in only that case) as a
> > valid pointer instead of actually allocating one byte and treating it as
> > zero, what we have is...a zero-size pointer.
>
> No, what we have is a sizeof(pointer) sized pointer pointing to an object of
> size zero. ZERO_SIZE_PTR is butt-ugly. With a really ugly butt.

sizeof(pointer) is the object. ZERO_SIZE_PTR is the value stored in that
object. Would you prefer PTR_TO_ZERO_SIZE_OBJ_VAL?

Maybe you would prefer ZERO_SIZE_OBJ instead. What you have is "a pointer
object which points to a zero-sized object".

What if there were some construct in the kernel that never got deleted?
We'll call it "struct foo * bar_ctl". What would you call a pointer to
this? "bar_ctl_ptr". Or even "foo_ptr". So "ZERO_SIZE_OBJ_PTR" is the most
correct form, and "ZERO_SIZE_PTR" is a convenient shortening. "ZERO_PTR"
is too short and also confuses with NULL because NULL is a zero-value
object, rather than a non-zero--value pointer to a zero-size object.

2007-06-05 14:35:24

by Rene Herman

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On 06/05/2007 03:58 PM, John Anthony Kazos Jr. wrote:

> So "ZERO_SIZE_OBJ_PTR" is the most correct form, and "ZERO_SIZE_PTR" is a
> convenient shortening. "ZERO_PTR" is too short and also confuses with NULL
> because NULL is a zero-value object, rather than a non-zero--value pointer to
> a zero-size object.

Like I said, I would prefer NIL of if absolutely need be NIL_PTR. NULL is a
pointer that doesn't point anywhere valid -- NIL is a pointer that points to a
zero sized object.

If I'd have a few small children around I'd show them the word ZERO_SIZE_PTR and
make pictures of them bursting out in tears.

Rene.

2007-06-05 14:41:08

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

Rene Herman wrote:
> No, what we have is a sizeof(pointer) sized pointer pointing to an
> object of size zero. ZERO_SIZE_PTR is butt-ugly. With a really ugly butt.

It doesn't matter. It will never, ever, be used by anything except the
kmalloc internals. No client code should ever use the constant for
anything.

J

2007-06-05 14:59:29

by Rene Herman

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On 06/05/2007 04:40 PM, Jeremy Fitzhardinge wrote:

> Rene Herman wrote:

>> No, what we have is a sizeof(pointer) sized pointer pointing to an
>> object of size zero. ZERO_SIZE_PTR is butt-ugly. With a really ugly butt.
>
> It doesn't matter. It will never, ever, be used by anything except the
> kmalloc internals. No client code should ever use the constant for
> anything.

Yes, I'm aware of this (I should snip less) but I still feel it's not a good
name. When I read say "a 64-bit pointer" I immediately take that to mean a
pointer of size 64-bit, not a pointer to 64-bits and only it not making any
sense would stop me from interpreting "ZERO_SIZE_PTR" similarly.

Yes, it's internal but given that this is open-source which, optimistically, is
read many more times than it's written one should still strive for code that
reads nice as far as I'm concerned. It's obviously also not hugely important but
it's just that ZERO_SIZE_PTR makes my neck hair stand up.

Rene.

2007-06-05 17:23:44

by Christoph Lameter

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On Tue, 5 Jun 2007, Pekka Enberg wrote:

> FWIW, I am happy :-). We should add a comment to kmalloc() that we return
> non-unique pointers for zero-length allocations though.

Would you do that? Could you also add ZERO_SIZE_PTR support to SLAB?

2007-06-05 19:07:23

by Pekka Enberg

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

Rene Herman wrote:
> If I'd have a few small children around I'd show them the word
> ZERO_SIZE_PTR and make pictures of them bursting out in tears.

I strongly recommend against it, linux/mm/*.c is for adults only.

Pekka

2007-06-06 21:34:57

by Pavel Machek

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

Hi!

> >>No, what we have is a sizeof(pointer) sized pointer
> >>pointing to an
> >>object of size zero. ZERO_SIZE_PTR is butt-ugly. With
> >>a really ugly butt.
> >
> >It doesn't matter. It will never, ever, be used by
> >anything except the
> >kmalloc internals. No client code should ever use the
> >constant for
> >anything.
>
> Yes, I'm aware of this (I should snip less) but I still
> feel it's not a good name. When I read say "a 64-bit
> pointer" I immediately take that to mean a pointer of
> size 64-bit, not a pointer to 64-bits and only it not
> making any sense would stop me from interpreting
> "ZERO_SIZE_PTR" similarly.
>
> Yes, it's internal but given that this is open-source
> which, optimistically, is read many more times than it's
> written one should still strive for code that reads nice
> as far as I'm concerned. It's obviously also not hugely
> important but it's just that ZERO_SIZE_PTR makes my neck
> hair stand up.

PTR_TO_NOTHING?
PTR_TO_0BYTES?
PTR_TO_0SIZE?
PTR_TO_ZERO_SIZE?
NOT_QUITE_NULL? :-)
FULL?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2007-06-06 22:02:16

by Rene Herman

[permalink] [raw]
Subject: Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

On 06/06/2007 11:26 PM, Pavel Machek wrote:

>> Yes, it's internal but given that this is open-source
>> which, optimistically, is read many more times than it's
>> written one should still strive for code that reads nice
>> as far as I'm concerned. It's obviously also not hugely
>> important but it's just that ZERO_SIZE_PTR makes my neck
>> hair stand up.
>
> PTR_TO_NOTHING?
> PTR_TO_0BYTES?
> PTR_TO_0SIZE?
> PTR_TO_ZERO_SIZE?
> NOT_QUITE_NULL? :-)

Particularly like that one. Wouldn't object to that at all. Hey, they tell
us it's just some inconsequential internal thingy anyway... :-)

> FULL?

Almost as nice. DULL? BULL?

NOTHING?
EMPTY? <-- nice one as far as I'm concerned.

Rene.