From 560164d8da8fd61edebb3e86867e111c0f872e07 Mon Sep 17 00:00:00 2001 From: Konrad Sztyber Date: Thu, 9 Mar 2023 11:45:11 +0100 Subject: [PATCH] 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 Change-Id: I6db2c79b15cb96a1b07c6cf5514004c76b9d2e92 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17198 Community-CI: Mellanox Build Bot Reviewed-by: Changpeng Liu Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Aleksey Marchuk --- module/bdev/malloc/bdev_malloc.c | 59 +++++++++++++++++++++++--------- test/bdev/chaining.sh | 4 +-- 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/module/bdev/malloc/bdev_malloc.c b/module/bdev/malloc/bdev_malloc.c index 632c74a9e..add33bc75 100644 --- a/module/bdev/malloc/bdev_malloc.c +++ b/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; - void *src; + int res = 0; 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; - - for (i = 0; i < bdev_io->u.bdev.iovcnt; i++) { - task->num_outstanding++; - res = spdk_accel_submit_copy(ch, bdev_io->u.bdev.iovs[i].iov_base, src, - bdev_io->u.bdev.iovs[i].iov_len, 0, malloc_done, task); - - if (res != 0) { - malloc_done(task, res); - break; - } - - src += bdev_io->u.bdev.iovs[i].iov_len; - len -= bdev_io->u.bdev.iovs[i].iov_len; + task->num_outstanding++; + res = spdk_accel_append_copy(&bdev_io->u.bdev.accel_sequence, ch, + bdev_io->u.bdev.iovs, bdev_io->u.bdev.iovcnt, + bdev_io->u.bdev.memory_domain, + bdev_io->u.bdev.memory_domain_ctx, + &task->iov, 1, NULL, NULL, 0, NULL, NULL); + if (spdk_unlikely(res != 0)) { + malloc_sequence_fail(task, res); + return; } + 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; } diff --git a/test/bdev/chaining.sh b/test/bdev/chaining.sh index 48fd1850d..1313a2ada 100755 --- a/test/bdev/chaining.sh +++ b/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))