2009-03-07 00:13:34

by Timur Tabi

[permalink] [raw]
Subject: [PATCH] add function spin_event_timeout()

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

Signed-off-by: Timur Tabi <[email protected]>
---
include/linux/delay.h | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/include/linux/delay.h b/include/linux/delay.h
index fd832c6..00ac466 100644
--- a/include/linux/delay.h
+++ b/include/linux/delay.h
@@ -51,4 +51,25 @@ 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 for the event to wait for
+ * @timeout: timeout, in jiffies
+ *
+ * The process spins until the @condition evaluates to true or the @timeout
+ * elapses.
+ *
+ * The function returns non-zero if the @condition evaluated to true, or
+ * zero if the @timeout elapsed. If both occurs (e.g. the loop was
+ * pre-empted and the @condition became true in the meantime, but when the
+ * loop resumed the @timeout had already elapsed), then non-zero will be
+ * returned.
+ */
+#define spin_event_timeout(condition, timeout) \
+({ \
+ long __timeout = jiffies + (timeout); \
+ while (!(condition) && (jiffies < __timeout)); \
+ (condition); \
+})
+
#endif /* defined(_LINUX_DELAY_H) */
--
1.6.1.3


2009-03-07 01:11:19

by Roland Dreier

[permalink] [raw]
Subject: Re: [PATCH] add function spin_event_timeout()

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

What's the motivation for this? Where do you plan to use it?

> + * The function returns non-zero if the @condition evaluated to true, or

It's not a function but rather a macro.

> +#define spin_event_timeout(condition, timeout) \
> +({ \
> + long __timeout = jiffies + (timeout); \
> + while (!(condition) && (jiffies < __timeout)); \
> + (condition); \
> +})

If we're going to make this core infrastructure, it seems we should
implement it with all best practices... eg I would think the while loop
should include cpu_relax().

2009-03-07 01:25:27

by Timur Tabi

[permalink] [raw]
Subject: Re: [PATCH] add function spin_event_timeout()

On Fri, Mar 6, 2009 at 7:11 PM, Roland Dreier <[email protected]> wrote:
> > The function spin_event_timeout() takes a condition and timeout value
> > (in jiffies) as parameters. It spins until either the condition is true
> > or the timeout expires. It returns non-zero if the condition is true,
> > zero otherwise.
>
> What's the motivation for this? Where do you plan to use it?

I expect it would be useful for drivers that need to wait until a
specific register has changed its value, but don't want to risk an
infinite loop if the hardware is not functioning. For example, to
replace the while loop and subsequent check in this code:

long timeout = jiffies + 10;

setbits32(&ssi->scr, CCSR_SSI_SCR_SSIEN | CCSR_SSI_SCR_RE);

/* Wait until the SSI has filled its FIFO. Without this
* delay, ALSA complains about overruns. When the FIFO
* is full, the DMA controller initiates its first
* transfer. Until then, however, the DMA's DAR
* register is zero, which translates to an
* out-of-bounds pointer. This makes ALSA think an
* overrun has occurred.
*/
while (!(in_be32(&ssi->sisr) & CCSR_SSI_SISR_RFF0) &&
(jiffies < timeout));
if (!(in_be32(&ssi->sisr) & CCSR_SSI_SISR_RFF0))
return -EIO;

> If we're going to make this core infrastructure, it seems we should
> implement it with all best practices... eg I would think the while loop
> should include cpu_relax().

Ok. I'll post version 2 on Monday. Anything else I'm missing?

--
Timur Tabi
Linux kernel developer at Freescale

2009-03-07 09:13:54

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH] add function spin_event_timeout()

On 7.3.2009 02:25, Timur Tabi wrote:
> Ok. I'll post version 2 on Monday. Anything else I'm missing?

Use time_after().

2009-03-09 20:48:10

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: [PATCH] add function spin_event_timeout()

Timur Tabi wrote:
> On Fri, Mar 6, 2009 at 7:11 PM, Roland Dreier <[email protected]> wrote:
>
>> > The function spin_event_timeout() takes a condition and timeout value
>> > (in jiffies) as parameters. It spins until either the condition is true
>> > or the timeout expires. It returns non-zero if the condition is true,
>> > zero otherwise.
>>
>> What's the motivation for this? Where do you plan to use it?
>>
>
> I expect it would be useful for drivers that need to wait until a
> specific register has changed its value, but don't want to risk an
> infinite loop if the hardware is not functioning. For example, to
> replace the while loop and subsequent check in this code:
>

A jiffy is a really long time to be spinning, let alone liffies plural.
In this use case are you expecting that the loop will iterate a very
small number of times before returning, and the timeout case is a very
unlikely error condition?

J

2009-03-09 21:06:41

by Timur Tabi

[permalink] [raw]
Subject: Re: [PATCH] add function spin_event_timeout()

Jeremy Fitzhardinge wrote:

> A jiffy is a really long time to be spinning, let alone liffies plural.
> In this use case are you expecting that the loop will iterate a very
> small number of times before returning, and the timeout case is a very
> unlikely error condition?

Yes, in version 3 of this patch, I switch to using udelay(). I might
need to post a v4 though.

--
Timur Tabi
Linux kernel developer at Freescale