Commit 4e8f5d39 authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

bdev/nvme: Fix bdev_nvme_delete() to call ctrlr_get_by_name() and set destruct to true in a lock 



bdev_nvme_delete() had called nvme_bdev_ctrlr_get_by_name() and
then called remove_cb(). But nvme_bdev_ctrlr_get_by_name() had not
been guarded by lock and remove_cb() parsed the list again by using
spdk_nvme_ctrlr as a key.

These were not safe and efficient.

This patch refines bdev_nvme_delete() to inline remove_cb() into
it and acquire mutex before calling nvme_bdev_ctrlr_get_by_name().

Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ibf39859809efa6f22899353c53b7b1eef58aa470
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/4842


Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent edbb7ace

module/bdev/nvme/bdev_nvme.c

+26 −2
Original line number Diff line number Diff line
@@ -1984,29 +1984,53 @@ bdev_nvme_create(struct spdk_nvme_transport_id *trid, 
int

bdev_nvme_delete(const char *name)

{

	struct nvme_bdev_ctrlr *nvme_bdev_ctrlr = NULL;

	struct nvme_bdev_ctrlr *nvme_bdev_ctrlr;

	struct nvme_probe_skip_entry *entry;



	if (name == NULL) {

		return -EINVAL;

	}



	pthread_mutex_lock(&g_bdev_nvme_mutex);



	nvme_bdev_ctrlr = nvme_bdev_ctrlr_get_by_name(name);

	if (nvme_bdev_ctrlr == NULL) {

		pthread_mutex_unlock(&g_bdev_nvme_mutex);

		SPDK_ERRLOG("Failed to find NVMe controller\n");

		return -ENODEV;

	}



	/* The controller's destruction was already started */

	if (nvme_bdev_ctrlr->destruct) {

		pthread_mutex_unlock(&g_bdev_nvme_mutex);

		return 0;

	}



	if (nvme_bdev_ctrlr->connected_trid->trtype == SPDK_NVME_TRANSPORT_PCIE) {

		entry = calloc(1, sizeof(*entry));

		if (!entry) {

			pthread_mutex_unlock(&g_bdev_nvme_mutex);

			return -ENOMEM;

		}

		entry->trid = *nvme_bdev_ctrlr->connected_trid;

		TAILQ_INSERT_TAIL(&g_skipped_nvme_ctrlrs, entry, tailq);

	}



	remove_cb(NULL, nvme_bdev_ctrlr->ctrlr);

	nvme_bdev_ctrlr->destruct = true;

	pthread_mutex_unlock(&g_bdev_nvme_mutex);



	nvme_ctrlr_depopulate_namespaces(nvme_bdev_ctrlr);



	pthread_mutex_lock(&g_bdev_nvme_mutex);

	assert(nvme_bdev_ctrlr->ref > 0);

	nvme_bdev_ctrlr->ref--;

	if (nvme_bdev_ctrlr->ref == 0) {

		pthread_mutex_unlock(&g_bdev_nvme_mutex);

		nvme_bdev_ctrlr_destruct(nvme_bdev_ctrlr);

	} else {

		pthread_mutex_unlock(&g_bdev_nvme_mutex);

	}



	return 0;

}