[PATCH]micro optimization of kcalloc

[PATCH]micro optimization of kcalloc

[Oliver Neukum]

Hi,

this optimises away a division in kcalloc by letting the compiler
do it. It is redone to allow size==0.

	Regards
		Oliver

Signed-off-by: Oliver Neukum <oliver@neukum.name>

--- linux-2.6.16-rc6-vanilla/include/linux/slab.h	2006-03-11 23:12:55.000000000 +0100
+++ linux-2.6.16-rc6/include/linux/slab.h	2006-03-20 14:39:36.000000000 +0100
@@ -118,7 +118,7 @@
  */
 static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
 {
-	if (n != 0 && size > INT_MAX / n)
+	if (unlikely(size != 0 && n > INT_MAX / size ))
 		return NULL;
 	return kzalloc(n * size, flags);
 }

Re: [PATCH]micro optimization of kcalloc

[Benjamin LaHaise]

On Mon, Mar 20, 2006 at 03:45:23PM +0100, Oliver Neukum wrote:
>  static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
>  {
> -	if (n != 0 && size > INT_MAX / n)
> +	if (unlikely(size != 0 && n > INT_MAX / size ))
>  		return NULL;
>  	return kzalloc(n * size, flags);
>  }

This function shouldn't be inlined.  We have no need to optimize the 
unlikely case like this.

		-ben
-- 
"Time is of no importance, Mr. President, only life is important."
Don't Email: <dont@kvack.org>.

Re: [PATCH]micro optimization of kcalloc

[Oliver Neukum]

Am Montag, 20. März 2006 16:14 schrieb Benjamin LaHaise:
> On Mon, Mar 20, 2006 at 03:45:23PM +0100, Oliver Neukum wrote:
> >  static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
> >  {
> > -	if (n != 0 && size > INT_MAX / n)
> > +	if (unlikely(size != 0 && n > INT_MAX / size ))
> >  		return NULL;
> >  	return kzalloc(n * size, flags);
> >  }
> 
> This function shouldn't be inlined.  We have no need to optimize the 
> unlikely case like this.

The likely case is passing constants. Without inlining precisely the
likely case cannot be optimised.

	Regards
		Oliver

Re: [PATCH]micro optimization of kcalloc

[Benjamin LaHaise]

On Mon, Mar 20, 2006 at 04:33:29PM +0100, Oliver Neukum wrote:
> The likely case is passing constants. Without inlining precisely the
> likely case cannot be optimised.

That's what __builtin_constant_p() is for.

		-ben
-- 
"Time is of no importance, Mr. President, only life is important."
Don't Email: <dont@kvack.org>.

Re: [PATCH]micro optimization of kcalloc

[Pekka Enberg]

Hi,

On Mon, Mar 20, 2006 at 03:45:23PM +0100, Oliver Neukum wrote:
> >  static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
> >  {
> > -     if (n != 0 && size > INT_MAX / n)
> > +     if (unlikely(size != 0 && n > INT_MAX / size ))
> >               return NULL;
> >       return kzalloc(n * size, flags);
> >  }

On 3/20/06, Benjamin LaHaise <bcrl@kvack.org> wrote:
> This function shouldn't be inlined.  We have no need to optimize the
> unlikely case like this.

IIRC, I made it static inline in the first place because that actually
reduced kernel text size. (And I think it was Adrian who made me do it
:-).

                                Pekka

Re: [PATCH]micro optimization of kcalloc

[David Lang]

On Mon, 20 Mar 2006, Pekka Enberg wrote:

> Hi,
>
> On Mon, Mar 20, 2006 at 03:45:23PM +0100, Oliver Neukum wrote:
>>>  static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
>>>  {
>>> -     if (n != 0 && size > INT_MAX / n)
>>> +     if (unlikely(size != 0 && n > INT_MAX / size ))
>>>               return NULL;
>>>       return kzalloc(n * size, flags);
>>>  }
>
> On 3/20/06, Benjamin LaHaise <bcrl@kvack.org> wrote:
>> This function shouldn't be inlined.  We have no need to optimize the
>> unlikely case like this.
>
> IIRC, I made it static inline in the first place because that actually
> reduced kernel text size. (And I think it was Adrian who made me do it
> :-).

I wonder if this is still needed with the new inline changes that were 
made to allow GCC to make the decision (for recent GCC's)

David Lang

-- 
There are two ways of constructing a software design. One way is to make it so simple that there are obviously no deficiencies. And the other way is to make it so complicated that there are no obvious deficiencies.
  -- C.A.R. Hoare

Re: [PATCH]micro optimization of kcalloc

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1052 bytes --]

On Mon, 20 Mar 2006 10:44:00 PST, David Lang said:
> On Mon, 20 Mar 2006, Pekka Enberg wrote:

> > On Mon, Mar 20, 2006 at 03:45:23PM +0100, Oliver Neukum wrote:
> >>>  static inline void *kcalloc(size_t n, size_t size, gfp_t flags)

> > On 3/20/06, Benjamin LaHaise <bcrl@kvack.org> wrote:
> >> This function shouldn't be inlined.  We have no need to optimize the
> >> unlikely case like this.
> >
> > IIRC, I made it static inline in the first place because that actually
> > reduced kernel text size. (And I think it was Adrian who made me do it
> > :-).
> 
> I wonder if this is still needed with the new inline changes that were 
> made to allow GCC to make the decision (for recent GCC's)

One non-obvious reason to inline it (at least in -mm kernels) is because the
slab leak detector stuff wants to find where it was called from - and if you
don't inline kcalloc(), you end up with the kzalloc() call it makes showing
kcalloc() as the caller.  If you inline it, you end up showing the caller
of kcalloc() instead, which is far more useful.....

[-- Attachment #2: Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH]micro optimization of kcalloc

[Pekka Enberg]

On 3/20/06, Oliver Neukum <neukum@fachschaft.cup.uni-muenchen.de> wrote:
> --- linux-2.6.16-rc6-vanilla/include/linux/slab.h       2006-03-11 23:12:55.000000000 +0100
> +++ linux-2.6.16-rc6/include/linux/slab.h       2006-03-20 14:39:36.000000000 +0100
> @@ -118,7 +118,7 @@
>   */
>  static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
>  {
> -       if (n != 0 && size > INT_MAX / n)
> +       if (unlikely(size != 0 && n > INT_MAX / size ))

Please fix whitespace damage at the end.

                                     Pekka

Re: [PATCH]micro optimization of kcalloc

[Pekka Enberg]

On 3/20/06, Oliver Neukum <neukum@fachschaft.cup.uni-muenchen.de> wrote:
> this optimises away a division in kcalloc by letting the compiler
> do it. It is redone to allow size==0.

Does this shrink kernel text and if yes, how much?

                                        Pekka

[PATCH]micro optimization of kcalloc

[Oliver Neukum]

Hi,

this transforms the limit check of kcalloc() so that size becomes the
divisor. This saves some kernel code, because size is always a constant,
thus turning the check into a simple comparison saving a full division.
This saved 18K in allyesconfig's kernel size.

	Regards
		Oliver

Signed-off-by: Oliver Neukum <oliver@neukum.name>

--- a/include/linux/slab.h	2006-03-11 23:12:55.000000000 +0100
+++ b/include/linux/slab.h	2006-03-20 09:00:41.000000000 +0100
@@ -118,7 +118,7 @@
  */
 static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
 {
-	if (n != 0 && size > INT_MAX / n)
+	if (unlikely(n > INT_MAX / size ))
 		return NULL;
 	return kzalloc(n * size, flags);
 }