From d7ad7bca3c9fce85ca069070c2e468ac1931de6e Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Wed, 7 Dec 2022 17:09:42 +0900 Subject: [PATCH] bdev: Add mode to bdev_reset_iostat RPC to reset only max/min fields Both max and min should be reset periodically. We can use the queue depth sampling poller to reset these but the queue depth sampling poller is optional. We extend the bdev_reset_iostat RPC to support mode to reset all or only max/min fields. Signed-off-by: Shuhei Matsumoto Change-Id: I9ce54892f6e808f6a82754b6930092f3a16d51ff Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15444 Reviewed-by: Jim Harris Reviewed-by: Aleksey Marchuk Tested-by: SPDK CI Jenkins Community-CI: Mellanox Build Bot --- doc/jsonrpc.md | 1 + lib/bdev/bdev.c | 21 +++++++++++++++------ lib/bdev/bdev_internal.h | 9 +++++++-- lib/bdev/bdev_rpc.c | 31 +++++++++++++++++++++++++++---- python/spdk/rpc/bdev.py | 6 +++++- scripts/rpc.py | 3 ++- 6 files changed, 57 insertions(+), 14 deletions(-) diff --git a/doc/jsonrpc.md b/doc/jsonrpc.md index d27d231a3..2fd7ce6e3 100644 --- a/doc/jsonrpc.md +++ b/doc/jsonrpc.md @@ -2132,6 +2132,7 @@ a block device may be specified by name. Name | Optional | Type | Description ----------------------- | -------- | ----------- | ----------- name | Optional | string | Block device name +mode | Optional | string | Mode to reset I/O statistics: all, maxmin (default: all) #### Example diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index 07ebb80ff..05bf7fd1d 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -3718,7 +3718,7 @@ bdev_io_stat_get(struct spdk_bdev_io_stat *to_stat, struct spdk_bdev_io_stat *fr } static void -bdev_io_stat_reset(struct spdk_bdev_io_stat *stat) +bdev_io_stat_reset(struct spdk_bdev_io_stat *stat, enum bdev_reset_stat_mode mode) { stat->max_read_latency_ticks = 0; stat->min_read_latency_ticks = UINT64_MAX; @@ -3728,6 +3728,11 @@ bdev_io_stat_reset(struct spdk_bdev_io_stat *stat) stat->min_unmap_latency_ticks = UINT64_MAX; stat->max_copy_latency_ticks = 0; stat->min_copy_latency_ticks = UINT64_MAX; + + if (mode != BDEV_RESET_STAT_ALL) { + return; + } + stat->bytes_read = 0; stat->num_read_ops = 0; stat->bytes_written = 0; @@ -3746,7 +3751,7 @@ bdev_io_stat_alloc(void) stat = malloc(sizeof(struct spdk_bdev_io_stat)); if (stat != NULL) { - bdev_io_stat_reset(stat); + bdev_io_stat_reset(stat, BDEV_RESET_STAT_ALL); } return stat; @@ -5758,6 +5763,7 @@ spdk_bdev_get_device_stat(struct spdk_bdev *bdev, struct spdk_bdev_io_stat *stat } struct bdev_iostat_reset_ctx { + enum bdev_reset_stat_mode mode; bdev_reset_device_stat_cb cb; void *cb_arg; }; @@ -5774,17 +5780,19 @@ bdev_reset_device_stat_done(struct spdk_bdev *bdev, void *_ctx, int status) static void bdev_reset_each_channel_stat(struct spdk_bdev_channel_iter *i, struct spdk_bdev *bdev, - struct spdk_io_channel *ch, void *ctx) + struct spdk_io_channel *ch, void *_ctx) { + struct bdev_iostat_reset_ctx *ctx = _ctx; struct spdk_bdev_channel *channel = __io_ch_to_bdev_ch(ch); - bdev_io_stat_reset(channel->stat); + bdev_io_stat_reset(channel->stat, ctx->mode); spdk_bdev_for_each_channel_continue(i, 0); } void -bdev_reset_device_stat(struct spdk_bdev *bdev, bdev_reset_device_stat_cb cb, void *cb_arg) +bdev_reset_device_stat(struct spdk_bdev *bdev, enum bdev_reset_stat_mode mode, + bdev_reset_device_stat_cb cb, void *cb_arg) { struct bdev_iostat_reset_ctx *ctx; @@ -5798,11 +5806,12 @@ bdev_reset_device_stat(struct spdk_bdev *bdev, bdev_reset_device_stat_cb cb, voi return; } + ctx->mode = mode; ctx->cb = cb; ctx->cb_arg = cb_arg; spdk_spin_lock(&bdev->internal.spinlock); - bdev_io_stat_reset(bdev->internal.stat); + bdev_io_stat_reset(bdev->internal.stat, mode); spdk_spin_unlock(&bdev->internal.spinlock); spdk_bdev_for_each_channel(bdev, diff --git a/lib/bdev/bdev_internal.h b/lib/bdev/bdev_internal.h index acbf38d68..6af961acc 100644 --- a/lib/bdev/bdev_internal.h +++ b/lib/bdev/bdev_internal.h @@ -25,9 +25,14 @@ struct spdk_bdev_io_stat *bdev_io_stat_alloc(void); void bdev_io_stat_free(struct spdk_bdev_io_stat *stat); void bdev_io_stat_dump_json(struct spdk_bdev_io_stat *stat, struct spdk_json_write_ctx *w); +enum bdev_reset_stat_mode { + BDEV_RESET_STAT_ALL, + BDEV_RESET_STAT_MAXMIN, +}; + typedef void (*bdev_reset_device_stat_cb)(struct spdk_bdev *bdev, void *cb_arg, int rc); -void bdev_reset_device_stat(struct spdk_bdev *bdev, bdev_reset_device_stat_cb cb, - void *cb_arg); +void bdev_reset_device_stat(struct spdk_bdev *bdev, enum bdev_reset_stat_mode mode, + bdev_reset_device_stat_cb cb, void *cb_arg); #endif /* SPDK_BDEV_INTERNAL_H */ diff --git a/lib/bdev/bdev_rpc.c b/lib/bdev/bdev_rpc.c index 396906ebf..c125f16e0 100644 --- a/lib/bdev/bdev_rpc.c +++ b/lib/bdev/bdev_rpc.c @@ -462,6 +462,7 @@ struct rpc_reset_iostat_ctx { int rc; struct spdk_jsonrpc_request *request; struct spdk_json_write_ctx *w; + enum bdev_reset_stat_mode mode; }; struct bdev_reset_iostat_ctx { @@ -527,13 +528,14 @@ bdev_reset_iostat(void *ctx, struct spdk_bdev *bdev) rpc_ctx->bdev_count++; bdev_ctx->rpc_ctx = rpc_ctx; - bdev_reset_device_stat(bdev, bdev_reset_iostat_done, bdev_ctx); + bdev_reset_device_stat(bdev, rpc_ctx->mode, bdev_reset_iostat_done, bdev_ctx); return 0; } struct rpc_bdev_reset_iostat { char *name; + enum bdev_reset_stat_mode mode; }; static void @@ -542,14 +544,32 @@ free_rpc_bdev_reset_iostat(struct rpc_bdev_reset_iostat *r) free(r->name); } +static int +rpc_decode_reset_iostat_mode(const struct spdk_json_val *val, void *out) +{ + enum bdev_reset_stat_mode *mode = out; + + if (spdk_json_strequal(val, "all") == true) { + *mode = BDEV_RESET_STAT_ALL; + } else if (spdk_json_strequal(val, "maxmin") == true) { + *mode = BDEV_RESET_STAT_MAXMIN; + } else { + SPDK_NOTICELOG("Invalid parameter value: mode\n"); + return -EINVAL; + } + + return 0; +} + static const struct spdk_json_object_decoder rpc_bdev_reset_iostat_decoders[] = { {"name", offsetof(struct rpc_bdev_reset_iostat, name), spdk_json_decode_string, true}, + {"mode", offsetof(struct rpc_bdev_reset_iostat, mode), rpc_decode_reset_iostat_mode, true}, }; static void rpc_bdev_reset_iostat(struct spdk_jsonrpc_request *request, const struct spdk_json_val *params) { - struct rpc_bdev_reset_iostat req = {}; + struct rpc_bdev_reset_iostat req = { .mode = BDEV_RESET_STAT_ALL, }; struct spdk_bdev_desc *desc = NULL; struct rpc_reset_iostat_ctx *rpc_ctx; struct bdev_reset_iostat_ctx *bdev_ctx; @@ -577,12 +597,12 @@ rpc_bdev_reset_iostat(struct spdk_jsonrpc_request *request, const struct spdk_js } } - free_rpc_bdev_reset_iostat(&req); rpc_ctx = calloc(1, sizeof(struct rpc_reset_iostat_ctx)); if (rpc_ctx == NULL) { SPDK_ERRLOG("Failed to allocate rpc_iostat_ctx struct\n"); spdk_jsonrpc_send_error_response(request, -ENOMEM, spdk_strerror(ENOMEM)); + free_rpc_bdev_reset_iostat(&req); return; } @@ -592,6 +612,9 @@ rpc_bdev_reset_iostat(struct spdk_jsonrpc_request *request, const struct spdk_js */ rpc_ctx->bdev_count++; rpc_ctx->request = request; + rpc_ctx->mode = req.mode; + + free_rpc_bdev_reset_iostat(&req); if (desc != NULL) { bdev_ctx = calloc(1, sizeof(struct bdev_reset_iostat_ctx)); @@ -605,7 +628,7 @@ rpc_bdev_reset_iostat(struct spdk_jsonrpc_request *request, const struct spdk_js rpc_ctx->bdev_count++; bdev_ctx->rpc_ctx = rpc_ctx; - bdev_reset_device_stat(spdk_bdev_desc_get_bdev(desc), + bdev_reset_device_stat(spdk_bdev_desc_get_bdev(desc), rpc_ctx->mode, bdev_reset_iostat_done, bdev_ctx); } } else { diff --git a/python/spdk/rpc/bdev.py b/python/spdk/rpc/bdev.py index 869810fc7..7819233d1 100644 --- a/python/spdk/rpc/bdev.py +++ b/python/spdk/rpc/bdev.py @@ -1523,15 +1523,19 @@ def bdev_get_iostat(client, name=None, per_channel=None): return client.call('bdev_get_iostat', params) -def bdev_reset_iostat(client, name=None): +def bdev_reset_iostat(client, name=None, mode=None): """Reset I/O statistics for block devices. Args: name: bdev name to reset (optional; if omitted, reset all bdevs) + mode: mode to reset: all, maxmin (optional: if omitted, reset all fields) """ params = {} if name: params['name'] = name + if mode: + params['mode'] = mode + return client.call('bdev_reset_iostat', params) diff --git a/scripts/rpc.py b/scripts/rpc.py index c42f3553f..7d774f44b 100755 --- a/scripts/rpc.py +++ b/scripts/rpc.py @@ -1142,11 +1142,12 @@ if __name__ == "__main__": p.set_defaults(func=bdev_get_iostat) def bdev_reset_iostat(args): - rpc.bdev.bdev_reset_iostat(args.client, name=args.name) + rpc.bdev.bdev_reset_iostat(args.client, name=args.name, mode=args.mode) p = subparsers.add_parser('bdev_reset_iostat', help='Reset I/O statistics of all the blockdevs or specified blockdev.') p.add_argument('-b', '--name', help="Name of the Blockdev. Example: Nvme0n1", required=False) + p.add_argument('-m', '--mode', help="Mode to reset I/O statistics", choices=['all', 'maxmin'], required=False) p.set_defaults(func=bdev_reset_iostat) def bdev_enable_histogram(args):