From 8fcb8b966d93950e0aaccce886d6722e865833a6 Mon Sep 17 00:00:00 2001 From: Rafal Stefanowski Date: Mon, 26 Apr 2021 16:45:56 +0200 Subject: [PATCH] bdev/ocf: Improve cache line size handling - Use consistent cache line size units in KiB across RPC calls and config files. The KiB units are much easier to use then the bytes units and are more human readable. - Properly handle cache start when cache line size is incorrect. - Add test to check if cache line size value is reported correctly. - Add cache line size info to JSON RPC documentation. Fixes #1858 Signed-off-by: Rafal Stefanowski Change-Id: Iec9ede85f6884b64605d2d112947b3f175cbd938 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/7614 Community-CI: Broadcom CI Tested-by: SPDK CI Jenkins Reviewed-by: Shuhei Matsumoto Reviewed-by: Tomasz Zawadzki --- doc/jsonrpc.md | 4 ++- module/bdev/ocf/utils.c | 6 ++++ module/bdev/ocf/utils.h | 3 ++ module/bdev/ocf/vbdev_ocf.c | 11 +++++-- test/ocf/management/configuration-change.sh | 33 +++++++++++++++++++-- test/ocf/management/create-destruct.sh | 4 +-- 6 files changed, 52 insertions(+), 9 deletions(-) diff --git a/doc/jsonrpc.md b/doc/jsonrpc.md index e5e4d748a..572802b86 100644 --- a/doc/jsonrpc.md +++ b/doc/jsonrpc.md @@ -2107,6 +2107,7 @@ Name | Optional | Type | Description ----------------------- | -------- | ----------- | ----------- name | Required | string | Bdev name to use mode | Required | string | OCF cache mode: wb, wt, pt, wa, wi, wo +cache_line_size | Optional | int | OCF cache line size in KiB: 4, 8, 16, 32, 64 cache_bdev_name | Required | string | Name of underlying cache bdev core_bdev_name | Required | string | Name of underlying core bdev @@ -2123,7 +2124,8 @@ Example request: "params": { "name": "ocf0", "mode": "wt", - "cache_bdev_name": "Nvme0n1" + "cache_line_size": 64, + "cache_bdev_name": "Nvme0n1", "core_bdev_name": "aio0" }, "jsonrpc": "2.0", diff --git a/module/bdev/ocf/utils.c b/module/bdev/ocf/utils.c index 3a1df3c9e..dacab10b7 100644 --- a/module/bdev/ocf/utils.c +++ b/module/bdev/ocf/utils.c @@ -69,6 +69,12 @@ ocf_get_cache_modename(ocf_cache_mode_t mode) } } +int +ocf_get_cache_line_size(ocf_cache_t cache) +{ + return ocf_cache_get_line_size(cache) / KiB; +} + int vbdev_ocf_mngt_start(struct vbdev_ocf *vbdev, vbdev_ocf_mngt_fn *path, vbdev_ocf_mngt_callback cb, void *cb_arg) diff --git a/module/bdev/ocf/utils.h b/module/bdev/ocf/utils.h index 73bf6c93a..eb9f26d31 100644 --- a/module/bdev/ocf/utils.h +++ b/module/bdev/ocf/utils.h @@ -40,6 +40,9 @@ ocf_cache_mode_t ocf_get_cache_mode(const char *cache_mode); const char *ocf_get_cache_modename(ocf_cache_mode_t mode); +/* Get cache line size in KiB units */ +int ocf_get_cache_line_size(ocf_cache_t cache); + /* Initiate management operation * Receives NULL terminated array of functions (path) * and callback (cb) diff --git a/module/bdev/ocf/vbdev_ocf.c b/module/bdev/ocf/vbdev_ocf.c index fcc7ef3ee..04491a0c1 100644 --- a/module/bdev/ocf/vbdev_ocf.c +++ b/module/bdev/ocf/vbdev_ocf.c @@ -198,7 +198,11 @@ 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); + + if (vbdev->ocf_cache) { + ocf_mngt_cache_put(vbdev->ocf_cache); + } + vbdev_ocf_cache_ctx_put(vbdev->cache_ctx); vbdev_ocf_mngt_continue(vbdev, 0); } @@ -740,7 +744,7 @@ vbdev_ocf_dump_info_json(void *opaque, struct spdk_json_write_ctx *w) spdk_json_write_named_string(w, "mode", ocf_get_cache_modename(ocf_cache_get_mode(vbdev->ocf_cache))); spdk_json_write_named_uint32(w, "cache_line_size", - ocf_cache_get_line_size(vbdev->ocf_cache)); + ocf_get_cache_line_size(vbdev->ocf_cache)); spdk_json_write_named_bool(w, "metadata_volatile", vbdev->cfg.cache.metadata_volatile); @@ -761,7 +765,7 @@ vbdev_ocf_write_json_config(struct spdk_bdev *bdev, struct spdk_json_write_ctx * spdk_json_write_named_string(w, "mode", ocf_get_cache_modename(ocf_cache_get_mode(vbdev->ocf_cache))); spdk_json_write_named_uint32(w, "cache_line_size", - ocf_cache_get_line_size(vbdev->ocf_cache)); + ocf_get_cache_line_size(vbdev->ocf_cache)); spdk_json_write_named_string(w, "cache_bdev_name", vbdev->cache.name); spdk_json_write_named_string(w, "core_bdev_name", vbdev->core.name); spdk_json_write_object_end(w); @@ -1092,6 +1096,7 @@ start_cache(struct vbdev_ocf *vbdev) rc = ocf_mngt_cache_start(vbdev_ocf_ctx, &vbdev->ocf_cache, &vbdev->cfg.cache); if (rc) { + SPDK_ERRLOG("Could not start cache %s: %d\n", vbdev->name, rc); vbdev_ocf_mngt_exit(vbdev, unregister_path_dirty, rc); return; } diff --git a/test/ocf/management/configuration-change.sh b/test/ocf/management/configuration-change.sh index e307d9b41..faccebc9d 100755 --- a/test/ocf/management/configuration-change.sh +++ b/test/ocf/management/configuration-change.sh @@ -5,6 +5,7 @@ rootdir=$(readlink -f $curdir/../../..) source $rootdir/test/common/autotest_common.sh rpc_py=$rootdir/scripts/rpc.py +cache_line_sizes=(4 8 16 32 64) cache_modes=(wt wb pt wa wi wo) $SPDK_BIN_DIR/iscsi_tgt & @@ -12,17 +13,43 @@ spdk_pid=$! waitforlisten $spdk_pid -# Prepare OCF cache +# Create OCF cache with different cache line sizes +for cache_line_size in "${cache_line_sizes[@]}"; do + $rpc_py bdev_malloc_create 101 512 -b Malloc0 + $rpc_py bdev_malloc_create 101 512 -b Malloc1 + $rpc_py bdev_ocf_create Cache0 wt Malloc0 Malloc1 --cache-line-size $cache_line_size + + $rpc_py bdev_ocf_get_bdevs | jq -e \ + '.[0] | .started and .cache.attached and .core.attached' + + # Check if cache line size values are reported correctly + $rpc_py bdev_get_bdevs -b Cache0 | jq -e \ + ".[0] | .driver_specific.cache_line_size == $cache_line_size" + $rpc_py save_subsystem_config -n bdev | jq -e \ + ".config | .[] | select(.method == \"bdev_ocf_create\") | .params.cache_line_size == $cache_line_size" + + $rpc_py bdev_ocf_delete Cache0 + $rpc_py bdev_malloc_delete Malloc0 + $rpc_py bdev_malloc_delete Malloc1 +done + +# Prepare OCF cache for dynamic configuration switching $rpc_py bdev_malloc_create 101 512 -b Malloc0 $rpc_py bdev_malloc_create 101 512 -b Malloc1 -$rpc_py bdev_ocf_create Cache wt Malloc0 Malloc1 +$rpc_py bdev_ocf_create Cache0 wt Malloc0 Malloc1 $rpc_py bdev_ocf_get_bdevs | jq -e \ '.[0] | .started and .cache.attached and .core.attached' # Change cache mode for cache_mode in "${cache_modes[@]}"; do - $rpc_py bdev_ocf_set_cache_mode Cache $cache_mode + $rpc_py bdev_ocf_set_cache_mode Cache0 $cache_mode + + # Check if cache mode values are reported correctly + $rpc_py bdev_get_bdevs -b Cache0 | jq -e \ + ".[0] | .driver_specific.mode == \"$cache_mode\"" + $rpc_py save_subsystem_config -n bdev | jq -e \ + ".config | .[] | select(.method == \"bdev_ocf_create\") | .params.mode == \"$cache_mode\"" done trap - SIGINT SIGTERM EXIT diff --git a/test/ocf/management/create-destruct.sh b/test/ocf/management/create-destruct.sh index c1fd66b36..162f7a679 100755 --- a/test/ocf/management/create-destruct.sh +++ b/test/ocf/management/create-destruct.sh @@ -43,7 +43,7 @@ if bdev_check_claimed Malloc0; then exit 1 fi -$rpc_py bdev_ocf_create FullCache wt Malloc0 Malloc1 --cache-line-size 8 +$rpc_py bdev_ocf_create FullCache wt Malloc0 Malloc1 $rpc_py bdev_ocf_get_bdevs FullCache | jq -e \ '.[0] | .started and .cache.attached and .core.attached' @@ -59,7 +59,7 @@ if bdev_check_claimed Malloc0 && bdev_check_claimed Malloc1; then exit 1 fi -$rpc_py bdev_ocf_create HotCache wt Malloc0 Malloc1 --cache-line-size 16 +$rpc_py bdev_ocf_create HotCache wt Malloc0 Malloc1 if ! (bdev_check_claimed Malloc0 && bdev_check_claimed Malloc1); then echo >&2 "Base devices expected to be claimed now"