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

nvme: always use wait for completion with lock 



In flows where nvme_wait_for_completion_timeout is used, the controller
mutex is already held. Since the mutex is recursive and for simplicity,
always use the version that takes the lock.

Remove the function that does not take the lock, as it is no longer used.

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


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

lib/nvme/nvme.c

+0 −23
Original line number Diff line number Diff line
@@ -336,29 +336,6 @@ nvme_wait_for_completion_robust_lock_timeout( 
	return rc;

}



/**

 * Poll qpair for completions until a command completes.

 *

 * \param qpair queue to poll

 * \param status completion status. The user must fill this structure with zeroes before calling

 * this function

 * \param timeout_in_usecs optional timeout

 *

 * \return 0 if command completed without error,

 * -EIO if command completed with error,

 * -ECANCELED if command is not completed due to transport/device error or time expired

 *

 * The command to wait upon must be submitted with nvme_completion_poll_cb as the callback

 * and status as the callback argument.

 */

int

nvme_wait_for_completion_timeout(struct spdk_nvme_qpair *qpair,

				 struct nvme_completion_poll_status *status,

				 uint64_t timeout_in_usecs)

{

	return nvme_wait_for_completion_robust_lock_timeout(qpair, status, NULL, timeout_in_usecs);

}



static void

nvme_user_copy_cmd_complete(void *arg, const struct spdk_nvme_cpl *cpl)

{

lib/nvme/nvme_ctrlr.c

+4 −4
Original line number Diff line number Diff line
@@ -968,7 +968,7 @@ nvme_ctrlr_set_arbitration_feature(struct spdk_nvme_ctrlr *ctrlr) 
		return;

	}



	if (nvme_wait_for_completion_timeout(ctrlr->adminq, status,

	if (nvme_wait_for_completion_robust_lock_timeout(ctrlr->adminq, status, &ctrlr->ctrlr_lock,

			ctrlr->opts.admin_timeout_ms * 1000)) {

		NVME_CTRLR_ERRLOG(ctrlr, "Timeout to set arbitration feature\n");

	}
@@ -3290,7 +3290,7 @@ nvme_ctrlr_clear_changed_ns_log(struct spdk_nvme_ctrlr_aer_completion *async_eve 
		goto free_buffer;

	}



	rc = nvme_wait_for_completion_timeout(ctrlr->adminq, status,

	rc = nvme_wait_for_completion_robust_lock_timeout(ctrlr->adminq, status, &ctrlr->ctrlr_lock,

			ctrlr->opts.admin_timeout_ms * 1000);

	if (!status->timed_out) {

		free(status);

lib/nvme/nvme_fabric.c

+4 −3
Original line number Diff line number Diff line
@@ -366,7 +366,8 @@ nvme_fabric_get_discovery_log_page(struct spdk_nvme_ctrlr *ctrlr, 
		return -1;

	}



	if (nvme_wait_for_completion_timeout(ctrlr->adminq, status, ctrlr->opts.admin_timeout_ms * 1000)) {

	if (nvme_wait_for_completion_robust_lock_timeout(ctrlr->adminq, status, &ctrlr->ctrlr_lock,

			ctrlr->opts.admin_timeout_ms * 1000)) {

		if (!status->timed_out) {

			free(status);

		}
@@ -427,8 +428,8 @@ nvme_fabric_ctrlr_scan(struct spdk_nvme_probe_ctx *probe_ctx, 
		return rc;

	}



	if (nvme_wait_for_completion_timeout(discovery_ctrlr->adminq, status,

					     discovery_ctrlr->opts.admin_timeout_ms * 1000)) {

	if (nvme_wait_for_completion_robust_lock_timeout(discovery_ctrlr->adminq, status,

			&discovery_ctrlr->ctrlr_lock, discovery_ctrlr->opts.admin_timeout_ms * 1000)) {

		SPDK_ERRLOG("nvme_identify_controller failed!\n");

		nvme_ctrlr_destruct(discovery_ctrlr);

		if (!status->timed_out) {

lib/nvme/nvme_internal.h

+0 −3
Original line number Diff line number Diff line
@@ -1299,9 +1299,6 @@ int nvme_ctrlr_cmd_sanitize(struct spdk_nvme_ctrlr *ctrlr, uint32_t nsid, 
				struct spdk_nvme_sanitize *sanitize, uint32_t cdw11,

				spdk_nvme_cmd_cb cb_fn, void *cb_arg);

void	nvme_completion_poll_cb(void *arg, const struct spdk_nvme_cpl *cpl);

int	nvme_wait_for_completion_timeout(struct spdk_nvme_qpair *qpair,

		struct nvme_completion_poll_status *status,

		uint64_t timeout_in_usecs);

int	nvme_wait_for_completion_robust_lock_timeout(struct spdk_nvme_qpair *qpair,

		struct nvme_completion_poll_status *status,

		pthread_mutex_t *robust_mutex,

lib/nvme/nvme_pcie_common.c

+4 −2
Original line number Diff line number Diff line
@@ -1150,7 +1150,8 @@ nvme_pcie_ctrlr_delete_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_ 
		free(status);

		goto free;

	}

	if (nvme_wait_for_completion_timeout(ctrlr->adminq, status, ctrlr->opts.admin_timeout_ms * 1000)) {

	if (nvme_wait_for_completion_robust_lock_timeout(ctrlr->adminq, status, &ctrlr->ctrlr_lock,

			ctrlr->opts.admin_timeout_ms * 1000)) {

		if (!status->timed_out) {

			free(status);

		}
@@ -1172,7 +1173,8 @@ nvme_pcie_ctrlr_delete_io_qpair(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_ 
		free(status);

		goto free;

	}

	if (nvme_wait_for_completion_timeout(ctrlr->adminq, status, ctrlr->opts.admin_timeout_ms * 1000)) {

	if (nvme_wait_for_completion_robust_lock_timeout(ctrlr->adminq, status, &ctrlr->ctrlr_lock,

			ctrlr->opts.admin_timeout_ms * 1000)) {

		if (!status->timed_out) {

			free(status);

		}