Re: [PATCH v2 1/2] sched/wait: introduce wait_event_freezable_hrtimeout

[Ingo Molnar <mingo@kernel.org>]

* Hugo Lefeuvre <hle@owl.eu.com> wrote:

> introduce wait_event_freezable_hrtimeout, an interruptible and freezable
> version of wait_event_hrtimeout.
> 
> This helper will allow for simplifications in staging/android/vsoc.c, among
> others.
> 
> Signed-off-by: Hugo Lefeuvre <hle@owl.eu.com>
> ---
> Changes in v2:
>   - No change.
> 
>  include/linux/wait.h | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/wait.h b/include/linux/wait.h
> index 5f3efabc36f4..c4cf5113f58a 100644
> --- a/include/linux/wait.h
> +++ b/include/linux/wait.h
> @@ -483,7 +483,7 @@ do {										\
>  	__ret;									\
>  })
>  
> -#define __wait_event_hrtimeout(wq_head, condition, timeout, state)		\
> +#define __wait_event_hrtimeout(wq_head, condition, timeout, state, cmd)		\
>  ({										\
>  	int __ret = 0;								\
>  	struct hrtimer_sleeper __t;						\
> @@ -500,7 +500,7 @@ do {										\
>  			__ret = -ETIME;						\
>  			break;							\
>  		}								\
> -		schedule());							\
> +		cmd);							        \
>  										\
>  	hrtimer_cancel(&__t.timer);						\
>  	destroy_hrtimer_on_stack(&__t.timer);					\
> @@ -529,7 +529,23 @@ do {										\
>  	might_sleep();								\
>  	if (!(condition))							\
>  		__ret = __wait_event_hrtimeout(wq_head, condition, timeout,	\
> -					       TASK_UNINTERRUPTIBLE);		\
> +					       TASK_UNINTERRUPTIBLE,		\
> +					       schedule());			\
> +	__ret;									\
> +})
> +
> +/*
> + * like wait_event_hrtimeout() -- except it uses TASK_INTERRUPTIBLE to avoid
> + * increasing load and is freezable.
> + */
> +#define wait_event_freezable_hrtimeout(wq_head, condition, timeout)		\
> +({										\
> +	int __ret = 0;								\
> +	might_sleep();								\
> +	if (!(condition))							\
> +		__ret = __wait_event_hrtimeout(wq_head, condition, timeout,	\
> +					       TASK_INTERRUPTIBLE,		\
> +					       freezable_schedule());		\
>  	__ret;									\
>  })
>  
> @@ -555,7 +571,8 @@ do {										\
>  	might_sleep();								\
>  	if (!(condition))							\
>  		__ret = __wait_event_hrtimeout(wq, condition, timeout,		\
> -					       TASK_INTERRUPTIBLE);		\
> +					       TASK_INTERRUPTIBLE,		\
> +					       schedule());			\
>  	__ret;									\
>  })

Looks good to me - unless PeterZ objects I suspect this wants to go 
upstream via the driver tree, not the scheduler tree:

  Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo