Re: [PATCH 13/32] wait: Add wait_event_hrtimeout()

From: Kent Overstreet
Date: Mon Jan 07 2013 - 19:09:53 EST


On Thu, Jan 03, 2013 at 03:08:16PM -0800, Andrew Morton wrote:
> On Wed, 26 Dec 2012 17:59:51 -0800
> Kent Overstreet <koverstreet@xxxxxxxxxx> wrote:
>
> > Analagous to wait_event_timeout() and friends, this adds
> > wait_event_hrtimeout() and wait_event_interruptible_hrtimeout().
> >
> > Note that unlike the versions that use regular timers, these don't
> > return the amount of time remaining when they return - instead, they
> > return 0 or -ETIME if they timed out.
>
> Why is this?

I was uncomfortable with the semantics of doing it the other way (that I
could get it right, anyways).

If the timer expires, there's no real guarantee that expire_time -
current_time would be <= 0 - due to timer slack certainly, and I'm not
sure I want to know the implications of the different clock bases in
hrtimers.

If the timer does expire and the code calculates that the time remaining
is nonnegative, that could be even worse if the calling code then reuses
that timeout. Probably safer to just return 0 then, but I could imagine
weird bugs or at least unintended behaviour arising from that too.

I came to the conclusion that if other users end up actually needing the
amount of time remaining, the sanest thing to do would be to create a
version that uses absolute timeouts instead of relative.

>
> > +/**
> > + * wait_event_hrtimeout - sleep until a condition gets true or a timeout elapses
> > + * @wq: the waitqueue to wait on
> > + * @condition: a C expression for the event to wait for
> > + * @timeout: timeout, in jiffies
> > + *
> > + * The process is put to sleep (TASK_UNINTERRUPTIBLE) until the
> > + * @condition evaluates to true or a signal is received.
> > + * The @condition is checked each time the waitqueue @wq is woken up.
> > + *
> > + * wake_up() has to be called after changing any variable that could
> > + * change the result of the wait condition.
> > + *
> > + * The function returns 0 if @condition became true, or -ETIME if the timeout
> > + * elapsed.
> > + */
>
> this, methinks:

Whoops, yes.

> --- a/include/linux/wait.h~wait-add-wait_event_hrtimeout-fix
> +++ a/include/linux/wait.h
> @@ -370,7 +370,7 @@ do { \
> * wait_event_hrtimeout - sleep until a condition gets true or a timeout elapses
> * @wq: the waitqueue to wait on
> * @condition: a C expression for the event to wait for
> - * @timeout: timeout, in jiffies
> + * @timeout: timeout, as a ktime_t
> *
> * The process is put to sleep (TASK_UNINTERRUPTIBLE) until the
> * @condition evaluates to true or a signal is received.
> @@ -395,7 +395,7 @@ do { \
> * wait_event_interruptible_hrtimeout - sleep until a condition gets true or a timeout elapses
> * @wq: the waitqueue to wait on
> * @condition: a C expression for the event to wait for
> - * @timeout: timeout, in jiffies
> + * @timeout: timeout, as a ktime_t
> *
> * The process is put to sleep (TASK_INTERRUPTIBLE) until the
> * @condition evaluates to true or a signal is received.
> _
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/