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

bdev/nvme: Configure all for nvme_bdev before spdk_bdev_register() call 



After spdk_bdev_register() completes, the bdev is ready to use. However,
nvme_bdev_create() continue configuration even after
spdk_bdev_register() call.

This is a potential issue but if spdk_bdev_unregister() is called on a
thread different from the thread which called spdk_bdev_register(),
the race between nvme_bdev_create() and bdev_nvme_destruct() can cause
unexpected behavior. Even on the current SPDK master, this is possible
when the bdev susystem starts shutdown when there still exists any NVMe
bdev.

To fix the potential issue, move some operations prior to the
spdk_bdev_register() call and revert these if spdk_bdev_register()
fails. Additionally, for readability, move enqueueing nvme_ns to
bdev->nvme_ns_list close to the similar operations.

Signed-off-by: default avatarAlexey Marchuk <alexeymar@nvidia.com>
Signed-off-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Change-Id: I3b70dfe64e1860fa9ed1704f7982415d61d4ecc0
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17712


Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 07d0c549

module/bdev/nvme/bdev_nvme.c

+11 −8
Original line number Diff line number Diff line
@@ -3535,6 +3535,7 @@ static int 
nvme_bdev_create(struct nvme_ctrlr *nvme_ctrlr, struct nvme_ns *nvme_ns)

{

	struct nvme_bdev *bdev;

	struct nvme_bdev_ctrlr *nbdev_ctrlr = nvme_ctrlr->nbdev_ctrlr;

	int rc;



	bdev = nvme_bdev_alloc();
@@ -3543,10 +3544,9 @@ nvme_bdev_create(struct nvme_ctrlr *nvme_ctrlr, struct nvme_ns *nvme_ns) 
		return -ENOMEM;

	}



	TAILQ_INSERT_TAIL(&bdev->nvme_ns_list, nvme_ns, tailq);

	bdev->opal = nvme_ctrlr->opal_dev != NULL;



	rc = nvme_disk_create(&bdev->disk, nvme_ctrlr->nbdev_ctrlr->name, nvme_ctrlr->ctrlr,

	rc = nvme_disk_create(&bdev->disk, nbdev_ctrlr->name, nvme_ctrlr->ctrlr,

			      nvme_ns->ns, nvme_ctrlr->opts.prchk_flags, bdev);

	if (rc != 0) {

		SPDK_ERRLOG("Failed to create NVMe disk\n");
@@ -3560,20 +3560,23 @@ nvme_bdev_create(struct nvme_ctrlr *nvme_ctrlr, struct nvme_ns *nvme_ns) 
				sizeof(struct nvme_bdev_channel),

				bdev->disk.name);



	nvme_ns->bdev = bdev;

	bdev->nsid = nvme_ns->id;

	TAILQ_INSERT_TAIL(&bdev->nvme_ns_list, nvme_ns, tailq);



	bdev->nbdev_ctrlr = nbdev_ctrlr;

	TAILQ_INSERT_TAIL(&nbdev_ctrlr->bdevs, bdev, tailq);



	rc = spdk_bdev_register(&bdev->disk);

	if (rc != 0) {

		SPDK_ERRLOG("spdk_bdev_register() failed\n");

		spdk_io_device_unregister(bdev, NULL);

		nvme_ns->bdev = NULL;

		TAILQ_REMOVE(&nbdev_ctrlr->bdevs, bdev, tailq);

		nvme_bdev_free(bdev);

		return rc;

	}



	nvme_ns->bdev = bdev;

	bdev->nsid = nvme_ns->id;



	bdev->nbdev_ctrlr = nvme_ctrlr->nbdev_ctrlr;

	TAILQ_INSERT_TAIL(&nvme_ctrlr->nbdev_ctrlr->bdevs, bdev, tailq);



	return 0;

}