[CRIU] Re: [PATCH] lock: Restore ability to abort on futex waiting

[Andrew Vagin]

On Tue, Apr 03, 2012 at 12:52:38AM +0400, Cyrill Gorcunov wrote:
> In commit 71cc2733a79efba65d3466f784b19d17805cf50d
> I occasionally dropped the ability to abort on waiting
> (because we used signed -1 value to inform waiters that
> something is wrong and waiting should be aborted, but
> the type was changed to unsigned one and as result
> this condition never triggers).
> 
> So to resolve it futex_abort_and_wake() is added and
> should be used explicitly where appropriate instead
> if signess hack.
>

Acked-by: Andrew Vagin <avagin at openvz.org> 
> Reported-by: Andrew Vagin <avagin at parallels.com>
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
> 
> Andrew, does it looks good for you?
> 
>  cr-restore.c   |    2 +-
>  include/lock.h |   13 ++++++++++++-
>  restorer.c     |    2 +-
>  3 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/cr-restore.c b/cr-restore.c
> index 4f38c7f..f5fb91c 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -1132,7 +1132,7 @@ static void sigchld_handler(int signal, siginfo_t *siginfo, void *data)
>  		pr_err("%d killed by signal %d\n",
>  			siginfo->si_pid, siginfo->si_status);
>  
> -	futex_set_and_wake(&task_entries->nr_in_progress, -1);
> +	futex_abort_and_wake(&task_entries->nr_in_progress);
>  }
>  
>  static int restore_task_with_children(void *_arg)
> diff --git a/include/lock.h b/include/lock.h
> index 6f77bfd..be9ffdf 100644
> --- a/include/lock.h
> +++ b/include/lock.h
> @@ -15,6 +15,9 @@ typedef struct {
>  	u32	raw;
>  } futex_t;
>  
> +#define FUTEX_ABORT_FLAG	(0x80000000)
> +#define FUTEX_ABORT_RAW		(-1U)
> +
>  /* Get current futex @f value */
>  static inline u32 futex_get(futex_t *f)
>  {
> @@ -37,7 +40,8 @@ static inline void futex_set(futex_t *f, u32 v)
>  								\
>  		while (1) {					\
>  			tmp = (__f)->raw;			\
> -			if (tmp __cond (__v))			\
> +			if ((tmp & FUTEX_ABORT_FLAG) ||		\
> +			    (tmp __cond (__v)))			\
>  				break;				\
>  			ret = sys_futex(&(__f)->raw, FUTEX_WAIT,\
>  					tmp, NULL, NULL, 0);	\
> @@ -52,6 +56,13 @@ static inline void futex_set_and_wake(futex_t *f, u32 v)
>  	BUG_ON(sys_futex(&f->raw, FUTEX_WAKE, INT_MAX, NULL, NULL, 0) < 0);
>  }
>  
> +/* Mark futex @f as wait abort needed and wake up all waiters */
> +static inline void futex_abort_and_wake(futex_t *f)
> +{
> +	BUILD_BUG_ON(!(FUTEX_ABORT_RAW & FUTEX_ABORT_FLAG));
> +	futex_set_and_wake(f, FUTEX_ABORT_RAW);
> +}
> +
>  /* Decrement futex @f value and wake up all waiters */
>  static inline void futex_dec_and_wake(futex_t *f)
>  {
> diff --git a/restorer.c b/restorer.c
> index 18e5b6c..97798e9 100644
> --- a/restorer.c
> +++ b/restorer.c
> @@ -46,7 +46,7 @@ static void sigchld_handler(int signal, siginfo_t *siginfo, void *data)
>  		write_string(" killed by signal ");
>  	write_num_n(siginfo->si_status);
>  
> -	futex_set_and_wake(&task_entries->nr_in_progress, -1);
> +	futex_abort_and_wake(&task_entries->nr_in_progress);
>  	/* sa_restorer may be unmaped, so we can't go back to userspace*/
>  	sys_kill(sys_getpid(), SIGSTOP);
>  	sys_exit(1);
> -- 
> 1.7.7.6
>