[PATCH v3] add function spin_event_timeout()

[PATCH v3] add function spin_event_timeout()

[Timur Tabi]

The macro spin_event_timeout() takes a condition and timeout value
(in microseconds) as parameters.  It spins until either the condition is true
or the timeout expires.  It returns zero if the timeout expires first, non-zero
otherwise.

This primary purpose of this macro is to poll on a hardware register until a
status bit changes.  The timeout ensures that the loop still terminates if the
bit doesn't change as expected.  This macro makes it easier for driver
developers to perform this kind of operation properly.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

v3: eliminated secondary evaluation of condition, replaced jiffies with udelay

v2: added cpu_relax and time_before

 include/linux/delay.h |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/include/linux/delay.h b/include/linux/delay.h
index fd832c6..3e793b4 100644
--- a/include/linux/delay.h
+++ b/include/linux/delay.h
@@ -51,4 +51,33 @@ static inline void ssleep(unsigned int seconds)
 	msleep(seconds * 1000);
 }
 
+/**
+ * spin_event_timeout - spin until a condition gets true or a timeout elapses
+ * @condition: a C expression to evalate
+ * @timeout: timeout, in microseconds
+ *
+ * The process spins until the @condition evaluates to true (non-zero) or
+ * the @timeout elapses.
+ *
+ * This primary purpose of this macro is to poll on a hardware register
+ * until a status bit changes.  The timeout ensures that the loop still
+ * terminates if the bit never changes.
+ *
+ * The return value is the number of microseconds left in the timeout, so if
+ * the return value is non-zero, then it means the condition is true.
+ *
+ * Short-circuit evaluation in the while-loop ensures that if the condition
+ * becomes true exactly when the timeout expires, non-zero will still be
+ * returned.
+ */
+#define spin_event_timeout(condition, timeout)				\
+({									\
+	int __timeout = timeout;					\
+	while (!(condition) && --__timeout) {				\
+		udelay(1);						\
+		cpu_relax();						\
+	}								\
+	__timeout;							\
+})
+
 #endif /* defined(_LINUX_DELAY_H) */
-- 
1.6.1.3

Re: [PATCH v3] add function spin_event_timeout()

[Jiri Slaby]

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...

And I would make timeout UL like delay functions.

Re: [PATCH v3] add function spin_event_timeout()

[Timur Tabi]

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()?

> 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.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH v3] add function spin_event_timeout()

[Robert Hancock]

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.
>