From cfc372c2ec9e5252f66ec3d891737c5c002bd08f Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Mon, 1 Oct 2018 14:36:51 -0700 Subject: [PATCH] reduce: write path of pm file to backing dev Write pm filepath first to offset 4K. Then write the super block to offset 0. This ensures the backing device isn't really valid until both are written (avoiding the case where the super block got written but not the path.) Signed-off-by: Jim Harris Change-Id: I55508aa67b4179e658827c982cd955d009a3f321 Reviewed-on: https://review.gerrithub.io/432505 Chandler-Test-Pool: SPDK Automated Test System Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Shuhei Matsumoto Reviewed-by: Ben Walker Reviewed-by: --- lib/reduce/reduce.c | 47 ++++++++++++++++++++--- test/unit/lib/reduce/reduce.c/reduce_ut.c | 11 ++++++ 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/lib/reduce/reduce.c b/lib/reduce/reduce.c index 0907b98c0..26dce548e 100644 --- a/lib/reduce/reduce.c +++ b/lib/reduce/reduce.c @@ -45,6 +45,9 @@ #define SPDK_REDUCE_SIGNATURE "SPDKREDU" +/* Offset into the backing device where the persistent memory file's path is stored. */ +#define REDUCE_BACKING_DEV_PATH_OFFSET 4096 + /* Structure written to offset 0 of both the pm file and the backing device. */ struct spdk_reduce_vol_superblock { uint8_t signature[8]; @@ -83,7 +86,7 @@ _get_pm_logical_map_size(uint64_t vol_size, uint64_t chunk_size) logical_map_size = chunks_in_logical_map * sizeof(uint64_t); /* Round up to next cacheline. */ - return divide_round_up(logical_map_size, 64) * 64; + return divide_round_up(logical_map_size, REDUCE_PM_SIZE_ALIGNMENT) * REDUCE_PM_SIZE_ALIGNMENT; } static uint64_t @@ -171,6 +174,7 @@ struct reduce_init_load_ctx { spdk_reduce_vol_op_with_handle_complete cb_fn; void *cb_arg; struct iovec iov; + void *path; }; static void @@ -182,6 +186,22 @@ _init_write_super_cpl(void *cb_arg, int ziperrno) free(init_ctx); } +static void +_init_write_path_cpl(void *cb_arg, int ziperrno) +{ + struct reduce_init_load_ctx *init_ctx = cb_arg; + struct spdk_reduce_vol *vol = init_ctx->vol; + + spdk_dma_free(init_ctx->path); + init_ctx->iov.iov_base = vol->backing_super; + init_ctx->iov.iov_len = sizeof(*vol->backing_super); + init_ctx->backing_cb_args.cb_fn = _init_write_super_cpl; + init_ctx->backing_cb_args.cb_arg = init_ctx; + vol->backing_dev->writev(vol->backing_dev, &init_ctx->iov, 1, + 0, sizeof(*vol->backing_super) / vol->backing_dev->blocklen, + &init_ctx->backing_cb_args); +} + void spdk_reduce_vol_init(struct spdk_reduce_vol_params *params, struct spdk_reduce_backing_dev *backing_dev, @@ -255,6 +275,15 @@ spdk_reduce_vol_init(struct spdk_reduce_vol_params *params, return; } + init_ctx->path = spdk_dma_zmalloc(REDUCE_PATH_MAX, 0, NULL); + if (init_ctx->path == NULL) { + free(init_ctx); + spdk_dma_free(vol->backing_super); + free(vol); + cb_fn(cb_arg, NULL, -ENOMEM); + return; + } + memcpy(&vol->uuid, ¶ms->uuid, sizeof(params->uuid)); vol->backing_dev = backing_dev; @@ -274,12 +303,20 @@ spdk_reduce_vol_init(struct spdk_reduce_vol_params *params, init_ctx->vol = vol; init_ctx->cb_fn = cb_fn; init_ctx->cb_arg = cb_arg; - init_ctx->iov.iov_base = vol->backing_super; - init_ctx->iov.iov_len = sizeof(*vol->backing_super); - init_ctx->backing_cb_args.cb_fn = _init_write_super_cpl; + + memcpy(init_ctx->path, vol->pm_file.path, REDUCE_PATH_MAX); + init_ctx->iov.iov_base = init_ctx->path; + init_ctx->iov.iov_len = REDUCE_PATH_MAX; + init_ctx->backing_cb_args.cb_fn = _init_write_path_cpl; init_ctx->backing_cb_args.cb_arg = init_ctx; + /* Write path to offset 4K on backing device - just after where the super + * block will be written. We wait until this is committed before writing the + * super block to guarantee we don't get the super block written without the + * the path if the system crashed in the middle of a write operation. + */ vol->backing_dev->writev(vol->backing_dev, &init_ctx->iov, 1, - 0, sizeof(*vol->backing_super) / vol->backing_dev->blocklen, + REDUCE_BACKING_DEV_PATH_OFFSET / vol->backing_dev->blocklen, + REDUCE_PATH_MAX / vol->backing_dev->blocklen, &init_ctx->backing_cb_args); } diff --git a/test/unit/lib/reduce/reduce.c/reduce_ut.c b/test/unit/lib/reduce/reduce.c/reduce_ut.c index 32a36c02f..9793a579c 100644 --- a/test/unit/lib/reduce/reduce.c/reduce_ut.c +++ b/test/unit/lib/reduce/reduce.c/reduce_ut.c @@ -46,6 +46,8 @@ static char *g_persistent_pm_buf; static bool g_backing_dev_closed; static char *g_backing_dev_buf; +#define TEST_MD_PATH "/tmp/meta.data" + static void sync_pm_buf(const void *addr, size_t length) { @@ -177,6 +179,9 @@ pm_file_init(struct spdk_reduce_pm_file *pm_file, struct spdk_reduce_vol_params g_volatile_pm_buf = calloc(1, pm_file->size); SPDK_CU_ASSERT_FATAL(g_volatile_pm_buf != NULL); + memcpy(pm_file->path, TEST_MD_PATH, strlen(TEST_MD_PATH)); + pm_file->path[strlen(TEST_MD_PATH)] = '\0'; + pm_file->pm_buf = g_volatile_pm_buf; pm_file->close = pm_file_close; @@ -379,6 +384,12 @@ init_backing_dev(void) persistent_params = (struct spdk_reduce_vol_params *)(g_backing_dev_buf + 8); CU_ASSERT(memcmp(persistent_params, ¶ms, sizeof(params)) == 0); CU_ASSERT(backing_dev.close != NULL); + /* Confirm that the path to the persistent memory metadata file was persisted to + * the backing device. + */ + CU_ASSERT(strncmp(TEST_MD_PATH, + g_backing_dev_buf + REDUCE_BACKING_DEV_PATH_OFFSET, + REDUCE_PATH_MAX) == 0); g_ziperrno = -1; g_backing_dev_closed = false;