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 <konrad.sztyber@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/900
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Wojciech Malikowski <wojciech.malikowski@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>

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) {

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) }

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;

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) {

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));
 

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);

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();
+	dev = setup_device(1);
-	ioch = spdk_io_channel_get_ctx(dev->ioch);
+	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();
+	dev = setup_device(1);
-	ioch = spdk_io_channel_get_ctx(dev->ioch);
+	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();
+	dev = setup_device(1);
-	ioch = spdk_io_channel_get_ctx(dev->ioch);
+	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();
+	dev = setup_device(1);
-	ioch = spdk_io_channel_get_ctx(dev->ioch);
+	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();
+	dev = setup_device(1);
-	ioch = spdk_io_channel_get_ctx(dev->ioch);
+	ioch = ftl_io_channel_get_ctx(dev->ioch);
 	pool_size = spdk_mempool_count(ioch->io_pool);
 
 	/* Verify correct behaviour when children finish first */

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);
 }