(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>
---
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
