bdev/malloc: use appends for read requests

This only changes the interface bdev_malloc uses for scheduling the copy
to appends, but it won't chain those copies to an existing sequence, as
bdev_malloc doesn't report support for accel sequences yet.  That will
be changed in one of the subsequent patches.

Signed-off-by: Konrad Sztyber <konrad.sztyber@intel.com>
Change-Id: I6db2c79b15cb96a1b07c6cf5514004c76b9d2e92
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17198
Community-CI: Mellanox Build Bot
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@nvidia.com>

module/bdev/malloc/bdev_malloc.c

@@ -10,6 +10,7 @@
 #include "spdk/endian.h"
 #include "spdk/env.h"
 #include "spdk/accel.h"
+#include "spdk/likely.h"
 #include "spdk/string.h"
 
 #include "spdk/log.h"
@@ -22,6 +23,7 @@ struct malloc_disk {
 };
 
 struct malloc_task {
+	struct iovec			iov;
 	int				num_outstanding;
 	enum spdk_bdev_io_status	status;
 	TAILQ_ENTRY(malloc_task)	tailq;
@@ -119,6 +121,7 @@ malloc_done(void *ref, int status)
 		}
 	}
 
+	assert(!bdev_io->u.bdev.accel_sequence || task->status == SPDK_BDEV_IO_STATUS_NOMEM);
 	spdk_bdev_io_complete(spdk_bdev_io_from_ctx(task), task->status);
 }
 
@@ -192,13 +195,37 @@ bdev_malloc_check_iov_len(struct iovec *iovs, int iovcnt, size_t nbytes)
 	return nbytes != 0;
 }
 
+static void
+malloc_sequence_fail(struct malloc_task *task, int status)
+{
+	struct spdk_bdev_io *bdev_io = spdk_bdev_io_from_ctx(task);
+
+	/* For ENOMEM, the IO will be retried by the bdev layer, so we don't abort the sequence */
+	if (status != -ENOMEM) {
+		spdk_accel_sequence_abort(bdev_io->u.bdev.accel_sequence);
+		bdev_io->u.bdev.accel_sequence = NULL;
+	}
+
+	malloc_done(task, status);
+}
+
+static void
+malloc_sequence_done(void *ctx, int status)
+{
+	struct malloc_task *task = ctx;
+	struct spdk_bdev_io *bdev_io = spdk_bdev_io_from_ctx(task);
+
+	bdev_io->u.bdev.accel_sequence = NULL;
+	/* Prevent bdev layer from retrying the request if the sequence failed with ENOMEM */
+	malloc_done(task, status != -ENOMEM ? status : -EFAULT);
+}
+
 static void
 bdev_malloc_readv(struct malloc_disk *mdisk, struct spdk_io_channel *ch,
 		  struct malloc_task *task, struct spdk_bdev_io *bdev_io)
 {
 	uint64_t len, offset, md_offset;
-	int i, res = 0;
+	int res = 0;
-	void *src;
 	size_t md_len;
 
 	len = bdev_io->u.bdev.num_blocks * bdev_io->bdev->blocklen;
@@ -212,26 +239,26 @@ bdev_malloc_readv(struct malloc_disk *mdisk, struct spdk_io_channel *ch,
 
 	task->status = SPDK_BDEV_IO_STATUS_SUCCESS;
 	task->num_outstanding = 0;
+	task->iov.iov_base = mdisk->malloc_buf + offset;
+	task->iov.iov_len = len;
 
 	SPDK_DEBUGLOG(bdev_malloc, "read %zu bytes from offset %#" PRIx64 ", iovcnt=%d\n",
 		      len, offset, bdev_io->u.bdev.iovcnt);
 
-	src = mdisk->malloc_buf + offset;
+	task->num_outstanding++;
-
+	res = spdk_accel_append_copy(&bdev_io->u.bdev.accel_sequence, ch,
-	for (i = 0; i < bdev_io->u.bdev.iovcnt; i++) {
+				     bdev_io->u.bdev.iovs, bdev_io->u.bdev.iovcnt,
-		task->num_outstanding++;
+				     bdev_io->u.bdev.memory_domain,
-		res = spdk_accel_submit_copy(ch, bdev_io->u.bdev.iovs[i].iov_base, src,
+				     bdev_io->u.bdev.memory_domain_ctx,
-					     bdev_io->u.bdev.iovs[i].iov_len, 0, malloc_done, task);
+				     &task->iov, 1, NULL, NULL, 0, NULL, NULL);
-
+	if (spdk_unlikely(res != 0)) {
-		if (res != 0) {
+		malloc_sequence_fail(task, res);
-			malloc_done(task, res);
+		return;
-			break;
-		}
-
-		src += bdev_io->u.bdev.iovs[i].iov_len;
-		len -= bdev_io->u.bdev.iovs[i].iov_len;
 	}
 
+	spdk_accel_sequence_reverse(bdev_io->u.bdev.accel_sequence);
+	spdk_accel_sequence_finish(bdev_io->u.bdev.accel_sequence, malloc_sequence_done, task);
+
 	if (bdev_io->u.bdev.md_buf == NULL) {
 		return;
 	}

test/bdev/chaining.sh

@@ -88,7 +88,7 @@ update_stats
 
 # Now read that 64K, verify the stats and check that it matches what was written
 spdk_dd --of "$output" --ib Nvme0n1 --bs $((64 * 1024)) --count 1
-(($(get_stat sequence_executed) == stats[sequence_executed] + 1))
+(($(get_stat sequence_executed) == stats[sequence_executed] + 2))
 (($(get_stat executed encrypt) == stats[encrypt_executed]))
 (($(get_stat executed decrypt) == stats[decrypt_executed] + 2))
 (($(get_stat executed copy) == stats[copy_executed] + 1))
@@ -107,7 +107,7 @@ update_stats
 # Check the reads
 : > "$output"
 spdk_dd --of "$output" --ib Nvme0n1 --bs 4096 --count 16
-(($(get_stat sequence_executed) == stats[sequence_executed] + 16))
+(($(get_stat sequence_executed) == stats[sequence_executed] + 32))
 (($(get_stat executed encrypt) == stats[encrypt_executed]))
 (($(get_stat executed decrypt) == stats[decrypt_executed] + 32))
 (($(get_stat executed copy) == stats[copy_executed] + 16))