Commit 3512714b authored by Alexey Marchuk's avatar Alexey Marchuk Committed by Jim Harris
Browse files

nvme_fabrics: Lock mutext when prcessing set/get regs 



That is possible to get/set registers from any thread,
during regs processing we are polling admin qpair to
get a completion. At the same time, another thread
can also poll admin qpair and that can lead to
undefined behavior.

This patch fixes an issue when bdev_nvme is configured
with io_timeout. If remote target becomes unresponsive
(e.g. due to link down), IO timeout occurs and bdev_nvme
tries to get csts registers in timeout_cb. At the same
time another thread can process adminq, so we may have
2 simultaneous adminq polls. If admin qpair is disconnecting
at that time (RDMA transport) we may destroy resources
twice from different threads.

We don't see a problem with set_regs function but it
won't be redundant to lock mutex in set_regs as well.

Signed-off-by: default avatarAlexey Marchuk <alexeymar@nvidia.com>
Change-Id: I7ec3984d25d0249061005533d13b22315b44ddf2
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/13687


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarKonrad Sztyber <konrad.sztyber@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 6f338d4b

lib/nvme/nvme_fabric.c

+3 −3
Original line number Diff line number Diff line 
/*   SPDX-License-Identifier: BSD-3-Clause

 *   Copyright (c) Intel Corporation. All rights reserved.

 *   Copyright (c) 2020 Mellanox Technologies LTD. All rights reserved.

 *   Copyright (c) 2021 NVIDIA CORPORATION & AFFILIATES. All rights reserved.

 *   Copyright (c) 2021, 2022 NVIDIA CORPORATION & AFFILIATES. All rights reserved.

 */



/*
@@ -59,7 +59,7 @@ nvme_fabric_prop_set_cmd_sync(struct spdk_nvme_ctrlr *ctrlr, 
		return rc;

	}



	if (nvme_wait_for_completion(ctrlr->adminq, status)) {

	if (nvme_wait_for_completion_robust_lock(ctrlr->adminq, status, &ctrlr->ctrlr_lock)) {

		if (!status->timed_out) {

			free(status);

		}
@@ -145,7 +145,7 @@ nvme_fabric_prop_get_cmd_sync(struct spdk_nvme_ctrlr *ctrlr, 
		return rc;

	}



	if (nvme_wait_for_completion(ctrlr->adminq, status)) {

	if (nvme_wait_for_completion_robust_lock(ctrlr->adminq, status, &ctrlr->ctrlr_lock)) {

		if (!status->timed_out) {

			free(status);

		}

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

+12 −0
Original line number Diff line number Diff line 
/*   SPDX-License-Identifier: BSD-3-Clause

 *   Copyright (c) Intel Corporation.

 *   All rights reserved.

 *   Copyright (c) 2022 NVIDIA CORPORATION & AFFILIATES. All rights reserved.

 */



#include "spdk/stdinc.h"
@@ -166,6 +167,17 @@ nvme_wait_for_completion(struct spdk_nvme_qpair *qpair, 
	return 0;

}



DEFINE_RETURN_MOCK(nvme_wait_for_completion_robust_lock, int);

int

nvme_wait_for_completion_robust_lock(struct spdk_nvme_qpair *qpair,

				     struct nvme_completion_poll_status *status,

				     pthread_mutex_t *robust_mutex)

{

	status->timed_out = false;

	HANDLE_RETURN_MOCK(nvme_wait_for_completion_robust_lock);

	return 0;

}



DEFINE_RETURN_MOCK(spdk_nvme_ctrlr_cmd_admin_raw, int);

int

spdk_nvme_ctrlr_cmd_admin_raw(struct spdk_nvme_ctrlr *ctrlr,