libdm/libdm-stats.c | 223 ++++++++++++++++++++++++++++++---------------------- tools/dmsetup.c | 1 + 2 files changed, 131 insertions(+), 93 deletions(-) diff --git a/libdm/libdm-stats.c b/libdm/libdm-stats.c index 93c7760..ee62fe6 100644 --- a/libdm/libdm-stats.c +++ b/libdm/libdm-stats.c @@ -144,17 +144,24 @@ struct dm_stats *dm_stats_create(const char *program_id) return_NULL; /* FIXME: better hint. */ - if (!(dms->mem = dm_pool_create("stats_pool", 4096))) - goto_bad; + if (!(dms->mem = dm_pool_create("stats_pool", 4096))) { + dm_free(dms); + return_NULL; + } if (!(dms->hist_mem = dm_pool_create("histogram_pool", hist_hint))) - return_0; + goto_bad; if (!program_id || !strlen(program_id)) dms->program_id = _program_id_from_proc(); else dms->program_id = dm_strdup(program_id); + if (!dms->program_id) { + dm_pool_destroy(dms->hist_mem); + goto_bad; + } + dms->major = -1; dms->minor = -1; dms->name = NULL; @@ -171,6 +178,7 @@ struct dm_stats *dm_stats_create(const char *program_id) return dms; bad: + dm_pool_destroy(dms->mem); dm_free(dms); return NULL; } @@ -186,15 +194,15 @@ static int _stats_region_present(const struct dm_stats_region *region) static void _stats_histograms_destroy(struct dm_pool *mem, struct dm_stats_region *region) { - uint64_t n; - /* Unpopulated handle. */ if (!region->counters) return; - for (n = _nr_areas_region(region) - 1; n; n--) - if (region->counters[n].histogram) - dm_pool_free(mem, region->counters[n].histogram); + /* + * Only the first histogram needs to be freed explicitly. + */ + if (region->counters[0].histogram) + dm_pool_free(mem, region->counters[0].histogram); } static void _stats_region_destroy(struct dm_stats_region *region) @@ -316,11 +324,54 @@ int dm_stats_driver_supports_histogram(void) return _stats_check_precise_timestamps(NULL); } +static int _fill_hist_arg(char *hist_arg, size_t hist_len, uint64_t scale, + struct dm_histogram *bounds) +{ + int i, l, len = 0, nr_bins; + char *arg = hist_arg; + uint64_t value; + + nr_bins = bounds->nr_bins; + + for (i = 0; i < nr_bins; i++) { + value = bounds->bins[i].upper / scale; + if ((l = dm_snprintf(arg, hist_len - len, FMTu64"%s", value, + (i == (nr_bins - 1)) ? "" : ",")) < 0) + return_0; + len += l; + arg += l; + } + return 1; +} + +static void *_get_hist_arg(struct dm_histogram *bounds, uint64_t scale, + size_t *len) +{ + struct dm_histogram_bin *entry, *bins; + size_t hist_len = 1; /* terminating '\0' */ + double value; + + entry = bins = bounds->bins; + + entry += bounds->nr_bins - 1; + while(entry >= bins) { + value = (double) (entry--)->upper; + /* Use lround to avoid size_t -> double cast warning. */ + hist_len += 1 + (size_t) lround(log10(value / scale)); + if (entry != bins) + hist_len++; /* ',' */ + } + + *len = hist_len; + + return dm_zalloc(hist_len); +} + static char *_build_histogram_arg(struct dm_histogram *bounds, int *precise) { struct dm_histogram_bin *entry, *bins; - size_t hist_len = 1, len = 0; - char *hist_arg, *arg = NULL; + size_t hist_len; + char *hist_arg; uint64_t scale; entry = bins = bounds->bins; @@ -331,53 +382,37 @@ static char *_build_histogram_arg(struct dm_histogram *bounds, int *precise) return NULL; } + /* Validate entries and set *precise if precision < 1ms. */ entry += bounds->nr_bins - 1; - while(entry >= bins) { - double value; + while (entry >= bins) { if (entry != bins) { if (entry->upper < (entry - 1)->upper) { log_error("Histogram boundaries must be in " "order of increasing magnitude."); return 0; } - hist_len++; /* ',' */ } /* * Only enable precise_timestamps automatically if any * value in the histogram bounds uses precision < 1ms. */ - if (!*precise && (entry->upper % NSEC_PER_MSEC)) + if (((entry--)->upper % NSEC_PER_MSEC) && !*precise) *precise = 1; - - value = (double) (entry--)->upper; - /* Use lround to avoid size_t -> double cast warning. */ - hist_len += 1 + (size_t) lround(log10(value)); } - if (!(hist_arg = dm_zalloc(hist_len))) { + scale = (*precise) ? 1 : NSEC_PER_MSEC; + + /* Calculate hist_len and allocate a character buffer. */ + if (!(hist_arg = _get_hist_arg(bounds, scale, &hist_len))) { log_error("Could not allocate memory for histogram argument."); return 0; } - arg = hist_arg; - - if (*precise) - scale = 1; - else - scale = (*precise) ? 1 : NSEC_PER_MSEC; + /* Fill hist_arg with boundary strings. */ + if (!_fill_hist_arg(hist_arg, hist_len, scale, bounds)) + goto_bad; - for (entry = bins; entry < (bins + bounds->nr_bins); entry++) { - uint64_t value; - ssize_t l = 0; - int last = !(entry < (bins + bounds->nr_bins - 1)); - value = entry->upper / scale; - if ((l = dm_snprintf(arg, hist_len - len, FMTu64"%s", value, - (last) ? "" : ",")) < 0) - goto_bad; - len += (size_t) l; - arg += (size_t) l; - } return hist_arg; bad: @@ -419,13 +454,13 @@ static int _stats_parse_histogram_spec(struct dm_stats *dms, const char *histogram) { static const char *_valid_chars = "0123456789,"; - uint64_t scale = region->timescale; + uint64_t scale = region->timescale, this_val = 0; struct dm_pool *mem = dms->hist_mem; struct dm_histogram_bin cur; struct dm_histogram hist; int nr_bins = 1; - const char *c, *v; - char *p; + const char *c, *v, *val_start; + char *p, *endptr = NULL; /* Advance past "histogram:". */ histogram = strchr(histogram, ':'); @@ -466,9 +501,8 @@ static int _stats_parse_histogram_spec(struct dm_stats *dms, histogram); goto bad; } else { - const char *val_start = c; - char *endptr = NULL; - uint64_t this_val = 0; + val_start = c; + endptr = NULL; this_val = strtoull(val_start, &endptr, 10); if (!endptr) { @@ -592,11 +626,11 @@ static int _stats_parse_list_region(struct dm_stats *dms, static int _stats_parse_list(struct dm_stats *dms, const char *resp) { - struct dm_pool *mem = dms->mem; - struct dm_stats_region cur; uint64_t max_region = 0, nr_regions = 0; + struct dm_stats_region cur, fill; + struct dm_pool *mem = dms->mem; FILE *list_rows; - /* FIXME: determine correct maximum line length based on kernel format */ + /* FIXME: use correct maximum line length for kernel format */ char line[256]; if (!resp) { @@ -631,7 +665,6 @@ static int _stats_parse_list(struct dm_stats *dms, const char *resp) /* handle holes in the list of region_ids */ if (cur.region_id > max_region) { - struct dm_stats_region fill; memset(&fill, 0, sizeof(fill)); fill.region_id = DM_STATS_REGION_NOT_PRESENT; do { @@ -707,54 +740,51 @@ static int _stats_parse_histogram(struct dm_pool *mem, char *hist_str, struct dm_histogram **histogram, struct dm_stats_region *region) { + struct dm_histogram hist, *bounds = region->bounds; static const char *_valid_chars = "0123456789:"; int nr_bins = region->bounds->nr_bins; - struct dm_histogram hist, *bounds = region->bounds; + const char *c, *v, *val_start; struct dm_histogram_bin cur; - uint64_t sum = 0; - const char *c, *v; + uint64_t sum = 0, this_val; + char *endptr = NULL; int bin = 0; c = hist_str; - dm_pool_begin_object(mem, sizeof(cur)); + if (!dm_pool_begin_object(mem, sizeof(cur))) + return_0; hist.nr_bins = nr_bins; - dm_pool_grow_object(mem, &hist, sizeof(hist)); + if (!dm_pool_grow_object(mem, &hist, sizeof(hist))) + goto_bad; do { memset(&cur, 0, sizeof(cur)); for (v = _valid_chars; *v; v++) if (*c == *v) break; - if (!*v) { - stack; + if (!*v) goto badchar; - } - if (*c == ',') { - log_error("Invalid histogram: %s", hist_str); - return 0; - } else { - const char *val_start = c; - char *endptr = NULL; - uint64_t this_val = 0; + if (*c == ',') + goto badchar; + else { + val_start = c; + endptr = NULL; this_val = strtoull(val_start, &endptr, 10); if (!endptr) { log_error("Could not parse histogram value."); - return 0; + goto bad; } c = endptr; /* Advance to colon, or end. */ if (*c == ':') c++; - else if (*c & (*c != '\n')) { + else if (*c & (*c != '\n')) /* Expected ':', '\n', or NULL. */ - stack; goto badchar; - } if (*c == ':') c++; @@ -763,7 +793,8 @@ static int _stats_parse_histogram(struct dm_pool *mem, char *hist_str, cur.count = this_val; sum += this_val; - dm_pool_grow_object(mem, &cur, sizeof(cur)); + if (!dm_pool_grow_object(mem, &cur, sizeof(cur))) + goto_bad; bin++; } @@ -778,6 +809,8 @@ static int _stats_parse_histogram(struct dm_pool *mem, char *hist_str, badchar: log_error("Invalid character in histogram data: '%c' (0x%x)", *c, *c); +bad: + dm_pool_abandon_object(mem); return 0; } @@ -786,8 +819,8 @@ static int _stats_parse_region(struct dm_stats *dms, const char *resp, uint64_t timescale) { struct dm_histogram *hist = NULL; - struct dm_stats_counters cur; struct dm_pool *mem = dms->mem; + struct dm_stats_counters cur; FILE *stats_rows = NULL; uint64_t start, len; char row[256]; @@ -1040,12 +1073,12 @@ static int _stats_create_region(struct dm_stats *dms, uint64_t *region_id, int precise, const char *hist_arg, const char *program_id, const char *aux_data) { - struct dm_task *dmt = NULL; - char msg[1024], range[64]; const char *err_fmt = "Could not prepare @stats_create %s."; const char *precise_str = PRECISE_ARG; const char *resp, *opt_args = NULL; - int r = 0, nr_opt = 0; /* number of optional args. */ + char msg[1024], range[64], *endptr = NULL; + struct dm_task *dmt = NULL; + int r = 0, nr_opt = 0; if (!_stats_bound(dms)) return_0; @@ -1105,7 +1138,6 @@ static int _stats_create_region(struct dm_stats *dms, uint64_t *region_id, } if (region_id) { - char *endptr = NULL; *region_id = strtoull(resp, &endptr, 10); if (resp == endptr) goto_out; @@ -1195,11 +1227,11 @@ static struct dm_task *_stats_print_region(struct dm_stats *dms, uint64_t region_id, unsigned start_line, unsigned num_lines, unsigned clear) { - struct dm_task *dmt = NULL; /* @stats_print[_clear] [ ] */ const char *clear_str = "_clear", *lines_fmt = "%u %u"; const char *msg_fmt = "@stats_print%s " FMTu64 " %s"; const char *err_fmt = "Could not prepare @stats_print %s."; + struct dm_task *dmt = NULL; char msg[1024], lines[64]; if (start_line || num_lines) @@ -1292,6 +1324,8 @@ int dm_stats_populate(struct dm_stats *dms, const char *program_id, uint64_t region_id) { int all_regions = (region_id == DM_STATS_REGIONS_ALL); + struct dm_task *dmt = NULL; /* @stats_print task */ + const char *resp; if (!_stats_bound(dms)) return_0; @@ -1311,9 +1345,6 @@ int dm_stats_populate(struct dm_stats *dms, const char *program_id, dm_stats_walk_start(dms); do { - struct dm_task *dmt = NULL; /* @stats_print task */ - const char *resp; - region_id = (all_regions) ? dm_stats_get_current_region(dms) : region_id; @@ -1967,10 +1998,12 @@ static struct dm_histogram *_alloc_dm_histogram(int nr_bins) struct dm_histogram *dm_histogram_bounds_from_string(const char *bounds_str) { static const char *_valid_chars = "0123456789,muns"; - struct dm_histogram *dmh; + uint64_t this_val = 0, mult = 1; + const char *c, *v, *val_start; struct dm_histogram_bin *cur; - const char *c, *v; + struct dm_histogram *dmh; int nr_entries = 1; + char *endptr; c = bounds_str; @@ -2003,9 +2036,8 @@ struct dm_histogram *dm_histogram_bounds_from_string(const char *bounds_str) bounds_str); goto bad; } else { - const char *val_start = c; - char *endptr = NULL; - uint64_t this_val = 0, mult = 1; + val_start = c; + endptr = NULL; this_val = strtoull(val_start, &endptr, 10); if (!endptr) { @@ -2058,10 +2090,10 @@ bad: struct dm_histogram *dm_histogram_bounds_from_uint64(const uint64_t *bounds) { - struct dm_histogram *dmh; + const uint64_t *entry = bounds; struct dm_histogram_bin *cur; + struct dm_histogram *dmh; int nr_entries = 1; - const uint64_t *entry = bounds; if (!bounds || !bounds[0]) { log_error("Could not parse empty histogram bounds array"); @@ -2113,6 +2145,7 @@ void dm_histogram_bounds_destroy(struct dm_histogram *bounds) */ static void _scale_bound_value_to_suffix(uint64_t *bound, const char **suffix) { + *suffix = "ns"; if (!(*bound % NSEC_PER_SEC)) { *bound /= NSEC_PER_SEC; *suffix = "s"; @@ -2191,12 +2224,14 @@ const char *dm_histogram_to_string(const struct dm_histogram *dmh, int bin, int width, int flags) { int minwidth, bounds, values, start, last; - uint64_t lower, upper; /* bounds of the current bin. */ + uint64_t lower, upper, val_u64; /* bounds of the current bin. */ /* Use the histogram pool for string building. */ struct dm_pool *mem = dmh->dms->hist_mem; char buf[64], bounds_buf[64]; const char *sep = ""; + int bounds_width; ssize_t len = 0; + float val_flt; bounds = flags & DM_HISTOGRAM_BOUNDS_MASK; values = flags & DM_HISTOGRAM_VALUES; @@ -2222,12 +2257,11 @@ const char *dm_histogram_to_string(const struct dm_histogram *dmh, int bin, /* Set bounds string to the empty string. */ bounds_buf[0] = '\0'; - dm_pool_begin_object(mem, 64); + if (!dm_pool_begin_object(mem, 64)) + return_0; for (bin = start; bin <= last; bin++) { if (bounds) { - int bounds_width; - /* Default bounds width depends on time suffixes. */ bounds_width = (!(flags & DM_HISTOGRAM_SUFFIX)) ? BOUND_WIDTH_NOSUFFIX @@ -2260,15 +2294,14 @@ const char *dm_histogram_to_string(const struct dm_histogram *dmh, int bin, if (flags & DM_HISTOGRAM_PERCENT) { dm_percent_t pr; - float value; pr = dm_histogram_get_bin_percent(dmh, bin); - value = dm_percent_to_float(pr); + val_flt = dm_percent_to_float(pr); len = dm_snprintf(buf, sizeof(buf), "%s%*.2f%%%s", - bounds_buf, width, value, sep); + bounds_buf, width, val_flt, sep); } else if (values) { - uint64_t value = dmh->bins[bin].count; + val_u64 = dmh->bins[bin].count; len = dm_snprintf(buf, sizeof(buf), "%s%*"PRIu64"%s", - bounds_buf, width, value, sep); + bounds_buf, width, val_u64, sep); } else if (bounds) len = dm_snprintf(buf, sizeof(buf), "%s%s", bounds_buf, sep); @@ -2277,9 +2310,13 @@ const char *dm_histogram_to_string(const struct dm_histogram *dmh, int bin, goto_bad; width = minwidth; /* re-set histogram column width. */ - dm_pool_grow_object(mem, buf, (size_t) len); + if (!dm_pool_grow_object(mem, buf, (size_t) len)) + goto_bad; } - dm_pool_grow_object(mem, "\0", 1); + + if (!dm_pool_grow_object(mem, "\0", 1)) + goto_bad; + return (const char *) dm_pool_end_object(mem); bad: diff --git a/tools/dmsetup.c b/tools/dmsetup.c index 61ad5a9..8b7ad74 100644 --- a/tools/dmsetup.c +++ b/tools/dmsetup.c @@ -4674,6 +4674,7 @@ static int _do_stats_create_regions(struct dm_stats *dms, return_0; if (!(dmt = dm_task_create(DM_DEVICE_TABLE))) { + dm_histogram_bounds_destroy(bounds); dm_stats_destroy(dms); return_0; }