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 <[email protected]>
---
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;
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
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
On 9/24/05, Nick Piggin <[email protected]> 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 <[email protected]>
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
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
On 9/28/05, Nick Piggin <[email protected]> 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/
On 9/28/05, Coywolf Qi Hunt <[email protected]> wrote:
> On 9/28/05, Nick Piggin <[email protected]> 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 <[email protected]>
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