linux-kernel - Re: [PATCH 13/32] wait: Add wait_event

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 7 Jan 2013 16:09:58 -0800
From:	Kent Overstreet <koverstreet@...gle.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	linux-kernel@...r.kernel.org, linux-aio@...ck.org,
	linux-fsdevel@...r.kernel.org, zab@...hat.com, bcrl@...ck.org,
	jmoyer@...hat.com, axboe@...nel.dk, viro@...iv.linux.org.uk,
	tytso@....edu
Subject: Re: [PATCH 13/32] wait: Add wait_event_hrtimeout()

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@...gle.com> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/