Commit 06955fd3 authored by Jim Harris's avatar Jim Harris
Browse files

nvme: free request in nvme_complete_request 



All existing callers of nvme_complete_request also immediately call
nvme_free_request as well. So just free the request inside of
nvme_complete_request.

nvme_tcp_req_complete() was the exception here. Commit 21d15cb0
tried to support immediately reusing a completed req, by calling
nvme_complete_request *before* nvme_free_request. This is non-intuitive,
and actually breaks an upcoming patch needed to fix a different issue.
This patch removes this non-intuitive ordering.

Fixes commit 21d15cb0 ("nvme: cache values in nvme_tcp_req_complete")

Signed-off-by: default avatarJim Harris <jim.harris@samsung.com>
Change-Id: Iee5ce0ebd03ce0eef29399a45359ffd6f56c44e7
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/20648


Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: default avatarVasuki Manikarnike <vasuki.manikarnike@hpe.com>
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarBen Walker <ben@nvidia.com>
parent 30a4eea3

lib/nvme/nvme_ctrlr.c

+0 −1
Original line number Diff line number Diff line
@@ -1615,7 +1615,6 @@ nvme_ctrlr_abort_queued_aborts(struct spdk_nvme_ctrlr *ctrlr) 
		ctrlr->outstanding_aborts++;



		nvme_complete_request(req->cb_fn, req->cb_arg, req->qpair, req, &cpl);

		nvme_free_request(req);

	}

}

lib/nvme/nvme_ctrlr_cmd.c

+0 −3
Original line number Diff line number Diff line
@@ -567,7 +567,6 @@ nvme_ctrlr_retry_queued_abort(struct spdk_nvme_ctrlr *ctrlr) 
			next->cpl.status.sc = SPDK_NVME_SC_INTERNAL_DEVICE_ERROR;

			next->cpl.status.dnr = 1;

			nvme_complete_request(next->cb_fn, next->cb_arg, next->qpair, next, &next->cpl);

			nvme_free_request(next);

		} else {

			/* If the first abort succeeds, stop iterating. */

			break;
@@ -659,7 +658,6 @@ nvme_complete_abort_request(void *ctx, const struct spdk_nvme_cpl *cpl) 
	if (parent->num_children == 0) {

		nvme_complete_request(parent->cb_fn, parent->cb_arg, parent->qpair,

				      parent, &parent->parent_status);

		nvme_free_request(parent);

	}

}
@@ -780,7 +778,6 @@ spdk_nvme_ctrlr_cmd_abort_ext(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_qp 
				 */

				nvme_complete_request(parent->cb_fn, parent->cb_arg, parent->qpair,

						      parent, &parent->parent_status);

				nvme_free_request(parent);

			} else {

				/* There was no queued request to abort. */

				rc = -ENOENT;

lib/nvme/nvme_internal.h

+2 −1
Original line number Diff line number Diff line
@@ -1391,6 +1391,8 @@ nvme_complete_request(spdk_nvme_cmd_cb cb_fn, void *cb_arg, struct spdk_nvme_qpa 
		}

	}



	nvme_free_request(req);



	if (cb_fn) {

		cb_fn(cb_arg, cpl);

	}
@@ -1438,7 +1440,6 @@ nvme_cb_complete_child(void *child_arg, const struct spdk_nvme_cpl *cpl) 
	if (parent->num_children == 0) {

		nvme_complete_request(parent->cb_fn, parent->cb_arg, parent->qpair,

				      parent, &parent->parent_status);

		nvme_free_request(parent);

	}

}

lib/nvme/nvme_pcie_common.c

+0 −2
Original line number Diff line number Diff line
@@ -328,7 +328,6 @@ nvme_pcie_qpair_complete_pending_admin_request(struct spdk_nvme_qpair *qpair) 
		assert(req->pid == pid);



		nvme_complete_request(req->cb_fn, req->cb_arg, qpair, req, &req->cpl);

		nvme_free_request(req);

	}

}
@@ -707,7 +706,6 @@ nvme_pcie_qpair_complete_tracker(struct spdk_nvme_qpair *qpair, struct nvme_trac 
			nvme_pcie_qpair_insert_pending_admin_request(qpair, req, cpl);

		} else {

			nvme_complete_request(tr->cb_fn, tr->cb_arg, qpair, req, cpl);

			nvme_free_request(req);

		}



		tr->req = NULL;

lib/nvme/nvme_qpair.c

+0 −1
Original line number Diff line number Diff line
@@ -561,7 +561,6 @@ nvme_qpair_manual_complete_request(struct spdk_nvme_qpair *qpair, 
	}



	nvme_complete_request(req->cb_fn, req->cb_arg, qpair, req, &cpl);

	nvme_free_request(req);

}



void