From bc006662710d3c824f67721a01a0ff921dc528ef Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Thu, 21 Dec 2017 11:56:38 -0700 Subject: [PATCH] util: enable dynamic spdk_histogram_data sizing Currently, each histogram range is hardcoded to 128 buckets (1ULL << 7), resulting in 58 ranges (64 - 7 + 1) and a total size of 58 * 128 * sizeof(uint64_t) = 59392 bytes. To allow for more usage models in cases where this size is prohibitive, enable the following changes: 1) specify number of buckets per range (in number of bits) 2) specify max datapoint value (in number of bits) The latter can be useful for cases where datapoints are never close to requiring all 64 bits - and allows reducing the number of ranges. Any data points that exceed the max will be tallied in the last bucket in the last range. Testing shows no performance disadvantage using the dynamic approach. Signed-off-by: Jim Harris Change-Id: I5979bcdff6209faaa9dee293918ef2a78679bcd4 Reviewed-on: https://review.gerrithub.io/392707 Tested-by: SPDK Automated Test System Reviewed-by: Daniel Verkamp Reviewed-by: Ben Walker --- examples/nvme/perf/perf.c | 11 ++-- include/spdk/histogram_data.h | 58 ++++++++++++++++--- test/lib/nvme/overhead/overhead.c | 18 +++--- test/lib/util/histogram_perf/histogram_perf.c | 8 ++- 4 files changed, 70 insertions(+), 25 deletions(-) diff --git a/examples/nvme/perf/perf.c b/examples/nvme/perf/perf.c index e3645e0b7..71de1db5e 100644 --- a/examples/nvme/perf/perf.c +++ b/examples/nvme/perf/perf.c @@ -128,7 +128,7 @@ struct ns_worker_ctx { struct ns_worker_ctx *next; - struct spdk_histogram_data histogram; + struct spdk_histogram_data *histogram; }; struct perf_task { @@ -626,7 +626,7 @@ task_complete(struct perf_task *task) ns_ctx->max_tsc = tsc_diff; } if (g_latency_sw_tracking_level > 0) { - spdk_histogram_data_tally(&ns_ctx->histogram, tsc_diff); + spdk_histogram_data_tally(ns_ctx->histogram, tsc_diff); } /* @@ -950,7 +950,7 @@ print_performance(void) printf("Summary latency data for %-43.43s from core %u:\n", ns_ctx->entry->name, worker->lcore); printf("=================================================================================\n"); - spdk_histogram_data_iterate(&ns_ctx->histogram, check_cutoff, &cutoff); + spdk_histogram_data_iterate(ns_ctx->histogram, check_cutoff, &cutoff); printf("\n"); ns_ctx = ns_ctx->next; @@ -970,7 +970,7 @@ print_performance(void) printf("==============================================================================\n"); printf(" Range in us Cumulative IO count\n"); - spdk_histogram_data_iterate(&ns_ctx->histogram, print_bucket, NULL); + spdk_histogram_data_iterate(ns_ctx->histogram, print_bucket, NULL); printf("\n"); ns_ctx = ns_ctx->next; } @@ -1325,6 +1325,7 @@ unregister_workers(void) while (ns_ctx) { struct ns_worker_ctx *next_ns_ctx = ns_ctx->next; + spdk_histogram_data_free(ns_ctx->histogram); free(ns_ctx); ns_ctx = next_ns_ctx; } @@ -1487,7 +1488,7 @@ associate_workers_with_ns(void) ns_ctx->min_tsc = UINT64_MAX; ns_ctx->entry = entry; ns_ctx->next = worker->ns_ctx; - spdk_histogram_data_reset(&ns_ctx->histogram); + ns_ctx->histogram = spdk_histogram_data_alloc(); worker->ns_ctx = ns_ctx; worker = worker->next; diff --git a/include/spdk/histogram_data.h b/include/spdk/histogram_data.h index de9d8dda0..25649397a 100644 --- a/include/spdk/histogram_data.h +++ b/include/spdk/histogram_data.h @@ -45,11 +45,14 @@ extern "C" { #endif -#define SPDK_HISTOGRAM_BUCKET_SHIFT(h) 7 +#define SPDK_HISTOGRAM_BUCKET_SHIFT_DEFAULT 7 +#define SPDK_HISTOGRAM_BUCKET_SHIFT(h) h->bucket_shift #define SPDK_HISTOGRAM_BUCKET_LSB(h) (64 - SPDK_HISTOGRAM_BUCKET_SHIFT(h)) #define SPDK_HISTOGRAM_NUM_BUCKETS_PER_RANGE(h) (1ULL << SPDK_HISTOGRAM_BUCKET_SHIFT(h)) #define SPDK_HISTOGRAM_BUCKET_MASK(h) (SPDK_HISTOGRAM_NUM_BUCKETS_PER_RANGE(h) - 1) #define SPDK_HISTOGRAM_NUM_BUCKET_RANGES(h) (SPDK_HISTOGRAM_BUCKET_LSB(h) + 1) +#define SPDK_HISTOGRAM_NUM_BUCKETS(h) (SPDK_HISTOGRAM_NUM_BUCKETS_PER_RANGE(h) * \ + SPDK_HISTOGRAM_NUM_BUCKET_RANGES(h)) /* * SPDK histograms are implemented using ranges of bucket arrays. The most common usage @@ -83,30 +86,30 @@ extern "C" { struct spdk_histogram_data { - /* - * "x" is just a filler here for now - a future patch will make this a dynamic array of - * uint64_t's and this will go away. - */ - uint64_t bucket[SPDK_HISTOGRAM_NUM_BUCKET_RANGES(x)][SPDK_HISTOGRAM_NUM_BUCKETS_PER_RANGE(x)]; + uint32_t bucket_shift; + uint64_t *bucket; }; static inline void __spdk_histogram_increment(struct spdk_histogram_data *h, uint32_t range, uint32_t index) { - h->bucket[range][index]++; + uint64_t *count; + + count = &h->bucket[(range << SPDK_HISTOGRAM_BUCKET_SHIFT(h)) + index]; + (*count)++; } static inline uint64_t __spdk_histogram_get_count(const struct spdk_histogram_data *h, uint32_t range, uint32_t index) { - return h->bucket[range][index]; + return h->bucket[(range << SPDK_HISTOGRAM_BUCKET_SHIFT(h)) + index]; } static inline void spdk_histogram_data_reset(struct spdk_histogram_data *histogram) { - memset(histogram, 0, sizeof(*histogram)); + memset(histogram->bucket, 0, SPDK_HISTOGRAM_NUM_BUCKETS(histogram) * sizeof(uint64_t)); } static inline uint32_t @@ -200,6 +203,43 @@ spdk_histogram_data_iterate(const struct spdk_histogram_data *histogram, } } +static inline struct spdk_histogram_data * +spdk_histogram_data_alloc_sized(uint32_t bucket_shift) +{ + struct spdk_histogram_data *h; + + h = (struct spdk_histogram_data *)calloc(1, sizeof(*h)); + if (h == NULL) { + return NULL; + } + + h->bucket_shift = bucket_shift; + h->bucket = (uint64_t *)calloc(SPDK_HISTOGRAM_NUM_BUCKETS(h), sizeof(uint64_t)); + if (h->bucket == NULL) { + free(h); + return NULL; + } + + return h; +} + +static inline struct spdk_histogram_data * +spdk_histogram_data_alloc(void) +{ + return spdk_histogram_data_alloc_sized(SPDK_HISTOGRAM_BUCKET_SHIFT_DEFAULT); +} + +static inline void +spdk_histogram_data_free(struct spdk_histogram_data *h) +{ + if (h == NULL) { + return; + } + + free(h->bucket); + free(h); +} + #ifdef __cplusplus } #endif diff --git a/test/lib/nvme/overhead/overhead.c b/test/lib/nvme/overhead/overhead.c index d6b91686b..36c3c4ef1 100644 --- a/test/lib/nvme/overhead/overhead.c +++ b/test/lib/nvme/overhead/overhead.c @@ -80,8 +80,8 @@ struct ns_entry { uint32_t current_queue_depth; char name[1024]; - struct spdk_histogram_data submit_histogram; - struct spdk_histogram_data complete_histogram; + struct spdk_histogram_data *submit_histogram; + struct spdk_histogram_data *complete_histogram; }; struct perf_task { @@ -150,8 +150,8 @@ register_ns(struct spdk_nvme_ctrlr *ctrlr, struct spdk_nvme_ns *ns) entry->size_in_ios = spdk_nvme_ns_get_size(ns) / g_io_size_bytes; entry->io_size_blocks = g_io_size_bytes / spdk_nvme_ns_get_sector_size(ns); - spdk_histogram_data_reset(&entry->submit_histogram); - spdk_histogram_data_reset(&entry->complete_histogram); + entry->submit_histogram = spdk_histogram_data_alloc(); + entry->complete_histogram = spdk_histogram_data_alloc(); snprintf(entry->name, 44, "%-20.20s (%-20.20s)", cdata->mn, cdata->sn); @@ -314,7 +314,7 @@ submit_single_io(void) g_tsc_submit_max = tsc_submit; } if (g_enable_histogram) { - spdk_histogram_data_tally(&entry->submit_histogram, tsc_submit); + spdk_histogram_data_tally(entry->submit_histogram, tsc_submit); } if (rc != 0) { @@ -369,7 +369,7 @@ check_io(void) g_tsc_complete_max = tsc_complete; } if (g_enable_histogram) { - spdk_histogram_data_tally(&g_ns->complete_histogram, tsc_complete); + spdk_histogram_data_tally(g_ns->complete_histogram, tsc_complete); } g_io_completed++; if (!g_ns->is_draining) { @@ -521,13 +521,13 @@ print_stats(void) printf("Submit histogram\n"); printf("================\n"); printf(" Range in us Cumulative Count\n"); - spdk_histogram_data_iterate(&g_ns->submit_histogram, print_bucket, NULL); + spdk_histogram_data_iterate(g_ns->submit_histogram, print_bucket, NULL); printf("\n"); printf("Complete histogram\n"); printf("==================\n"); printf(" Range in us Cumulative Count\n"); - spdk_histogram_data_iterate(&g_ns->complete_histogram, print_bucket, NULL); + spdk_histogram_data_iterate(g_ns->complete_histogram, print_bucket, NULL); printf("\n"); } @@ -675,6 +675,8 @@ int main(int argc, char **argv) print_stats(); cleanup: + spdk_histogram_data_free(g_ns->submit_histogram); + spdk_histogram_data_free(g_ns->complete_histogram); free(g_ns); if (g_ctrlr) { spdk_nvme_detach(g_ctrlr->ctrlr); diff --git a/test/lib/util/histogram_perf/histogram_perf.c b/test/lib/util/histogram_perf/histogram_perf.c index fa143e38c..eeff61746 100644 --- a/test/lib/util/histogram_perf/histogram_perf.c +++ b/test/lib/util/histogram_perf/histogram_perf.c @@ -57,7 +57,7 @@ usage(const char *prog) int main(int argc, char **argv) { - struct spdk_histogram_data h; + struct spdk_histogram_data *h; struct spdk_env_opts opts; uint64_t tsc[128], t, end_tsc, count; uint32_t i; @@ -81,11 +81,11 @@ main(int argc, char **argv) end_tsc = spdk_get_ticks() + (10 * spdk_get_ticks_hz()); count = 0; - spdk_histogram_data_reset(&h); + h = spdk_histogram_data_alloc(); while (true) { t = spdk_get_ticks(); - spdk_histogram_data_tally(&h, t - tsc[count % 128]); + spdk_histogram_data_tally(h, t - tsc[count % 128]); count++; if (t > end_tsc) { break; @@ -93,5 +93,7 @@ main(int argc, char **argv) } printf("count = %ju\n", count); + spdk_histogram_data_free(h); + return rc; }