SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Christoph Lameter]

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 <clameter@sgi.com>

---
 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);
 }

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[John Anthony Kazos Jr.]

> +	 * 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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Christoph Lameter]

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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[John Anthony Kazos Jr.]

> > > +	 * 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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Linus Torvalds]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Christoph Lameter]

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 <clameter@sgi.com>


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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Andrew Morton]

> On Fri, 1 Jun 2007 18:37:46 -0700 (PDT) Christoph Lameter <clameter@sgi.com> 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?

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Christoph Lameter]

On Fri, 1 Jun 2007, Andrew Morton wrote:

> > On Fri, 1 Jun 2007 18:37:46 -0700 (PDT) Christoph Lameter <clameter@sgi.com> 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?

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Andrew Morton]

On Fri, 1 Jun 2007 21:01:09 -0700 (PDT) Christoph Lameter <clameter@sgi.com> wrote:

> On Fri, 1 Jun 2007, Andrew Morton wrote:
> 
> > > On Fri, 1 Jun 2007 18:37:46 -0700 (PDT) Christoph Lameter <clameter@sgi.com> 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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Christoph Lameter]

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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Andrew Morton]

On Fri, 1 Jun 2007 21:45:15 -0700 (PDT) Christoph Lameter <clameter@sgi.com> 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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Matt Mackall]

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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[William Lee Irwin III]

On Fri, 1 Jun 2007 21:45:15 -0700 (PDT) Christoph Lameter <clameter@sgi.com> 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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Matt Mackall]

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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Alan Cox]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Pekka Enberg]

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 <clameter@sgi.com> 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?

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Christoph Lameter]

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 <clameter@sgi.com> 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).
 

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Pekka Enberg]

On 6/4/07, Christoph Lameter <clameter@sgi.com> 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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Linus Torvalds]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Jeremy Fitzhardinge]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Pekka Enberg]

On 6/4/07, Linus Torvalds <torvalds@linux-foundation.org> 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 <torvalds@linux-foundation.org> 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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Jeremy Fitzhardinge]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Roland Dreier]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Pekka Enberg]

On 6/4/07, Roland Dreier <rdreier@cisco.com> 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 ;-)

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Linus Torvalds]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Cyrill Gorcunov]

[Pekka Enberg - Mon, Jun 04, 2007 at 07:37:01PM +0300]
| On 6/4/07, Linus Torvalds <torvalds@linux-foundation.org> 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 <torvalds@linux-foundation.org> 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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Alan Cox]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Linus Torvalds]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[William Lee Irwin III]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Linus Torvalds]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Christoph Lameter]

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 <clameter@sgi.com>

---
 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);
 }

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Pekka Enberg]

Hi Christoph,

On 6/4/07, Christoph Lameter <clameter@sgi.com> 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?

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Christoph Lameter]

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
 

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Pekka Enberg]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Christoph Lameter]

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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Andrew Morton]

On Mon, 4 Jun 2007 15:39:08 -0700 (PDT)
Christoph Lameter <clameter@sgi.com> 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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Christoph Lameter]

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 <clameter@sgi.com>

---
 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);
 }

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Pekka Enberg]

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 <penberg@cs.helsinki.fi>

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Christoph Lameter]

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?

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Rene Herman]

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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[John Anthony Kazos Jr.]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Rene Herman]

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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[John Anthony Kazos Jr.]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Rene Herman]

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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Pekka Enberg]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Jeremy Fitzhardinge]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Rene Herman]

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.

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Pavel Machek]

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

Re: SLUB: Return ZERO_SIZE_PTR for kmalloc(0)

[Rene Herman]

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.