RE: [PATCH] unneeded memory barrier

RE: [PATCH] unneeded memory barrier

[Grover, Andrew]

> From: Felipe W Damasio [mailto:felipewd-y7mWNqJcIDpfJ/NunPodnw@public.gmane.org] 
> 	Patch against 2.6-test5 to remove an, IMHO, unneeded 
> memory barrier 
> on acpi/bus.c. Feel free to tell me I'm totally wrong, though :)
> 
> 	If it looks good, please consider applying.

-		set_current_state(TASK_RUNNING);
+		__set_current_state(TASK_RUNNING);

My understanding of functions starting with __ was "use them but you
better know what you're doing". This patch may be technically correct
but does the unneeded barrier really hurt anything? If not then I think
the regular version is ok.

Regards -- Andy


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

Re: [PATCH] unneeded memory barrier

[Felipe W Damasio]

	Hi Andy,

	First of all, thanks for your review.

Grover, Andrew wrote:
>>From: Felipe W Damasio [mailto:felipewd-y7mWNqJcIDpfJ/NunPodnw@public.gmane.org] 
>>	Patch against 2.6-test5 to remove an, IMHO, unneeded 
>>memory barrier 
>>on acpi/bus.c. Feel free to tell me I'm totally wrong, though :)
>>
>>	If it looks good, please consider applying.
> 
> 
> -		set_current_state(TASK_RUNNING);
> +		__set_current_state(TASK_RUNNING);
> 
> My understanding of functions starting with __ was "use them but you
> better know what you're doing". This patch may be technically correct
> but does the unneeded barrier really hurt anything? If not then I think
> the regular version is ok.

	I don't think it hurts in terms of "enforcing ordering too much", but 
that barrier isn't needed at all since it sets the state to 
TASK_RUNNING...

	But hey, it's not my driver to decide :)

	Thanks anyway,

Felipe



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

Re: Re: [PATCH] unneeded memory barrier

[Pavel Machek]

Hi!

> >>	Patch against 2.6-test5 to remove an, IMHO, unneeded 
> >>memory barrier 
> >>on acpi/bus.c. Feel free to tell me I'm totally wrong, though :)
> >>
> >>	If it looks good, please consider applying.
> >
> >
> >-		set_current_state(TASK_RUNNING);
> >+		__set_current_state(TASK_RUNNING);
> >
> >My understanding of functions starting with __ was "use them but you
> >better know what you're doing". This patch may be technically correct
> >but does the unneeded barrier really hurt anything? If not then I 
> >think
> >the regular version is ok.
> 
> 	I don't think it hurts in terms of "enforcing ordering too 
> 	much", but that barrier isn't needed at all since it sets the state 
> to TASK_RUNNING...

If setting to TASK_RUNNING never requires barier, I guess some
__builtin_constant and ==TASK_RUNNING check in 
set_task_state is better idea.

-- 
				Pavel
Written on sharp zaurus, because my Velo1 broke. If you have Velo you don't need...



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

[PATCH] unneeded memory barrier

[Felipe W Damasio]

[-- Attachment #1: Type: text/plain, Size: 220 bytes --]

	Hi Andrew/Patrick,

	Patch against 2.6-test5 to remove an, IMHO, unneeded memory barrier 
on acpi/bus.c. Feel free to tell me I'm totally wrong, though :)

	If it looks good, please consider applying.

	Cheers,

Felipe

[-- Attachment #2: acpi-bus_state.patch --]
[-- Type: text/plain, Size: 389 bytes --]

--- linux-2.6.0-test5/drivers/acpi/bus.c.orig	2003-09-11 17:23:06.000000000 -0300
+++ linux-2.6.0-test5/drivers/acpi/bus.c	2003-09-11 17:23:14.000000000 -0300
@@ -333,7 +333,7 @@
 			schedule();
 
 		remove_wait_queue(&acpi_bus_event_queue, &wait);
-		set_current_state(TASK_RUNNING);
+		__set_current_state(TASK_RUNNING);
 
 		if (signal_pending(current))
 			return_VALUE(-ERESTARTSYS);