diff --git a/lib/nvme/nvme_qpair.c b/lib/nvme/nvme_qpair.c index 2e662fcfd..97eb23de9 100644 --- a/lib/nvme/nvme_qpair.c +++ b/lib/nvme/nvme_qpair.c @@ -546,11 +546,6 @@ nvme_qpair_submit_request(struct spdk_nvme_qpair *qpair, struct nvme_request *re struct spdk_nvme_ctrlr *ctrlr = qpair->ctrlr; bool child_req_failed = false; - if (ctrlr->is_failed) { - nvme_free_request(req); - return -ENXIO; - } - nvme_qpair_check_enabled(qpair); if (req->num_children) { @@ -559,17 +554,28 @@ nvme_qpair_submit_request(struct spdk_nvme_qpair *qpair, struct nvme_request *re * request itself, since the parent is the original unsplit request. */ TAILQ_FOREACH_SAFE(child_req, &req->children, child_tailq, tmp) { - if (!child_req_failed) { + if (spdk_likely(!child_req_failed)) { rc = nvme_qpair_submit_request(qpair, child_req); - if (rc != 0) { + if (spdk_unlikely(rc != 0)) { child_req_failed = true; } } else { /* free remaining child_reqs since one child_req fails */ nvme_request_remove_child(req, child_req); + nvme_request_free_children(child_req); nvme_free_request(child_req); } } + if (spdk_unlikely(child_req_failed)) { + /* part of children requests have been submitted, + * return success for this case. + */ + if (req->num_children) { + return 0; + } + goto error; + } + return rc; } @@ -593,6 +599,11 @@ nvme_qpair_submit_request(struct spdk_nvme_qpair *qpair, struct nvme_request *re } } + if (spdk_unlikely(ctrlr->is_failed)) { + rc = -ENXIO; + goto error; + } + /* assign submit_tick before submitting req to specific transport */ if (spdk_unlikely(ctrlr->timeout_enabled)) { if (req->submit_tick == 0) { /* req submitted for the first time */ @@ -604,12 +615,12 @@ nvme_qpair_submit_request(struct spdk_nvme_qpair *qpair, struct nvme_request *re } if (spdk_likely(qpair->is_enabled)) { - return nvme_transport_qpair_submit_request(qpair, req); + rc = nvme_transport_qpair_submit_request(qpair, req); } else if (nvme_qpair_is_admin_queue(qpair) && req->cmd.opc == SPDK_NVME_OPC_FABRIC) { /* Always allow fabrics commands through on the admin qpair - these get * the controller out of reset state. */ - return nvme_transport_qpair_submit_request(qpair, req); + rc = nvme_transport_qpair_submit_request(qpair, req); } else { /* The controller is being reset - queue this request and * submit it later when the reset is completed. @@ -617,6 +628,18 @@ nvme_qpair_submit_request(struct spdk_nvme_qpair *qpair, struct nvme_request *re STAILQ_INSERT_TAIL(&qpair->queued_req, req, stailq); return 0; } + + if (spdk_likely(rc == 0)) { + return 0; + } + +error: + if (req->parent != NULL) { + nvme_request_remove_child(req->parent, req); + } + nvme_free_request(req); + + return rc; } void diff --git a/test/unit/lib/nvme/nvme_qpair.c/nvme_qpair_ut.c b/test/unit/lib/nvme/nvme_qpair.c/nvme_qpair_ut.c index 16e0d974c..b2995f33e 100644 --- a/test/unit/lib/nvme/nvme_qpair.c/nvme_qpair_ut.c +++ b/test/unit/lib/nvme/nvme_qpair.c/nvme_qpair_ut.c @@ -368,6 +368,76 @@ test_nvme_qpair_add_cmd_error_injection(void) cleanup_submit_request_test(&qpair); } +static void +test_nvme_qpair_submit_request(void) +{ + int rc; + struct spdk_nvme_qpair qpair = {}; + struct spdk_nvme_ctrlr ctrlr = {}; + struct nvme_request *req, *req1, *req2, *req3, *req2_1, *req2_2, *req2_3; + + prepare_submit_request_test(&qpair, &ctrlr); + + /* + * Build a request chain like the following: + * req + * | + * --------------- + * | | | + * req1 req2 req3 + * | + * --------------- + * | | | + * req2_1 req2_2 req2_3 + */ + req = nvme_allocate_request_null(&qpair, NULL, NULL); + CU_ASSERT(req != NULL); + TAILQ_INIT(&req->children); + + req1 = nvme_allocate_request_null(&qpair, NULL, NULL); + CU_ASSERT(req1 != NULL); + req->num_children++; + TAILQ_INSERT_TAIL(&req->children, req1, child_tailq); + req1->parent = req; + + req2 = nvme_allocate_request_null(&qpair, NULL, NULL); + CU_ASSERT(req2 != NULL); + TAILQ_INIT(&req2->children); + req->num_children++; + TAILQ_INSERT_TAIL(&req->children, req2, child_tailq); + req2->parent = req; + + req3 = nvme_allocate_request_null(&qpair, NULL, NULL); + CU_ASSERT(req3 != NULL); + req->num_children++; + TAILQ_INSERT_TAIL(&req->children, req3, child_tailq); + req3->parent = req; + + req2_1 = nvme_allocate_request_null(&qpair, NULL, NULL); + CU_ASSERT(req2_1 != NULL); + req2->num_children++; + TAILQ_INSERT_TAIL(&req2->children, req2_1, child_tailq); + req2_1->parent = req2; + + req2_2 = nvme_allocate_request_null(&qpair, NULL, NULL); + CU_ASSERT(req2_2 != NULL); + req2->num_children++; + TAILQ_INSERT_TAIL(&req2->children, req2_2, child_tailq); + req2_2->parent = req2; + + req2_3 = nvme_allocate_request_null(&qpair, NULL, NULL); + CU_ASSERT(req2_3 != NULL); + req2->num_children++; + TAILQ_INSERT_TAIL(&req2->children, req2_3, child_tailq); + req2_3->parent = req2; + + ctrlr.is_failed = true; + rc = nvme_qpair_submit_request(&qpair, req); + SPDK_CU_ASSERT_FATAL(rc == -ENXIO); + + cleanup_submit_request_test(&qpair); +} + int main(int argc, char **argv) { CU_pSuite suite = NULL; @@ -394,6 +464,8 @@ int main(int argc, char **argv) #endif || CU_add_test(suite, "spdk_nvme_qpair_add_cmd_error_injection", test_nvme_qpair_add_cmd_error_injection) == NULL + || CU_add_test(suite, "spdk_nvme_qpair_submit_request", + test_nvme_qpair_submit_request) == NULL ) { CU_cleanup_registry(); return CU_get_error();