From 5fb87228998dd51ba9c2967c52933ed04c1eb4d0 Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Thu, 14 Sep 2017 11:34:36 -0700 Subject: [PATCH] bdev: add callback to free part_base This puts responsibility on the caller to free the buffer and do any other cleanup associated with the partition base (for example, GPT buffer). While here, also clean up a bunch of places where on various failures during initialization, it would just free the buffer instead of calling spdk_bdev_part_base_free(). The latter is required to make sure the bdev descriptor gets closed as well. Signed-off-by: Jim Harris Change-Id: Ic000339459eca4a4a1d103da2e1f3feffe7e764f Reviewed-on: https://review.gerrithub.io/378653 Reviewed-by: Daniel Verkamp Reviewed-by: Dariusz Stojaczyk Tested-by: SPDK Automated Test System Reviewed-by: Changpeng Liu --- include/spdk_internal/bdev.h | 6 ++++++ lib/bdev/bdev.c | 13 +++++++++---- lib/bdev/error/vbdev_error.c | 16 +++++++++++----- lib/bdev/gpt/vbdev_gpt.c | 17 +++++++++-------- lib/bdev/split/vbdev_split.c | 11 ++++++++--- 5 files changed, 43 insertions(+), 20 deletions(-) diff --git a/include/spdk_internal/bdev.h b/include/spdk_internal/bdev.h index 4c65ab4d4..8854caf50 100644 --- a/include/spdk_internal/bdev.h +++ b/include/spdk_internal/bdev.h @@ -433,11 +433,16 @@ spdk_bdev_io_from_ctx(void *ctx) ((uintptr_t)ctx - offsetof(struct spdk_bdev_io, driver_ctx)); } +struct spdk_bdev_part_base; + +typedef void (*spdk_bdev_part_base_free_fn)(struct spdk_bdev_part_base *base); + struct spdk_bdev_part_base { struct spdk_bdev *bdev; struct spdk_bdev_desc *desc; uint32_t ref; uint32_t channel_size; + spdk_bdev_part_base_free_fn base_free_fn; bool claimed; struct spdk_bdev_module_if *module; struct spdk_bdev_fn_table *fn_table; @@ -469,6 +474,7 @@ int spdk_bdev_part_base_construct(struct spdk_bdev_part_base *base, struct spdk_ struct spdk_bdev_module_if *module, struct spdk_bdev_fn_table *fn_table, struct bdev_part_tailq *tailq, + spdk_bdev_part_base_free_fn free_fn, uint32_t channel_size, spdk_io_channel_create_cb ch_create_cb, spdk_io_channel_destroy_cb ch_destroy_cb); diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index e59ea21be..47332e14d 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -1880,10 +1880,11 @@ spdk_bdev_module_list_add(struct spdk_bdev_module_if *bdev_module) void spdk_bdev_part_base_free(struct spdk_bdev_part_base *base) { - assert(base->bdev); - assert(base->desc); - spdk_bdev_close(base->desc); - free(base); + if (base->desc) { + spdk_bdev_close(base->desc); + base->desc = NULL; + } + base->base_free_fn(base); } void @@ -2043,6 +2044,7 @@ int spdk_bdev_part_base_construct(struct spdk_bdev_part_base *base, struct spdk_bdev *bdev, spdk_bdev_remove_cb_t remove_cb, struct spdk_bdev_module_if *module, struct spdk_bdev_fn_table *fn_table, struct bdev_part_tailq *tailq, + spdk_bdev_part_base_free_fn free_fn, uint32_t channel_size, spdk_io_channel_create_cb ch_create_cb, spdk_io_channel_destroy_cb ch_destroy_cb) { @@ -2052,6 +2054,7 @@ spdk_bdev_part_base_construct(struct spdk_bdev_part_base *base, struct spdk_bdev fn_table->io_type_supported = spdk_bdev_part_io_type_supported; base->bdev = bdev; + base->desc = NULL; base->ref = 0; base->module = module; base->fn_table = fn_table; @@ -2060,9 +2063,11 @@ spdk_bdev_part_base_construct(struct spdk_bdev_part_base *base, struct spdk_bdev base->channel_size = channel_size; base->ch_create_cb = ch_create_cb; base->ch_destroy_cb = ch_destroy_cb; + base->base_free_fn = free_fn; rc = spdk_bdev_open(bdev, false, remove_cb, bdev, &base->desc); if (rc) { + spdk_bdev_part_base_free(base); SPDK_ERRLOG("could not open bdev %s\n", spdk_bdev_get_name(bdev)); return -1; } diff --git a/lib/bdev/error/vbdev_error.c b/lib/bdev/error/vbdev_error.c index 4bd73c3f1..6a39ae3d2 100644 --- a/lib/bdev/error/vbdev_error.c +++ b/lib/bdev/error/vbdev_error.c @@ -70,6 +70,12 @@ struct error_channel { static pthread_mutex_t g_vbdev_error_mutex = PTHREAD_MUTEX_INITIALIZER; static SPDK_BDEV_PART_TAILQ g_error_disks = TAILQ_HEAD_INITIALIZER(g_error_disks); +static void +spdk_error_free_base(struct spdk_bdev_part_base *base) +{ + free(base); +} + int spdk_vbdev_inject_error(char *name, uint32_t io_type, uint32_t error_type, uint32_t error_num) { @@ -223,25 +229,25 @@ spdk_vbdev_error_create(struct spdk_bdev *base_bdev) rc = spdk_bdev_part_base_construct(base, base_bdev, NULL, SPDK_GET_BDEV_MODULE(error), &vbdev_error_fn_table, - &g_error_disks, sizeof(struct error_channel), NULL, NULL); + &g_error_disks, spdk_error_free_base, + sizeof(struct error_channel), NULL, NULL); if (rc) { SPDK_ERRLOG("could not construct part base for bdev %s\n", spdk_bdev_get_name(base_bdev)); - free(base); return -1; } disk = calloc(1, sizeof(*disk)); if (!disk) { SPDK_ERRLOG("Memory allocation failure\n"); - free(base); + spdk_error_free_base(base); return -1; } name = spdk_sprintf_alloc("EE_%s", spdk_bdev_get_name(base_bdev)); if (!name) { SPDK_ERRLOG("name allocation failure\n"); + spdk_error_free_base(base); free(disk); - free(base); return -1; } @@ -250,8 +256,8 @@ spdk_vbdev_error_create(struct spdk_bdev *base_bdev) if (rc) { SPDK_ERRLOG("could not construct part for bdev %s\n", spdk_bdev_get_name(base_bdev)); /* spdk_bdev_part_construct will free name on failure */ + spdk_error_free_base(base); free(disk); - free(base); return -1; } diff --git a/lib/bdev/gpt/vbdev_gpt.c b/lib/bdev/gpt/vbdev_gpt.c index 1dee6e514..7ec417d26 100644 --- a/lib/bdev/gpt/vbdev_gpt.c +++ b/lib/bdev/gpt/vbdev_gpt.c @@ -75,10 +75,12 @@ static SPDK_BDEV_PART_TAILQ g_gpt_disks = TAILQ_HEAD_INITIALIZER(g_gpt_disks); static bool g_gpt_disabled; static void -spdk_gpt_base_free(struct gpt_base *gpt_base) +spdk_gpt_base_free(struct spdk_bdev_part_base *base) { + struct gpt_base *gpt_base = SPDK_CONTAINEROF(base, struct gpt_base, part_base); + spdk_dma_free(gpt_base->gpt.buf); - spdk_bdev_part_base_free(&gpt_base->part_base); + free(gpt_base); } static void @@ -113,11 +115,10 @@ spdk_gpt_base_bdev_init(struct spdk_bdev *bdev) rc = spdk_bdev_part_base_construct(&gpt_base->part_base, bdev, spdk_gpt_base_bdev_hotremove_cb, SPDK_GET_BDEV_MODULE(gpt), &vbdev_gpt_fn_table, - &g_gpt_disks, sizeof(struct gpt_channel), - NULL, NULL); + &g_gpt_disks, spdk_gpt_base_free, + sizeof(struct gpt_channel), NULL, NULL); if (rc) { SPDK_ERRLOG("cannot construct gpt_base"); - free(gpt_base); return NULL; } @@ -125,7 +126,7 @@ spdk_gpt_base_bdev_init(struct spdk_bdev *bdev) gpt->buf = spdk_dma_zmalloc(SPDK_GPT_BUFFER_SIZE, 0x1000, NULL); if (!gpt->buf) { SPDK_ERRLOG("Cannot alloc buf\n"); - free(gpt_base); + spdk_bdev_part_base_free(&gpt_base->part_base); return NULL; } @@ -309,7 +310,7 @@ end: if (gpt_base->part_base.ref == 0) { /* If no gpt_disk instances were created, free the base context */ - spdk_gpt_base_free(gpt_base); + spdk_bdev_part_base_free(&gpt_base->part_base); } } @@ -331,7 +332,7 @@ vbdev_gpt_read_gpt(struct spdk_bdev *bdev) SPDK_GPT_BUFFER_SIZE, spdk_gpt_bdev_complete, gpt_base); if (rc < 0) { spdk_put_io_channel(gpt_base->ch); - spdk_gpt_base_free(gpt_base); + spdk_bdev_part_base_free(&gpt_base->part_base); SPDK_ERRLOG("Failed to send bdev_io command\n"); return -1; } diff --git a/lib/bdev/split/vbdev_split.c b/lib/bdev/split/vbdev_split.c index e397e6f4e..dc366c8fd 100644 --- a/lib/bdev/split/vbdev_split.c +++ b/lib/bdev/split/vbdev_split.c @@ -56,6 +56,12 @@ struct vbdev_split_channel { struct spdk_bdev_part_channel part_ch; }; +static void +vbdev_split_base_free(struct spdk_bdev_part_base *base) +{ + free(base); +} + static int vbdev_split_destruct(void *ctx) { @@ -151,11 +157,10 @@ vbdev_split_create(struct spdk_bdev *base_bdev, uint64_t split_count, uint64_t s rc = spdk_bdev_part_base_construct(split_base, base_bdev, vbdev_split_base_bdev_hotremove_cb, SPDK_GET_BDEV_MODULE(split), &vbdev_split_fn_table, - &g_split_disks, sizeof(struct vbdev_split_channel), - NULL, NULL); + &g_split_disks, vbdev_split_base_free, + sizeof(struct vbdev_split_channel), NULL, NULL); if (rc) { SPDK_ERRLOG("Cannot construct bdev part base\n"); - free(split_base); return -1; }