Commit 1350922d authored by Tomasz Zawadzki's avatar Tomasz Zawadzki Committed by Jim Harris
Browse files

bdev/ocf: take additional reference for ocf_cache



Fixes #1498

When shutting down the application, it was possible to
reference stale ocf_cache pointer. This was the case
when two or more vbdev_ocf devices were based on top
of single cache bdev.

This issue did not occur outside of the shutdown case,
since RPC only allows deletion of the vbdev_ocf.
This erases on disk metadata and next run of the application,
would not detect such vbdev_ocf.

Shutdown meanwhile works different, by first stopping
the instance of running "ocf_mngt_cache" and later detaching
"core" devices (the ones being cached). This prevented
erasing the on disk metadata and allowed for restarted
application to detect vbdev_ocf.
See patch (1292ef24) for details.

Since references to ocf_cache are copied between vbdev_ocf
[see start_cache()], the reference count inside ocf_cache
was limited to original ocf_mngt_cache_start() and
management queue creation. First call into ocf_mngt_cache_stop()
released all references to ocf_cache. Leaving other
vbdev_ocfs pointing to released memory.

This patch works around this issue by increasing ref cnt
on ocf_cache for each vbdev based on top of it.
It allows to call into ocf_mngt_cache_stop(), but not
release the memory for ocf_cache until last vbdev.

Note:
A proper redesign here is in order:
- either rearranging structures to be based around single ocf_cache,
rather than multiple vbdev_ocf instances
- better use of OCF API to reduce book keeping logic in vbdev

There are plans to implement detach/attach in RPC,
so it should be a focus during that effort.

Signed-off-by: default avatarTomasz Zawadzki <tomasz.zawadzki@intel.com>
Change-Id: I560a7fbb1c052bf53970e655bdb60803c561a252
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3574


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarVitaliy Mysak <vitaliy.mysak@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
parent 868ba177
Loading
Loading
Loading
Loading
+3 −0
Original line number Diff line number Diff line
@@ -199,6 +199,7 @@ static void
unregister_finish(struct vbdev_ocf *vbdev)
{
	spdk_bdev_destruct_done(&vbdev->exp_bdev, vbdev->state.stop_status);
	ocf_mngt_cache_put(vbdev->ocf_cache);
	vbdev_ocf_cache_ctx_put(vbdev->cache_ctx);
	vbdev_ocf_mngt_continue(vbdev, 0);
}
@@ -1059,6 +1060,7 @@ start_cache(struct vbdev_ocf *vbdev)
		SPDK_NOTICELOG("OCF bdev %s connects to existing cache device %s\n",
			       vbdev->name, vbdev->cache.name);
		vbdev->ocf_cache = existing;
		ocf_mngt_cache_get(vbdev->ocf_cache);
		vbdev->cache_ctx = ocf_cache_get_priv(existing);
		vbdev_ocf_cache_ctx_get(vbdev->cache_ctx);
		vbdev_ocf_mngt_continue(vbdev, 0);
@@ -1079,6 +1081,7 @@ start_cache(struct vbdev_ocf *vbdev)
		vbdev_ocf_mngt_exit(vbdev, unregister_path_dirty, rc);
		return;
	}
	ocf_mngt_cache_get(vbdev->ocf_cache);

	ocf_cache_set_priv(vbdev->ocf_cache, vbdev->cache_ctx);

+1 −2
Original line number Diff line number Diff line
@@ -10,6 +10,5 @@ run_test "ocf_bdevperf_iotypes" "$testdir/integrity/bdevperf-iotypes.sh"
run_test "ocf_stats" "$testdir/integrity/stats.sh"
run_test "ocf_create_destruct" "$testdir/management/create-destruct.sh"
run_test "ocf_multicore" "$testdir/management/multicore.sh"
# Disabled due to issue #1498
# run_test "ocf_persistent_metadata" "$testdir/management/persistent-metadata.sh"
run_test "ocf_persistent_metadata" "$testdir/management/persistent-metadata.sh"
run_test "ocf_remove" "$testdir/management/remove.sh"