Re: [PATCH v3] add function spin_event_timeout()

[Robert Hancock <hancockrwd@gmail.com>]

Timur Tabi wrote:
> Jiri Slaby wrote:
>> On 9.3.2009 21:32, Timur Tabi wrote:
>>> +#define spin_event_timeout(condition, timeout)				\
>>> +({									\
>>> +	int __timeout = timeout;					\
>>> +	while (!(condition)&&  --__timeout) {				\
>>> +		udelay(1);						\
>>> +		cpu_relax();						\
>> So you don't need cpu_relax anymore...
> 
> I checked the udelay() code.  It varies per platform, but I didn't see
> how it always replicated the functionality of cpu_relax().  For example,
> in x86_64, cpu_relax is a "rep; nop;".  But I don't see that code
> sequence in arch/x86/lib/delay.c.
> 
> So I presume that something in the delay functions makes cpu_relax()
> unnecessary.  What exactly is the purpose of cpu_relax()?

On platforms where it's possible and matters, it tells the CPU that the 
thread that's executing isn't very important and to give more resources 
to other threads (typically this is on a CPU with hyperthreading where 
it's supposed to make the other sibling get more of the execution 
resources). On x86, "rep nop" is the magic otherwise do-nothing 
instruction that does this.

I'd suspect that the delay functions should use it, except that it may 
skew the delay timing longer than specified. On a hyperthreaded CPU, 
that's kind of unavoidable, however, since we don't know what may be 
running on the sibling thread. Normally usages of the delay functions 
don't care that they may sleep a bit longer than specified, they mainly 
care about a minimum delay..

> 
>> And I would make timeout UL like delay functions.
> 
> I made it an integer because I don't expect anyone to pass a value
> larger than 2^31, but I'll change it.
>