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