[CRIU] Re: [PATCH] lock: Restore ability to abort on futex waiting
Andrew Vagin
avagin at parallels.com
Tue Apr 3 04:27:55 EDT 2012
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
>
More information about the CRIU
mailing list