Commit ff20880b authored by Jacek Kalwas's avatar Jacek Kalwas Committed by Tomasz Zawadzki
Browse files

nvme: simplify wait for completion poll interface 



There is no need to pass the mutex explicitly, since the decision
to lock and the particular mutex can be obtained from the qpair.

Additionally, introduce a fix by using the controller’s utility to get
the mutex, which allows tracking of the lock depth.

Change-Id: I485931ed31bb750b4f4ac72a05390df783d018ab
Signed-off-by: default avatarJacek Kalwas <jacek.kalwas@nutanix.com>
Reviewed-on: https://review.spdk.io/c/spdk/spdk/+/26519


Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK Automated Test System <spdkbot@gmail.com>
Reviewed-by: default avatarTomasz Zawadzki <tomasz@tzawadzki.com>
Reviewed-by: default avatarJim Harris <jim.harris@nvidia.com>
parent 4b8f8e91

lib/nvme/nvme.c

+7 −8
Original line number Diff line number Diff line
@@ -237,14 +237,13 @@ dummy_disconnected_qpair_cb(struct spdk_nvme_qpair *qpair, void *poll_group_ctx) 
}



int

nvme_wait_for_completion_robust_lock_timeout_poll(struct spdk_nvme_qpair *qpair,

		struct nvme_completion_poll_status *status,

		pthread_mutex_t *robust_mutex)

nvme_wait_for_completion_poll(struct spdk_nvme_qpair *qpair,

			      struct nvme_completion_poll_status *status)

{

	int rc;



	if (robust_mutex) {

		nvme_robust_mutex_lock(robust_mutex);

	if (nvme_qpair_is_admin_queue(qpair)) {

		nvme_ctrlr_lock(qpair->ctrlr);

	}



	if (qpair->poll_group) {
@@ -254,8 +253,8 @@ nvme_wait_for_completion_robust_lock_timeout_poll(struct spdk_nvme_qpair *qpair, 
		rc = spdk_nvme_qpair_process_completions(qpair, 0);

	}



	if (robust_mutex) {

		nvme_robust_mutex_unlock(robust_mutex);

	if (nvme_qpair_is_admin_queue(qpair)) {

		nvme_ctrlr_unlock(qpair->ctrlr);

	}



	if (rc < 0) {
@@ -326,7 +325,7 @@ nvme_wait_for_adminq_completion(struct spdk_nvme_ctrlr *ctrlr, 


	status->cpl.status_raw = 0;

	do {

		rc = nvme_wait_for_completion_robust_lock_timeout_poll(ctrlr->adminq, status, &ctrlr->ctrlr_lock);

		rc = nvme_wait_for_completion_poll(ctrlr->adminq, status);

	} while (rc == -EAGAIN);



	return rc;

lib/nvme/nvme_auth.c

+5 −5
Original line number Diff line number Diff line
@@ -1101,7 +1101,7 @@ nvme_fabric_qpair_authenticate_poll(struct spdk_nvme_qpair *qpair) 
			nvme_auth_set_state(qpair, NVME_QPAIR_AUTH_STATE_AWAIT_NEGOTIATE);

			break;

		case NVME_QPAIR_AUTH_STATE_AWAIT_NEGOTIATE:

			rc = nvme_wait_for_completion_robust_lock_timeout_poll(qpair, status, NULL);

			rc = nvme_wait_for_completion_poll(qpair, status);

			if (rc != 0) {

				if (rc != -EAGAIN) {

					nvme_auth_print_cpl(qpair, "AUTH_negotiate");
@@ -1120,7 +1120,7 @@ nvme_fabric_qpair_authenticate_poll(struct spdk_nvme_qpair *qpair) 
			nvme_auth_set_state(qpair, NVME_QPAIR_AUTH_STATE_AWAIT_CHALLENGE);

			break;

		case NVME_QPAIR_AUTH_STATE_AWAIT_CHALLENGE:

			rc = nvme_wait_for_completion_robust_lock_timeout_poll(qpair, status, NULL);

			rc = nvme_wait_for_completion_poll(qpair, status);

			if (rc != 0) {

				if (rc != -EAGAIN) {

					nvme_auth_print_cpl(qpair, "DH-HMAC-CHAP_challenge");
@@ -1142,7 +1142,7 @@ nvme_fabric_qpair_authenticate_poll(struct spdk_nvme_qpair *qpair) 
			nvme_auth_set_state(qpair, NVME_QPAIR_AUTH_STATE_AWAIT_REPLY);

			break;

		case NVME_QPAIR_AUTH_STATE_AWAIT_REPLY:

			rc = nvme_wait_for_completion_robust_lock_timeout_poll(qpair, status, NULL);

			rc = nvme_wait_for_completion_poll(qpair, status);

			if (rc != 0) {

				if (rc != -EAGAIN) {

					nvme_auth_print_cpl(qpair, "DH-HMAC-CHAP_reply");
@@ -1161,7 +1161,7 @@ nvme_fabric_qpair_authenticate_poll(struct spdk_nvme_qpair *qpair) 
			nvme_auth_set_state(qpair, NVME_QPAIR_AUTH_STATE_AWAIT_SUCCESS1);

			break;

		case NVME_QPAIR_AUTH_STATE_AWAIT_SUCCESS1:

			rc = nvme_wait_for_completion_robust_lock_timeout_poll(qpair, status, NULL);

			rc = nvme_wait_for_completion_poll(qpair, status);

			if (rc != 0) {

				if (rc != -EAGAIN) {

					nvme_auth_print_cpl(qpair, "DH-HMAC-CHAP_success1");
@@ -1189,7 +1189,7 @@ nvme_fabric_qpair_authenticate_poll(struct spdk_nvme_qpair *qpair) 
			break;

		case NVME_QPAIR_AUTH_STATE_AWAIT_SUCCESS2:

		case NVME_QPAIR_AUTH_STATE_AWAIT_FAILURE2:

			rc = nvme_wait_for_completion_robust_lock_timeout_poll(qpair, status, NULL);

			rc = nvme_wait_for_completion_poll(qpair, status);

			if (rc == -EAGAIN) {

				break;

			}

lib/nvme/nvme_fabric.c

+1 −1
Original line number Diff line number Diff line
@@ -591,7 +591,7 @@ nvme_fabric_qpair_connect_poll(struct spdk_nvme_qpair *qpair) 
	ctrlr = qpair->ctrlr;

	status = qpair->fabric_poll_status;



	if (nvme_wait_for_completion_robust_lock_timeout_poll(qpair, status, NULL) == -EAGAIN) {

	if (nvme_wait_for_completion_poll(qpair, status) == -EAGAIN) {

		return -EAGAIN;

	}

lib/nvme/nvme_internal.h

+2 −3
Original line number Diff line number Diff line
@@ -1331,9 +1331,8 @@ int nvme_ctrlr_cmd_sanitize(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, 
void	nvme_completion_poll_cb(void *arg, const struct spdk_nvme_cpl *cpl);

int	nvme_wait_for_adminq_completion(struct spdk_nvme_ctrlr *ctrlr,

					struct nvme_completion_poll_status *status);

int	nvme_wait_for_completion_robust_lock_timeout_poll(struct spdk_nvme_qpair *qpair,

		struct nvme_completion_poll_status *status,

		pthread_mutex_t *robust_mutex);

int	nvme_wait_for_completion_poll(struct spdk_nvme_qpair *qpair,

				      struct nvme_completion_poll_status *status);



struct spdk_nvme_ctrlr_process *nvme_ctrlr_get_process(struct spdk_nvme_ctrlr *ctrlr,

		pid_t pid);

test/unit/lib/nvme/nvme_fabric.c/nvme_fabric_ut.c

+2 −3
Original line number Diff line number Diff line
@@ -85,9 +85,8 @@ nvme_completion_poll_cb(void *arg, const struct spdk_nvme_cpl *cpl) 
static bool g_nvme_wait_for_completion_timeout;



int

nvme_wait_for_completion_robust_lock_timeout_poll(struct spdk_nvme_qpair *qpair,

		struct nvme_completion_poll_status *status,

		pthread_mutex_t *robust_mutex)

nvme_wait_for_completion_poll(struct spdk_nvme_qpair *qpair,

			      struct nvme_completion_poll_status *status)

{

	struct spdk_nvmf_fabric_connect_rsp *rsp = (void *)&status->cpl;