From 2b768821bf579cbbfcf8950160a26583ec6989a0 Mon Sep 17 00:00:00 2001 From: Ben Walker Date: Mon, 4 Dec 2017 11:05:49 -0700 Subject: [PATCH] blob: spdk_bs_destroy now only zeroes the super block The function just needs to zero out metadata so that the blobstore is effectively destroyed. If the user wants to unmap the rest of the disk after the blobstore is destroyed, they are free to do so. On future initializations of blobstores the code will do the unmapping, so performance is not impacted. While here, implement the zeroing using the new write_zeroes functionality instead of allocating a buffer full of zeroes. Change-Id: I7f18be0fd5e13a48b171ab3f4d5f5e12876023bc Signed-off-by: Ben Walker Reviewed-on: https://review.gerrithub.io/390307 Tested-by: SPDK Automated Test System Reviewed-by: Jim Harris Reviewed-by: Tomasz Zawadzki Reviewed-by: Daniel Verkamp --- include/spdk/blob.h | 8 +++---- include/spdk/lvol.h | 3 +-- include/spdk_internal/lvolstore.h | 1 - lib/bdev/lvol/vbdev_lvol.c | 2 +- lib/blob/blobstore.c | 24 +++++-------------- lib/lvol/lvol.c | 7 +++--- .../lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c | 4 ++-- test/unit/lib/blob/blob.c/blob_ut.c | 2 +- test/unit/lib/lvol/lvol.c/lvol_ut.c | 18 +++++++------- 9 files changed, 26 insertions(+), 43 deletions(-) diff --git a/include/spdk/blob.h b/include/spdk/blob.h index 2d15d4749..0fe263358 100644 --- a/include/spdk/blob.h +++ b/include/spdk/blob.h @@ -167,14 +167,12 @@ void spdk_bs_load(struct spdk_bs_dev *dev, struct spdk_bs_opts *opts, void spdk_bs_init(struct spdk_bs_dev *dev, struct spdk_bs_opts *opts, spdk_bs_op_with_handle_complete cb_fn, void *cb_arg); -/* Destroy a blob store by unmapping super block and destroying in-memory structures. - * If unmap_device is set to true, entire device will be unmapped. Otherwise only - * super block will be unmapped. +/* Destroy a blob store by zeroing the metadata and freeing in-memory structures. */ -void spdk_bs_destroy(struct spdk_blob_store *bs, bool unmap_device, spdk_bs_op_complete cb_fn, +void spdk_bs_destroy(struct spdk_blob_store *bs, spdk_bs_op_complete cb_fn, void *cb_arg); -/* Flush all volatile data to disk and destroy in-memory structures. */ +/* Flush all volatile data to disk and free in-memory structures. */ void spdk_bs_unload(struct spdk_blob_store *bs, spdk_bs_op_complete cb_fn, void *cb_arg); /* Set the given blob as the super blob. This will be retrievable immediately after an diff --git a/include/spdk/lvol.h b/include/spdk/lvol.h index 5b5d63bca..e3df90965 100644 --- a/include/spdk/lvol.h +++ b/include/spdk/lvol.h @@ -127,12 +127,11 @@ int spdk_lvs_unload(struct spdk_lvol_store *lvol_store, * All lvols have to be closed beforehand, when doing destroy. * * \param lvol_store Handle to lvolstore - * \param umap_device When set to true, unmaps whole lvolstore, otherwise writes zeros in first block * \param cb_fn Completion callback * \param cb_arg Completion callback custom arguments * \return error */ -int spdk_lvs_destroy(struct spdk_lvol_store *lvol_store, bool unmap_device, +int spdk_lvs_destroy(struct spdk_lvol_store *lvol_store, spdk_lvs_op_complete cb_fn, void *cb_arg); /** diff --git a/include/spdk_internal/lvolstore.h b/include/spdk_internal/lvolstore.h index 4474c5077..fd5f92f1a 100644 --- a/include/spdk_internal/lvolstore.h +++ b/include/spdk_internal/lvolstore.h @@ -70,7 +70,6 @@ struct spdk_lvs_destroy_req { spdk_lvs_op_complete cb_fn; void *cb_arg; struct spdk_lvol_store *lvs; - bool unmap_device; }; struct spdk_lvol_with_handle_req { diff --git a/lib/bdev/lvol/vbdev_lvol.c b/lib/bdev/lvol/vbdev_lvol.c index 36a4e5f7d..b2bd2e2b7 100644 --- a/lib/bdev/lvol/vbdev_lvol.c +++ b/lib/bdev/lvol/vbdev_lvol.c @@ -238,7 +238,7 @@ _vbdev_lvs_remove(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, void if (all_lvols_closed == true) { if (destroy) { - spdk_lvs_destroy(lvs, false, _vbdev_lvs_remove_cb, lvs_bdev); + spdk_lvs_destroy(lvs, _vbdev_lvs_remove_cb, lvs_bdev); } else { spdk_lvs_unload(lvs, _vbdev_lvs_remove_cb, lvs_bdev); } diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c index b13a31499..5c815fdb2 100644 --- a/lib/blob/blobstore.c +++ b/lib/blob/blobstore.c @@ -2335,12 +2335,11 @@ _spdk_bs_destroy_trim_cpl(spdk_bs_sequence_t *seq, void *cb_arg, int bserrno) spdk_bs_sequence_finish(seq, bserrno); _spdk_bs_free(bs); - spdk_dma_free(ctx->super); free(ctx); } void -spdk_bs_destroy(struct spdk_blob_store *bs, bool unmap_device, spdk_bs_op_complete cb_fn, +spdk_bs_destroy(struct spdk_blob_store *bs, spdk_bs_op_complete cb_fn, void *cb_arg) { struct spdk_bs_cpl cpl; @@ -2365,31 +2364,20 @@ spdk_bs_destroy(struct spdk_blob_store *bs, bool unmap_device, spdk_bs_op_comple return; } - ctx->super = spdk_dma_zmalloc(sizeof(*ctx->super), 0x1000, NULL); - if (!ctx->super) { - free(ctx); - cb_fn(cb_arg, -ENOMEM); - return; - } - ctx->bs = bs; seq = spdk_bs_sequence_start(bs->md_target.md_channel, &cpl); if (!seq) { - spdk_dma_free(ctx->super); free(ctx); cb_fn(cb_arg, -ENOMEM); return; } - if (unmap_device) { - /* TRIM the entire device */ - spdk_bs_sequence_unmap(seq, 0, bs->dev->blockcnt, _spdk_bs_destroy_trim_cpl, ctx); - } else { - /* Write zeroes to the super block */ - spdk_bs_sequence_write(seq, ctx->super, _spdk_bs_page_to_lba(bs, 0), _spdk_bs_byte_to_lba(bs, - sizeof(*ctx->super)), _spdk_bs_destroy_trim_cpl, ctx); - } + /* Write zeroes to the super block */ + spdk_bs_sequence_write_zeroes(seq, + _spdk_bs_page_to_lba(bs, 0), + _spdk_bs_byte_to_lba(bs, sizeof(struct spdk_bs_super_block)), + _spdk_bs_destroy_trim_cpl, ctx); } /* END spdk_bs_destroy */ diff --git a/lib/lvol/lvol.c b/lib/lvol/lvol.c index 265ae11dc..63d3299f0 100644 --- a/lib/lvol/lvol.c +++ b/lib/lvol/lvol.c @@ -693,12 +693,12 @@ _lvs_destroy_super_cb(void *cb_arg, int bserrno) assert(lvs != NULL); SPDK_INFOLOG(SPDK_TRACE_LVOL, "Destroying lvol store\n"); - spdk_bs_destroy(lvs->blobstore, lvs_req->unmap_device, _lvs_destroy_cb, lvs_req); + spdk_bs_destroy(lvs->blobstore, _lvs_destroy_cb, lvs_req); _spdk_lvs_free(lvs); } int -spdk_lvs_destroy(struct spdk_lvol_store *lvs, bool unmap_device, spdk_lvs_op_complete cb_fn, +spdk_lvs_destroy(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, void *cb_arg) { struct spdk_lvs_destroy_req *lvs_req; @@ -735,7 +735,6 @@ spdk_lvs_destroy(struct spdk_lvol_store *lvs, bool unmap_device, spdk_lvs_op_com lvs_req->cb_fn = cb_fn; lvs_req->cb_arg = cb_arg; lvs_req->lvs = lvs; - lvs_req->unmap_device = unmap_device; SPDK_INFOLOG(SPDK_TRACE_LVOL, "Deleting super blob\n"); spdk_bs_md_delete_blob(lvs->blobstore, lvs->super_blob_id, _lvs_destroy_super_cb, lvs_req); @@ -799,7 +798,7 @@ _spdk_lvol_delete_blob_cb(void *cb_arg, int lvolerrno) if (lvol->lvol_store->destruct_req && TAILQ_EMPTY(&lvol->lvol_store->lvols)) { if (lvol->lvol_store->destruct) - spdk_lvs_destroy(lvol->lvol_store, false, _spdk_lvs_destruct_cb, lvol->lvol_store->destruct_req); + spdk_lvs_destroy(lvol->lvol_store, _spdk_lvs_destruct_cb, lvol->lvol_store->destruct_req); } SPDK_INFOLOG(SPDK_TRACE_LVOL, "Lvol %s deleted\n", lvol->old_name); diff --git a/test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c b/test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c index 602605b7e..da91bb87d 100644 --- a/test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c +++ b/test/unit/lib/bdev/vbdev_lvol.c/vbdev_lvol_ut.c @@ -208,7 +208,7 @@ spdk_lvs_unload(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, void *c } int -spdk_lvs_destroy(struct spdk_lvol_store *lvs, bool unmap_device, spdk_lvs_op_complete cb_fn, +spdk_lvs_destroy(struct spdk_lvol_store *lvs, spdk_lvs_op_complete cb_fn, void *cb_arg) { struct spdk_lvol *lvol, *tmp; @@ -297,7 +297,7 @@ spdk_lvol_destroy(struct spdk_lvol *lvol, spdk_lvol_op_complete cb_fn, void *cb_ if (!lvol->lvol_store->destruct) { spdk_lvs_unload(lvol->lvol_store, destruct_req->cb_fn, destruct_req->cb_arg); } else { - spdk_lvs_destroy(lvol->lvol_store, false, destruct_req->cb_fn, destruct_req->cb_arg); + spdk_lvs_destroy(lvol->lvol_store, destruct_req->cb_fn, destruct_req->cb_arg); free(destruct_req); } } diff --git a/test/unit/lib/blob/blob.c/blob_ut.c b/test/unit/lib/blob/blob.c/blob_ut.c index ebf03b294..b92d97a25 100644 --- a/test/unit/lib/blob/blob.c/blob_ut.c +++ b/test/unit/lib/blob/blob.c/blob_ut.c @@ -1495,7 +1495,7 @@ bs_destroy(void) /* Destroy the blob store */ g_bserrno = -1; - spdk_bs_destroy(g_bs, 0, bs_op_complete, NULL); + spdk_bs_destroy(g_bs, bs_op_complete, NULL); /* Callback is called after device is destroyed in next scheduler run. */ _bs_flush_scheduler(); CU_ASSERT(TAILQ_EMPTY(&g_scheduled_ops)); diff --git a/test/unit/lib/lvol/lvol.c/lvol_ut.c b/test/unit/lib/lvol/lvol.c/lvol_ut.c index 668975b2d..6a4e9eccb 100644 --- a/test/unit/lib/lvol/lvol.c/lvol_ut.c +++ b/test/unit/lib/lvol/lvol.c/lvol_ut.c @@ -256,7 +256,7 @@ spdk_bs_unload(struct spdk_blob_store *bs, spdk_bs_op_complete cb_fn, void *cb_a } void -spdk_bs_destroy(struct spdk_blob_store *bs, bool unmap_device, spdk_bs_op_complete cb_fn, +spdk_bs_destroy(struct spdk_blob_store *bs, spdk_bs_op_complete cb_fn, void *cb_arg) { free(bs); @@ -482,7 +482,7 @@ lvs_init_destroy_success(void) /* Lvol store contains one lvol, this destroy should fail. */ g_lvserrno = -1; - rc = spdk_lvs_destroy(g_lvol_store, true, lvol_store_op_complete, NULL); + rc = spdk_lvs_destroy(g_lvol_store, lvol_store_op_complete, NULL); CU_ASSERT(rc == -EBUSY); CU_ASSERT(g_lvserrno == -EBUSY); SPDK_CU_ASSERT_FATAL(g_lvol_store != NULL); @@ -493,7 +493,7 @@ lvs_init_destroy_success(void) spdk_lvol_destroy(g_lvol, destroy_cb, NULL); g_lvserrno = -1; - rc = spdk_lvs_destroy(g_lvol_store, true, lvol_store_op_complete, NULL); + rc = spdk_lvs_destroy(g_lvol_store, lvol_store_op_complete, NULL); CU_ASSERT(rc == 0); CU_ASSERT(g_lvserrno == 0); g_lvol_store = NULL; @@ -607,7 +607,7 @@ lvs_names(void) /* Now destroy lvolstore 'x' and then confirm we can create a new lvolstore with name 'x'. */ g_lvserrno = -1; - rc = spdk_lvs_destroy(lvs_x, false, lvol_store_op_complete, NULL); + rc = spdk_lvs_destroy(lvs_x, lvol_store_op_complete, NULL); CU_ASSERT(rc == 0); CU_ASSERT(g_lvserrno == 0); g_lvol_store = NULL; @@ -636,7 +636,7 @@ lvs_names(void) /* Destroy the second lvolstore 'x'. Then we should be able to load the first lvolstore 'x'. */ g_lvserrno = -1; - rc = spdk_lvs_destroy(lvs_x2, false, lvol_store_op_complete, NULL); + rc = spdk_lvs_destroy(lvs_x2, lvol_store_op_complete, NULL); CU_ASSERT(rc == 0); CU_ASSERT(g_lvserrno == 0); g_lvserrno = -1; @@ -646,12 +646,12 @@ lvs_names(void) lvs_x = g_lvol_store; g_lvserrno = -1; - rc = spdk_lvs_destroy(lvs_x, false, lvol_store_op_complete, NULL); + rc = spdk_lvs_destroy(lvs_x, lvol_store_op_complete, NULL); CU_ASSERT(rc == 0); CU_ASSERT(g_lvserrno == 0); g_lvserrno = -1; - rc = spdk_lvs_destroy(lvs_y, false, lvol_store_op_complete, NULL); + rc = spdk_lvs_destroy(lvs_y, lvol_store_op_complete, NULL); CU_ASSERT(rc == 0); CU_ASSERT(g_lvserrno == 0); @@ -1231,7 +1231,7 @@ lvol_open(void) } g_lvserrno = -1; - spdk_lvs_destroy(g_lvol_store, false, lvol_store_op_complete, NULL); + spdk_lvs_destroy(g_lvol_store, lvol_store_op_complete, NULL); free(req); free(blob1); @@ -1311,7 +1311,7 @@ lvol_names(void) spdk_lvol_destroy(lvol2, destroy_cb, NULL); g_lvserrno = -1; - rc = spdk_lvs_destroy(lvs, false, lvol_store_op_complete, NULL); + rc = spdk_lvs_destroy(lvs, lvol_store_op_complete, NULL); CU_ASSERT(rc == 0); CU_ASSERT(g_lvserrno == 0); g_lvol_store = NULL;