From 78097953f7311e83abc51f300c2a0148273d8218 Mon Sep 17 00:00:00 2001 From: Konrad Sztyber Date: Mon, 24 Jun 2019 15:46:26 +0200 Subject: [PATCH] lib/ftl: notify init/fini callbacks on proper threads This patch makes sure we're on the thread that requested creation / deletion of the device when calling the notification callback. Change-Id: Ia11a8054692874f6b57d4ebe3e3cb290c58e83b6 Signed-off-by: Konrad Sztyber Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/459618 Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Mateusz Kozlowski Reviewed-by: Wojciech Malikowski Reviewed-by: Darek Stojaczyk --- include/spdk/ftl.h | 2 +- lib/bdev/nvme/bdev_ftl.c | 2 +- lib/ftl/ftl_core.h | 26 +++++----- lib/ftl/ftl_init.c | 101 +++++++++++++++++++++++---------------- 4 files changed, 78 insertions(+), 53 deletions(-) diff --git a/include/spdk/ftl.h b/include/spdk/ftl.h index 2862e7cad..59c5c450a 100644 --- a/include/spdk/ftl.h +++ b/include/spdk/ftl.h @@ -216,7 +216,7 @@ int spdk_ftl_dev_init(const struct spdk_ftl_dev_init_opts *opts, spdk_ftl_init_f * * \return 0 if successfully scheduled free, negative errno otherwise. */ -int spdk_ftl_dev_free(struct spdk_ftl_dev *dev, spdk_ftl_fn cb, void *cb_arg); +int spdk_ftl_dev_free(struct spdk_ftl_dev *dev, spdk_ftl_init_fn cb, void *cb_arg); /** * Initialize FTL configuration structure with default values. diff --git a/lib/bdev/nvme/bdev_ftl.c b/lib/bdev/nvme/bdev_ftl.c index 831bac737..c19ab3e8d 100644 --- a/lib/bdev/nvme/bdev_ftl.c +++ b/lib/bdev/nvme/bdev_ftl.c @@ -181,7 +181,7 @@ out: } static void -bdev_ftl_free_cb(void *ctx, int status) +bdev_ftl_free_cb(struct spdk_ftl_dev *dev, void *ctx, int status) { struct ftl_bdev *ftl_bdev = ctx; bool finish_done; diff --git a/lib/ftl/ftl_core.h b/lib/ftl/ftl_core.h index 18602caa7..5d19bffdc 100644 --- a/lib/ftl/ftl_core.h +++ b/lib/ftl/ftl_core.h @@ -130,6 +130,17 @@ struct ftl_nv_cache { pthread_spinlock_t lock; }; +struct ftl_init_context { + /* User's callback */ + spdk_ftl_init_fn cb_fn; + /* Callback's argument */ + void *cb_arg; + /* Thread to call the callback on */ + struct spdk_thread *thread; + /* Poller to check if the device has been destroyed/initialized */ + struct spdk_poller *poller; +}; + struct spdk_ftl_dev { /* Device instance */ struct spdk_uuid uuid; @@ -143,17 +154,10 @@ struct spdk_ftl_dev { /* Indicates the device is about to be stopped */ int halt; - /* Init callback */ - spdk_ftl_init_fn init_cb; - /* Init callback's context */ - void *init_arg; - - /* Halt callback */ - spdk_ftl_fn halt_cb; - /* Halt callback's context */ - void *halt_arg; - /* Halt poller, checks if the device has been halted */ - struct spdk_poller *halt_poller; + /* Initializaton context */ + struct ftl_init_context init_ctx; + /* Destruction context */ + struct ftl_init_context fini_ctx; /* IO channel */ struct spdk_io_channel *ioch; diff --git a/lib/ftl/ftl_init.c b/lib/ftl/ftl_init.c index e440955b8..8ea0b48e9 100644 --- a/lib/ftl/ftl_init.c +++ b/lib/ftl/ftl_init.c @@ -811,6 +811,17 @@ ftl_dev_l2p_alloc(struct spdk_ftl_dev *dev) return 0; } +static void +ftl_call_init_complete_cb(void *_ctx) +{ + struct ftl_init_context *ctx = _ctx; + struct spdk_ftl_dev *dev = SPDK_CONTAINEROF(ctx, struct spdk_ftl_dev, init_ctx); + + if (ctx->cb_fn != NULL) { + ctx->cb_fn(dev, ctx->cb_arg, 0); + } +} + static void ftl_init_complete(struct spdk_ftl_dev *dev) { @@ -820,44 +831,37 @@ ftl_init_complete(struct spdk_ftl_dev *dev) dev->initialized = 1; - if (dev->init_cb) { - dev->init_cb(dev, dev->init_arg, 0); - } - - dev->init_cb = NULL; - dev->init_arg = NULL; + spdk_thread_send_msg(dev->init_ctx.thread, ftl_call_init_complete_cb, &dev->init_ctx); } -struct ftl_init_fail_ctx { - spdk_ftl_init_fn cb; - void *arg; -}; - static void -ftl_init_fail_cb(void *ctx, int status) +ftl_init_fail_cb(struct spdk_ftl_dev *dev, void *_ctx, int status) { - struct ftl_init_fail_ctx *fail_cb = ctx; + struct ftl_init_context *ctx = _ctx; - fail_cb->cb(NULL, fail_cb->arg, -ENODEV); - free(fail_cb); + if (ctx->cb_fn != NULL) { + ctx->cb_fn(NULL, ctx->cb_arg, -ENODEV); + } + + free(ctx); } +static int _spdk_ftl_dev_free(struct spdk_ftl_dev *dev, spdk_ftl_init_fn cb_fn, void *cb_arg, + struct spdk_thread *thread); + static void ftl_init_fail(struct spdk_ftl_dev *dev) { - struct ftl_init_fail_ctx *fail_cb; + struct ftl_init_context *ctx; - fail_cb = malloc(sizeof(*fail_cb)); - if (!fail_cb) { + ctx = malloc(sizeof(*ctx)); + if (!ctx) { SPDK_ERRLOG("Unable to allocate context to free the device\n"); return; } - fail_cb->cb = dev->init_cb; - fail_cb->arg = dev->init_arg; - dev->halt_cb = NULL; - - if (spdk_ftl_dev_free(dev, ftl_init_fail_cb, fail_cb)) { + *ctx = dev->init_ctx; + if (_spdk_ftl_dev_free(dev, ftl_init_fail_cb, ctx, ctx->thread)) { SPDK_ERRLOG("Unable to free the device\n"); assert(0); } @@ -1108,7 +1112,7 @@ ftl_dev_init_io_channel(struct spdk_ftl_dev *dev) } int -spdk_ftl_dev_init(const struct spdk_ftl_dev_init_opts *_opts, spdk_ftl_init_fn cb, void *cb_arg) +spdk_ftl_dev_init(const struct spdk_ftl_dev_init_opts *_opts, spdk_ftl_init_fn cb_fn, void *cb_arg) { struct spdk_ftl_dev *dev; struct spdk_ftl_dev_init_opts opts = *_opts; @@ -1124,8 +1128,9 @@ spdk_ftl_dev_init(const struct spdk_ftl_dev_init_opts *_opts, spdk_ftl_init_fn c TAILQ_INIT(&dev->retry_queue); dev->conf = *opts.conf; - dev->init_cb = cb; - dev->init_arg = cb_arg; + dev->init_ctx.cb_fn = cb_fn; + dev->init_ctx.cb_arg = cb_arg; + dev->init_ctx.thread = spdk_get_thread(); dev->range = opts.range; dev->limit = SPDK_FTL_LIMIT_MAX; @@ -1287,25 +1292,33 @@ ftl_dev_free_sync(struct spdk_ftl_dev *dev) free(dev); } +static void +ftl_call_fini_complete_cb(void *_ctx) +{ + struct spdk_ftl_dev *dev = _ctx; + struct ftl_init_context ctx = dev->fini_ctx; + + ftl_dev_free_sync(dev); + + if (ctx.cb_fn != NULL) { + ctx.cb_fn(NULL, ctx.cb_arg, 0); + } +} + static int ftl_halt_poller(void *ctx) { struct spdk_ftl_dev *dev = ctx; - spdk_ftl_fn halt_cb = dev->halt_cb; - void *halt_arg = dev->halt_arg; if (!dev->core_thread.poller && !dev->read_thread.poller) { - spdk_poller_unregister(&dev->halt_poller); + spdk_poller_unregister(&dev->fini_ctx.poller); ftl_dev_free_thread(dev, &dev->read_thread); ftl_dev_free_thread(dev, &dev->core_thread); ftl_anm_unregister_device(dev); - ftl_dev_free_sync(dev); - if (halt_cb) { - halt_cb(halt_arg, 0); - } + spdk_thread_send_msg(dev->fini_ctx.thread, ftl_call_fini_complete_cb, dev); } return 0; @@ -1318,19 +1331,21 @@ ftl_add_halt_poller(void *ctx) _ftl_halt_defrag(dev); - assert(!dev->halt_poller); - dev->halt_poller = spdk_poller_register(ftl_halt_poller, dev, 100); + assert(!dev->fini_ctx.poller); + dev->fini_ctx.poller = spdk_poller_register(ftl_halt_poller, dev, 100); } -int -spdk_ftl_dev_free(struct spdk_ftl_dev *dev, spdk_ftl_fn cb, void *cb_arg) +static int +_spdk_ftl_dev_free(struct spdk_ftl_dev *dev, spdk_ftl_init_fn cb_fn, void *cb_arg, + struct spdk_thread *thread) { - if (dev->halt_cb) { + if (dev->fini_ctx.cb_fn != NULL) { return -EBUSY; } - dev->halt_cb = cb; - dev->halt_arg = cb_arg; + dev->fini_ctx.cb_fn = cb_fn; + dev->fini_ctx.cb_arg = cb_arg; + dev->fini_ctx.thread = thread; dev->halt = 1; ftl_rwb_disable_interleaving(dev->rwb); @@ -1339,6 +1354,12 @@ spdk_ftl_dev_free(struct spdk_ftl_dev *dev, spdk_ftl_fn cb, void *cb_arg) return 0; } +int +spdk_ftl_dev_free(struct spdk_ftl_dev *dev, spdk_ftl_init_fn cb_fn, void *cb_arg) +{ + return _spdk_ftl_dev_free(dev, cb_fn, cb_arg, spdk_get_thread()); +} + int spdk_ftl_module_init(const struct ftl_module_init_opts *opts, spdk_ftl_fn cb, void *cb_arg) {