[PATCH -mm 6/6] cpu_relax(): ptrace.c coding style fix

[PATCH -mm 6/6] cpu_relax(): ptrace.c coding style fix

[Andreas Mohr]

Fix existing cpu_relax() loop to have proper kernel style.


Tested on 2.6.17-mm1.

Signed-off-by: Andreas Mohr <andi@lisas.de>


diff -urN linux-2.6.17-mm1.orig/kernel/ptrace.c linux-2.6.17-mm1.my/kernel/ptrace.c
--- linux-2.6.17-mm1.orig/kernel/ptrace.c	2006-06-21 14:28:20.000000000 +0200
+++ linux-2.6.17-mm1.my/kernel/ptrace.c	2006-06-21 14:43:24.000000000 +0200
@@ -182,9 +182,8 @@
 	if (!write_trylock(&tasklist_lock)) {
 		local_irq_enable();
 		task_unlock(task);
-		do {
+		while (!write_can_lock(&tasklist_lock))
 			cpu_relax();
-		} while (!write_can_lock(&tasklist_lock));
 		goto repeat;
 	}
 

Re: [PATCH -mm 6/6] cpu_relax(): ptrace.c coding style fix

[Nathan Lynch]

Andreas Mohr wrote:
> 
> Fix existing cpu_relax() loop to have proper kernel style.
> 

...

> @@ -182,9 +182,8 @@
>  	if (!write_trylock(&tasklist_lock)) {
>  		local_irq_enable();
>  		task_unlock(task);
> -		do {
> +		while (!write_can_lock(&tasklist_lock))
>  			cpu_relax();
> -		} while (!write_can_lock(&tasklist_lock));

This is a change in behavior, not just style.  (And there is nothing
wrong with the current style.)

Re: [PATCH -mm 6/6] cpu_relax(): ptrace.c coding style fix

[Pavel Machek]

Hi!

> Fix existing cpu_relax() loop to have proper kernel style.

..but you change the code, not its style.

> @@ -182,9 +182,8 @@
>  	if (!write_trylock(&tasklist_lock)) {
>  		local_irq_enable();
>  		task_unlock(task);
> -		do {
> +		while (!write_can_lock(&tasklist_lock))
>  			cpu_relax();
> -		} while (!write_can_lock(&tasklist_lock));
>  		goto repeat;
>  	}

I guess that barrier was needed before write_can_lock can succeed.
=> your version does one unneccessary test.

-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         

Re: [PATCH -mm 6/6] cpu_relax(): ptrace.c coding style fix

[Andreas Mohr]

On Thu, Jun 22, 2006 at 09:32:23AM -0500, Nathan Lynch wrote:
> Andreas Mohr wrote:
> > 
> > Fix existing cpu_relax() loop to have proper kernel style.
> > 
> 
> ...
> 
> > @@ -182,9 +182,8 @@
> >  	if (!write_trylock(&tasklist_lock)) {
> >  		local_irq_enable();
> >  		task_unlock(task);
> > -		do {
> > +		while (!write_can_lock(&tasklist_lock))
> >  			cpu_relax();
> > -		} while (!write_can_lock(&tasklist_lock));
> 
> This is a change in behavior, not just style.  (And there is nothing
> wrong with the current style.)

Ick, right, this could cause the new state to be visible in the 2nd iteration
only.

Thanks! Discard this change please.

Andreas Mohr