doc: update dmstats.8.in for --filemap and --nogroup

Add a new option to the create command to create regions that map the
extents of a file:

  # dmstats create --filemap /path/to/file
  /path/to/file: Created new group with 10 region(s) as group ID 0.

When performing a --filemap no device argument is required (and
supplying one results in error) since the device to bind to is implied
by the file path and is obtained directly from an fstat().

Grouping may be optionally disabled by the --nogroup switch: in this
case the command will report each region individually:

  # dmstats create --nogroup --filemap /path/to/file
  /path/to/file: Created new region with 1 area as region ID 0.
  /path/to/file: Created new region with 1 area as region ID 1.
  /path/to/file: Created new region with 1 area as region ID 2.

When grouping regions the group alias is automatically set to the
basename (as returned by dm_basename()) of the provided file.

This can be overridden to a user-defined value at the command line by
use of the --alias option.

If grouping is disabled no alias can be set.

Use of offset and subdivision options (--start, --length, --segments,
--areas, --areasize).

Setting aux_data and histograms for groups is possible but is not
currently implemented.

WHATS_NEW_DM

@@ -1,13 +1,5 @@
 Version 1.02.131 -
 ================================
-  Automatically group regions with 'create --segments' unless --nogroup.
-  Fix resource leak when deleting the first member of a group.
-  Allow --bounds with 'create --filemap' for dmstats.
-  Enable creation of filemap regions with histograms.
-  Enable histogram aggregation for regions with more than one area.
-  Enable histogram aggregation for groups of regions.
-  Add a --filemap option to 'dmstats create' to allow mapping of files.
-  Add dm_stats_create_regions_from_fd() to map file extents to regions.
 
 Version 1.02.130 - 6th July 2016
 ================================

libdm/libdm-stats.c

@@ -85,7 +85,6 @@ struct dm_stats_region {
 	char *aux_data;
 	uint64_t timescale; /* precise_timestamps is per-region */
 	struct dm_histogram *bounds; /* histogram configuration */
-	struct dm_histogram *histogram; /* aggregate cache */
 	struct dm_stats_counters *counters;
 };
 
@@ -93,7 +92,6 @@ struct dm_stats_group {
 	uint64_t group_id;
 	const char *alias;
 	dm_bitset_t regions;
-	struct dm_histogram *histogram;
 };
 
 struct dm_stats {
@@ -341,8 +339,6 @@ static void _stats_group_destroy(struct dm_stats_group *group)
 	if (!_stats_group_present(group))
 		return;
 
-	group->histogram = NULL;
-
 	if (group->alias) {
 		dm_free((char *) group->alias);
 		group->alias = NULL;
@@ -903,9 +899,6 @@ static int _stats_parse_list_region(struct dm_stats *dms,
 	} else
 		region->bounds = NULL;
 
-	/* clear aggregate cache */
-	region->histogram = NULL;
-
 	region->group_id = DM_STATS_GROUP_NOT_PRESENT;
 
 	if (!(region->program_id = dm_strdup(program_id)))
@@ -3184,131 +3177,20 @@ int dm_stats_get_current_region_precise_timestamps(const struct dm_stats *dms)
  * Histogram access methods.
  */
 
-static void _sum_histogram_bins(const struct dm_stats *dms,
-				struct dm_histogram *dmh_aggr,
-				uint64_t region_id, uint64_t area_id)
-{
-	struct dm_stats_region *region;
-	struct dm_histogram_bin *bins;
-	struct dm_histogram *dmh_cur;
-	uint64_t bin;
-
-	region = &dms->regions[region_id];
-	dmh_cur = region->counters[area_id].histogram;
-	bins = dmh_aggr->bins;
-
-	for (bin = 0; bin < dmh_aggr->nr_bins; bin++)
-		bins[bin].count += dmh_cur->bins[bin].count;
-}
-
-/*
- * Create an aggregate histogram for a sub-divided region or a group.
- */
-static struct dm_histogram *_aggregate_histogram(const struct dm_stats *dms,
-						 uint64_t region_id,
-						 uint64_t area_id)
-{
-	struct dm_histogram *dmh_aggr, *dmh_cur, **dmh_cachep;
-	int bin, nr_bins, group = 1;
-	uint64_t group_id;
-	size_t hist_size;
-
-	if (area_id == DM_STATS_WALK_REGION) {
-		/* region aggregation */
-		group = 0;
-		if (!_stats_region_present(&dms->regions[region_id]))
-			return_NULL;
-
-		if (!dms->regions[region_id].bounds)
-			return_NULL;
-
-		if (!dms->regions[region_id].counters)
-			return dms->regions[region_id].bounds;
-
-		if (dms->regions[region_id].histogram)
-			return dms->regions[region_id].histogram;
-
-		dmh_cur = dms->regions[region_id].counters[0].histogram;
-		dmh_cachep = &dms->regions[region_id].histogram;
-		nr_bins = dms->regions[region_id].bounds->nr_bins;
-	} else {
-		/* group aggregation */
-		group_id = region_id;
-		area_id = DM_STATS_WALK_GROUP;
-		if (!_stats_group_id_present(dms, group_id))
-			return_NULL;
-
-		if (!dms->regions[group_id].bounds)
-			return_NULL;
-
-		if (!dms->regions[group_id].counters)
-			return dms->regions[group_id].bounds;
-
-		if (dms->groups[group_id].histogram)
-			return dms->groups[group_id].histogram;
-
-		dmh_cur = dms->regions[group_id].counters[0].histogram;
-		dmh_cachep = &dms->groups[group_id].histogram;
-		nr_bins = dms->regions[group_id].bounds->nr_bins;
-	}
-
-	hist_size = sizeof(*dmh_aggr)
-		     + nr_bins * sizeof(struct dm_histogram_bin);
-
-	if (!(dmh_aggr = dm_pool_zalloc(dms->hist_mem, hist_size))) {
-		log_error("Could not allocate group histogram");
-		return 0;
-	}
-
-	dmh_aggr->nr_bins = dmh_cur->nr_bins;
-	dmh_aggr->dms = dms;
-
-	if (!group)
-		_foreach_region_area(dms, region_id, area_id) {
-			_sum_histogram_bins(dms, dmh_aggr, region_id, area_id);
-		}
-	else {
-		_foreach_group_area(dms, group_id, region_id, area_id) {
-			_sum_histogram_bins(dms, dmh_aggr, region_id, area_id);
-		}
-	}
-
-	for (bin = 0; bin < nr_bins; bin++) {
-		dmh_aggr->sum += dmh_aggr->bins[bin].count;
-		dmh_aggr->bins[bin].upper = dmh_cur->bins[bin].upper;
-	}
-
-	/* cache aggregate histogram for subsequent access */
-	*dmh_cachep = dmh_aggr;
-
-	return dmh_aggr;
-}
-
 struct dm_histogram *dm_stats_get_histogram(const struct dm_stats *dms,
 					    uint64_t region_id,
 					    uint64_t area_id)
 {
-	int aggr = 0;
-
-	if (region_id == DM_STATS_REGION_CURRENT) {
-		region_id = dms->cur_region;
-		if (region_id & DM_STATS_WALK_GROUP) {
-			region_id = dms->cur_group;
-			aggr = 1;
-		}
-	} else if (region_id & DM_STATS_WALK_GROUP) {
-		region_id &= ~DM_STATS_WALK_GROUP;
-		aggr = 1;
-	}
-
+	region_id = (region_id == DM_STATS_REGION_CURRENT)
+		     ? dms->cur_region : region_id ;
 	area_id = (area_id == DM_STATS_AREA_CURRENT)
-		   ? dms->cur_area : area_id ;
+		     ? dms->cur_area : area_id ;
 
-	if (area_id == DM_STATS_WALK_REGION)
-		aggr = 1;
-
-	if (aggr)
-		return _aggregate_histogram(dms, region_id, area_id);
+	/* FIXME return histogram sum? Requires bounds check at group time */
+	if (region_id & DM_STATS_WALK_GROUP) {
+		log_err_once("Group histogram data is not supported");
+		return NULL;
+	}
 
 	if (region_id & DM_STATS_WALK_REGION)
 		region_id &= ~DM_STATS_WALK_REGION;
@@ -3860,39 +3742,6 @@ merge:
 	return overlap;
 }
 
-static void _stats_copy_histogram_bounds(struct dm_histogram *to,
-					 struct dm_histogram *from)
-{
-	uint64_t i;
-
-	to->nr_bins = from->nr_bins;
-
-	for (i = 0; i < to->nr_bins; i++)
-		to->bins[i].upper = from->bins[i].upper;
-}
-
-/*
- * Compare histogram bounds h1 and h2, and return 1 if they match (i.e.
- * have the same number of bins and identical bin boundary values), or 0
- * otherwise.
- */
-static int _stats_check_histogram_bounds(struct dm_histogram *h1,
-					 struct dm_histogram *h2)
-{
-	uint64_t i;
-
-	if (!h1 || !h2)
-		return 0;
-
-	if (h1->nr_bins != h2->nr_bins)
-		return 0;
-
-	for (i = 0; i < h1->nr_bins; i++)
-		if (h1->bins[i].upper != h2->bins[i].upper)
-			return 0;
-	return 1;
-}
-
 /*
  * Create a new group in stats handle dms from the group description
  * passed in group.
@@ -3900,7 +3749,6 @@ static int _stats_check_histogram_bounds(struct dm_histogram *h1,
 int dm_stats_create_group(struct dm_stats *dms, const char *members,
 			  const char *alias, uint64_t *group_id)
 {
-	struct dm_histogram *check = NULL, *bounds;
 	int i, count = 0, precise = 0;
 	dm_bitset_t regions;
 
@@ -3911,11 +3759,6 @@ int dm_stats_create_group(struct dm_stats *dms, const char *members,
 
 	if (!(regions = dm_bitset_parse_list(members, NULL))) {
 		log_error("Could not parse list: '%s'", members);
-		return 0;
-	}
-
-	if (!(check = dm_pool_zalloc(dms->hist_mem, sizeof(*check)))) {
-		log_error("Could not allocate memory for bounds check");
 		goto bad;
 	}
 
@@ -3941,20 +3784,14 @@ int dm_stats_create_group(struct dm_stats *dms, const char *members,
 				  FMTu64, i, dms->regions[i].group_id);
 			goto bad;
 		}
+		if (dms->regions[i].bounds) {
+			log_error("Region ID %d: grouping regions with "
+				  "histograms is not yet supported", i);
+			goto bad;
+		}
 		if (dms->regions[i].timescale == 1)
 			precise++;
 
-		/* check for matching histogram bounds */
-		bounds = dms->regions[i].bounds;
-		if (bounds && !check->nr_bins)
-			_stats_copy_histogram_bounds(check, bounds);
-		else if (bounds) {
-			if (!_stats_check_histogram_bounds(check, bounds)) {
-				log_error("All region histogram bounds "
-					  "must match exactly");
-				goto bad;
-			}
-		}
 		count++;
 	}
 
@@ -3968,11 +3805,8 @@ int dm_stats_create_group(struct dm_stats *dms, const char *members,
 	if (!_stats_create_group(dms, regions, alias, group_id))
 		goto bad;
 
-	dm_pool_free(dms->hist_mem, check);
 	return 1;
-
 bad:
-	dm_pool_free(dms->hist_mem, check);
 	dm_bitset_destroy(regions);
 	return 0;
 }
@@ -4247,7 +4081,6 @@ static uint64_t *_stats_create_file_regions(struct dm_stats *dms, int fd,
 {
 	struct _extent *extents = NULL;
 	uint64_t *regions = NULL, i;
-	char *hist_arg = NULL;
 	struct statfs fsbuf;
 	struct stat buf;
 
@@ -4280,12 +4113,6 @@ static uint64_t *_stats_create_file_regions(struct dm_stats *dms, int fd,
 	if (!(extents = _stats_get_extents_for_file(dms->mem, fd, count)))
 		return_0;
 
-        if (bounds) {
-                /* _build_histogram_arg enables precise if vals < 1ms. */
-                if (!(hist_arg = _build_histogram_arg(bounds, &precise)))
-                        goto_out;
-        }
-
 	/* make space for end-of-table marker */
 	if (!(regions = dm_malloc((1 + *count) * sizeof(*regions)))) {
 		log_error("Could not allocate memory for region IDs.");
@@ -4293,9 +4120,9 @@ static uint64_t *_stats_create_file_regions(struct dm_stats *dms, int fd,
 	}
 
 	for (i = 0; i < *count; i++) {
-		if (!_stats_create_region(dms, regions + i, extents[i].start,
-					  extents[i].len, -1, precise, hist_arg,
-					  dms->program_id, "")) {
+		if (!_stats_create_region(dms, regions + i,
+					  extents[i].start, extents[i].len, -1,
+					  precise, NULL, dms->program_id, "")) {
 			log_error("Failed to create region " FMTu64 " of "
 				  FMTu64 " at " FMTu64 ".", i, *count,
 				  extents[i].start);
@@ -4328,6 +4155,12 @@ uint64_t *dm_stats_create_regions_from_fd(struct dm_stats *dms, int fd,
 {
 	uint64_t *regions, count = 0;
 
+	if (bounds) {
+		log_error("File mapped groups with histograms are not "
+			  "yet supported.");
+		return NULL;
+	}
+
 	if (alias && !group) {
 		log_error("Cannot set alias without grouping regions.");
 		return NULL;

man/dmstats.8.in

@@ -148,6 +148,8 @@ dmstats \(em device-mapper statistics management
 .  RI [ device_name ]
 .  RB [ \-\-histogram ]
 .  OPT_PROGRAMS
+.  RB [ \-\-statstype
+.  IR type_list ]
 .  RB [ \-\-units
 .  IR units ]
 .  OPT_OBJECTS
@@ -191,6 +193,8 @@ dmstats \(em device-mapper statistics management
 .  IR sort_fields ]
 .  RB [ \-S | \-\-select
 .  IR selection ]
+.  RB [ \-\-statstype
+.  IR type_list ]
 .  RB [ \-\-units
 .  IR units ]
 .  RB [ \-\-nosuffix ]
@@ -456,13 +460,23 @@ optional suffix selects units of:
 .HP
 .BR \-\-segments
 .br
-When used with \fBcreate\fP, create a new statistics region for each
-target contained in the given device(s). This causes a separate region
-to be allocated for each segment of the device.
+Create a new statistics region for each target contained in the target
+device. This causes a separate region to be allocated for each segment
+of the device.
+.
+.HP
+.BR \-\-statstype
+.IR type_list
+.br
+Filter the types of statistics object included in a report or listing
+according to the provided list. A report may include areas, regions,
+and user-defined groups of regions that report aggregate data for all
+group members.
 
-The newly created regions are automatically placed into a group unless
-the \fB\-\-nogroup\fP option is given. When grouping is enabled a group
-alias may be specified using the \fB\-\-alias\fP option.
+The list may be a single object type, a comma separated list of types,
+or the special value 'all'.
+
+The currently available object types are 'area', 'region' and 'group'.
 .
 .HP
 .BR \-\-units
@@ -560,8 +574,8 @@ By default regions that map a file are placed into a group and the
 group alias is set to the basename of the file. This behaviour can be
 overridden with the \fB\-\-alias\fP and \fB\-\-nogroup\fP options.
 
-Use the \fB\-\-group\fP option to only display information for groups
-when listing and reporting.
+To display only group information when listing and reporting, use the
+\fB\-\-statstype\fP option with the 'group' type.
 .
 .HP
 .CMD_DELETE
@@ -595,9 +609,6 @@ regions is given as a comma-separated list of region identifiers. A
 continuous range of identifers spanning from \fBR1\fP to \fBR2\fP may
 be expressed as '\fBR1\fP-\fBR2\fP'.
 
-Regions that have a histogram configured can be grouped: in this case
-the number of histogram bins and their bounds must match exactly.
-
 On success the group list and newly created \fBgroup_id\fP are
 printed to stdout.
 .
@@ -617,13 +628,13 @@ regardless of region program ID values.
 By default only regions and groups are included in list output. If
 \fB\-v\fP or \fB\-\-verbose\fP is given the report will also include a
 row of information for each configured group and for each area contained
-in each region displayed.
+in each region displayed (regions that contain a single area are by
+default omitted from the verbose list since their properties are
+identical to the area that they contain - to view all regions regardless
+of the number of areas they contain use \fB\-\-statstype\fP).
 
-Regions that contain a single area are by default omitted from the
-verbose list since their properties are identical to the area that they
-contain - to view all regions regardless of the number of areas present
-use \fB\-\-region\fP). To also view the areas contained within regions
-use \fB\-\-area\fP.
+Specific combinations of objects may be selected using the
+\fB\-\-statstype\fP option.
 
 If \fB\-\-histogram\fP is given the report will include the bin count
 and latency boundary values for any configured histograms.
@@ -650,9 +661,8 @@ values and latency boundaries.
 If the \fB\-\-relative\fP is used the default histogram field displays
 bin values as a percentage of the total number of I/Os.
 
-Object types (areas, regions and groups) to include in the report are
-selected using the \fB\-\-area\fP, \fB\-\-region\fP, and \fB\-\-group\fP
-options.
+Object types (areas, regions and groups) are selected using the
+\fB\-\-statstype\fP option.
 .
 .HP
 .CMD_UNGROUP

tools/dmsetup.c

@@ -4768,56 +4768,6 @@ static uint64_t _nr_areas_from_step(uint64_t len, int64_t step)
 	return (len / step) + !!(len % (uint64_t) step);
 }
 
-/* maximum length of a string representation of an integer */
-#define max_int_strlen(i) (strlen(#i))
-#define MAX_UINT64_STRLEN max_int_strlen(UINT64_MAX)
-static int _stats_group_segments(struct dm_stats *dms, uint64_t *region_ids,
-				 int count, const char *alias)
-{
-	/* NULL, commas, and count * region_id */
-	size_t bufsize = 1 + count + count * MAX_UINT64_STRLEN;
-	char *this_region, *regions = NULL;
-	uint64_t group_id;
-	int r, i;
-
-	this_region = regions = dm_malloc(bufsize);
-
-	if (!regions) {
-		log_error("Could not allocate memory for region_id table.");
-		return 0;
-	}
-
-	for (i = 0; i < count; i++) {
-		/*
-		 * We don't expect large numbers of segments (compared to e.g.
-		 * --filemap): use a fixed-size buffer based on the number of
-		 *  region identifiers and do not collapse continuous ranges
-		 *  of identifiers in the group descriptor argument.
-		 */
-		r = dm_snprintf(this_region, bufsize, FMTu64 "%s", region_ids[i],
-				(i < (count - 1)) ? "," : "");
-		if (r < 0)
-			goto_bad;
-		this_region += r;
-		bufsize -= r;
-	}
-
-	/* refresh handle */
-	if (!(r = dm_stats_list(dms, NULL)))
-		goto bad;
-
-	if ((r = dm_stats_create_group(dms, regions, alias, &group_id)))
-		printf("Grouped regions %s as group ID " FMTu64 "%s%s\n",
-		       regions, group_id, (alias) ? " with alias " : "",
-		       (alias) ? : "");
-	else
-		log_error("Failed to create group for regions %s", regions);
-
-bad:
-	dm_free(regions);
-	return r;
-}
-
 /*
  * Create a single region starting at start and spanning len sectors,
  * or, if the segments argument is no-zero create one region for each
@@ -4834,9 +4784,8 @@ static int _do_stats_create_regions(struct dm_stats *dms,
 {
 	uint64_t this_start = 0, this_len = len, region_id = UINT64_C(0);
 	const char *devname = NULL, *histogram = _string_args[BOUNDS_ARG];
-	int r = 0, count = 0, precise = _switches[PRECISE_ARG];
+	int r = 0, precise = _switches[PRECISE_ARG];
 	struct dm_histogram *bounds = NULL; /* histogram bounds */
-	uint64_t *region_ids = NULL; /* segments */
 	char *target_type, *params; /* unused */
 	struct dm_task *dmt;
 	struct dm_info info;
@@ -4845,12 +4794,6 @@ static int _do_stats_create_regions(struct dm_stats *dms,
 	if (histogram && !(bounds = dm_histogram_bounds_from_string(histogram)))
 		return_0;
 
-	if (_switches[ALIAS_ARG] && _switches[NOGROUP_ARG]) {
-		log_error("Cannot set alias with --nogroup.");
-		dm_stats_destroy(dms);
-		return 0;
-	}
-
 	if (!(dmt = dm_task_create(DM_DEVICE_TABLE))) {
 		dm_histogram_bounds_destroy(bounds);
 		dm_stats_destroy(dms);
@@ -4875,11 +4818,6 @@ static int _do_stats_create_regions(struct dm_stats *dms,
 	if (!(devname = dm_task_get_name(dmt)))
 		goto_out;
 
-	if (!segments || (info.target_count == 1))
-		region_ids = &region_id;
-	else
-		region_ids = dm_malloc(info.target_count * sizeof(*region_ids));
-
 	do {
 		uint64_t segment_start, segment_len;
 		next = dm_get_next_target(dmt, next, &segment_start, &segment_len,
@@ -4900,10 +4838,10 @@ static int _do_stats_create_regions(struct dm_stats *dms,
 			 */
 			this_start = (segments) ? segment_start : start;
 			this_len = (segments) ? segment_len : this_len;
-			if (!(r = dm_stats_create_region(dms, &region_ids[count],
-							 this_start, this_len, step,
-							 precise, bounds,
-							 program_id, user_data))) {
+			if (!dm_stats_create_region(dms, &region_id,
+						    this_start, this_len, step,
+						    precise, bounds,
+						    program_id, user_data)) {
 				log_error("%s: Could not create statistics region.",
 					  devname);
 				goto out;
@@ -4911,19 +4849,12 @@ static int _do_stats_create_regions(struct dm_stats *dms,
 
 			printf("%s: Created new region with "FMTu64" area(s) as "
 			       "region ID "FMTu64"\n", devname,
-			       _nr_areas_from_step(this_len, step),
-			       region_ids[count++]);
+			       _nr_areas_from_step(this_len, step), region_id);
 		}
 	} while (next);
-
-	if (!_switches[NOGROUP_ARG] && segments)
-		r = _stats_group_segments(dms, region_ids, count,
-					  _string_args[ALIAS_ARG]);
+	r = 1;
 
 out:
-	if (region_ids != &region_id)
-		dm_free(region_ids);
-
 	dm_task_destroy(dmt);
 	dm_stats_destroy(dms);
 	dm_histogram_bounds_destroy(bounds);
@@ -4951,9 +4882,7 @@ static char *_get_abspath(const char *path)
 static int _stats_create_file(CMD_ARGS)
 {
 	const char *alias, *program_id = DM_STATS_PROGRAM_ID;
-	const char *histogram = _string_args[BOUNDS_ARG];
 	uint64_t *regions, *region, count = 0;
-	struct dm_histogram *bounds = NULL;
 	char *path, *abspath = NULL;
 	int group, fd, precise;
 	struct dm_stats *dms;
@@ -4978,6 +4907,11 @@ static int _stats_create_file(CMD_ARGS)
 		return 0;
 	}
 
+	if (_switches[BOUNDS_ARG]) {
+		log_error("--bounds is not yet supported with --filemap.");
+		return 0;
+	}
+
 	/* _stats_create_file does not use _process_all() */
 	if (names) {
 		log_error("Device argument not compatible with --filemap.");
@@ -5011,9 +4945,6 @@ static int _stats_create_file(CMD_ARGS)
 		}
 	}
 
-	if (histogram && !(bounds = dm_histogram_bounds_from_string(histogram)))
-		return_0;
-
 	if (_switches[PROGRAM_ID_ARG])
 		program_id = _string_args[PROGRAM_ID_ARG];
 	if (!strlen(program_id) && !_switches[FORCE_ARG])
@@ -5034,7 +4965,7 @@ static int _stats_create_file(CMD_ARGS)
 	fd = open(abspath, O_RDONLY);
 
 	if (fd < 0) {
-		log_error("Could not open %s for reading", abspath);
+		log_error("Could not open %s for reading", path);
 		goto bad;
 	}
 
@@ -5057,15 +4988,15 @@ static int _stats_create_file(CMD_ARGS)
 	}
 
 	regions = dm_stats_create_regions_from_fd(dms, fd, group, precise,
-						  bounds, alias);
+						  NULL, alias);
 
 	if (close(fd))
-		log_error("Error closing %s", abspath);
+		log_error("Error closing %s", path);
 
 	fd = -1;
 
 	if (!regions) {
-		log_error("Could not create regions from file %s", abspath);
+		log_error("Could not create regions from file %s", path);
 		goto bad;
 	}