[PATCH] micro optimization in kernel/params.c; don't call to_module_kobject before we really have to.

[PATCH] micro optimization in kernel/params.c; don't call to_module_kobject before we really have to.

[Jesper Juhl]

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;
 

Re: [PATCH] micro optimization in kernel/params.c; don't call to_module_kobject before we really have to.

[Brian Gerst]

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

Re: [PATCH] micro optimization in kernel/params.c; don't call to_module_kobject before we really have to.

[Jesper Juhl]

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

Re: [PATCH] micro optimization in kernel/params.c; don't call to_module_kobject before we really have to.

[Rusty Russell]

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