Commit 1b6d1c80 authored by Marcin Dziegielewski's avatar Marcin Dziegielewski Committed by Tomasz Zawadzki
Browse files

lib/bdev/ocf: fix potential issue around base->management_channel 



This patch improves management of base->management_channel in code and
fixes potential issue described below. In situation when we want to create
configuration like this:

$rpc_py bdev_ocf_create C1 wt Malloc NonExisting
$rpc_py bdev_ocf_create C2 wt Malloc Core

In current code on C1 vbdev creation we are creating not fully
initialized vbdev_ocf_base for cache (without management channel).

On C2 creation, because in vbdev_ocf_volume_open we are looking for
right base by name, we can grab cache base from C1 and then, there is
possibility, to hit segmentation fault due to uninitialized
mangament_channel.

This patch moves initialisation of managemnet channel to attach
base function and thanks to that we have guarrnte that each copy of
cache base (we have copy in each parent in case of multicore cache)
are valid.

Signed-off-by: default avatarMarcin Dziegielewski <marcin.dziegielewski@intel.com>
Change-Id: Iab1ced3a4bf8ba0fec947bbf77b55dc3620a58a7
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/468131


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: default avatarVitaliy Mysak <vitaliy.mysak@intel.com>
parent 9d660f18

module/bdev/ocf/ctx.h

+0 −1
Original line number Diff line number Diff line
@@ -47,7 +47,6 @@ extern ocf_ctx_t vbdev_ocf_ctx; 
struct vbdev_ocf_cache_ctx {

	ocf_queue_t                  mngt_queue;

	ocf_queue_t                  cleaner_queue;

	struct spdk_io_channel      *management_channel;

	pthread_mutex_t              lock;

	env_atomic                   refcnt;

};

module/bdev/ocf/vbdev_ocf.c

+13 −11
Original line number Diff line number Diff line
@@ -162,13 +162,13 @@ static void 
remove_base_bdev(struct vbdev_ocf_base *base)

{

	if (base->attached) {

		if (base->management_channel) {

			spdk_put_io_channel(base->management_channel);

		}



		spdk_bdev_module_release_bdev(base->bdev);

		spdk_bdev_close(base->desc);

		base->attached = false;



		if (base->management_channel && !base->is_cache) {

			spdk_put_io_channel(base->management_channel);

		}

	}

}
@@ -252,7 +252,6 @@ stop_vbdev_cmpl(ocf_cache_t cache, void *priv, int error) 


	vbdev_ocf_queue_put(vbdev->cache_ctx->mngt_queue);

	ocf_mngt_cache_unlock(cache);

	spdk_put_io_channel(vbdev->cache.management_channel);



	vbdev_ocf_mngt_continue(vbdev, error);

}
@@ -903,8 +902,6 @@ finish_register(struct vbdev_ocf *vbdev) 
{

	int result;



	vbdev->cache.management_channel = vbdev->cache_ctx->management_channel;



	/* Copy properties of the base bdev */

	vbdev->exp_bdev.blocklen = vbdev->core.bdev->blocklen;

	vbdev->exp_bdev.write_cache = vbdev->core.bdev->write_cache;
@@ -945,7 +942,6 @@ add_core_cmpl(ocf_cache_t cache, ocf_core_t core, void *priv, int error) 
		vbdev->core.id  = ocf_core_get_id(core);

	}



	vbdev->core.management_channel = spdk_bdev_get_io_channel(vbdev->core.desc);

	vbdev_ocf_mngt_continue(vbdev, error);

}
@@ -1057,9 +1053,6 @@ start_cache(struct vbdev_ocf *vbdev) 
		return;

	}



	vbdev->cache_ctx->management_channel = spdk_bdev_get_io_channel(vbdev->cache.desc);

	vbdev->cache.management_channel = vbdev->cache_ctx->management_channel;



	if (vbdev->cfg.loadq) {

		ocf_mngt_cache_load(vbdev->ocf_cache, &vbdev->cfg.device, start_cache_cmpl, vbdev);

	} else {
@@ -1347,6 +1340,7 @@ attach_base(struct vbdev_ocf_base *base) 
		struct vbdev_ocf_base *existing = get_other_cache_base(base);

		if (existing) {

			base->desc = existing->desc;

			base->management_channel = existing->management_channel;

			base->attached = true;

			return 0;

		}
@@ -1366,6 +1360,14 @@ attach_base(struct vbdev_ocf_base *base) 
		return status;

	}



	base->management_channel = spdk_bdev_get_io_channel(base->desc);

	if (!base->management_channel) {

		SPDK_ERRLOG("Unable to get io channel '%s'\n", base->name);

		spdk_bdev_module_release_bdev(base->bdev);

		spdk_bdev_close(base->desc);

		return -ENOMEM;

	}



	base->attached = true;

	return status;

}