Commit 73358c99 authored by Dariusz Stojaczyk's avatar Dariusz Stojaczyk Committed by Daniel Verkamp
Browse files

util/nvme: added io_device unregister callback 



Patch afe860ae deferred freeing the io_device. However, for nvme, the
io_device context (spdk_nvme_ctrlr) is still being destructed before
io_channels are destroyed, causing segfaults on hotremove.

This patch defers io_device context destruction and fixes nvme
hotremove.

Fixes: afe860ae ("channel: Correctly defer unregisters if channels exist")
Fixes: 5533c3d2 ("util: defer put_io_channel")

Change-Id: I7af699174cac0c6c6a6faa2cc65418c47347eb9a
Signed-off-by: default avatarDariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/370459


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
parent 542b5415

include/spdk/io_channel.h

+6 −3
Original line number Diff line number Diff line
@@ -52,6 +52,8 @@ typedef void (*spdk_thread_pass_msg)(spdk_thread_fn fn, void *ctx, 
typedef int (*spdk_io_channel_create_cb)(void *io_device, void *ctx_buf);

typedef void (*spdk_io_channel_destroy_cb)(void *io_device, void *ctx_buf);



typedef void (*spdk_io_device_unregister_cb)(void *io_device);



typedef void (*spdk_channel_msg)(void *io_device, struct spdk_io_channel *ch,

				 void *ctx);

typedef void (*spdk_channel_for_each_cpl)(void *io_device, void *ctx);
@@ -110,10 +112,11 @@ void spdk_io_device_register(void *io_device, spdk_io_channel_create_cb create_c 
/**

 * \brief Unregister the opaque io_device context as an I/O device.

 *

 * Callers must ensure they release references to any I/O channel related to this

 *  device before calling this function.

 * The actual unregistration might be deferred until all active I/O channels are destroyed.

 *  unregister_cb is an optional callback function invoked to release any references to

 *  this I/O device.

 */

void spdk_io_device_unregister(void *io_device);

void spdk_io_device_unregister(void *io_device, spdk_io_device_unregister_cb unregister_cb);



/**

 * \brief Gets an I/O channel for the specified io_device to be used by the calling thread.

lib/bdev/bdev.c

+2 −2
Original line number Diff line number Diff line
@@ -522,7 +522,7 @@ spdk_bdev_finish(void) 
	spdk_mempool_free(g_bdev_mgr.buf_small_pool);

	spdk_mempool_free(g_bdev_mgr.buf_large_pool);



	spdk_io_device_unregister(&g_bdev_mgr);

	spdk_io_device_unregister(&g_bdev_mgr, NULL);



	return 0;

}
@@ -1462,7 +1462,7 @@ spdk_bdev_unregister(struct spdk_bdev *bdev) 


	pthread_mutex_destroy(&bdev->mutex);



	spdk_io_device_unregister(bdev);

	spdk_io_device_unregister(bdev, NULL);



	rc = bdev->fn_table->destruct(bdev->ctxt);

	if (rc < 0) {

lib/bdev/nvme/bdev_nvme.c

+9 −2
Original line number Diff line number Diff line
@@ -224,6 +224,14 @@ bdev_nvme_poll_adminq(void *arg) 
	spdk_nvme_ctrlr_process_admin_completions(ctrlr);

}



static void

bdev_nvme_unregister_cb(void *io_device)

{

	struct spdk_nvme_ctrlr *ctrlr = io_device;



	spdk_nvme_detach(ctrlr);

}



static int

bdev_nvme_destruct(void *ctx)

{
@@ -240,9 +248,8 @@ bdev_nvme_destruct(void *ctx) 
	if (nvme_ctrlr->ref == 0) {

		TAILQ_REMOVE(&g_nvme_ctrlrs, nvme_ctrlr, tailq);

		pthread_mutex_unlock(&g_bdev_nvme_mutex);

		spdk_io_device_unregister(nvme_ctrlr->ctrlr);

		spdk_io_device_unregister(nvme_ctrlr->ctrlr, bdev_nvme_unregister_cb);

		spdk_bdev_poller_stop(&nvme_ctrlr->adminq_timer_poller);

		spdk_nvme_detach(nvme_ctrlr->ctrlr);

		free(nvme_ctrlr->name);

		free(nvme_ctrlr);

		return 0;

lib/blob/blobstore.c

+2 −2
Original line number Diff line number Diff line
@@ -1141,8 +1141,8 @@ _spdk_bs_free(struct spdk_blob_store *bs) 
	struct spdk_blob	*blob, *blob_tmp;



	spdk_bs_unregister_md_thread(bs);

	spdk_io_device_unregister(&bs->io_target);

	spdk_io_device_unregister(&bs->md_target);

	spdk_io_device_unregister(&bs->io_target, NULL);

	spdk_io_device_unregister(&bs->md_target, NULL);



	TAILQ_FOREACH_SAFE(blob, &bs->blobs, link, blob_tmp) {

		TAILQ_REMOVE(&bs->blobs, blob, link);

lib/blobfs/blobfs.c

+9 −9
Original line number Diff line number Diff line
@@ -440,10 +440,10 @@ spdk_fs_init(struct spdk_bs_dev *dev, fs_send_request_fn send_request_fn, 
	req = alloc_fs_request(fs->md_target.md_fs_channel);

	if (req == NULL) {

		spdk_put_io_channel(fs->md_target.md_io_channel);

		spdk_io_device_unregister(&fs->md_target);

		spdk_io_device_unregister(&fs->md_target, NULL);

		spdk_put_io_channel(fs->sync_target.sync_io_channel);

		spdk_io_device_unregister(&fs->sync_target);

		spdk_io_device_unregister(&fs->io_target);

		spdk_io_device_unregister(&fs->sync_target, NULL);

		spdk_io_device_unregister(&fs->io_target, NULL);

		free(fs);

		cb_fn(cb_arg, NULL, -ENOMEM);

		return;
@@ -566,10 +566,10 @@ spdk_fs_load(struct spdk_bs_dev *dev, fs_send_request_fn send_request_fn, 
	req = alloc_fs_request(fs->md_target.md_fs_channel);

	if (req == NULL) {

		spdk_put_io_channel(fs->md_target.md_io_channel);

		spdk_io_device_unregister(&fs->md_target);

		spdk_io_device_unregister(&fs->md_target, NULL);

		spdk_put_io_channel(fs->sync_target.sync_io_channel);

		spdk_io_device_unregister(&fs->sync_target);

		spdk_io_device_unregister(&fs->io_target);

		spdk_io_device_unregister(&fs->sync_target, NULL);

		spdk_io_device_unregister(&fs->io_target, NULL);

		free(fs);

		cb_fn(cb_arg, NULL, -ENOMEM);

		return;
@@ -600,9 +600,9 @@ unload_cb(void *ctx, int bserrno) 
	args->fn.fs_op(args->arg, bserrno);

	free(req);



	spdk_io_device_unregister(&fs->io_target);

	spdk_io_device_unregister(&fs->sync_target);

	spdk_io_device_unregister(&fs->md_target);

	spdk_io_device_unregister(&fs->io_target, NULL);

	spdk_io_device_unregister(&fs->sync_target, NULL);

	spdk_io_device_unregister(&fs->md_target, NULL);



	free(fs);

}