[RFC][PATCH] inline a few tiny functions in init/initramfs.c

[RFC][PATCH] inline a few tiny functions in init/initramfs.c

[Jesper Juhl]

A few functions in init/initramfs.c are so simple that I don't see why *any* 
point in them having to bear the cost of a function call.
Wouldn't something like the patch below make sense ?


Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
---

 init/initramfs.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

--- linux-2.6.14-rc2-git3-orig/init/initramfs.c	2005-09-22 00:28:39.000000000 +0200
+++ linux-2.6.14-rc2-git3/init/initramfs.c	2005-09-24 01:21:55.000000000 +0200
@@ -14,12 +14,12 @@ static void __init error(char *x)
 		message = x;
 }
 
-static void __init *malloc(size_t size)
+static inline void __init *malloc(size_t size)
 {
 	return kmalloc(size, GFP_KERNEL);
 }
 
-static void __init free(void *where)
+static inline void __init free(void *where)
 {
 	kfree(where);
 }
@@ -155,7 +155,7 @@ static void __init read_into(char *buf, 
 
 static __initdata char *header_buf, *symlink_buf, *name_buf;
 
-static int __init do_start(void)
+static inline int __init do_start(void)
 {
 	read_into(header_buf, 110, GotHeader);
 	return 0;

Re: [RFC][PATCH] inline a few tiny functions in init/initramfs.c

[Con Kolivas]

On Sat, 24 Sep 2005 09:26, Jesper Juhl wrote:
> A few functions in init/initramfs.c are so simple that I don't see why
> *any* point in them having to bear the cost of a function call.
> Wouldn't something like the patch below make sense ?

> -static void __init *malloc(size_t size)
> +static inline void __init *malloc(size_t size)
>  {
>  	return kmalloc(size, GFP_KERNEL);

maybe it looks like it would, but kmalloc looks like this:

85 static inline void *kmalloc(size_t size, int flags)
86 {
87         if (__builtin_constant_p(size)) {
88                 int i = 0;
89 #define CACHE(x) \
90                 if (size <= x) \
91                         goto found; \
92                 else \
93                         i++;
94 #include "kmalloc_sizes.h"
95 #undef CACHE
96                 {
97                         extern void __you_cannot_kmalloc_that_much(void);
98                         __you_cannot_kmalloc_that_much();
99                 }
100 found:
101                 return kmem_cache_alloc((flags & GFP_DMA) ?
102                         malloc_sizes[i].cs_dmacachep :
103                         malloc_sizes[i].cs_cachep, flags);
104         }
105         return __kmalloc(size, flags);
106 }

which is not a one liner to inline at all

> -static void __init free(void *where)
> +static inline void __init free(void *where)
>  {
>  	kfree(where);

kfree ok since it's a void function of its own

>  static __initdata char *header_buf, *symlink_buf, *name_buf;
>
> -static int __init do_start(void)
> +static inline int __init do_start(void)
>  {
>  	read_into(header_buf, 110, GotHeader);

read_into ok as well.

Cheers,
Con

Re: [RFC][PATCH] inline a few tiny functions in init/initramfs.c

[Nick Piggin]

Con Kolivas wrote:

>On Sat, 24 Sep 2005 09:26, Jesper Juhl wrote:
>
>>A few functions in init/initramfs.c are so simple that I don't see why
>>*any* point in them having to bear the cost of a function call.
>>Wouldn't something like the patch below make sense ?
>>
>
>>-static void __init *malloc(size_t size)
>>+static inline void __init *malloc(size_t size)
>> {
>> 	return kmalloc(size, GFP_KERNEL);
>>
>
>maybe it looks like it would, but kmalloc looks like this:
>
>85 static inline void *kmalloc(size_t size, int flags)
>86 {
>87         if (__builtin_constant_p(size)) {
>88                 int i = 0;
>89 #define CACHE(x) \
>90                 if (size <= x) \
>91                         goto found; \
>92                 else \
>93                         i++;
>94 #include "kmalloc_sizes.h"
>95 #undef CACHE
>96                 {
>97                         extern void __you_cannot_kmalloc_that_much(void);
>98                         __you_cannot_kmalloc_that_much();
>99                 }
>100 found:
>101                 return kmem_cache_alloc((flags & GFP_DMA) ?
>102                         malloc_sizes[i].cs_dmacachep :
>103                         malloc_sizes[i].cs_cachep, flags);
>104         }
>105         return __kmalloc(size, flags);
>106 }
>
>which is not a one liner to inline at all
>
>

Actually, this is even better, because the inline 'malloc' should be
able to propogate the builtin_constantness of 'size' while an out of
line version cannot.

IMO the best policy is not to second guess the API implementor's
choice of inline / noinline. That is - if kmalloc was too big to
inline then it should be fixed in kmalloc or another interface
introduced.

Nick


Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: [RFC][PATCH] inline a few tiny functions in init/initramfs.c

[Jesper Juhl]

On 9/24/05, Nick Piggin <nickpiggin@yahoo.com.au> wrote:
> Con Kolivas wrote:
>
> >On Sat, 24 Sep 2005 09:26, Jesper Juhl wrote:
> >
> >>A few functions in init/initramfs.c are so simple that I don't see why
> >>*any* point in them having to bear the cost of a function call.
> >>Wouldn't something like the patch below make sense ?
> >>
> >
> >>-static void __init *malloc(size_t size)
> >>+static inline void __init *malloc(size_t size)
> >> {
> >>      return kmalloc(size, GFP_KERNEL);
> >>
> >
> >maybe it looks like it would, but kmalloc looks like this:
> >
> >85 static inline void *kmalloc(size_t size, int flags)
> >86 {
> >87         if (__builtin_constant_p(size)) {
> >88                 int i = 0;
> >89 #define CACHE(x) \
> >90                 if (size <= x) \
> >91                         goto found; \
> >92                 else \
> >93                         i++;
> >94 #include "kmalloc_sizes.h"
> >95 #undef CACHE
> >96                 {
> >97                         extern void __you_cannot_kmalloc_that_much(void);
> >98                         __you_cannot_kmalloc_that_much();
> >99                 }
> >100 found:
> >101                 return kmem_cache_alloc((flags & GFP_DMA) ?
> >102                         malloc_sizes[i].cs_dmacachep :
> >103                         malloc_sizes[i].cs_cachep, flags);
> >104         }
> >105         return __kmalloc(size, flags);
> >106 }
> >
> >which is not a one liner to inline at all
> >
> >
>
> Actually, this is even better, because the inline 'malloc' should be
> able to propogate the builtin_constantness of 'size' while an out of
> line version cannot.
>
> IMO the best policy is not to second guess the API implementor's
> choice of inline / noinline. That is - if kmalloc was too big to
> inline then it should be fixed in kmalloc or another interface
> introduced.
>

Ok, so it seems that there's agreement that the other two inlines in
the patch makes sense, but the malloc() is not clear cut.

Since this is in initramfs after all it doesn't make that big a
difference overall, so I'll just send in a patch that inlines the
other two functions but leaves malloc() alone.

Thank you both, Nick, Con, for commenting.


--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: [RFC][PATCH] inline a few tiny functions in init/initramfs.c

[Nick Piggin]

On Wed, 2005-09-28 at 02:07 +0200, Jesper Juhl wrote:

> Ok, so it seems that there's agreement that the other two inlines in
> the patch makes sense, but the malloc() is not clear cut.
> 
> Since this is in initramfs after all it doesn't make that big a
> difference overall, so I'll just send in a patch that inlines the
> other two functions but leaves malloc() alone.
> 

Well, they're not particularly performance critical, and everything
is marked init anyway so I don't know why you would bother changing
anything ;)

-- 
SUSE Labs, Novell Inc.



Send instant messages to your online friends http://au.messenger.yahoo.com 

Re: [RFC][PATCH] inline a few tiny functions in init/initramfs.c

[Coywolf Qi Hunt]

On 9/28/05, Nick Piggin <nickpiggin@yahoo.com.au> wrote:
> On Wed, 2005-09-28 at 02:07 +0200, Jesper Juhl wrote:
>
> > Ok, so it seems that there's agreement that the other two inlines in
> > the patch makes sense, but the malloc() is not clear cut.
> >
> > Since this is in initramfs after all it doesn't make that big a
> > difference overall, so I'll just send in a patch that inlines the
> > other two functions but leaves malloc() alone.
> >
>
> Well, they're not particularly performance critical, and everything
> is marked init anyway so I don't know why you would bother changing
> anything ;)
>

Don't you feel "static inline void __init " stupid? (inline + __init)
Anyway don't do things like that manually. Leave the optimization job
to gcc.
--
Coywolf Qi Hunt
http://sosdg.org/~coywolf/

Re: [RFC][PATCH] inline a few tiny functions in init/initramfs.c

[Jesper Juhl]

On 9/28/05, Coywolf Qi Hunt <coywolf@gmail.com> wrote:
> On 9/28/05, Nick Piggin <nickpiggin@yahoo.com.au> wrote:
> > On Wed, 2005-09-28 at 02:07 +0200, Jesper Juhl wrote:
> >
> > > Ok, so it seems that there's agreement that the other two inlines in
> > > the patch makes sense, but the malloc() is not clear cut.
> > >
> > > Since this is in initramfs after all it doesn't make that big a
> > > difference overall, so I'll just send in a patch that inlines the
> > > other two functions but leaves malloc() alone.
> > >
> >
> > Well, they're not particularly performance critical, and everything
> > is marked init anyway so I don't know why you would bother changing
> > anything ;)
> >
>
> Don't you feel "static inline void __init " stupid? (inline + __init)
> Anyway don't do things like that manually. Leave the optimization job
> to gcc.

Hmm, I guess you are right. They just looked like so obvious
candidates for inlining, __init or no __init, but I guess it doesn't
matter - I'll find better things to spend my time on.
Thanks.


--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html