Commit a7d61bef authored by Konrad Sztyber's avatar Konrad Sztyber Committed by Tomasz Zawadzki
Browse files

nvme: guard admin qpair error injection queue 



Admin commands can be sent and polled from any thread, which also means
that the error injection queue on the admin qpair can be accessed from
multiple threads.  Therefore, any modifications to that queue should be
done under the ctrlr lock.

Signed-off-by: default avatarKonrad Sztyber <konrad.sztyber@intel.com>
Change-Id: Ib1ed194405cb5b93f65a007b9749fd4433dc367d
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/11099


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarMichael Haeuptle <michaelhaeuptle@gmail.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
parent 88e676f7

lib/nvme/nvme_qpair.c

+14 −4
Original line number Diff line number Diff line
@@ -1076,9 +1076,11 @@ spdk_nvme_qpair_add_cmd_error_injection(struct spdk_nvme_ctrlr *ctrlr, 
					uint8_t sct, uint8_t sc)

{

	struct nvme_error_cmd *entry, *cmd = NULL;

	int rc = 0;



	if (qpair == NULL) {

		qpair = ctrlr->adminq;

		nvme_robust_mutex_lock(&ctrlr->ctrlr_lock);

	}



	TAILQ_FOREACH(entry, &qpair->err_cmd_head, link) {
@@ -1091,7 +1093,8 @@ spdk_nvme_qpair_add_cmd_error_injection(struct spdk_nvme_ctrlr *ctrlr, 
	if (cmd == NULL) {

		cmd = spdk_zmalloc(sizeof(*cmd), 64, NULL, SPDK_ENV_LCORE_ID_ANY, SPDK_MALLOC_DMA);

		if (!cmd) {

			return -ENOMEM;

			rc = -ENOMEM;

			goto out;

		}

		TAILQ_INSERT_TAIL(&qpair->err_cmd_head, cmd, link);

	}
@@ -1102,8 +1105,12 @@ spdk_nvme_qpair_add_cmd_error_injection(struct spdk_nvme_ctrlr *ctrlr, 
	cmd->opc = opc;

	cmd->status.sct = sct;

	cmd->status.sc = sc;

out:

	if (nvme_qpair_is_admin_queue(qpair)) {

		nvme_robust_mutex_unlock(&ctrlr->ctrlr_lock);

	}



	return 0;

	return rc;

}



void
@@ -1115,17 +1122,20 @@ spdk_nvme_qpair_remove_cmd_error_injection(struct spdk_nvme_ctrlr *ctrlr, 


	if (qpair == NULL) {

		qpair = ctrlr->adminq;

		nvme_robust_mutex_lock(&ctrlr->ctrlr_lock);

	}



	TAILQ_FOREACH_SAFE(cmd, &qpair->err_cmd_head, link, entry) {

		if (cmd->opc == opc) {

			TAILQ_REMOVE(&qpair->err_cmd_head, cmd, link);

			spdk_free(cmd);

			return;

			break;

		}

	}



	return;

	if (nvme_qpair_is_admin_queue(qpair)) {

		nvme_robust_mutex_unlock(&ctrlr->ctrlr_lock);

	}

}



uint16_t

test/unit/lib/nvme/nvme_qpair.c/nvme_qpair_ut.c

+7 −0
Original line number Diff line number Diff line
@@ -445,11 +445,17 @@ test_nvme_qpair_add_cmd_error_injection(void) 
{

	struct spdk_nvme_qpair qpair = {};

	struct spdk_nvme_ctrlr ctrlr = {};

	pthread_mutexattr_t attr;

	int rc;



	prepare_submit_request_test(&qpair, &ctrlr);

	ctrlr.adminq = &qpair;



	SPDK_CU_ASSERT_FATAL(pthread_mutexattr_init(&attr) == 0);

	SPDK_CU_ASSERT_FATAL(pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE) == 0);

	SPDK_CU_ASSERT_FATAL(pthread_mutex_init(&ctrlr.ctrlr_lock, &attr) == 0);

	pthread_mutexattr_destroy(&attr);



	/* Admin error injection at submission path */

	MOCK_CLEAR(spdk_zmalloc);

	rc = spdk_nvme_qpair_add_cmd_error_injection(&ctrlr, NULL,
@@ -498,6 +504,7 @@ test_nvme_qpair_add_cmd_error_injection(void) 


	CU_ASSERT(TAILQ_EMPTY(&qpair.err_cmd_head));



	pthread_mutex_destroy(&ctrlr.ctrlr_lock);

	cleanup_submit_request_test(&qpair);

}