From 085bfa0d13580b6e0c5cd71e7371d1975f9d2e2c Mon Sep 17 00:00:00 2001 From: Konrad Sztyber Date: Thu, 30 Jan 2020 12:44:56 +0100 Subject: [PATCH] lib/ftl: allocate separate memory for IO channels This is the first patch of a series that replaces single global write buffer with per-io_channel buffers. This change is intended to improve performance for multithreaded workloads. This patch changes the way the ftl_io_channels are allocated by only keeping an ftl_io_channel pointer inside spdk_io_channel's context. It allows for delaying IO channel destruction, which in turn allows the FTL to iterate over all exisiting IO channels without locking. Change-Id: I5e0cab8043a2b5f747e971dd3d65ed2546c8cf26 Signed-off-by: Konrad Sztyber Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/900 Tested-by: SPDK CI Jenkins Reviewed-by: Wojciech Malikowski Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto --- lib/ftl/ftl_core.c | 16 +++--- lib/ftl/ftl_core.h | 4 +- lib/ftl/ftl_init.c | 34 ++++++++++-- lib/ftl/ftl_io.c | 6 +-- lib/ftl/ftl_restore.c | 2 +- test/unit/lib/ftl/common/utils.c | 2 +- test/unit/lib/ftl/ftl_io.c/ftl_io_ut.c | 68 ++++++++++++++++++------ test/unit/lib/ftl/ftl_wptr/ftl_wptr_ut.c | 18 ++++++- 8 files changed, 115 insertions(+), 35 deletions(-) diff --git a/lib/ftl/ftl_core.c b/lib/ftl/ftl_core.c index 1e50a9957..dd4510cde 100644 --- a/lib/ftl/ftl_core.c +++ b/lib/ftl/ftl_core.c @@ -353,7 +353,7 @@ ftl_submit_erase(struct ftl_io *io) int rc = 0; size_t i; - ioch = spdk_io_channel_get_ctx(ftl_get_io_channel(dev)); + ioch = ftl_io_channel_get_ctx(ftl_get_io_channel(dev)); for (i = 0; i < io->num_blocks; ++i) { if (i != 0) { @@ -858,7 +858,7 @@ ftl_wptr_process_shutdown(struct ftl_wptr *wptr) static int ftl_shutdown_complete(struct spdk_ftl_dev *dev) { - struct ftl_io_channel *ioch = spdk_io_channel_get_ctx(dev->ioch); + struct ftl_io_channel *ioch = ftl_io_channel_get_ctx(dev->ioch); return !__atomic_load_n(&dev->num_inflight, __ATOMIC_SEQ_CST) && LIST_EMPTY(&dev->wptr_list) && TAILQ_EMPTY(&ioch->retry_queue); @@ -1018,7 +1018,7 @@ ftl_submit_read(struct ftl_io *io) struct ftl_addr addr; int rc = 0, num_blocks; - ioch = spdk_io_channel_get_ctx(io->ioch); + ioch = ftl_io_channel_get_ctx(io->ioch); assert(LIST_EMPTY(&io->children)); @@ -1221,7 +1221,7 @@ ftl_submit_nv_cache(void *ctx) struct ftl_io_channel *ioch; int rc; - ioch = spdk_io_channel_get_ctx(io->ioch); + ioch = ftl_io_channel_get_ctx(io->ioch); thread = spdk_io_channel_get_thread(io->ioch); rc = spdk_bdev_write_blocks_with_md(nv_cache->bdev_desc, ioch->cache_ioch, @@ -1328,7 +1328,7 @@ ftl_nv_cache_write_header(struct ftl_nv_cache *nv_cache, bool shutdown, struct ftl_io_channel *ioch; bdev = spdk_bdev_desc_get_bdev(nv_cache->bdev_desc); - ioch = spdk_io_channel_get_ctx(ftl_get_io_channel(dev)); + ioch = ftl_io_channel_get_ctx(ftl_get_io_channel(dev)); memset(hdr, 0, spdk_bdev_get_block_size(bdev)); @@ -1350,7 +1350,7 @@ ftl_nv_cache_scrub(struct ftl_nv_cache *nv_cache, spdk_bdev_io_completion_cb cb_ struct ftl_io_channel *ioch; struct spdk_bdev *bdev; - ioch = spdk_io_channel_get_ctx(ftl_get_io_channel(dev)); + ioch = ftl_io_channel_get_ctx(ftl_get_io_channel(dev)); bdev = spdk_bdev_desc_get_bdev(nv_cache->bdev_desc); return spdk_bdev_write_zeroes_blocks(nv_cache->bdev_desc, ioch->cache_ioch, 1, @@ -1573,7 +1573,7 @@ ftl_submit_child_write(struct ftl_wptr *wptr, struct ftl_io *io) struct ftl_addr addr; int rc; - ioch = spdk_io_channel_get_ctx(io->ioch); + ioch = ftl_io_channel_get_ctx(io->ioch); if (spdk_likely(!wptr->direct_mode)) { addr = wptr->addr; @@ -1808,7 +1808,7 @@ ftl_rwb_fill(struct ftl_io *io) struct ftl_addr addr = { .cached = 1 }; int flags = ftl_rwb_flags_from_io(io); - ioch = spdk_io_channel_get_ctx(io->ioch); + ioch = ftl_io_channel_get_ctx(io->ioch); while (io->pos < io->num_blocks) { if (ftl_io_current_lba(io) == FTL_LBA_INVALID) { diff --git a/lib/ftl/ftl_core.h b/lib/ftl/ftl_core.h index 9bb044fde..fe113447d 100644 --- a/lib/ftl/ftl_core.h +++ b/lib/ftl/ftl_core.h @@ -253,9 +253,9 @@ int ftl_nv_cache_scrub(struct ftl_nv_cache *nv_cache, spdk_bdev_io_completion_cb void *cb_arg); void ftl_get_media_events(struct spdk_ftl_dev *dev); int ftl_io_channel_poll(void *arg); +struct spdk_io_channel *ftl_get_io_channel(const struct spdk_ftl_dev *dev); +struct ftl_io_channel *ftl_io_channel_get_ctx(struct spdk_io_channel *ioch); -struct spdk_io_channel * -ftl_get_io_channel(const struct spdk_ftl_dev *dev); #define ftl_to_addr(address) \ (struct ftl_addr) { .offset = (uint64_t)(address) } diff --git a/lib/ftl/ftl_init.c b/lib/ftl/ftl_init.c index 725c5786c..1a97273d2 100644 --- a/lib/ftl/ftl_init.c +++ b/lib/ftl/ftl_init.c @@ -945,17 +945,37 @@ ftl_dev_init_zones(struct ftl_dev_init_ctx *init_ctx) return 0; } +struct _ftl_io_channel { + struct ftl_io_channel *ioch; +}; + +struct ftl_io_channel * +ftl_io_channel_get_ctx(struct spdk_io_channel *ioch) +{ + struct _ftl_io_channel *_ioch = spdk_io_channel_get_ctx(ioch); + + return _ioch->ioch; +} + static int ftl_io_channel_create_cb(void *io_device, void *ctx) { struct spdk_ftl_dev *dev = io_device; - struct ftl_io_channel *ioch = ctx; + struct _ftl_io_channel *_ioch = ctx; + struct ftl_io_channel *ioch; char mempool_name[32]; int rc; + ioch = calloc(1, sizeof(*ioch)); + if (ioch == NULL) { + SPDK_ERRLOG("Failed to allocate IO channel\n"); + return -1; + } + rc = snprintf(mempool_name, sizeof(mempool_name), "ftl_io_%p", ioch); if (rc < 0 || rc >= (int)sizeof(mempool_name)) { SPDK_ERRLOG("Failed to create IO channel pool name\n"); + free(ioch); return -1; } @@ -969,6 +989,7 @@ ftl_io_channel_create_cb(void *io_device, void *ctx) SPDK_ENV_SOCKET_ID_ANY); if (!ioch->io_pool) { SPDK_ERRLOG("Failed to create IO channel's IO pool\n"); + free(ioch); return -1; } @@ -994,6 +1015,7 @@ ftl_io_channel_create_cb(void *io_device, void *ctx) goto fail_poller; } + _ioch->ioch = ioch; return 0; fail_poller: @@ -1004,6 +1026,8 @@ fail_cache: spdk_put_io_channel(ioch->base_ioch); fail_ioch: spdk_mempool_free(ioch->io_pool); + free(ioch); + return -1; } @@ -1011,24 +1035,26 @@ fail_ioch: static void ftl_io_channel_destroy_cb(void *io_device, void *ctx) { - struct ftl_io_channel *ioch = ctx; + struct _ftl_io_channel *_ioch = ctx; + struct ftl_io_channel *ioch = _ioch->ioch; spdk_poller_unregister(&ioch->poller); spdk_mempool_free(ioch->io_pool); - spdk_put_io_channel(ioch->base_ioch); if (ioch->cache_ioch) { spdk_put_io_channel(ioch->cache_ioch); } + + free(ioch); } static int ftl_dev_init_io_channel(struct spdk_ftl_dev *dev) { spdk_io_device_register(dev, ftl_io_channel_create_cb, ftl_io_channel_destroy_cb, - sizeof(struct ftl_io_channel), + sizeof(struct _ftl_io_channel), NULL); return 0; diff --git a/lib/ftl/ftl_io.c b/lib/ftl/ftl_io.c index e14b47efb..a8b4b9a89 100644 --- a/lib/ftl/ftl_io.c +++ b/lib/ftl/ftl_io.c @@ -353,7 +353,7 @@ struct ftl_io * ftl_io_user_init(struct spdk_io_channel *_ioch, uint64_t lba, size_t num_blocks, struct iovec *iov, size_t iov_cnt, spdk_ftl_fn cb_fn, void *cb_ctx, int type) { - struct ftl_io_channel *ioch = spdk_io_channel_get_ctx(_ioch); + struct ftl_io_channel *ioch = ftl_io_channel_get_ctx(_ioch); struct spdk_ftl_dev *dev = ioch->dev; struct ftl_io *io; @@ -388,7 +388,7 @@ _ftl_io_free(struct ftl_io *io) SPDK_ERRLOG("pthread_spin_destroy failed\n"); } - ioch = spdk_io_channel_get_ctx(io->ioch); + ioch = ftl_io_channel_get_ctx(io->ioch); spdk_mempool_put(ioch->io_pool, io); } @@ -473,7 +473,7 @@ struct ftl_io * ftl_io_alloc(struct spdk_io_channel *ch) { struct ftl_io *io; - struct ftl_io_channel *ioch = spdk_io_channel_get_ctx(ch); + struct ftl_io_channel *ioch = ftl_io_channel_get_ctx(ch); io = spdk_mempool_get(ioch->io_pool); if (!io) { diff --git a/lib/ftl/ftl_restore.c b/lib/ftl/ftl_restore.c index f7eb3f084..ae8bad353 100644 --- a/lib/ftl/ftl_restore.c +++ b/lib/ftl/ftl_restore.c @@ -1032,7 +1032,7 @@ ftl_restore_nv_cache(struct ftl_restore *restore, ftl_restore_fn cb, void *cb_ar size_t alignment; int rc, i; - ioch = spdk_io_channel_get_ctx(ftl_get_io_channel(dev)); + ioch = ftl_io_channel_get_ctx(ftl_get_io_channel(dev)); bdev = spdk_bdev_desc_get_bdev(nv_cache->bdev_desc); alignment = spdk_max(spdk_bdev_get_buf_align(bdev), sizeof(uint64_t)); diff --git a/test/unit/lib/ftl/common/utils.c b/test/unit/lib/ftl/common/utils.c index 145dd646f..723693218 100644 --- a/test/unit/lib/ftl/common/utils.c +++ b/test/unit/lib/ftl/common/utils.c @@ -77,7 +77,7 @@ test_init_ftl_dev(const struct base_bdev_geometry *geo) dev->core_thread = spdk_thread_create("unit_test_thread", NULL); spdk_set_thread(dev->core_thread); dev->ioch = calloc(1, sizeof(*dev->ioch) - + sizeof(struct ftl_io_channel)); + + sizeof(struct ftl_io_channel *)); dev->num_bands = geo->blockcnt / (geo->zone_size * geo->optimal_open_zones); dev->bands = calloc(dev->num_bands, sizeof(*dev->bands)); SPDK_CU_ASSERT_FATAL(dev->bands != NULL); diff --git a/test/unit/lib/ftl/ftl_io.c/ftl_io_ut.c b/test/unit/lib/ftl/ftl_io.c/ftl_io_ut.c index 0f62c015e..c7f2e2bc4 100644 --- a/test/unit/lib/ftl/ftl_io.c/ftl_io_ut.c +++ b/test/unit/lib/ftl/ftl_io.c/ftl_io_ut.c @@ -34,33 +34,66 @@ #include "spdk/stdinc.h" #include "spdk_cunit.h" -#include "common/lib/test_env.c" +#include "common/lib/ut_multithread.c" #include "ftl/ftl_io.c" +#include "ftl/ftl_init.c" DEFINE_STUB(ftl_trace_alloc_id, uint64_t, (struct spdk_ftl_dev *dev), 0); DEFINE_STUB(spdk_bdev_io_get_append_location, uint64_t, (struct spdk_bdev_io *bdev_io), 0); DEFINE_STUB_V(ftl_band_acquire_lba_map, (struct ftl_band *band)); DEFINE_STUB_V(ftl_band_release_lba_map, (struct ftl_band *band)); +DEFINE_STUB(ftl_io_channel_poll, int, (void *ioch), 0); + +struct spdk_io_channel * +spdk_bdev_get_io_channel(struct spdk_bdev_desc *bdev_desc) +{ + return spdk_get_io_channel(bdev_desc); +} + +static int +channel_create_cb(void *io_device, void *ctx) +{ + return 0; +} + +static void +channel_destroy_cb(void *io_device, void *ctx) +{} static struct spdk_ftl_dev * -setup_device(void) +setup_device(uint32_t num_threads) { struct spdk_ftl_dev *dev; + struct _ftl_io_channel *_ioch; struct ftl_io_channel *ioch; + int rc; + + allocate_threads(num_threads); + set_thread(0); dev = calloc(1, sizeof(*dev)); SPDK_CU_ASSERT_FATAL(dev != NULL); - dev->ioch = calloc(1, sizeof(*ioch) + sizeof(struct spdk_io_channel)); + + dev->ioch = calloc(1, sizeof(*_ioch) + sizeof(struct spdk_io_channel)); SPDK_CU_ASSERT_FATAL(dev->ioch != NULL); - ioch = spdk_io_channel_get_ctx(dev->ioch); + _ioch = (struct _ftl_io_channel *)(dev->ioch + 1); + ioch = _ioch->ioch = calloc(1, sizeof(*ioch)); + SPDK_CU_ASSERT_FATAL(ioch != NULL); ioch->elem_size = sizeof(struct ftl_md_io); ioch->io_pool = spdk_mempool_create("io-pool", 4096, ioch->elem_size, 0, 0); SPDK_CU_ASSERT_FATAL(ioch->io_pool != NULL); + dev->conf = g_default_conf; + dev->base_bdev_desc = (struct spdk_bdev_desc *)0xdeadbeef; + spdk_io_device_register(dev->base_bdev_desc, channel_create_cb, channel_destroy_cb, 0, NULL); + + rc = ftl_dev_init_io_channel(dev); + CU_ASSERT_EQUAL(rc, 0); + return dev; } @@ -69,8 +102,13 @@ free_device(struct spdk_ftl_dev *dev) { struct ftl_io_channel *ioch; - ioch = spdk_io_channel_get_ctx(dev->ioch); + ioch = ftl_io_channel_get_ctx(dev->ioch); spdk_mempool_free(ioch->io_pool); + free(ioch); + + spdk_io_device_unregister(dev, NULL); + spdk_io_device_unregister(dev->base_bdev_desc, NULL); + free_threads(); free(dev->ioch); free(dev); @@ -111,8 +149,8 @@ test_completion(void) int req, status = 0; size_t pool_size; - dev = setup_device(); - ioch = spdk_io_channel_get_ctx(dev->ioch); + dev = setup_device(1); + ioch = ftl_io_channel_get_ctx(dev->ioch); pool_size = spdk_mempool_count(ioch->io_pool); io = alloc_io(dev, io_complete_cb, &status); @@ -153,8 +191,8 @@ test_alloc_free(void) int parent_status = -1; size_t pool_size; - dev = setup_device(); - ioch = spdk_io_channel_get_ctx(dev->ioch); + dev = setup_device(1); + ioch = ftl_io_channel_get_ctx(dev->ioch); pool_size = spdk_mempool_count(ioch->io_pool); parent = alloc_io(dev, io_complete_cb, &parent_status); @@ -199,8 +237,8 @@ test_child_requests(void) int status[MAX_CHILDREN + 1], i; size_t pool_size; - dev = setup_device(); - ioch = spdk_io_channel_get_ctx(dev->ioch); + dev = setup_device(1); + ioch = ftl_io_channel_get_ctx(dev->ioch); pool_size = spdk_mempool_count(ioch->io_pool); /* Verify correct behaviour when children finish first */ @@ -299,8 +337,8 @@ test_child_status(void) int parent_status, child_status[2]; size_t pool_size, i; - dev = setup_device(); - ioch = spdk_io_channel_get_ctx(dev->ioch); + dev = setup_device(1); + ioch = ftl_io_channel_get_ctx(dev->ioch); pool_size = spdk_mempool_count(ioch->io_pool); /* Verify the first error is returned by the parent */ @@ -386,8 +424,8 @@ test_multi_generation(void) size_t pool_size; int i, j; - dev = setup_device(); - ioch = spdk_io_channel_get_ctx(dev->ioch); + dev = setup_device(1); + ioch = ftl_io_channel_get_ctx(dev->ioch); pool_size = spdk_mempool_count(ioch->io_pool); /* Verify correct behaviour when children finish first */ diff --git a/test/unit/lib/ftl/ftl_wptr/ftl_wptr_ut.c b/test/unit/lib/ftl/ftl_wptr/ftl_wptr_ut.c index 4091afd17..ea055bdfb 100644 --- a/test/unit/lib/ftl/ftl_wptr/ftl_wptr_ut.c +++ b/test/unit/lib/ftl/ftl_wptr/ftl_wptr_ut.c @@ -38,6 +38,7 @@ #include "ftl/ftl_core.c" #include "ftl/ftl_band.c" +#include "ftl/ftl_init.c" #include "../common/utils.c" struct base_bdev_geometry g_geo = { @@ -73,6 +74,12 @@ DEFINE_STUB(spdk_bdev_zone_management, int, (struct spdk_bdev_desc *desc, spdk_bdev_io_completion_cb cb, void *cb_arg), 0); DEFINE_STUB(spdk_bdev_io_get_append_location, uint64_t, (struct spdk_bdev_io *bdev_io), 0); +struct spdk_io_channel * +spdk_bdev_get_io_channel(struct spdk_bdev_desc *bdev_desc) +{ + return spdk_get_io_channel(bdev_desc); +} + struct ftl_io * ftl_io_erase_init(struct ftl_band *band, size_t num_blocks, ftl_io_fn cb) { @@ -105,8 +112,9 @@ ftl_io_complete(struct ftl_io *io) static void setup_wptr_test(struct spdk_ftl_dev **dev, const struct base_bdev_geometry *geo) { - size_t i; struct spdk_ftl_dev *t_dev; + struct _ftl_io_channel *_ioch; + size_t i; t_dev = test_init_ftl_dev(geo); for (i = 0; i < ftl_get_num_bands(t_dev); ++i) { @@ -115,12 +123,17 @@ setup_wptr_test(struct spdk_ftl_dev **dev, const struct base_bdev_geometry *geo) ftl_band_set_state(&t_dev->bands[i], FTL_BAND_STATE_FREE); } + _ioch = (struct _ftl_io_channel *)(t_dev->ioch + 1); + _ioch->ioch = calloc(1, sizeof(*_ioch->ioch)); + SPDK_CU_ASSERT_FATAL(_ioch->ioch != NULL); + *dev = t_dev; } static void cleanup_wptr_test(struct spdk_ftl_dev *dev) { + struct _ftl_io_channel *_ioch; size_t i; for (i = 0; i < ftl_get_num_bands(dev); ++i) { @@ -128,6 +141,9 @@ cleanup_wptr_test(struct spdk_ftl_dev *dev) test_free_ftl_band(&dev->bands[i]); } + _ioch = (struct _ftl_io_channel *)(dev->ioch + 1); + free(_ioch->ioch); + test_free_ftl_dev(dev); }