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] <region_id> [<start_line> <num_lines>] */
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;
}