lib/ftl: Stop using ftl_io for internal relocation logic

Added ftl_reloc_move structure to internally track
relocation requests. This patch remove unnecessary
dependency between relocation module and ftl_io.

Change-Id: I9dd1bca6039b7c8961cb22a9f85ea1c5737a0a6f
Signed-off-by: Wojciech Malikowski <wojciech.malikowski@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/454006
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>

lib/ftl/ftl_io.c

@@ -304,6 +304,14 @@ ftl_io_init_internal(const struct ftl_io_init_opts *opts)
 		return NULL;
 	}
 
+	if (opts->flags & FTL_IO_VECTOR_LBA) {
+		io->lba.vector = calloc(io->lbk_cnt, sizeof(uint64_t));
+		if (!io->lba.vector) {
+			ftl_io_free(io);
+			return NULL;
+		}
+	}
+
 	return io;
 }
 
@@ -395,6 +403,10 @@ _ftl_io_free(struct ftl_io *io)
 
 	assert(LIST_EMPTY(&io->children));
 
+	if (io->flags & FTL_IO_VECTOR_LBA) {
+		free(io->lba.vector);
+	}
+
 	if (pthread_spin_destroy(&io->lock)) {
 		SPDK_ERRLOG("pthread_spin_destroy failed\n");
 	}

lib/ftl/ftl_reloc.c

@@ -34,6 +34,7 @@
 #include "spdk/likely.h"
 #include "spdk_internal/log.h"
 #include "spdk/ftl.h"
+
 #include "ftl_reloc.h"
 #include "ftl_core.h"
 #include "ftl_io.h"
@@ -41,10 +42,27 @@
 #include "ftl_band.h"
 #include "ftl_debug.h"
 
+/* Maximum active reloc moves */
+#define FTL_RELOC_MAX_MOVES 256
+
 struct ftl_reloc;
 struct ftl_band_reloc;
 
-typedef int (*ftl_reloc_fn)(struct ftl_band_reloc *, struct ftl_io *);
+struct ftl_reloc_move {
+	struct ftl_band_reloc			*breloc;
+
+	/* Start ppa */
+	struct ftl_ppa				ppa;
+
+	/* Number of logical blocks */
+	size_t					lbk_cnt;
+
+	/* Data buffer */
+	void					*data;
+
+	/* IO associated with move */
+	struct ftl_io				*io;
+};
 
 struct ftl_band_reloc {
 	struct ftl_reloc			*parent;
@@ -70,6 +88,9 @@ struct ftl_band_reloc {
 		size_t				chk_current;
 	} iter;
 
+	/* Pool of move objects */
+	struct ftl_reloc_move			*moves;
+
 	/* Free IO queue */
 	struct spdk_ring			*free_queue;
 
@@ -89,9 +110,6 @@ struct ftl_reloc {
 	/* Maximum number of IOs per band */
 	size_t					max_qdepth;
 
-	/* IO buffer */
-	struct ftl_io				**io;
-
 	/* Maximum number of active band relocates */
 	size_t					max_active;
 
@@ -114,11 +132,7 @@ struct ftl_reloc {
 	TAILQ_HEAD(, ftl_band_reloc)		pending_queue;
 };
 
-static struct ftl_band_reloc *
+typedef int (*ftl_reloc_fn)(struct ftl_band_reloc *, struct ftl_reloc_move *);
-ftl_io_get_band_reloc(struct ftl_io *io)
-{
-	return &io->dev->reloc->brelocs[io->band->id];
-}
 
 static size_t
 ftl_reloc_iter_chk_offset(struct ftl_band_reloc *breloc)
@@ -151,21 +165,21 @@ ftl_reloc_clr_lbk(struct ftl_band_reloc *breloc, size_t lbkoff)
 static void
 _ftl_reloc_prep(struct ftl_band_reloc *breloc)
 {
-	struct ftl_io *io;
 	struct ftl_reloc *reloc = breloc->parent;
-	struct spdk_ftl_dev *dev = reloc->dev;
+	struct ftl_reloc_move *move;
 	size_t i;
 
 	for (i = 0; i < reloc->max_qdepth; ++i) {
-		io = ftl_io_alloc(dev->ioch);
+		move = &breloc->moves[i];
-		spdk_ring_enqueue(breloc->free_queue, (void **)&io, 1, NULL);
+		spdk_ring_enqueue(breloc->free_queue, (void **)&move, 1, NULL);
 	}
 }
 
 static void
 ftl_reloc_read_lba_map_cb(struct ftl_io *io, void *arg, int status)
 {
-	struct ftl_band_reloc *breloc = arg;
+	struct ftl_reloc_move *move = arg;
+	struct ftl_band_reloc *breloc = move->breloc;
 
 	assert(status == 0);
 	_ftl_reloc_prep(breloc);
@@ -176,13 +190,16 @@ ftl_reloc_read_lba_map(struct ftl_band_reloc *breloc)
 {
 	struct ftl_band *band = breloc->band;
 	struct spdk_ftl_dev *dev = band->dev;
+	struct ftl_reloc_move *move = &breloc->moves[0];
+
+	move->breloc = breloc;
 
 	if (ftl_band_alloc_lba_map(band)) {
 		assert(false);
 	}
 
 	return ftl_band_read_lba_map(band, 0, ftl_num_band_lbks(dev),
-				     ftl_reloc_read_lba_map_cb, breloc);
+				     ftl_reloc_read_lba_map_cb, move);
 }
 
 static void
@@ -204,18 +221,20 @@ ftl_reloc_prep(struct ftl_band_reloc *breloc)
 }
 
 static void
-ftl_reloc_free_io(struct ftl_band_reloc *breloc, struct ftl_io *io)
+ftl_reloc_free_move(struct ftl_band_reloc *breloc, struct ftl_reloc_move *move)
 {
-	spdk_dma_free(io->iov[0].iov_base);
+	assert(move);
-	free(io->lba.vector);
+	spdk_dma_free(move->data);
-	spdk_ring_enqueue(breloc->free_queue, (void **)&io, 1, NULL);
+	memset(move, 0, sizeof(*move));
+	spdk_ring_enqueue(breloc->free_queue, (void **)&move, 1, NULL);
 }
 
 static void
 ftl_reloc_write_cb(struct ftl_io *io, void *arg, int status)
 {
-	struct ftl_ppa ppa = io->ppa;
+	struct ftl_reloc_move *move = arg;
-	struct ftl_band_reloc *breloc = ftl_io_get_band_reloc(io);
+	struct ftl_ppa ppa = move->ppa;
+	struct ftl_band_reloc *breloc = move->breloc;
 	size_t i;
 
 	if (status) {
@@ -224,19 +243,20 @@ ftl_reloc_write_cb(struct ftl_io *io, void *arg, int status)
 		return;
 	}
 
-	for (i = 0; i < io->lbk_cnt; ++i) {
+	for (i = 0; i < move->lbk_cnt; ++i) {
-		ppa.lbk = io->ppa.lbk + i;
+		ppa.lbk = move->ppa.lbk + i;
 		size_t lbkoff = ftl_band_lbkoff_from_ppa(breloc->band, ppa);
 		ftl_reloc_clr_lbk(breloc, lbkoff);
 	}
 
-	ftl_reloc_free_io(breloc, io);
+	ftl_reloc_free_move(breloc, move);
 }
 
 static void
 ftl_reloc_read_cb(struct ftl_io *io, void *arg, int status)
 {
-	struct ftl_band_reloc *breloc = ftl_io_get_band_reloc(io);
+	struct ftl_reloc_move *move = arg;
+	struct ftl_band_reloc *breloc = move->breloc;
 
 	/* TODO: We should handle fail on relocation read. We need to inform */
 	/* user that this group of blocks is bad (update l2p with bad block address and */
@@ -247,8 +267,8 @@ ftl_reloc_read_cb(struct ftl_io *io, void *arg, int status)
 		return;
 	}
 
-	io->flags &= ~FTL_IO_INITIALIZED;
+	move->io = NULL;
-	spdk_ring_enqueue(breloc->write_queue, (void **)&io, 1, NULL);
+	spdk_ring_enqueue(breloc->write_queue, (void **)&move, 1, NULL);
 }
 
 static void
@@ -361,8 +381,7 @@ ftl_reloc_next_lbks(struct ftl_band_reloc *breloc, struct ftl_ppa *ppa)
 	struct spdk_ftl_dev *dev = breloc->parent->dev;
 
 	for (i = 0; i < ftl_dev_num_punits(dev); ++i) {
-		lbk_cnt = ftl_reloc_find_valid_lbks(breloc,
+		lbk_cnt = ftl_reloc_find_valid_lbks(breloc, breloc->parent->xfer_size, ppa);
-						    breloc->parent->xfer_size, ppa);
 		ftl_reloc_iter_next_chk(breloc);
 
 		if (lbk_cnt || ftl_reloc_iter_done(breloc)) {
@@ -373,22 +392,34 @@ ftl_reloc_next_lbks(struct ftl_band_reloc *breloc, struct ftl_ppa *ppa)
 	return lbk_cnt;
 }
 
-static void
+static struct ftl_io *
-ftl_reloc_io_reinit(struct ftl_io *io, struct ftl_band_reloc *breloc,
+ftl_reloc_io_init(struct ftl_band_reloc *breloc, struct ftl_reloc_move *move,
 		  ftl_io_fn fn, enum ftl_io_type io_type, int flags)
 {
-	size_t i;
+	size_t lbkoff, i;
-	uint64_t lbkoff;
+	struct ftl_ppa ppa = move->ppa;
-	struct ftl_ppa ppa = io->ppa;
+	struct ftl_io *io = NULL;
+	struct ftl_io_init_opts opts = {
+		.dev		= breloc->parent->dev,
+		.band		= breloc->band,
+		.size		= sizeof(*io),
+		.flags		= flags | FTL_IO_INTERNAL | FTL_IO_PPA_MODE,
+		.type		= io_type,
+		.lbk_cnt	= move->lbk_cnt,
+		.data		= move->data,
+		.cb_fn		= fn,
+	};
 
-	ftl_io_reinit(io, fn, NULL, flags | FTL_IO_INTERNAL, io_type);
+	io = ftl_io_init_internal(&opts);
+	if (!io) {
+		return NULL;
+	}
 
-	io->ppa = ppa;
+	io->cb_ctx = move;
-	io->band = breloc->band;
+	io->ppa = move->ppa;
-	io->lba.vector = calloc(io->lbk_cnt, sizeof(uint64_t));
 
-	for (i = 0; i < io->lbk_cnt; ++i) {
+	if (flags & FTL_IO_VECTOR_LBA) {
-		ppa.lbk = io->ppa.lbk + i;
+		for (i = 0; i < io->lbk_cnt; ++i, ++ppa.lbk) {
 			lbkoff = ftl_band_lbkoff_from_ppa(breloc->band, ppa);
 
 			if (!ftl_band_lbkoff_valid(breloc->band, lbkoff)) {
@@ -398,74 +429,56 @@ ftl_reloc_io_reinit(struct ftl_io *io, struct ftl_band_reloc *breloc,
 
 			io->lba.vector[i] = breloc->band->lba_map.map[lbkoff];
 		}
+	}
 
 	ftl_trace_lba_io_init(io->dev, io);
+
+	return io;
 }
 
 static int
-ftl_reloc_write(struct ftl_band_reloc *breloc, struct ftl_io *io)
+ftl_reloc_write(struct ftl_band_reloc *breloc, struct ftl_reloc_move *move)
 {
-	if (!(io->flags & FTL_IO_INITIALIZED)) {
+	if (spdk_likely(!move->io)) {
-		ftl_reloc_io_reinit(io, breloc, ftl_reloc_write_cb,
+		move->io = ftl_reloc_io_init(breloc, move, ftl_reloc_write_cb,
-				    FTL_IO_WRITE,
+					     FTL_IO_WRITE, FTL_IO_WEAK | FTL_IO_VECTOR_LBA);
-				    FTL_IO_KEEP_ALIVE | FTL_IO_WEAK | FTL_IO_VECTOR_LBA);
+		if (!move->io) {
+			ftl_reloc_free_move(breloc, move);
+			return -ENOMEM;
+		}
 	}
 
-	ftl_io_write(io);
+	ftl_io_write(move->io);
 	return 0;
 }
 
 static int
-ftl_reloc_io_init(struct ftl_band_reloc *breloc, struct ftl_io *io,
+ftl_reloc_read(struct ftl_band_reloc *breloc, struct ftl_reloc_move *move)
-		  struct ftl_ppa ppa, size_t num_lbks)
-{
-	struct ftl_io_init_opts opts = {
-		.dev		= breloc->parent->dev,
-		.io		= io,
-		.rwb_batch	= NULL,
-		.band		= breloc->band,
-		.size		= sizeof(*io),
-		.flags		= FTL_IO_KEEP_ALIVE | FTL_IO_INTERNAL | FTL_IO_PPA_MODE,
-		.type		= FTL_IO_READ,
-		.lbk_cnt	= num_lbks,
-		.cb_fn		= ftl_reloc_read_cb,
-	};
-
-	opts.data = spdk_dma_malloc(PAGE_SIZE * num_lbks, PAGE_SIZE, NULL);
-	if (!opts.data) {
-		return -1;
-	}
-
-	io = ftl_io_init_internal(&opts);
-	if (!io) {
-		spdk_dma_free(opts.data);
-		return -1;
-	}
-
-	io->ppa = ppa;
-
-	return 0;
-}
-
-static int
-ftl_reloc_read(struct ftl_band_reloc *breloc, struct ftl_io *io)
 {
 	struct ftl_ppa ppa;
-	size_t num_lbks;
 
-	num_lbks = ftl_reloc_next_lbks(breloc, &ppa);
+	move->lbk_cnt = ftl_reloc_next_lbks(breloc, &ppa);
+	move->breloc = breloc;
+	move->ppa = ppa;
 
-	if (!num_lbks) {
+	if (!move->lbk_cnt) {
-		spdk_ring_enqueue(breloc->free_queue, (void **)&io, 1, NULL);
+		spdk_ring_enqueue(breloc->free_queue, (void **)&move, 1, NULL);
 		return 0;
 	}
 
-	if (ftl_reloc_io_init(breloc, io, ppa, num_lbks)) {
+	move->data = spdk_dma_malloc(PAGE_SIZE * move->lbk_cnt, PAGE_SIZE, NULL);
+	if (!move->data) {
+		return -1;
+	}
+
+	move->io = ftl_reloc_io_init(breloc, move, ftl_reloc_read_cb, FTL_IO_READ, 0);
+	if (!move->io) {
+		ftl_reloc_free_move(breloc, move);
 		SPDK_ERRLOG("Failed to initialize io for relocation.");
 		return -1;
 	}
 
-	ftl_io_read(io);
+	ftl_io_read(move->io);
 	return 0;
 }
 
@@ -473,13 +486,14 @@ static void
 ftl_reloc_process_queue(struct ftl_band_reloc *breloc, struct spdk_ring *queue,
 			ftl_reloc_fn fn)
 {
-	size_t i, num_ios;
+	size_t i, num_moves;
+	struct ftl_reloc_move *moves[FTL_RELOC_MAX_MOVES];
 	struct ftl_reloc *reloc = breloc->parent;
 
-	num_ios = spdk_ring_dequeue(queue, (void **)reloc->io, reloc->max_qdepth);
+	num_moves = spdk_ring_dequeue(queue, (void **)moves, reloc->max_qdepth);
 
-	for (i = 0; i < num_ios; ++i) {
+	for (i = 0; i < num_moves; ++i) {
-		if (fn(breloc, reloc->io[i])) {
+		if (fn(breloc, moves[i])) {
 			SPDK_ERRLOG("Reloc queue processing failed\n");
 			assert(false);
 		}
@@ -507,16 +521,12 @@ ftl_reloc_done(struct ftl_band_reloc *breloc)
 }
 
 static void
-ftl_reloc_release_io(struct ftl_band_reloc *breloc)
+ftl_reloc_release_moves(struct ftl_band_reloc *breloc)
 {
 	struct ftl_reloc *reloc = breloc->parent;
-	size_t i, num_ios;
+	struct ftl_reloc_move *moves[FTL_RELOC_MAX_MOVES];
 
-	num_ios = spdk_ring_dequeue(breloc->free_queue, (void **)reloc->io, reloc->max_qdepth);
+	spdk_ring_dequeue(breloc->free_queue, (void **)moves, reloc->max_qdepth);
-
-	for (i = 0; i < num_ios; ++i) {
-		ftl_io_free(reloc->io[i]);
-	}
 }
 
 static void
@@ -532,7 +542,7 @@ ftl_reloc_release(struct ftl_band_reloc *breloc)
 		TAILQ_REMOVE(&reloc->active_queue, breloc, entry);
 	}
 
-	ftl_reloc_release_io(breloc);
+	ftl_reloc_release_moves(breloc);
 	ftl_reloc_iter_reset(breloc);
 	ftl_band_release_lba_map(band);
 
@@ -597,6 +607,11 @@ ftl_band_reloc_init(struct ftl_reloc *reloc, struct ftl_band_reloc *breloc,
 		return -1;
 	}
 
+	breloc->moves = calloc(reloc->max_qdepth, sizeof(*breloc->moves));
+	if (!breloc->moves) {
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -604,8 +619,8 @@ static void
 ftl_band_reloc_free(struct ftl_band_reloc *breloc)
 {
 	struct ftl_reloc *reloc;
-	struct ftl_io *io;
+	struct ftl_reloc_move *moves[FTL_RELOC_MAX_MOVES] = {};
-	size_t i, num_ios;
+	size_t i, num_moves;
 
 	if (!breloc) {
 		return;
@@ -613,22 +628,22 @@ ftl_band_reloc_free(struct ftl_band_reloc *breloc)
 
 	reloc = breloc->parent;
 
+	/* Drain write queue if there is active band relocation during shutdown */
 	if (breloc->active) {
-		num_ios = spdk_ring_dequeue(breloc->write_queue, (void **)reloc->io, reloc->max_qdepth);
+		assert(reloc->halt);
-		for (i = 0; i < num_ios; ++i) {
+		num_moves = spdk_ring_dequeue(breloc->write_queue, (void **)&moves, reloc->max_qdepth);
-			io = reloc->io[i];
+		for (i = 0; i < num_moves; ++i) {
-			if (io->flags & FTL_IO_INITIALIZED) {
+			ftl_reloc_free_move(breloc, moves[i]);
-				ftl_reloc_free_io(breloc, io);
-			}
 		}
 
-		ftl_reloc_release_io(breloc);
+		ftl_reloc_release_moves(breloc);
 	}
 
 	spdk_ring_free(breloc->free_queue);
 	spdk_ring_free(breloc->write_queue);
 	spdk_bit_array_free(&breloc->reloc_map);
 	free(breloc->iter.chk_offset);
+	free(breloc->moves);
 }
 
 static void
@@ -661,13 +676,12 @@ ftl_reloc_init(struct spdk_ftl_dev *dev)
 	reloc->max_active = dev->conf.max_active_relocs;
 	reloc->xfer_size = dev->xfer_size;
 
-	reloc->brelocs =  calloc(ftl_dev_num_bands(dev), sizeof(*reloc->brelocs));
+	if (reloc->max_qdepth > FTL_RELOC_MAX_MOVES) {
-	if (!reloc->brelocs) {
 		goto error;
 	}
 
-	reloc->io = calloc(reloc->max_qdepth, sizeof(*reloc->io));
+	reloc->brelocs = calloc(ftl_dev_num_bands(dev), sizeof(*reloc->brelocs));
-	if (!reloc->io) {
+	if (!reloc->brelocs) {
 		goto error;
 	}
 
@@ -679,7 +693,7 @@ ftl_reloc_init(struct spdk_ftl_dev *dev)
 
 	rc = snprintf(pool_name, sizeof(pool_name), "%s-%s", dev->name, "reloc-io-pool");
 	if (rc < 0 || rc >= POOL_NAME_LEN) {
-		return NULL;
+		goto error;
 	}
 
 	TAILQ_INIT(&reloc->pending_queue);
@@ -706,7 +720,6 @@ ftl_reloc_free(struct ftl_reloc *reloc)
 	}
 
 	free(reloc->brelocs);
-	free(reloc->io);
 	free(reloc);
 }
 

test/unit/lib/ftl/ftl_reloc.c/ftl_reloc_ut.c

@@ -59,6 +59,7 @@ static struct spdk_ftl_punit_range g_range = {
 DEFINE_STUB(ftl_dev_tail_md_disk_size, size_t, (const struct spdk_ftl_dev *dev), 1);
 DEFINE_STUB_V(ftl_band_set_state, (struct ftl_band *band, enum ftl_band_state state));
 DEFINE_STUB_V(ftl_trace_lba_io_init, (struct spdk_ftl_dev *dev, const struct ftl_io *io));
+DEFINE_STUB_V(ftl_free_io, (struct ftl_io *io));
 
 int
 ftl_band_alloc_lba_map(struct ftl_band *band)
@@ -128,12 +129,15 @@ void
 ftl_io_read(struct ftl_io *io)
 {
 	io->cb_fn(io, io->cb_ctx, 0);
+	free(io);
 }
 
 void
 ftl_io_write(struct ftl_io *io)
 {
 	io->cb_fn(io, io->cb_ctx, 0);
+	free(io->lba.vector);
+	free(io);
 }
 
 struct ftl_io *
@@ -154,6 +158,12 @@ ftl_io_init_internal(const struct ftl_io_init_opts *opts)
 	io->cb_ctx = io;
 	io->lbk_cnt = opts->lbk_cnt;
 	io->iov[0].iov_base = opts->data;
+
+	if (opts->flags & FTL_IO_VECTOR_LBA) {
+		io->lba.vector = calloc(io->lbk_cnt, sizeof(uint64_t));
+		SPDK_CU_ASSERT_FATAL(io->lba.vector != NULL);
+	}
+
 	return io;
 }
 
@@ -165,12 +175,6 @@ ftl_io_alloc(struct spdk_io_channel *ch)
 	return malloc(io_size);
 }
 
-void
-ftl_io_free(struct ftl_io *io)
-{
-	free(io);
-}
-
 void
 ftl_io_reinit(struct ftl_io *io, ftl_io_fn fn, void *ctx, int flags, int type)
 {
@@ -211,7 +215,12 @@ cleanup_reloc(struct spdk_ftl_dev *dev, struct ftl_reloc *reloc)
 {
 	size_t i;
 
+	for (i = 0; i < ftl_dev_num_bands(reloc->dev); ++i) {
+		SPDK_CU_ASSERT_FATAL(reloc->brelocs[i].active == false);
+	}
+
 	ftl_reloc_free(reloc);
+
 	for (i = 0; i < ftl_dev_num_bands(dev); ++i) {
 		test_free_ftl_band(&dev->bands[i]);
 	}
@@ -457,6 +466,7 @@ test_reloc_empty_band(void)
 	ftl_reloc(reloc);
 
 	CU_ASSERT_EQUAL(breloc->num_lbks, 0);
+	SPDK_CU_ASSERT_FATAL(breloc->moves != NULL);
 
 	cleanup_reloc(dev, reloc);
 }