Quoting Oren Laadan (orenl@xxxxxxxxxxxxxxx):
> (This patch applies on top of ckpt-v20-rc1)
>
> The value of ctx->errno is protected by a semaphore to ensure that
> processes do not pique at it prematurely (that is - after the error
> bit is set, but before the value is committed).
>
> But, lockdep sometimes complains when an error is detected from a
> legitimately failed restart attempt. This is because we release
> ctx->errno_sem in a task context different from the one in which it
> was acquired.
>
> Replace the semaphore with an event completion: this sort of
> synchronization is what completion constructs are all about.
>
> Originally reported by Nathan Lynch.
>
> Signed-off-by: Oren Laadan <orenl@xxxxxxxxxxxxxxx>
Acked-by: Serge Hallyn <serue@xxxxxxxxxx>
> ---
> checkpoint/sys.c | 13 ++++---------
> include/linux/checkpoint.h | 5 ++---
> include/linux/checkpoint_types.h | 2 +-
> 3 files changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/checkpoint/sys.c b/checkpoint/sys.c
> index b605784..fe85ca7 100644
> --- a/checkpoint/sys.c
> +++ b/checkpoint/sys.c
> @@ -280,8 +280,7 @@ static struct ckpt_ctx *ckpt_ctx_alloc(int fd, unsigned long uflags,
> init_waitqueue_head(&ctx->ghostq);
> init_completion(&ctx->complete);
>
> - init_rwsem(&ctx->errno_sem);
> - down_write(&ctx->errno_sem);
> + init_completion(&ctx->errno_sync);
>
> #ifdef CONFIG_CHECKPOINT_DEBUG
> INIT_LIST_HEAD(&ctx->task_status);
> @@ -343,11 +342,8 @@ void ckpt_set_error(struct ckpt_ctx *ctx, int err)
> /* atomically set ctx->errno */
> if (!ckpt_test_and_set_ctx_kflag(ctx, CKPT_CTX_ERROR)) {
> ctx->errno = err;
> - /*
> - * We initialized ctx->errno_sem write-held to prevent
> - * other tasks from reading ctx->errno prematurely.
> - */
> - up_write(&ctx->errno_sem);
> + /* make ctx->errno visible to all other tasks */
> + complete_all(&ctx->errno_sync);
> /* on restart, notify all tasks in restarting subtree */
> if (ctx->kflags & CKPT_CTX_RESTART)
> restore_notify_error(ctx);
> @@ -357,8 +353,7 @@ void ckpt_set_error(struct ckpt_ctx *ctx, int err)
> void ckpt_set_success(struct ckpt_ctx *ctx)
> {
> ckpt_set_ctx_kflag(ctx, CKPT_CTX_SUCCESS);
> - /* avoid warning "lock still held" when freeing (was write-held) */
> - up_write(&ctx->errno_sem);
> + complete_all(&ctx->errno_sync);
> }
>
> /* helpers to handler log/dbg/err messages */
> diff --git a/include/linux/checkpoint.h b/include/linux/checkpoint.h
> index a25bac1..a8b9e21 100644
> --- a/include/linux/checkpoint.h
> +++ b/include/linux/checkpoint.h
> @@ -160,10 +160,9 @@ static inline int ckpt_get_error(struct ckpt_ctx *ctx)
> {
> /*
> * We may notice CKPT_CTX_ERROR before ctx->errno is set, but
> - * ctx->errno_sem remains (write) held until after it's done.
> + * ctx->errno_sync remains not-completed until after it's done.
> */
> - if (!ctx->errno)
> - down_read(&ctx->errno_sem);
> + wait_for_completion(&ctx->errno_sync);
> return ctx->errno;
> }
>
> diff --git a/include/linux/checkpoint_types.h b/include/linux/checkpoint_types.h
> index efc9357..e9cc1d8 100644
> --- a/include/linux/checkpoint_types.h
> +++ b/include/linux/checkpoint_types.h
> @@ -61,7 +61,7 @@ struct ckpt_ctx {
> char err_string[256]; /* checkpoint: error string */
>
> int errno; /* errno that caused failure */
> - struct rw_semaphore errno_sem; /* protect errno setting */
> + struct completion errno_sync; /* protect errno setting */
>
> struct list_head pgarr_list; /* page array to dump VMA contents */
> struct list_head pgarr_pool; /* pool of empty page arrays chain */
> --
> 1.6.3.3
>
> _______________________________________________
> Containers mailing list
> Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
> https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
