From bc6a14636acc7d14f45ccf12cc2dec75ac76fd46 Mon Sep 17 00:00:00 2001 From: Konrad Sztyber Date: Fri, 9 Dec 2022 16:16:25 +0100 Subject: [PATCH] accel: use iovecs for fill operations Also, make it possible to remove copy operations following a fill operation if they're using the same buffers. Signed-off-by: Konrad Sztyber Change-Id: I7da195ce80650a02c5db99d9400ee692f797b1f8 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15940 Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Aleksey Marchuk --- lib/accel/accel.c | 45 +++++++++----------------- lib/accel/accel_sw.c | 19 +++++++++-- module/accel/dsa/accel_dsa.c | 6 ++-- module/accel/ioat/accel_ioat.c | 15 +++++++-- test/unit/lib/accel/accel.c/accel_ut.c | 39 ++++++++++++++++++++-- 5 files changed, 84 insertions(+), 40 deletions(-) diff --git a/lib/accel/accel.c b/lib/accel/accel.c index 522dbfac1..2908ac947 100644 --- a/lib/accel/accel.c +++ b/lib/accel/accel.c @@ -322,9 +322,11 @@ spdk_accel_submit_fill(struct spdk_io_channel *ch, void *dst, return -ENOMEM; } - accel_task->dst = dst; + accel_task->d.iovs = &accel_task->aux_iovs[SPDK_ACCEL_AUX_IOV_DST]; + accel_task->d.iovs[0].iov_base = dst; + accel_task->d.iovs[0].iov_len = nbytes; + accel_task->d.iovcnt = 1; memset(&accel_task->fill_pattern, fill, sizeof(uint64_t)); - accel_task->nbytes = nbytes; accel_task->flags = flags; accel_task->op_code = ACCEL_OPC_FILL; accel_task->src_domain = NULL; @@ -821,11 +823,13 @@ spdk_accel_append_fill(struct spdk_accel_sequence **pseq, struct spdk_io_channel memset(&task->fill_pattern, pattern, sizeof(uint64_t)); + task->d.iovs = &task->aux_iovs[SPDK_ACCEL_AUX_IOV_DST]; + task->d.iovs[0].iov_base = buf; + task->d.iovs[0].iov_len = len; + task->d.iovcnt = 1; task->src_domain = NULL; task->dst_domain = domain; task->dst_domain_ctx = domain_ctx; - task->dst = buf; - task->nbytes = len; task->flags = flags; task->op_code = ACCEL_OPC_FILL; @@ -994,29 +998,13 @@ accel_sequence_set_virtbuf(struct spdk_accel_sequence *seq, struct accel_buffer * in a sequence that were using it. */ TAILQ_FOREACH(task, &seq->tasks, seq_link) { - switch (task->op_code) { - case ACCEL_OPC_DECOMPRESS: - case ACCEL_OPC_COPY: - if (task->src_domain == g_accel_domain && task->src_domain_ctx == buf) { - accel_update_iovs(task->s.iovs, task->s.iovcnt, buf); - task->src_domain = NULL; - } - if (task->dst_domain == g_accel_domain && task->dst_domain_ctx == buf) { - accel_update_iovs(task->d.iovs, task->d.iovcnt, buf); - task->dst_domain = NULL; - } - break; - case ACCEL_OPC_FILL: - /* Fill should have src_domain cleared */ - assert(task->src_domain == NULL); - if (task->dst_domain == g_accel_domain && task->dst_domain_ctx == buf) { - accel_update_buf(&task->dst, buf); - task->dst_domain = NULL; - } - break; - default: - assert(0 && "bad opcode"); - break; + if (task->src_domain == g_accel_domain && task->src_domain_ctx == buf) { + accel_update_iovs(task->s.iovs, task->s.iovcnt, buf); + task->src_domain = NULL; + } + if (task->dst_domain == g_accel_domain && task->dst_domain_ctx == buf) { + accel_update_iovs(task->d.iovs, task->d.iovcnt, buf); + task->dst_domain = NULL; } } } @@ -1189,6 +1177,7 @@ accel_sequence_merge_tasks(struct spdk_accel_sequence *seq, struct spdk_accel_ta TAILQ_INSERT_TAIL(&seq->completed, task, seq_link); break; case ACCEL_OPC_DECOMPRESS: + case ACCEL_OPC_FILL: /* We can only merge tasks when one of them is a copy */ if (next->op_code != ACCEL_OPC_COPY) { break; @@ -1209,8 +1198,6 @@ accel_sequence_merge_tasks(struct spdk_accel_sequence *seq, struct spdk_accel_ta TAILQ_REMOVE(&seq->tasks, next, seq_link); TAILQ_INSERT_TAIL(&seq->completed, next, seq_link); break; - case ACCEL_OPC_FILL: - break; default: assert(0 && "bad opcode"); break; diff --git a/lib/accel/accel_sw.c b/lib/accel/accel_sw.c index 799d3c9e0..ab1728333 100644 --- a/lib/accel/accel_sw.c +++ b/lib/accel/accel_sw.c @@ -209,9 +209,19 @@ _sw_accel_compare(struct iovec *src_iovs, uint32_t src_iovcnt, return memcmp(src_iovs[0].iov_base, src2_iovs[0].iov_base, src_iovs[0].iov_len); } -static void -_sw_accel_fill(void *dst, uint8_t fill, size_t nbytes, int flags) +static int +_sw_accel_fill(struct iovec *iovs, uint32_t iovcnt, uint8_t fill, int flags) { + void *dst; + size_t nbytes; + + if (spdk_unlikely(iovcnt != 1)) { + return -EINVAL; + } + + dst = iovs[0].iov_base; + nbytes = iovs[0].iov_len; + if (flags & ACCEL_FLAG_PERSISTENT) { #ifdef SPDK_CONFIG_PMDK int is_pmem = pmem_is_pmem(dst, nbytes); @@ -229,6 +239,8 @@ _sw_accel_fill(void *dst, uint8_t fill, size_t nbytes, int flags) } else { memset(dst, fill, nbytes); } + + return 0; } static void @@ -516,7 +528,8 @@ sw_accel_submit_tasks(struct spdk_io_channel *ch, struct spdk_accel_task *accel_ case ACCEL_OPC_FILL: rc = _check_flags(accel_task->flags); if (rc == 0) { - _sw_accel_fill(accel_task->dst, accel_task->fill_pattern, accel_task->nbytes, accel_task->flags); + rc = _sw_accel_fill(accel_task->d.iovs, accel_task->d.iovcnt, + accel_task->fill_pattern, accel_task->flags); } break; case ACCEL_OPC_DUALCAST: diff --git a/module/accel/dsa/accel_dsa.c b/module/accel/dsa/accel_dsa.c index 040cc6470..b07a66a31 100644 --- a/module/accel/dsa/accel_dsa.c +++ b/module/accel/dsa/accel_dsa.c @@ -175,14 +175,12 @@ _process_single_task(struct spdk_io_channel *ch, struct spdk_accel_task *task) dsa_done, idxd_task); break; case ACCEL_OPC_FILL: - diov.iov_base = task->dst; - diov.iov_len = task->nbytes; if (task->flags & ACCEL_FLAG_PERSISTENT) { flags |= SPDK_IDXD_FLAG_PERSISTENT; flags |= SPDK_IDXD_FLAG_NONTEMPORAL; } - rc = spdk_idxd_submit_fill(chan->chan, &diov, 1, task->fill_pattern, flags, dsa_done, - idxd_task); + rc = spdk_idxd_submit_fill(chan->chan, task->d.iovs, task->d.iovcnt, + task->fill_pattern, flags, dsa_done, idxd_task); break; case ACCEL_OPC_CRC32C: if (task->s.iovcnt == 0) { diff --git a/module/accel/ioat/accel_ioat.c b/module/accel/ioat/accel_ioat.c index bbf43a15b..1bdcce012 100644 --- a/module/accel/ioat/accel_ioat.c +++ b/module/accel/ioat/accel_ioat.c @@ -129,6 +129,18 @@ ioat_supports_opcode(enum accel_opcode opc) } +static int +ioat_submit_fill(struct ioat_io_channel *ioat_ch, struct spdk_accel_task *task) +{ + if (spdk_unlikely(task->d.iovcnt != 1)) { + return -EINVAL; + } + + return spdk_ioat_build_fill(ioat_ch->ioat_ch, task, ioat_done, + task->d.iovs[0].iov_base, task->fill_pattern, + task->d.iovs[0].iov_len); +} + static int ioat_submit_copy(struct ioat_io_channel *ioat_ch, struct spdk_accel_task *task) { @@ -160,8 +172,7 @@ ioat_submit_tasks(struct spdk_io_channel *ch, struct spdk_accel_task *accel_task do { switch (accel_task->op_code) { case ACCEL_OPC_FILL: - rc = spdk_ioat_build_fill(ioat_ch->ioat_ch, accel_task, ioat_done, - accel_task->dst, accel_task->fill_pattern, accel_task->nbytes); + rc = ioat_submit_fill(ioat_ch, accel_task); break; case ACCEL_OPC_COPY: rc = ioat_submit_copy(ioat_ch, accel_task); diff --git a/test/unit/lib/accel/accel.c/accel_ut.c b/test/unit/lib/accel/accel.c/accel_ut.c index 2d58aabd0..133659a20 100644 --- a/test/unit/lib/accel/accel.c/accel_ut.c +++ b/test/unit/lib/accel/accel.c/accel_ut.c @@ -330,10 +330,8 @@ test_spdk_accel_submit_fill(void) /* accel submission OK. */ rc = spdk_accel_submit_fill(g_ch, dst, fill, nbytes, flags, NULL, cb_arg); CU_ASSERT(rc == 0); - CU_ASSERT(task.dst == dst); CU_ASSERT(task.fill_pattern == fill64); CU_ASSERT(task.op_code == ACCEL_OPC_FILL); - CU_ASSERT(task.nbytes == nbytes); CU_ASSERT(task.flags == 0); CU_ASSERT(memcmp(dst, src, TEST_SUBMIT_SIZE) == 0); @@ -1668,6 +1666,43 @@ test_sequence_copy_elision(void) CU_ASSERT_EQUAL(ut_seq.status, 0); CU_ASSERT_EQUAL(g_seq_operations[ACCEL_OPC_COPY].count, 1); + /* Check fill + copy */ + seq = NULL; + completed = 0; + g_seq_operations[ACCEL_OPC_COPY].count = 0; + g_seq_operations[ACCEL_OPC_FILL].count = 0; + g_seq_operations[ACCEL_OPC_COPY].src_iovs = NULL; + g_seq_operations[ACCEL_OPC_COPY].dst_iovs = NULL; + g_seq_operations[ACCEL_OPC_FILL].dst_iovcnt = 1; + g_seq_operations[ACCEL_OPC_FILL].dst_iovs = &exp_iovs[0]; + exp_iovs[0].iov_base = buf; + exp_iovs[0].iov_len = sizeof(buf); + + rc = spdk_accel_append_fill(&seq, ioch, tmp[0], sizeof(tmp[0]), NULL, NULL, 0xa5, 0, + ut_sequence_step_cb, &completed); + CU_ASSERT_EQUAL(rc, 0); + + dst_iovs[0].iov_base = buf; + dst_iovs[0].iov_len = sizeof(buf); + src_iovs[0].iov_base = tmp[0]; + src_iovs[0].iov_len = sizeof(tmp[0]); + rc = spdk_accel_append_copy(&seq, ioch, &dst_iovs[0], 1, NULL, NULL, + &src_iovs[0], 1, NULL, NULL, 0, + ut_sequence_step_cb, &completed); + CU_ASSERT_EQUAL(rc, 0); + + ut_seq.complete = false; + rc = spdk_accel_sequence_finish(seq, ut_sequence_complete_cb, &ut_seq); + CU_ASSERT_EQUAL(rc, 0); + + poll_threads(); + + CU_ASSERT_EQUAL(completed, 2); + CU_ASSERT(ut_seq.complete); + CU_ASSERT_EQUAL(ut_seq.status, 0); + CU_ASSERT_EQUAL(g_seq_operations[ACCEL_OPC_COPY].count, 0); + CU_ASSERT_EQUAL(g_seq_operations[ACCEL_OPC_FILL].count, 1); + /* Cleanup module pointers to make subsequent tests work correctly */ for (i = 0; i < ACCEL_OPC_LAST; ++i) { g_modules_opc[i] = modules[i];