* [PATCH] micro optimization in kernel/params.c; don't call to_module_kobject before we really have to.
@ 2005-01-29 23:45 Jesper Juhl
2005-01-30 0:13 ` Brian Gerst
0 siblings, 1 reply; 4+ messages in thread
From: Jesper Juhl @ 2005-01-29 23:45 UTC (permalink / raw)
To: Paul `Rusty' Russell; +Cc: linux-kernel
In kernel/params.c::module_attr_show and
kernel/params.c::module_attr_store we call to_module_kobject() and save
the result in a local variable right before a conditional statement that
does not depend on the call to to_module_kobject() and may cause the
function to return. If the function returns before we use the result of
to_module_kobject() then the call is just a waste of time.
The patch moves the call to to_module_kobject() down just before it is
actually used, thus we save a call to the function in a few cases. I doubt
this is in any way measurable, but I see no reason to not move the call -
it should be an infinitesimal improvement with no ill sideeffects.
Please consider applying.
Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>
diff -up linux-2.6.11-rc2-bk7-orig/kernel/params.c linux-2.6.11-rc2-bk7/kernel/params.c
--- linux-2.6.11-rc2-bk7-orig/kernel/params.c 2005-01-29 23:54:53.000000000 +0100
+++ linux-2.6.11-rc2-bk7/kernel/params.c 2005-01-30 00:27:08.000000000 +0100
@@ -625,11 +625,12 @@ static ssize_t module_attr_show(struct k
int ret;
attribute = to_module_attr(attr);
- mk = to_module_kobject(kobj);
if (!attribute->show)
return -EPERM;
+ mk = to_module_kobject(kobj);
+
if (!try_module_get(mk->mod))
return -ENODEV;
@@ -649,11 +650,12 @@ static ssize_t module_attr_store(struct
int ret;
attribute = to_module_attr(attr);
- mk = to_module_kobject(kobj);
if (!attribute->store)
return -EPERM;
+ mk = to_module_kobject(kobj);
+
if (!try_module_get(mk->mod))
return -ENODEV;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] micro optimization in kernel/params.c; don't call to_module_kobject before we really have to.
2005-01-29 23:45 [PATCH] micro optimization in kernel/params.c; don't call to_module_kobject before we really have to Jesper Juhl
@ 2005-01-30 0:13 ` Brian Gerst
2005-01-30 12:43 ` Jesper Juhl
0 siblings, 1 reply; 4+ messages in thread
From: Brian Gerst @ 2005-01-30 0:13 UTC (permalink / raw)
To: Jesper Juhl; +Cc: Paul `Rusty' Russell, linux-kernel
Jesper Juhl wrote:
> In kernel/params.c::module_attr_show and
> kernel/params.c::module_attr_store we call to_module_kobject() and save
> the result in a local variable right before a conditional statement that
> does not depend on the call to to_module_kobject() and may cause the
> function to return. If the function returns before we use the result of
> to_module_kobject() then the call is just a waste of time.
> The patch moves the call to to_module_kobject() down just before it is
> actually used, thus we save a call to the function in a few cases. I doubt
> this is in any way measurable, but I see no reason to not move the call -
> it should be an infinitesimal improvement with no ill sideeffects.
> Please consider applying.
>
>
> Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>
>
> diff -up linux-2.6.11-rc2-bk7-orig/kernel/params.c linux-2.6.11-rc2-bk7/kernel/params.c
> --- linux-2.6.11-rc2-bk7-orig/kernel/params.c 2005-01-29 23:54:53.000000000 +0100
> +++ linux-2.6.11-rc2-bk7/kernel/params.c 2005-01-30 00:27:08.000000000 +0100
> @@ -625,11 +625,12 @@ static ssize_t module_attr_show(struct k
> int ret;
>
> attribute = to_module_attr(attr);
> - mk = to_module_kobject(kobj);
>
> if (!attribute->show)
> return -EPERM;
>
> + mk = to_module_kobject(kobj);
> +
> if (!try_module_get(mk->mod))
> return -ENODEV;
>
> @@ -649,11 +650,12 @@ static ssize_t module_attr_store(struct
> int ret;
>
> attribute = to_module_attr(attr);
> - mk = to_module_kobject(kobj);
>
> if (!attribute->store)
> return -EPERM;
>
> + mk = to_module_kobject(kobj);
> +
> if (!try_module_get(mk->mod))
> return -ENODEV;
>
>
>
>
I'd bet that the compiler already reorders the assignment since there is
no side effect to to_module_kobject(). Even with a patch like this you
can't control exactly when the assignment is done. There may not even
be an assignment, since mk is a constant offset of kobj.
--
Brian Gerst
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] micro optimization in kernel/params.c; don't call to_module_kobject before we really have to.
2005-01-30 0:13 ` Brian Gerst
@ 2005-01-30 12:43 ` Jesper Juhl
2005-01-31 3:43 ` Rusty Russell
0 siblings, 1 reply; 4+ messages in thread
From: Jesper Juhl @ 2005-01-30 12:43 UTC (permalink / raw)
To: Brian Gerst; +Cc: Paul `Rusty' Russell, linux-kernel
On Sat, 29 Jan 2005, Brian Gerst wrote:
> Jesper Juhl wrote:
> > In kernel/params.c::module_attr_show and kernel/params.c::module_attr_store
> > we call to_module_kobject() and save the result in a local variable right
> > before a conditional statement that does not depend on the call to
> > to_module_kobject() and may cause the function to return. If the function
> > returns before we use the result of to_module_kobject() then the call is
> > just a waste of time. The patch moves the call to to_module_kobject() down
> > just before it is actually used, thus we save a call to the function in a
> > few cases. I doubt this is in any way measurable, but I see no reason to not
> > move the call - it should be an infinitesimal improvement with no ill
> > sideeffects.
> > Please consider applying.
> >
> >
> > Signed-off-by: Jesper Juhl <juhl-lkml@dif.dk>
> >
> > diff -up linux-2.6.11-rc2-bk7-orig/kernel/params.c
> > linux-2.6.11-rc2-bk7/kernel/params.c
> > --- linux-2.6.11-rc2-bk7-orig/kernel/params.c 2005-01-29 23:54:53.000000000
> > +0100
> > +++ linux-2.6.11-rc2-bk7/kernel/params.c 2005-01-30 00:27:08.000000000
> > +0100
> > @@ -625,11 +625,12 @@ static ssize_t module_attr_show(struct k
> > int ret;
> > attribute = to_module_attr(attr);
> > - mk = to_module_kobject(kobj);
> > if (!attribute->show)
> > return -EPERM;
> > + mk = to_module_kobject(kobj);
> > +
> > if (!try_module_get(mk->mod))
> > return -ENODEV;
> > @@ -649,11 +650,12 @@ static ssize_t module_attr_store(struct int
> > ret;
> > attribute = to_module_attr(attr);
> > - mk = to_module_kobject(kobj);
> > if (!attribute->store)
> > return -EPERM;
> > + mk = to_module_kobject(kobj);
> > +
> > if (!try_module_get(mk->mod))
> > return -ENODEV;
> >
> >
>
> I'd bet that the compiler already reorders the assignment since there is no
> side effect to to_module_kobject(). Even with a patch like this you can't
> control exactly when the assignment is done. There may not even be an
> assignment, since mk is a constant offset of kobj.
>
True, the compiler is free to be clever, but I still think it's best to
write the code in the most optimal way as seen from a C perspective.
I just took a look at the compiled object files with and without the
patch, and it makes no difference what-so-ever - gcc generates the exact
same code. So you are right, gcc is clever about it.
Hmm, it's Rusty's code as far as I can see, I'll leave it up to him to
apply the patch or not.
--
Jesper
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] micro optimization in kernel/params.c; don't call to_module_kobject before we really have to.
2005-01-30 12:43 ` Jesper Juhl
@ 2005-01-31 3:43 ` Rusty Russell
0 siblings, 0 replies; 4+ messages in thread
From: Rusty Russell @ 2005-01-31 3:43 UTC (permalink / raw)
To: Jesper Juhl; +Cc: Brian Gerst, linux-kernel
On Sun, 2005-01-30 at 13:43 +0100, Jesper Juhl wrote:
> True, the compiler is free to be clever, but I still think it's best to
> write the code in the most optimal way as seen from a C perspective.
> I just took a look at the compiled object files with and without the
> patch, and it makes no difference what-so-ever - gcc generates the exact
> same code. So you are right, gcc is clever about it.
If the code were not already in the kernel, I'd probably apply this.
However, cleanups this trivial are not worthwhile IMHO.
Rusty.
--
A bad analogy is like a leaky screwdriver -- Richard Braakman
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2005-01-31 3:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-01-29 23:45 [PATCH] micro optimization in kernel/params.c; don't call to_module_kobject before we really have to Jesper Juhl
2005-01-30 0:13 ` Brian Gerst
2005-01-30 12:43 ` Jesper Juhl
2005-01-31 3:43 ` Rusty Russell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox