2006-03-20 14:49:23

by Oliver Neukum

[permalink] [raw]
Subject: [PATCH]micro optimization of kcalloc

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 <[email protected]>

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


2006-03-20 15:35:07

by Oliver Neukum

[permalink] [raw]
Subject: Re: [PATCH]micro optimization of kcalloc

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

2006-03-20 15:38:26

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH]micro optimization of kcalloc

On 3/20/06, Oliver Neukum <[email protected]> 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

2006-03-20 15:39:43

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: [PATCH]micro optimization of kcalloc

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: <[email protected]>.

2006-03-20 15:49:13

by Benjamin LaHaise

[permalink] [raw]
Subject: Re: [PATCH]micro optimization of kcalloc

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: <[email protected]>.

2006-03-20 15:52:19

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH]micro optimization of kcalloc

On 3/20/06, Oliver Neukum <[email protected]> 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

2006-03-20 16:15:15

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH]micro optimization of kcalloc

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 <[email protected]> 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

2006-03-20 18:51:56

by David Lang

[permalink] [raw]
Subject: Re: [PATCH]micro optimization of kcalloc

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 <[email protected]> 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

2006-03-22 05:44:03

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [PATCH]micro optimization of kcalloc

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 <[email protected]> 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.....


Attachments:
(No filename) (228.00 B)