From 52a9661d94aa3e2677badfe0cef4da3caf900d8a Mon Sep 17 00:00:00 2001 From: Wojciech Malikowski Date: Tue, 10 Sep 2019 09:01:27 -0400 Subject: [PATCH] bdev/ftl: Remove punit parameter from ftl bdev configuration FTL library is consuming whole OCSSD device so punit parameter is not needed for bdev ftl configuration. Change-Id: I56f62ea6d09b3157b70c02ccfffcd3cb07ba4597 Signed-off-by: Wojciech Malikowski Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/467950 Reviewed-by: Konrad Sztyber Reviewed-by: Shuhei Matsumoto Reviewed-by: Jim Harris Tested-by: SPDK CI Jenkins Community-CI: Broadcom SPDK FC-NVMe CI --- lib/ftl/ftl_init.c | 4 +- module/bdev/nvme/bdev_ftl.c | 72 +-------------------------------- module/bdev/nvme/bdev_ftl.h | 4 -- module/bdev/nvme/bdev_ftl_rpc.c | 14 ------- scripts/gen_ftl.sh | 21 +++++----- scripts/rpc.py | 3 -- scripts/rpc/bdev.py | 6 +-- test/ftl/bdevperf.sh | 2 +- test/ftl/common.sh | 10 +++++ test/ftl/dirty_shutdown.sh | 8 ++-- test/ftl/fio.sh | 4 +- test/ftl/ftl.sh | 2 +- test/ftl/json.sh | 12 ++---- test/ftl/restore.sh | 9 +++-- 14 files changed, 42 insertions(+), 129 deletions(-) diff --git a/lib/ftl/ftl_init.c b/lib/ftl/ftl_init.c index e8b6f3d64..afe2ccc59 100644 --- a/lib/ftl/ftl_init.c +++ b/lib/ftl/ftl_init.c @@ -1106,7 +1106,6 @@ spdk_ftl_dev_init(const struct spdk_ftl_dev_init_opts *_opts, spdk_ftl_init_fn c dev->init_ctx.cb_fn = cb_fn; dev->init_ctx.cb_arg = cb_arg; dev->init_ctx.thread = spdk_get_thread(); - dev->range = opts.range; dev->limit = SPDK_FTL_LIMIT_MAX; dev->name = strdup(opts.name); @@ -1127,6 +1126,9 @@ spdk_ftl_dev_init(const struct spdk_ftl_dev_init_opts *_opts, spdk_ftl_init_fn c goto fail_sync; } + dev->range.begin = 0; + dev->range.end = dev->geo.num_pu * dev->geo.num_grp - 1; + if (ftl_check_init_opts(&opts, &dev->geo)) { SPDK_ERRLOG("Invalid device configuration\n"); goto fail_sync; diff --git a/module/bdev/nvme/bdev_ftl.c b/module/bdev/nvme/bdev_ftl.c index 6bc57aa5f..4f9c98113 100644 --- a/module/bdev/nvme/bdev_ftl.c +++ b/module/bdev/nvme/bdev_ftl.c @@ -418,7 +418,6 @@ _bdev_ftl_write_config_info(struct ftl_bdev *ftl_bdev, struct spdk_json_write_ct } spdk_json_write_named_string(w, "traddr", ftl_bdev->ctrlr->trid.traddr); - spdk_json_write_named_string_fmt(w, "punits", "%d-%d", attrs.range.begin, attrs.range.end); if (ftl_bdev->cache_bdev_desc) { cache_bdev = spdk_bdev_get_name(spdk_bdev_desc_get_bdev(ftl_bdev->cache_bdev_desc)); @@ -492,59 +491,6 @@ static const struct spdk_bdev_fn_table ftl_fn_table = { .dump_info_json = bdev_ftl_dump_info_json, }; -int -bdev_ftl_parse_punits(struct spdk_ftl_punit_range *range, const char *range_string) -{ - regex_t range_regex; - regmatch_t range_match; - unsigned long begin = 0, end = 0; - char *str_ptr; - int rc = -1; - - if (regcomp(&range_regex, "\\b[[:digit:]]+-[[:digit:]]+\\b", REG_EXTENDED)) { - SPDK_ERRLOG("Regex init error\n"); - return -1; - } - - if (regexec(&range_regex, range_string, 1, &range_match, 0)) { - SPDK_WARNLOG("Invalid range\n"); - goto out; - } - - errno = 0; - begin = strtoul(range_string + range_match.rm_so, &str_ptr, 10); - if ((begin == ULONG_MAX && errno == ERANGE) || (begin == 0 && errno == EINVAL)) { - SPDK_WARNLOG("Invalid range '%s'\n", range_string); - goto out; - } - - errno = 0; - /* +1 to skip the '-' delimiter */ - end = strtoul(str_ptr + 1, NULL, 10); - if ((end == ULONG_MAX && errno == ERANGE) || (end == 0 && errno == EINVAL)) { - SPDK_WARNLOG("Invalid range '%s'\n", range_string); - goto out; - } - - if (begin > UINT_MAX || end > UINT_MAX) { - SPDK_WARNLOG("Invalid range '%s'\n", range_string); - goto out; - } - - if (begin > end) { - SPDK_WARNLOG("Invalid range '%s'\n", range_string); - goto out; - } - - range->begin = (unsigned int)begin; - range->end = (unsigned int)end; - - rc = 0; -out: - regfree(&range_regex); - return rc; -} - static int bdev_ftl_defer_init(struct ftl_bdev_init_opts *opts) { @@ -599,19 +545,6 @@ bdev_ftl_read_bdev_config(struct spdk_conf_section *sp, opts->name = val; val = spdk_conf_section_get_nmval(sp, "TransportID", i, 2); - if (!val) { - SPDK_ERRLOG("No punit range provided for TransportID: %s\n", trid); - rc = -1; - break; - } - - if (bdev_ftl_parse_punits(&opts->range, val)) { - SPDK_ERRLOG("Invalid punit range for TransportID: %s\n", trid); - rc = -1; - break; - } - - val = spdk_conf_section_get_nmval(sp, "TransportID", i, 3); if (!val) { SPDK_ERRLOG("No UUID provided for TransportID: %s\n", trid); rc = -1; @@ -631,7 +564,7 @@ bdev_ftl_read_bdev_config(struct spdk_conf_section *sp, opts->mode = 0; } - val = spdk_conf_section_get_nmval(sp, "TransportID", i, 4); + val = spdk_conf_section_get_nmval(sp, "TransportID", i, 3); if (!val) { continue; } @@ -741,8 +674,6 @@ bdev_ftl_create_cb(struct spdk_ftl_dev *dev, void *ctx, int status) SPDK_DEBUGLOG(SPDK_LOG_BDEV_FTL, "Creating bdev %s:\n", ftl_bdev->bdev.name); SPDK_DEBUGLOG(SPDK_LOG_BDEV_FTL, "\tblock_len:\t%zu\n", attrs.lbk_size); SPDK_DEBUGLOG(SPDK_LOG_BDEV_FTL, "\tblock_cnt:\t%"PRIu64"\n", attrs.lbk_cnt); - SPDK_DEBUGLOG(SPDK_LOG_BDEV_FTL, "\tpunits:\t\t%u-%u\n", attrs.range.begin, - attrs.range.end); ftl_bdev->bdev.ctxt = ftl_bdev; ftl_bdev->bdev.fn_table = &ftl_fn_table; @@ -841,7 +772,6 @@ bdev_ftl_create(struct spdk_nvme_ctrlr *ctrlr, const struct ftl_bdev_init_opts * opts.ctrlr = ctrlr; opts.trid = bdev_opts->trid; - opts.range = bdev_opts->range; opts.mode = bdev_opts->mode; opts.uuid = bdev_opts->uuid; opts.name = ftl_bdev->bdev.name; diff --git a/module/bdev/nvme/bdev_ftl.h b/module/bdev/nvme/bdev_ftl.h index a815b1344..6ce1a7702 100644 --- a/module/bdev/nvme/bdev_ftl.h +++ b/module/bdev/nvme/bdev_ftl.h @@ -41,7 +41,6 @@ #define FTL_MAX_CONTROLLERS 64 #define FTL_MAX_BDEVS (FTL_MAX_CONTROLLERS * 128) -#define FTL_RANGE_MAX_LENGTH 32 struct spdk_bdev; struct spdk_uuid; @@ -54,8 +53,6 @@ struct ftl_bdev_info { struct ftl_bdev_init_opts { /* NVMe controller's transport ID */ struct spdk_nvme_transport_id trid; - /* Parallel unit range */ - struct spdk_ftl_punit_range range; /* Bdev's name */ const char *name; /* Write buffer bdev's name */ @@ -70,7 +67,6 @@ struct ftl_bdev_init_opts { typedef void (*ftl_bdev_init_fn)(const struct ftl_bdev_info *, void *, int); -int bdev_ftl_parse_punits(struct spdk_ftl_punit_range *range, const char *range_string); int bdev_ftl_init_bdev(struct ftl_bdev_init_opts *opts, ftl_bdev_init_fn cb, void *cb_arg); void bdev_ftl_delete_bdev(const char *name, spdk_bdev_unregister_cb cb_fn, void *cb_arg); diff --git a/module/bdev/nvme/bdev_ftl_rpc.c b/module/bdev/nvme/bdev_ftl_rpc.c index 314482928..923cefb15 100644 --- a/module/bdev/nvme/bdev_ftl_rpc.c +++ b/module/bdev/nvme/bdev_ftl_rpc.c @@ -43,7 +43,6 @@ struct rpc_bdev_ftl_create { char *name; char *trtype; char *traddr; - char *punits; char *uuid; char *cache_bdev; struct spdk_ftl_conf ftl_conf; @@ -55,7 +54,6 @@ free_rpc_bdev_ftl_create(struct rpc_bdev_ftl_create *req) free(req->name); free(req->trtype); free(req->traddr); - free(req->punits); free(req->uuid); free(req->cache_bdev); } @@ -64,7 +62,6 @@ static const struct spdk_json_object_decoder rpc_bdev_ftl_create_decoders[] = { {"name", offsetof(struct rpc_bdev_ftl_create, name), spdk_json_decode_string}, {"trtype", offsetof(struct rpc_bdev_ftl_create, trtype), spdk_json_decode_string}, {"traddr", offsetof(struct rpc_bdev_ftl_create, traddr), spdk_json_decode_string}, - {"punits", offsetof(struct rpc_bdev_ftl_create, punits), spdk_json_decode_string}, {"uuid", offsetof(struct rpc_bdev_ftl_create, uuid), spdk_json_decode_string, true}, {"cache", offsetof(struct rpc_bdev_ftl_create, cache_bdev), spdk_json_decode_string, true}, { @@ -125,8 +122,6 @@ static const struct spdk_json_object_decoder rpc_bdev_ftl_create_decoders[] = { }, }; -#define FTL_RANGE_MAX_LENGTH 32 - static void _spdk_rpc_bdev_ftl_create_cb(const struct ftl_bdev_info *bdev_info, void *ctx, int status) { @@ -156,7 +151,6 @@ spdk_rpc_bdev_ftl_create(struct spdk_jsonrpc_request *request, { struct rpc_bdev_ftl_create req = {}; struct ftl_bdev_init_opts opts = {}; - char range[FTL_RANGE_MAX_LENGTH]; int rc; spdk_ftl_conf_init_defaults(&req.ftl_conf); @@ -198,14 +192,6 @@ spdk_rpc_bdev_ftl_create(struct spdk_jsonrpc_request *request, /* Parse traddr */ snprintf(opts.trid.traddr, sizeof(opts.trid.traddr), "%s", req.traddr); - snprintf(range, sizeof(range), "%s", req.punits); - - if (bdev_ftl_parse_punits(&opts.range, req.punits)) { - spdk_jsonrpc_send_error_response_fmt(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, - "Failed to parse parallel unit range: %s", - req.punits); - goto invalid; - } if (req.uuid) { if (spdk_uuid_parse(&opts.uuid, req.uuid) < 0) { diff --git a/scripts/gen_ftl.sh b/scripts/gen_ftl.sh index c68634ca7..e9d1311aa 100755 --- a/scripts/gen_ftl.sh +++ b/scripts/gen_ftl.sh @@ -5,20 +5,19 @@ set -e rootdir=$(readlink -f $(dirname $0))/.. function usage { - echo "Usage: [-j] $0 -a TRANSPORT_ADDR -n BDEV_NAME -l PUNITS [-u UUID] [-c CACHE]" + echo "Usage: [-j] $0 -a TRANSPORT_ADDR -n BDEV_NAME [-u UUID] [-c CACHE]" echo "UUID is required when restoring device state" echo echo "-j json format" echo "TRANSPORT_ADDR - SSD's PCIe address" echo "BDEV_NAME - name of the bdev" - echo "PUNITS - bdev's parallel unit range (e.g. 0-3)" echo "UUID - bdev's uuid (used when in restore mode)" echo "CACHE - name of the bdev to be used as write buffer cache" } function create_classic_config { echo "[Ftl]" - echo " TransportID \"trtype:PCIe traddr:$1\" $2 $3 $4 $5" + echo " TransportID \"trtype:PCIe traddr:$1\" $2 $3 $4" } function create_json_config() @@ -32,12 +31,11 @@ function create_json_config() echo "\"name\": \"$2\"," echo '"trtype": "PCIe",' echo "\"traddr\": \"$1\"," - echo "\"punits\": \"$3\"," - if [ -n "$5" ]; then - echo "\"uuid\": \"$4\"," - echo "\"cache\": \"$5\"" + if [ -n "$4" ]; then + echo "\"uuid\": \"$3\"," + echo "\"cache\": \"$4\"" else - echo "\"uuid\": \"$4\"" + echo "\"uuid\": \"$3\"" fi echo '}' echo '}' @@ -52,7 +50,6 @@ while getopts "ja:n:l:m:u:c:h" arg; do j) json=1 ;; a) addr=$OPTARG ;; n) name=$OPTARG ;; - l) punits=$OPTARG ;; u) uuid=$OPTARG ;; c) cache=$OPTARG ;; h) usage @@ -62,13 +59,13 @@ while getopts "ja:n:l:m:u:c:h" arg; do esac done -if [[ -z "$addr" || -z "$name" || -z "$punits" ]]; then +if [[ -z "$addr" || -z "$name" ]]; then usage exit 1 fi if [ -n "$json" ]; then - create_json_config $addr $name $punits $uuid $cache + create_json_config $addr $name $uuid $cache else - create_classic_config $addr $name $punits $uuid $cache + create_classic_config $addr $name $uuid $cache fi diff --git a/scripts/rpc.py b/scripts/rpc.py index 7683d9b9a..cb1bb3340 100755 --- a/scripts/rpc.py +++ b/scripts/rpc.py @@ -1542,7 +1542,6 @@ Format: 'user:u1 secret:s1 muser:mu1 msecret:ms1,user:u2 secret:s2 muser:mu2 mse name=args.name, trtype=args.trtype, traddr=args.traddr, - punits=args.punits, uuid=args.uuid, cache=args.cache, allow_open_bands=args.allow_open_bands, @@ -1556,8 +1555,6 @@ Format: 'user:u1 secret:s1 muser:mu1 msecret:ms1,user:u2 secret:s2 muser:mu2 mse help='NVMe target trtype: e.g., pcie', default='pcie') p.add_argument('-a', '--traddr', help='NVMe target address: e.g., an ip address or BDF', required=True) - p.add_argument('-l', '--punits', help='Parallel unit range in the form of start-end: e.g. 4-8', - required=True) p.add_argument('-u', '--uuid', help='UUID of restored bdev (not applicable when creating new ' 'instance): e.g. b286d19a-0059-4709-abcd-9f7732b1567d (optional)') p.add_argument('-c', '--cache', help='Name of the bdev to be used as a write buffer cache (optional)') diff --git a/scripts/rpc/bdev.py b/scripts/rpc/bdev.py index 9f11fa51a..2929947e9 100644 --- a/scripts/rpc/bdev.py +++ b/scripts/rpc/bdev.py @@ -884,20 +884,18 @@ def bdev_split_delete(client, base_bdev): @deprecated_alias('construct_ftl_bdev') -def bdev_ftl_create(client, name, trtype, traddr, punits, **kwargs): +def bdev_ftl_create(client, name, trtype, traddr, **kwargs): """Construct FTL bdev Args: name: name of the bdev trtype: transport type traddr: transport address - punit: parallel unit range kwargs: optional parameters """ params = {'name': name, 'trtype': trtype, - 'traddr': traddr, - 'punits': punits} + 'traddr': traddr} for key, value in kwargs.items(): if value is not None: params[key] = value diff --git a/test/ftl/bdevperf.sh b/test/ftl/bdevperf.sh index 1399d33b2..177e252cd 100755 --- a/test/ftl/bdevperf.sh +++ b/test/ftl/bdevperf.sh @@ -8,7 +8,7 @@ tests=('-q 1 -w randwrite -t 4 -o 69632' '-q 128 -w randwrite -t 4 -o 4096' '-q device=$1 ftl_bdev_conf=$testdir/config/ftl.conf -$rootdir/scripts/gen_ftl.sh -a $device -n nvme0 -l 0-3 > $ftl_bdev_conf +$rootdir/scripts/gen_ftl.sh -a $device -n nvme0 > $ftl_bdev_conf for (( i=0; i<${#tests[@]}; i++ )) do timing_enter "${tests[$i]}" diff --git a/test/ftl/common.sh b/test/ftl/common.sh index 73ffc8835..0e0fb5864 100644 --- a/test/ftl/common.sh +++ b/test/ftl/common.sh @@ -5,6 +5,16 @@ function get_chunk_size() { grep 'Logical blks per chunk' | sed 's/[^0-9]//g' } +function get_num_group() { + $rootdir/examples/nvme/identify/identify -r "trtype:PCIe traddr:$1" | + grep 'Groups' | sed 's/[^0-9]//g' +} + +function get_num_pu() { + $rootdir/examples/nvme/identify/identify -r "trtype:PCIe traddr:$1" | + grep 'PUs' | sed 's/[^0-9]//g' +} + function has_separate_md() { local md_type md_type=$($rootdir/examples/nvme/identify/identify -r "trtype:PCIe traddr:$1" | \ diff --git a/test/ftl/dirty_shutdown.sh b/test/ftl/dirty_shutdown.sh index 17f7a80f5..73055cc4c 100755 --- a/test/ftl/dirty_shutdown.sh +++ b/test/ftl/dirty_shutdown.sh @@ -6,8 +6,6 @@ source $rootdir/test/common/autotest_common.sh source $testdir/common.sh rpc_py=$rootdir/scripts/rpc.py -pu_start=0 -pu_end=3 while getopts ':u:c:' opt; do case $opt in @@ -33,7 +31,9 @@ restore_kill() { trap "restore_kill; exit 1" SIGINT SIGTERM EXIT chunk_size=$(get_chunk_size $device) -pu_count=$((pu_end - pu_start + 1)) +num_group=$(get_num_group $device) +num_pu=$(get_num_pu $device) +pu_count=$((num_group * num_pu)) # Write one band worth of data + one extra chunk data_size=$((chunk_size * (pu_count + 1))) @@ -45,7 +45,7 @@ if [ -n "$nv_cache" ]; then nvc_bdev=$(create_nv_cache_bdev nvc0 $device $nv_cache $pu_count) fi -ftl_construct_args="bdev_ftl_create -b nvme0 -a $device -l $pu_start-$pu_end -o" +ftl_construct_args="bdev_ftl_create -b nvme0 -a $device -o" [ -n "$nvc_bdev" ] && ftl_construct_args+=" -c $nvc_bdev" [ -n "$uuid" ] && ftl_construct_args+=" -u $uuid" diff --git a/test/ftl/fio.sh b/test/ftl/fio.sh index 018542902..d9ce20899 100755 --- a/test/ftl/fio.sh +++ b/test/ftl/fio.sh @@ -26,9 +26,9 @@ export FTL_BDEV_CONF=$testdir/config/ftl.conf export FTL_BDEV_NAME=nvme0 if [ -z "$uuid" ]; then - $rootdir/scripts/gen_ftl.sh -a $device -n nvme0 -l 0-3 > $FTL_BDEV_CONF + $rootdir/scripts/gen_ftl.sh -a $device -n nvme0 > $FTL_BDEV_CONF else - $rootdir/scripts/gen_ftl.sh -a $device -n nvme0 -l 0-3 -u $uuid > $FTL_BDEV_CONF + $rootdir/scripts/gen_ftl.sh -a $device -n nvme0 -u $uuid > $FTL_BDEV_CONF fi for test in ${tests}; do diff --git a/test/ftl/ftl.sh b/test/ftl/ftl.sh index dfbe251ee..fb7eaef9a 100755 --- a/test/ftl/ftl.sh +++ b/test/ftl/ftl.sh @@ -67,7 +67,7 @@ if [ $SPDK_TEST_FTL_EXTENDED -eq 1 ]; then trap 'killprocess $svc_pid; exit 1' SIGINT SIGTERM EXIT waitforlisten $svc_pid - uuid=$($rpc_py bdev_ftl_create -b nvme0 -a $device -l 0-3 | jq -r '.uuid') + uuid=$($rpc_py bdev_ftl_create -b nvme0 -a $device | jq -r '.uuid') killprocess $svc_pid trap - SIGINT SIGTERM EXIT diff --git a/test/ftl/json.sh b/test/ftl/json.sh index febc59e1c..463b5605e 100755 --- a/test/ftl/json.sh +++ b/test/ftl/json.sh @@ -20,21 +20,17 @@ $rootdir/app/spdk_tgt/spdk_tgt & svcpid=$! waitforlisten $svcpid # Create new bdev from json configuration -$rootdir/scripts/gen_ftl.sh -j -a $device -n nvme0 -l 0-1 | $rpc_py load_subsystem_config +$rootdir/scripts/gen_ftl.sh -j -a $device -n nvme0 | $rpc_py load_subsystem_config uuid=$($rpc_py bdev_get_bdevs | jq -r '.[0].uuid') $rpc_py bdev_ftl_delete -b nvme0 # Restore bdev from json configuration -$rootdir/scripts/gen_ftl.sh -j -a $device -n nvme0 -l 0-1 -u $uuid | $rpc_py load_subsystem_config -# Create new bdev from json configuration -$rootdir/scripts/gen_ftl.sh -j -a $device -n nvme1 -l 2-2 | $rpc_py load_subsystem_config -# Create new bdev from RPC -$rpc_py bdev_ftl_create -b nvme2 -a $device -l 3-3 - -$rpc_py bdev_ftl_delete -b nvme2 +$rootdir/scripts/gen_ftl.sh -j -a $device -n nvme0 -u $uuid | $rpc_py load_subsystem_config $rpc_py bdev_ftl_delete -b nvme0 +# Create new bdev from RPC +$rpc_py bdev_ftl_create -b nvme1 -a $device $rpc_py bdev_ftl_delete -b nvme1 # TODO: add negative test cases diff --git a/test/ftl/restore.sh b/test/ftl/restore.sh index 5661afc51..fc79d7b3a 100755 --- a/test/ftl/restore.sh +++ b/test/ftl/restore.sh @@ -8,8 +8,6 @@ source $testdir/common.sh rpc_py=$rootdir/scripts/rpc.py mount_dir=$(mktemp -d) -pu_start=0 -pu_end=3 while getopts ':u:c:' opt; do case $opt in @@ -20,6 +18,9 @@ while getopts ':u:c:' opt; do done shift $((OPTIND -1)) device=$1 +num_group=$(get_num_group $device) +num_pu=$(get_num_pu $device) +pu_count=$((num_group * num_pu)) restore_kill() { if mount | grep $mount_dir; then @@ -42,10 +43,10 @@ $rootdir/app/spdk_tgt/spdk_tgt & svcpid=$! waitforlisten $svcpid if [ -n "$nv_cache" ]; then - nvc_bdev=$(create_nv_cache_bdev nvc0 $device $nv_cache $((pu_end - pu_start + 1))) + nvc_bdev=$(create_nv_cache_bdev nvc0 $device $nv_cache $pu_count) fi -ftl_construct_args="bdev_ftl_create -b nvme0 -a $device -l ${pu_start}-${pu_end}" +ftl_construct_args="bdev_ftl_create -b nvme0 -a $device" [ -n "$uuid" ] && ftl_construct_args+=" -u $uuid" [ -n "$nv_cache" ] && ftl_construct_args+=" -c $nvc_bdev"