bdev/nvme: Submit abort command for admin command on the current thread

Previously only a single thread could submit abort commands for admin
commands and it was the thread of the corresponding controller.

When we support multipath, we need to traverse the list of controllers
to which the target admin command is submitted. Threads of controllers
may be different.

On the other hand, the previous implementation made the I/O flow very
clean, but the I/O flow will not be clean if there are many controllers
and the subsystem does not have its thread.

This patch changes the policy so that any SPDK thread can submit abort
commands for admin commands.

Then when multipath is supported, we will be able to traverse the
list of controllers simply on the current thread to abort either I/O
command or admin command.

We already are able to submit any admin command on any thread anytime
including abort command. Hence this will not cause any issue.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ib69de33f2e84b03861c7d95ce060035bdb589e4b
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8121
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
This commit is contained in:
Shuhei Matsumoto 2021-05-31 16:11:13 +09:00 committed by Tomasz Zawadzki
parent a4f96d93a8
commit 5863f95ae4
2 changed files with 53 additions and 55 deletions

View File

@ -3326,34 +3326,6 @@ bdev_nvme_io_passthru_md(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair,
(uint32_t)nbytes, md_buf, bdev_nvme_queued_done, bio);
}
static void
bdev_nvme_abort_admin_cmd(void *ctx)
{
struct nvme_bdev_io *bio = ctx;
struct spdk_bdev_io *bdev_io = spdk_bdev_io_from_ctx(bio);
struct nvme_io_channel *nvme_ch;
struct nvme_bdev_io *bio_to_abort;
int rc;
nvme_ch = spdk_io_channel_get_ctx(spdk_bdev_io_get_io_channel(bdev_io));
bio_to_abort = (struct nvme_bdev_io *)bdev_io->u.abort.bio_to_abort->driver_ctx;
rc = spdk_nvme_ctrlr_cmd_abort_ext(nvme_ch->ctrlr->ctrlr,
NULL,
bio_to_abort,
bdev_nvme_abort_done, bio);
if (rc == -ENOENT) {
/* If no admin command was found in admin qpair, complete the abort
* request with failure.
*/
bio->cpl.cdw0 |= 1U;
bio->cpl.status.sc = SPDK_NVME_SC_SUCCESS;
bio->cpl.status.sct = SPDK_NVME_SCT_GENERIC;
spdk_thread_send_msg(bio->orig_thread, bdev_nvme_abort_completion, bio);
}
}
static int
bdev_nvme_abort(struct nvme_io_channel *nvme_ch, struct nvme_bdev_io *bio,
struct nvme_bdev_io *bio_to_abort)
@ -3368,11 +3340,22 @@ bdev_nvme_abort(struct nvme_io_channel *nvme_ch, struct nvme_bdev_io *bio,
bdev_nvme_abort_done, bio);
if (rc == -ENOENT) {
/* If no command was found in I/O qpair, the target command may be
* admin command. Only a single thread tries aborting admin command
* to clean I/O flow.
* admin command.
*/
spdk_thread_send_msg(nvme_ch->ctrlr->thread,
bdev_nvme_abort_admin_cmd, bio);
rc = spdk_nvme_ctrlr_cmd_abort_ext(nvme_ch->ctrlr->ctrlr,
NULL,
bio_to_abort,
bdev_nvme_abort_done, bio);
}
if (rc == -ENOENT) {
/* If no command was found, complete the abort request with failure. */
bio->cpl.cdw0 |= 1U;
bio->cpl.status.sc = SPDK_NVME_SC_SUCCESS;
bio->cpl.status.sct = SPDK_NVME_SCT_GENERIC;
bdev_nvme_abort_completion(bio);
rc = 0;
}

View File

@ -1995,13 +1995,13 @@ test_abort(void)
const char *attached_names[STRING_SIZE];
struct nvme_bdev *bdev;
struct spdk_bdev_io *write_io, *admin_io, *abort_io;
struct spdk_io_channel *ch;
struct nvme_io_channel *nvme_ch;
struct spdk_io_channel *ch1, *ch2;
struct nvme_io_channel *nvme_ch1;
int rc;
/* Create ctrlr on thread 1 and submit I/O and admin requests to be aborted on
* thread 0. Abort requests are submitted on thread 0. Aborting I/O requests are
* done on thread 0 but aborting admin requests are done on thread 1.
* thread 0. Aborting I/O requests are submitted on thread 0. Aborting admin requests
* are submitted on thread 1. Both should succeed.
*/
ut_init_trid(&trid);
@ -2027,8 +2027,6 @@ test_abort(void)
bdev = nvme_bdev_ctrlr->namespaces[0]->bdev;
SPDK_CU_ASSERT_FATAL(bdev != NULL);
set_thread(0);
write_io = calloc(1, sizeof(struct spdk_bdev_io) + sizeof(struct nvme_bdev_io));
SPDK_CU_ASSERT_FATAL(write_io != NULL);
write_io->bdev = &bdev->disk;
@ -2046,17 +2044,23 @@ test_abort(void)
abort_io->bdev = &bdev->disk;
abort_io->type = SPDK_BDEV_IO_TYPE_ABORT;
ch = spdk_get_io_channel(nvme_bdev_ctrlr);
SPDK_CU_ASSERT_FATAL(ch != NULL);
nvme_ch = spdk_io_channel_get_ctx(ch);
set_thread(0);
write_io->internal.ch = (struct spdk_bdev_channel *)ch;
admin_io->internal.ch = (struct spdk_bdev_channel *)ch;
abort_io->internal.ch = (struct spdk_bdev_channel *)ch;
ch1 = spdk_get_io_channel(nvme_bdev_ctrlr);
SPDK_CU_ASSERT_FATAL(ch1 != NULL);
nvme_ch1 = spdk_io_channel_get_ctx(ch1);
set_thread(1);
ch2 = spdk_get_io_channel(nvme_bdev_ctrlr);
SPDK_CU_ASSERT_FATAL(ch2 != NULL);
write_io->internal.ch = (struct spdk_bdev_channel *)ch1;
abort_io->internal.ch = (struct spdk_bdev_channel *)ch1;
/* Aborting the already completed request should fail. */
write_io->internal.in_submit_request = true;
bdev_nvme_submit_request(ch, write_io);
bdev_nvme_submit_request(ch1, write_io);
poll_threads();
CU_ASSERT(write_io->internal.in_submit_request == false);
@ -2064,7 +2068,7 @@ test_abort(void)
abort_io->u.abort.bio_to_abort = write_io;
abort_io->internal.in_submit_request = true;
bdev_nvme_submit_request(ch, abort_io);
bdev_nvme_submit_request(ch1, abort_io);
poll_threads();
@ -2072,8 +2076,11 @@ test_abort(void)
CU_ASSERT(abort_io->internal.status == SPDK_BDEV_IO_STATUS_FAILED);
CU_ASSERT(ctrlr->adminq.num_outstanding_reqs == 0);
admin_io->internal.ch = (struct spdk_bdev_channel *)ch1;
abort_io->internal.ch = (struct spdk_bdev_channel *)ch2;
admin_io->internal.in_submit_request = true;
bdev_nvme_submit_request(ch, admin_io);
bdev_nvme_submit_request(ch1, admin_io);
spdk_delay_us(10000);
poll_threads();
@ -2082,7 +2089,7 @@ test_abort(void)
abort_io->u.abort.bio_to_abort = admin_io;
abort_io->internal.in_submit_request = true;
bdev_nvme_submit_request(ch, abort_io);
bdev_nvme_submit_request(ch2, abort_io);
poll_threads();
@ -2092,15 +2099,16 @@ test_abort(void)
/* Aborting the write request should succeed. */
write_io->internal.in_submit_request = true;
bdev_nvme_submit_request(ch, write_io);
bdev_nvme_submit_request(ch1, write_io);
CU_ASSERT(write_io->internal.in_submit_request == true);
CU_ASSERT(nvme_ch->qpair->num_outstanding_reqs == 1);
CU_ASSERT(nvme_ch1->qpair->num_outstanding_reqs == 1);
abort_io->internal.ch = (struct spdk_bdev_channel *)ch1;
abort_io->u.abort.bio_to_abort = write_io;
abort_io->internal.in_submit_request = true;
bdev_nvme_submit_request(ch, abort_io);
bdev_nvme_submit_request(ch1, abort_io);
spdk_delay_us(10000);
poll_threads();
@ -2110,19 +2118,20 @@ test_abort(void)
CU_ASSERT(ctrlr->adminq.num_outstanding_reqs == 0);
CU_ASSERT(write_io->internal.in_submit_request == false);
CU_ASSERT(write_io->internal.status == SPDK_BDEV_IO_STATUS_ABORTED);
CU_ASSERT(nvme_ch->qpair->num_outstanding_reqs == 0);
CU_ASSERT(nvme_ch1->qpair->num_outstanding_reqs == 0);
/* Aborting the admin request should succeed. */
admin_io->internal.in_submit_request = true;
bdev_nvme_submit_request(ch, admin_io);
bdev_nvme_submit_request(ch1, admin_io);
CU_ASSERT(admin_io->internal.in_submit_request == true);
CU_ASSERT(ctrlr->adminq.num_outstanding_reqs == 1);
abort_io->internal.ch = (struct spdk_bdev_channel *)ch2;
abort_io->u.abort.bio_to_abort = admin_io;
abort_io->internal.in_submit_request = true;
bdev_nvme_submit_request(ch, abort_io);
bdev_nvme_submit_request(ch2, abort_io);
spdk_delay_us(10000);
poll_threads();
@ -2134,7 +2143,13 @@ test_abort(void)
CU_ASSERT(admin_io->internal.status == SPDK_BDEV_IO_STATUS_ABORTED);
CU_ASSERT(ctrlr->adminq.num_outstanding_reqs == 0);
spdk_put_io_channel(ch);
set_thread(0);
spdk_put_io_channel(ch1);
set_thread(1);
spdk_put_io_channel(ch2);
poll_threads();