Re: [PATCH v3 5/6] sched/wait: Add wait_event_state()

Ingo Molnar
2022-09-04T09:55:07Z

* Peter Zijlstra <peterz@infradead.org> wrote:

> Allows waiting with a custom @state.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> include/linux/wait.h | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> --- a/include/linux/wait.h
> +++ b/include/linux/wait.h
> @@ -931,6 +931,34 @@ extern int do_wait_intr_irq(wait_queue_h
> __ret; \
> })
>
> +#define __wait_event_state(wq, condition, state) \
> + ___wait_event(wq, condition, state, 0, 0, schedule())
> +
> +/**
> + * wait_event_state - sleep until a condition gets true
> + * @wq_head: the waitqueue to wait on
> + * @condition: a C expression for the event to wait for
> + * @state: state to sleep in
> + *
> + * The process is put to sleep (@state) until the @condition evaluates to true
> + * or a signal is received. The @condition is checked each time the waitqueue
> + * @wq_head is woken up.

Documentation inconsistency nit: if TASK_INTERRUPTIBLE isn't in @state then
we won't wake up when a signal is received. This probably got copy-pasted
from a signal variant.

> + *
> + * wake_up() has to be called after changing any variable that could
> + * change the result of the wait condition.
> + *
> + * The function will return -ERESTARTSYS if it was interrupted by a
> + * signal and 0 if @condition evaluated to true.

That's not unconditionally true either if !TASK_INTERRUPTIBLE.

> +#define wait_event_state(wq_head, condition, state) \
> +({ \
> + int __ret = 0; \
> + might_sleep(); \

Very small style consistency nit, the above should have a newline after
local variables:

> +#define wait_event_state(wq_head, condition, state) \
> +({ \
> + int __ret = 0; \
> + \
> + might_sleep(); \

Like most (but not all ... :-/ ) of the existing primitives have.

Thanks,

Ingo