[PATCH v2 0/4] Modifications of some 'hist' APIs

[PATCH v2 0/4] Modifications of some 'hist' APIs

[Yordan Karadzhov (VMware)]

The patch-set contains various minor modifications inspired by my first
experience using the 'hist' APIs of libtracefs.

This patch-set applies on top of patches
'libtracefs: Fix code indentation' and
'libtracefs: Fix add_sort_key()'

Yordan Karadzhov (VMware) (4):
  libtracefs: Add new constructors for histograms
  libtracefs: Transform tracefs_hist_add_sort_key()
  libtracefs: Add new 'hist' APIs
  libtracefs: Update 'hist' documentation

 Documentation/libtracefs-hist-cont.txt |  30 ++++--
 Documentation/libtracefs-hist.txt      |  80 ++++++++-------
 include/tracefs.h                      |  26 ++++-
 src/tracefs-hist.c                     | 132 +++++++++++++++++++++----
 4 files changed, 207 insertions(+), 61 deletions(-)

-- 
2.30.2

[PATCH v2 1/4] libtracefs: Add new constructors for histograms

[Yordan Karadzhov (VMware)]

The current way to create an N-dimensional histogram is bit
counterintuitive. We first have to call tracefs_hist_alloc()
which is essentially a constructor of 1d histogram. Then we
have to call tracefs_hist_add_key() N-1 times in order to
increase the dimentions of the histogram. Here we transform
tracefs_hist_alloc() into a constructor of N-dimensional
histogram and we also add 2 helper constructors for 1d and
2d histograms.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 include/tracefs.h  | 19 +++++++++--
 src/tracefs-hist.c | 84 ++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 87 insertions(+), 16 deletions(-)

diff --git a/include/tracefs.h b/include/tracefs.h
index 9d84777..5c4141e 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -310,10 +310,25 @@ enum tracefs_compare {
 };
 
 void tracefs_hist_free(struct tracefs_hist *hist);
+struct tracefs_hist *
+tracefs_hist1d_alloc(struct tep_handle *tep,
+		     const char *system, const char *event_name,
+		     const char *key, enum tracefs_hist_key_type type);
+struct tracefs_hist *
+tracefs_hist2d_alloc(struct tep_handle *tep,
+		     const char *system, const char *event_name,
+		     const char *key1, enum tracefs_hist_key_type type1,
+		     const char *key2, enum tracefs_hist_key_type type2);
+
+struct tracefs_hist_axis {
+	const char *key;
+	enum tracefs_hist_key_type type;
+};
+
 struct tracefs_hist *
 tracefs_hist_alloc(struct tep_handle *tep,
-		   const char *system, const char *event,
-		   const char *key, enum tracefs_hist_key_type type);
+		   const char *system, const char *event_name,
+		   struct tracefs_hist_axis *axes);
 int tracefs_hist_add_key(struct tracefs_hist *hist, const char *key,
 			 enum tracefs_hist_key_type type);
 int tracefs_hist_add_value(struct tracefs_hist *hist, const char *value);
diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
index c112b68..ea12204 100644
--- a/src/tracefs-hist.c
+++ b/src/tracefs-hist.c
@@ -189,7 +189,7 @@ void tracefs_hist_free(struct tracefs_hist *hist)
 }
 
 /**
- * tracefs_hist_alloc - Initialize a histogram
+ * tracefs_hist1d_alloc - Initialize one-dimensional histogram
  * @tep: The tep handle that has the @system and @event.
  * @system: The system the histogram event is in.
  * @event_name: The name of the event that the histogram will be attached to.
@@ -205,15 +205,70 @@ void tracefs_hist_free(struct tracefs_hist *hist)
  * NULL on failure.
  */
 struct tracefs_hist *
+tracefs_hist1d_alloc(struct tep_handle *tep,
+		     const char *system, const char *event_name,
+		     const char *key, enum tracefs_hist_key_type type)
+{
+	struct tracefs_hist_axis axis[] = {{key, type}, {NULL, 0}};
+
+	return tracefs_hist_alloc(tep, system, event_name, axis);
+}
+
+/**
+ * tracefs_hist2d_alloc - Initialize two-dimensional histogram
+ * @tep: The tep handle that has the @system and @event.
+ * @system: The system the histogram event is in.
+ * @event: The event that the histogram will be attached to.
+ * @key1: The first primary key the histogram will use
+ * @type1: The format type of the first key.
+ * @key2: The second primary key the histogram will use
+ * @type2: The format type of the second key.
+ *
+ * Will initialize a histogram descriptor that will be attached to
+ * the @system/@event with the given @key1 and @key2 as the primaries.
+ * This only initializes the descriptor, it does not start the histogram
+ * in the kernel.
+ *
+ * Returns an initialized histogram on success.
+ * NULL on failure.
+ */
+struct tracefs_hist *
+tracefs_hist2d_alloc(struct tep_handle *tep,
+		     const char *system, const char *event_name,
+		     const char *key1, enum tracefs_hist_key_type type1,
+		     const char *key2, enum tracefs_hist_key_type type2)
+{
+	struct tracefs_hist_axis axis[] = {{key1, type1},
+					   {key2, type2},
+					   {NULL, 0}};
+
+	return tracefs_hist_alloc(tep, system, event_name, axis);
+}
+
+/**
+ * tracefs_hist_alloc - Initialize N-dimensional histogram
+ * @tep: The tep handle that has the @system and @event.
+ * @system: The system the histogram event is in
+ * @event: The event that the histogram will be attached to
+ * @axes: An array of histogram axes, terminated by a {NULL, 0} entry
+ *
+ * Will initialize a histogram descriptor that will be attached to
+ * the @system/@event. This only initializes the descriptor with the given
+ * @axes keys as primaries. This only initializes the descriptor, it does
+ * not start the histogram in the kernel.
+ *
+ * Returns an initialized histogram on success.
+ * NULL on failure.
+ */
+struct tracefs_hist *
 tracefs_hist_alloc(struct tep_handle *tep,
 		   const char *system, const char *event_name,
-		   const char *key, enum tracefs_hist_key_type type)
+		   struct tracefs_hist_axis *axes)
 {
 	struct tep_event *event;
 	struct tracefs_hist *hist;
-	int ret;
 
-	if (!system || !event_name || !key)
+	if (!system || !event_name)
 		return NULL;
 
 	event = tep_find_event_by_name(tep, system, event_name);
@@ -226,20 +281,21 @@ tracefs_hist_alloc(struct tep_handle *tep,
 
 	tep_ref(tep);
 	hist->tep = tep;
-
 	hist->event = event;
 	hist->system = strdup(system);
 	hist->event_name = strdup(event_name);
+	if (!hist->system || !hist->event_name)
+		goto fail;
 
-	ret = tracefs_hist_add_key(hist, key, type);
-
-	if (!hist->system || !hist->event || ret < 0) {
-		tracefs_hist_free(hist);
-		return NULL;
-	}
-
+	for (; axes && axes->key; axes++)
+		if (tracefs_hist_add_key(hist, axes->key, axes->type) < 0)
+			goto fail;
 
 	return hist;
+
+ fail:
+	tracefs_hist_free(hist);
+	return NULL;
 }
 
 /**
@@ -1768,8 +1824,8 @@ tracefs_synth_get_start_hist(struct tracefs_synth *synth)
 				return NULL;
 			}
 		} else {
-			hist = tracefs_hist_alloc(tep, system, event,
-						  key, type);
+			hist = tracefs_hist1d_alloc(tep, system, event,
+						    key, type);
 			if (!hist)
 				return NULL;
 		}
-- 
2.30.2

[PATCH v2 2/4] libtracefs: Transform tracefs_hist_add_sort_key()

[Yordan Karadzhov (VMware)]

The current version of the API makes it hard to add multiple sort keys
to a histogram. The only way to do this is to use the variadic arguments,
however in order to do this the caller has to know the number of sort
keys at compile time, because the method overwrite all previously added
keys. The problem is addressed by creating a tracefs_hist_add_sort_key()
into two methods - one that overwrite and one that does not.
The current version of 'tracefs_hist_add_sort_key()' gets renamed to
'tracefs_hist_set_sort_key()' without introducing any functional changes.
In the same time a new 'tracefs_hist_add_sort_key()' function is
defined. The new function can add one new sort key to the list of
previously added sort keys.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 include/tracefs.h  |  4 +++-
 src/tracefs-hist.c | 33 ++++++++++++++++++++++++++++++---
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/include/tracefs.h b/include/tracefs.h
index 5c4141e..230bc41 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -333,7 +333,9 @@ int tracefs_hist_add_key(struct tracefs_hist *hist, const char *key,
 			 enum tracefs_hist_key_type type);
 int tracefs_hist_add_value(struct tracefs_hist *hist, const char *value);
 int tracefs_hist_add_sort_key(struct tracefs_hist *hist,
-			      const char *sort_key, ...);
+			      const char *sort_key);
+int tracefs_hist_reset_sort_key(struct tracefs_hist *hist,
+				const char *sort_key, ...);
 int tracefs_hist_sort_key_direction(struct tracefs_hist *hist,
 				    const char *sort_key,
 				    enum tracefs_hist_sort_direction dir);
diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
index ea12204..a7c20de 100644
--- a/src/tracefs-hist.c
+++ b/src/tracefs-hist.c
@@ -437,16 +437,43 @@ add_sort_key(struct tracefs_hist *hist, const char *sort_key, char **list)
 /**
  * tracefs_hist_add_sort_key - add a key for sorting the histogram
  * @hist: The histogram to add the sort key to
+ * @sort_key: The key to sort
+ *
+ * Call the function to add to the list of sort keys of the histohram in
+ * the order of priority that the keys would be sorted on output.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int tracefs_hist_add_sort_key(struct tracefs_hist *hist,
+			      const char *sort_key)
+{
+	char **list = hist->sort;
+
+	if (!hist || !sort_key)
+		return -1;
+
+	list = add_sort_key(hist, sort_key, hist->sort);
+	if (!list)
+		return -1;
+
+	hist->sort = list;
+
+	return 0;
+}
+
+/**
+ * tracefs_hist_reset_sort_key - set a key for sorting the histogram
+ * @hist: The histogram to set the sort key to
  * @sort_key: The key to sort (and the strings after it)
  *  Last one must be NULL.
  *
- * Add a list of sort keys in the order of priority that the
+ * Set a list of sort keys in the order of priority that the
  * keys would be sorted on output. Keys must be added first.
  *
  * Returns 0 on success, -1 on error.
  */
-int tracefs_hist_add_sort_key(struct tracefs_hist *hist,
-			      const char *sort_key, ...)
+int tracefs_hist_reset_sort_key(struct tracefs_hist *hist,
+				const char *sort_key, ...)
 {
 	char **list = NULL;
 	char **tmp;
-- 
2.30.2

[PATCH v2 3/4] libtracefs: Add new 'hist' APIs

[Yordan Karadzhov (VMware)]

The new APIs will be valuable in a number of different scenarios.
For example if the user wants to implement a function that does
the readout of a histogram, the only argument that will have to
be passed to this function is the histogram descriptor. The same
applies for the case when the user wants to print an adequate
error message in a case of a failure.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 include/tracefs.h  |  3 +++
 src/tracefs-hist.c | 15 +++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/include/tracefs.h b/include/tracefs.h
index 230bc41..ebe83fe 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -329,6 +329,9 @@ struct tracefs_hist *
 tracefs_hist_alloc(struct tep_handle *tep,
 		   const char *system, const char *event_name,
 		   struct tracefs_hist_axis *axes);
+const char *tracefs_get_hist_name(struct tracefs_hist *hist);
+const char *tracefs_get_hist_event(struct tracefs_hist *hist);
+const char *tracefs_get_hist_system(struct tracefs_hist *hist);
 int tracefs_hist_add_key(struct tracefs_hist *hist, const char *key,
 			 enum tracefs_hist_key_type type);
 int tracefs_hist_add_value(struct tracefs_hist *hist, const char *value);
diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
index a7c20de..27bab00 100644
--- a/src/tracefs-hist.c
+++ b/src/tracefs-hist.c
@@ -39,6 +39,21 @@ struct tracefs_hist {
 	unsigned int		filter_state;
 };
 
+const char *tracefs_get_hist_name(struct tracefs_hist *hist)
+{
+	return hist ? hist->name : NULL;
+}
+
+const char *tracefs_get_hist_event(struct tracefs_hist *hist)
+{
+	return hist ? hist->event_name : NULL;
+}
+
+const char *tracefs_get_hist_system(struct tracefs_hist *hist)
+{
+	return hist ? hist->system : NULL;
+}
+
 static void add_list(struct trace_seq *seq, const char *start,
 		     char **list)
 {
-- 
2.30.2

[PATCH v2 4/4] libtracefs: Update 'hist' documentation

[Yordan Karadzhov (VMware)]

New 'hist' APIs have been added and some existing APIs have been
modified. Here we update the documentation accordingly.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 Documentation/libtracefs-hist-cont.txt | 30 ++++++++--
 Documentation/libtracefs-hist.txt      | 80 +++++++++++++++-----------
 2 files changed, 69 insertions(+), 41 deletions(-)

diff --git a/Documentation/libtracefs-hist-cont.txt b/Documentation/libtracefs-hist-cont.txt
index 1b0153c..2ee830d 100644
--- a/Documentation/libtracefs-hist-cont.txt
+++ b/Documentation/libtracefs-hist-cont.txt
@@ -11,18 +11,36 @@ SYNOPSIS
 --
 *#include <tracefs.h>*
 
-int tracefs_hist_pause(struct tracefs_hist pass:[*]hist);
-int tracefs_hist_continue(struct tracefs_hist pass:[*]hist);
-int tracefs_hist_reset(struct tracefs_hist pass:[*]hist);
+int tracefs_hist_start(struct tracefs_instance pass:[*]instance, struct tracefs_hist pass:[*]hist);
+int tracefs_hist_destory(struct tracefs_instance pass:[*]instance, struct tracefs_hist pass:[*]hist);
+int tracefs_hist_pause(struct tracefs_instance pass:[*]instance, struct tracefs_hist pass:[*]hist);
+int tracefs_hist_continue(struct tracefs_instance pass:[*]instance, struct tracefs_hist pass:[*]hist);
+int tracefs_hist_reset(struct tracefs_instance pass:[*]instance, struct tracefs_hist pass:[*]hist);
+
 --
 
 DESCRIPTION
 -----------
+
+*tracefs_hist_start* is called to actually start the histogram _hist_.
+The _instance_ is the instance to start the histogram in, NULL if it
+should start at the top level.
+
 *tracefs_hist_pause* is called to pause the histogram _hist_.
+The _instance_ is the instance to pause the histogram in, NULL if it
+is in the top level.
 
 *tracefs_hist_continue* is called to continue a paused histogram _hist_.
+The _instance_ is the instance to continue the histogram, NULL if it
+is in the top level.
 
 *tracefs_hist_reset* is called to clear / reset the histogram _hist_.
+The _instance_ is the instance to clear the histogram, NULL if it
+is in the top level.
+
+*tracefs_hist_destory* is called to delete the histogram where it will no longer
+exist.  The _instance_ is the instance to delete the histogram from, NULL if it
+is in the top level.
 
 RETURN VALUE
 ------------
@@ -83,14 +101,14 @@ int main (int argc, char **argv, char **env)
 	}
 
 	hist = tracefs_hist_alloc(instance, "kmem", "kmalloc",
-				  "call_site", TRACEFS_HIST_KEY_SYM);
+				  "call_site", TRACEFS_HIST_KEY_SYM,
+				  "bytes_req", 0);
 	if (!hist) {
 		fprintf(stderr, "Failed hist create\n");
 		exit(-1);
 	}
 
-	ret = tracefs_hist_add_key(hist, "bytes_req", 0);
-	ret |= tracefs_hist_add_value(hist, "bytes_alloc");
+	ret = tracefs_hist_add_value(hist, "bytes_alloc");
 	ret |= tracefs_hist_add_sort_key(hist, "bytes_req", "bytes_alloc", NULL);
 
 	ret |= tracefs_hist_sort_key_direction(hist, "bytes_alloc",
diff --git a/Documentation/libtracefs-hist.txt b/Documentation/libtracefs-hist.txt
index 03ac077..0139ba8 100644
--- a/Documentation/libtracefs-hist.txt
+++ b/Documentation/libtracefs-hist.txt
@@ -4,7 +4,7 @@ libtracefs(3)
 NAME
 ----
 tracefs_hist_alloc, tracefs_hist_free, tracefs_hist_add_key, tracefs_hist_add_value, tracefs_hist_add_name, tracefs_hist_start,
-tracefs_hist_destory, tracefs_hist_add_sort_key, tracefs_hist_sort_key_direction - Create and update event histograms
+tracefs_hist_destory, tracefs_hist_add_sort_key, tracefs_hist_reset_sort_key,tracefs_hist_sort_key_direction - Create and update event histograms
 
 SYNOPSIS
 --------
@@ -12,15 +12,24 @@ SYNOPSIS
 --
 *#include <tracefs.h>*
 
-struct tracefs_hist pass:[*]tracefs_hist_alloc(struct tracefs_tep pass:[*] tep,
+struct tracefs_hist pass:[*]tracefs_hist1d_alloc(struct tracefs_tep pass:[*] tep,
 			const char pass:[*]system, const char pass:[*]event,
 			const char pass:[*]key, enum tracefs_hist_key_type type);
+struct tracefs_hist pass:[*]tracefs_hist2d_alloc(struct tracefs_tep pass:[*] tep,
+			const char pass:[*]system, const char pass:[*]event,
+			const char pass:[*]key1, enum tracefs_hist_key_type type1,
+			const char pass:[*]key2, enum tracefs_hist_key_type type2));
+struct tracefs_hist pass:[*]tracefs_hist_alloc(struct tracefs_tep pass:[*] tep,
+			const char pass:[*]system, const char pass:[*]event,
+			struct tracefs_hist_axis [*]axes);
 void tracefs_hist_free(struct tracefs_hist pass:[*]hist);
 int tracefs_hist_add_key(struct tracefs_hist pass:[*]hist, const char pass:[*]key,
 			 enum tracefs_hist_key_type type);
 int tracefs_hist_add_value(struct tracefs_hist pass:[*]hist, const char pass:[*]value);
 int tracefs_hist_add_sort_key(struct tracefs_hist pass:[*]hist,
-			      const char pass:[*]sort_key, ...);
+			      const char pass:[*]sort_key);
+int tracefs_hist_reset_sort_key(struct tracefs_hist pass:[*]hist,
+			        const char pass:[*]sort_key, ...);
 int tracefs_hist_sort_key_direction(struct tracefs_hist pass:[*]hist,
 				    const char pass:[*]sort_key,
 				    enum tracefs_hist_sort_direction dir);
@@ -36,8 +45,7 @@ int tracefs_hist_show(struct trace_seq pass:[*]s, struct tracefs_instance pass:[
 int tracefs_hist_command(struct tracefs_instance pass:[*]instance,
 			 struct tracefs_hist pass:[*]hist,
 			 enum tracefs_hist_command command);
-int tracefs_hist_start(struct tracefs_instance pass:[*]instance, struct tracefs_hist pass:[*]hist);
-int tracefs_hist_destory(struct tracefs_instance pass:[*]instance, struct tracefs_hist pass:[*]hist);
+
 --
 
 DESCRIPTION
@@ -47,14 +55,31 @@ The syntax can be complex and difficult to get correct. This API handles the
 syntax, and facilitates the creation and interaction with the event histograms.
 See https://www.kernel.org/doc/html/latest/trace/histogram.html for more information.
 
-*tracefs_hist_alloc*() allocates a "struct tracefs_hist" descriptor and returns
-the address of it. This descriptor must be freed by *tracefs_hist_free*().
+*tracefs_hist1d_alloc*() allocates a "struct tracefs_hist" descriptor of a one-dimensional
+histogram and returns the address of it. This descriptor must be freed by *tracefs_hist_free*().
 The _tep_ is a trace event handle (see *tracefs_local_events*(3)), that holds the
 _system_ and _event_ that the histogram will be attached to. The _system_ is the
 system or group of the event. The _event_ is the event to attach the histogram to.
-The _key_ is a field of the event that will be used as the key of the histogram.
+The _key_ is a field of the event that will be used as the key(dimension) of the histogram.
 The _type_ is the type of the _key_. See KEY TYPES below.
 
+*tracefs_hist2d_alloc*() allocates a "struct tracefs_hist" descriptor of a two-dimensional
+histogram and returns the address of it. This descriptor must be freed by *tracefs_hist_free*().
+The _tep_ is a trace event handle (see *tracefs_local_events*(3)), that holds the
+_system_ and _event_ that the histogram will be attached to. The _system_ is the
+system or group of the event. The _event_ is the event to attach the histogram to.
+The _key1_ is the first field of the event that will be used as the key(dimension)
+of the histogram. The _type1_ is the type of the _key1_. See KEY TYPES below.
+The _key2_ is the second field of the event that will be used as the key(dimension)
+of the histogram. The _type2_ is the type of the _key2_. See KEY TYPES below.
+
+*tracefs_hist_alloc*() allocates a "struct tracefs_hist" descriptor of an N-dimensional
+histogram and returns the address of it. This descriptor must be freed by *tracefs_hist_free*().
+The _tep_ is a trace event handle (see *tracefs_local_events*(3)), that holds the
+_system_ and _event_ that the histogram will be attached to. The _system_ is the
+system or group of the event. The _event_ is the event to attach the histogram to.
+The _axes_ is an array of _key_ / _type_ pairs, defining the dimentions of the histogram.
+
 *tracefs_hist_free*() frees the _tracefs_hist_ descriptor. Note, it does not stop
 or disable the running histogram if it was started. *tracefs_hist_destroy*() needs
 to be called to do so.
@@ -73,8 +98,14 @@ key was hit. The _hist_ is the histogram descriptor to add the value to.
 The _value_ is a field in the histogram to add to when an instane of the
 key is hit.
 
-*tracefs_hist_add_sort_key*() will add a key to sort on. Th _hist_ is the
-the histrogram descriptor to add the sort key to. The _sort_key_ is a string
+*tracefs_hist_add_sort_key*() will add a key to sort on. The _hist_ is the
+histrogram descriptor to add the sort key to. The _sort_key_ is a string
+that must match either an already defined key of the histogram, or an already
+defined value. If _hist_  already has sorting keys (previously added) the new
+_sort_key_ will have lower priority(be secondary or so on) when sorting.
+
+*tracefs_hist_reset_sort_key*() will reset the list of key to sort on. The _hist_ is
+the histrogram descriptor to reset the sort key to. The _sort_key_ is a string
 that must match either an already defined key of the histogram, or an already
 defined value. Multiple sort keys may be added to denote a secondary, sort order
 and so on, but all sort keys must match an existing key or value, or be
@@ -146,27 +177,6 @@ The _cmd_ can be one of:
 The below functions are wrappers to tracefs_hist_command() to make the
 calling conventions a bit easier to understand what is happening.
 
-*tracefs_hist_start* is called to actually start the histogram _hist_.
-The _instance_ is the instance to start the histogram in, NULL if it
-should start at the top level.
-
-*tracefs_hist_pause* is called to pause the histogram _hist_.
-The _instance_ is the instance to pause the histogram in, NULL if it
-is in the top level.
-
-*tracefs_hist_continue* is called to continue a paused histogram _hist_.
-The _instance_ is the instance to continue the histogram, NULL if it
-is in the top level.
-
-*tracefs_hist_reset* is called to clear / reset the histogram _hist_.
-The _instance_ is the instance to clear the histogram, NULL if it
-is in the top level.
-
-*tracefs_hist_destory* is called to delete the histogram where it will no longer
-exist.  The _instance_ is the instance to delete the histogram from, NULL if it
-is in the top level.
-
-
 KEY TYPES
 ---------
 
@@ -263,15 +273,15 @@ int main (int argc, char **argv, char **env)
 	}
 
 	tep = tracefs_local_events(NULL);
-	hist = tracefs_hist_alloc(tep, "kmem", "kmalloc",
-				  "call_site", TRACEFS_HIST_KEY_SYM);
+	hist = tracefs_hist2d_alloc(tep, "kmem", "kmalloc",
+				    "call_site", TRACEFS_HIST_KEY_SYM,
+				    "bytes_req", 0);
 	if (!hist) {
 		fprintf(stderr, "Failed hist create\n");
 		exit(-1);
 	}
 
-	ret = tracefs_hist_add_key(hist, "bytes_req", 0);
-	ret |= tracefs_hist_add_value(hist, "bytes_alloc");
+	ret = tracefs_hist_add_value(hist, "bytes_alloc");
 	ret |= tracefs_hist_add_sort_key(hist, "bytes_req", "bytes_alloc", NULL);
 
 	ret |= tracefs_hist_sort_key_direction(hist, "bytes_alloc",
-- 
2.30.2

Re: [PATCH v2 1/4] libtracefs: Add new constructors for histograms

[Tzvetomir Stoyanov]

On Fri, Sep 24, 2021 at 12:59 PM Yordan Karadzhov (VMware)
<y.karadz@gmail.com> wrote:
>
> The current way to create an N-dimensional histogram is bit
> counterintuitive. We first have to call tracefs_hist_alloc()
> which is essentially a constructor of 1d histogram. Then we
> have to call tracefs_hist_add_key() N-1 times in order to
> increase the dimentions of the histogram. Here we transform
> tracefs_hist_alloc() into a constructor of N-dimensional
> histogram and we also add 2 helper constructors for 1d and
> 2d histograms.
>
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> ---
>  include/tracefs.h  | 19 +++++++++--
>  src/tracefs-hist.c | 84 ++++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 87 insertions(+), 16 deletions(-)
>
> diff --git a/include/tracefs.h b/include/tracefs.h
> index 9d84777..5c4141e 100644
> --- a/include/tracefs.h
> +++ b/include/tracefs.h
> @@ -310,10 +310,25 @@ enum tracefs_compare {
>  };
>
>  void tracefs_hist_free(struct tracefs_hist *hist);
> +struct tracefs_hist *
> +tracefs_hist1d_alloc(struct tep_handle *tep,
> +                    const char *system, const char *event_name,
> +                    const char *key, enum tracefs_hist_key_type type);
> +struct tracefs_hist *
> +tracefs_hist2d_alloc(struct tep_handle *tep,
> +                    const char *system, const char *event_name,
> +                    const char *key1, enum tracefs_hist_key_type type1,
> +                    const char *key2, enum tracefs_hist_key_type type2);
> +
> +struct tracefs_hist_axis {
> +       const char *key;
> +       enum tracefs_hist_key_type type;
> +};
> +
>  struct tracefs_hist *
>  tracefs_hist_alloc(struct tep_handle *tep,

For consistency with the newly added "alloc" APIs, this should be
renamed. The dimensions of the histogram are part of the name, this
one is for N dimensions:
  tracefs_hist_alloc_1d()
  tracefs_hist_alloc_2d()
  tracefs_hist_alloc_nd()
or something like that.

> -                  const char *system, const char *event,
> -                  const char *key, enum tracefs_hist_key_type type);
> +                  const char *system, const char *event_name,
> +                  struct tracefs_hist_axis *axes);
>  int tracefs_hist_add_key(struct tracefs_hist *hist, const char *key,
>                          enum tracefs_hist_key_type type);
>  int tracefs_hist_add_value(struct tracefs_hist *hist, const char *value);
> diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
> index c112b68..ea12204 100644
> --- a/src/tracefs-hist.c
> +++ b/src/tracefs-hist.c
> @@ -189,7 +189,7 @@ void tracefs_hist_free(struct tracefs_hist *hist)
>  }

[...]

--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

Re: [PATCH v2 2/4] libtracefs: Transform tracefs_hist_add_sort_key()

[Tzvetomir Stoyanov]

On Fri, Sep 24, 2021 at 12:59 PM Yordan Karadzhov (VMware)
<y.karadz@gmail.com> wrote:
>
> The current version of the API makes it hard to add multiple sort keys
> to a histogram. The only way to do this is to use the variadic arguments,
> however in order to do this the caller has to know the number of sort
> keys at compile time, because the method overwrite all previously added
> keys. The problem is addressed by creating a tracefs_hist_add_sort_key()
> into two methods - one that overwrite and one that does not.
> The current version of 'tracefs_hist_add_sort_key()' gets renamed to
> 'tracefs_hist_set_sort_key()' without introducing any functional changes.
> In the same time a new 'tracefs_hist_add_sort_key()' function is
> defined. The new function can add one new sort key to the list of
> previously added sort keys.
>
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> ---
>  include/tracefs.h  |  4 +++-
>  src/tracefs-hist.c | 33 ++++++++++++++++++++++++++++++---
>  2 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/include/tracefs.h b/include/tracefs.h
> index 5c4141e..230bc41 100644
> --- a/include/tracefs.h
> +++ b/include/tracefs.h
> @@ -333,7 +333,9 @@ int tracefs_hist_add_key(struct tracefs_hist *hist, const char *key,
>                          enum tracefs_hist_key_type type);
>  int tracefs_hist_add_value(struct tracefs_hist *hist, const char *value);
>  int tracefs_hist_add_sort_key(struct tracefs_hist *hist,
> -                             const char *sort_key, ...);
> +                             const char *sort_key);
> +int tracefs_hist_reset_sort_key(struct tracefs_hist *hist,
> +                               const char *sort_key, ...);

The new name of the function, mentioned in the patch description, is
    tracefs_hist_set_sort_key()
I like the name with "set" instead of "reset". The behaviour of the
function should be described in the function comments - to stress that
the old keys are overwritten.

>  int tracefs_hist_sort_key_direction(struct tracefs_hist *hist,
>                                     const char *sort_key,
>                                     enum tracefs_hist_sort_direction dir);
> diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
> index ea12204..a7c20de 100644
> --- a/src/tracefs-hist.c
> +++ b/src/tracefs-hist.c
> @@ -437,16 +437,43 @@ add_sort_key(struct tracefs_hist *hist, const char *sort_key, char **list)
>  /**
>   * tracefs_hist_add_sort_key - add a key for sorting the histogram
>   * @hist: The histogram to add the sort key to
> + * @sort_key: The key to sort
> + *
> + * Call the function to add to the list of sort keys of the histohram in
> + * the order of priority that the keys would be sorted on output.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +int tracefs_hist_add_sort_key(struct tracefs_hist *hist,
> +                             const char *sort_key)
> +{
> +       char **list = hist->sort;
> +
> +       if (!hist || !sort_key)
> +               return -1;
> +
> +       list = add_sort_key(hist, sort_key, hist->sort);
> +       if (!list)
> +               return -1;
> +
> +       hist->sort = list;
> +
> +       return 0;
> +}
> +
> +/**
> + * tracefs_hist_reset_sort_key - set a key for sorting the histogram
> + * @hist: The histogram to set the sort key to
>   * @sort_key: The key to sort (and the strings after it)
>   *  Last one must be NULL.
>   *
> - * Add a list of sort keys in the order of priority that the
> + * Set a list of sort keys in the order of priority that the
>   * keys would be sorted on output. Keys must be added first.
>   *
>   * Returns 0 on success, -1 on error.
>   */
> -int tracefs_hist_add_sort_key(struct tracefs_hist *hist,
> -                             const char *sort_key, ...)
> +int tracefs_hist_reset_sort_key(struct tracefs_hist *hist,
> +                               const char *sort_key, ...)
>  {
>         char **list = NULL;
>         char **tmp;
> --
> 2.30.2
>


--
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center

Re: [PATCH v2 1/4] libtracefs: Add new constructors for histograms

[Steven Rostedt]

On Fri, 24 Sep 2021 18:51:38 +0300
Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:

> >  struct tracefs_hist *
> >  tracefs_hist_alloc(struct tep_handle *tep,  
> 
> For consistency with the newly added "alloc" APIs, this should be
> renamed. The dimensions of the histogram are part of the name, this
> one is for N dimensions:
>   tracefs_hist_alloc_1d()
>   tracefs_hist_alloc_2d()
>   tracefs_hist_alloc_nd()
> or something like that.

I rather not have the _nd().

 tracefs_hist_alloc_1d()
 tracefs_hist_alloc_2d() are more just helper handlers for specific cases.

 tracefs_hist_alloc() should be the "main" function, and hence doesn't need
any extra annotation.

-- Steve

Re: [PATCH v2 1/4] libtracefs: Add new constructors for histograms

[Yordan Karadzhov]

On 24.09.21 г. 18:54, Steven Rostedt wrote:
> On Fri, 24 Sep 2021 18:51:38 +0300
> Tzvetomir Stoyanov <tz.stoyanov@gmail.com> wrote:
> 
>>>   struct tracefs_hist *
>>>   tracefs_hist_alloc(struct tep_handle *tep,
>>
>> For consistency with the newly added "alloc" APIs, this should be
>> renamed. The dimensions of the histogram are part of the name, this
>> one is for N dimensions:
>>    tracefs_hist_alloc_1d()
>>    tracefs_hist_alloc_2d()
>>    tracefs_hist_alloc_nd()
>> or something like that.
> 
> I rather not have the _nd().
> 
>   tracefs_hist_alloc_1d()
>   tracefs_hist_alloc_2d() are more just helper handlers for specific cases.
> 
>   tracefs_hist_alloc() should be the "main" function, and hence doesn't need
> any extra annotation.
> 

My idea was the same.

Thanks!
Yordan


> -- Steve
> 

Re: [PATCH v2 2/4] libtracefs: Transform tracefs_hist_add_sort_key()

[Yordan Karadzhov]

On 24.09.21 г. 18:51, Tzvetomir Stoyanov wrote:
> On Fri, Sep 24, 2021 at 12:59 PM Yordan Karadzhov (VMware)
> <y.karadz@gmail.com> wrote:
>>
>> The current version of the API makes it hard to add multiple sort keys
>> to a histogram. The only way to do this is to use the variadic arguments,
>> however in order to do this the caller has to know the number of sort
>> keys at compile time, because the method overwrite all previously added
>> keys. The problem is addressed by creating a tracefs_hist_add_sort_key()
>> into two methods - one that overwrite and one that does not.
>> The current version of 'tracefs_hist_add_sort_key()' gets renamed to
>> 'tracefs_hist_set_sort_key()' without introducing any functional changes.
>> In the same time a new 'tracefs_hist_add_sort_key()' function is
>> defined. The new function can add one new sort key to the list of
>> previously added sort keys.
>>
>> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
>> ---
>>   include/tracefs.h  |  4 +++-
>>   src/tracefs-hist.c | 33 ++++++++++++++++++++++++++++++---
>>   2 files changed, 33 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/tracefs.h b/include/tracefs.h
>> index 5c4141e..230bc41 100644
>> --- a/include/tracefs.h
>> +++ b/include/tracefs.h
>> @@ -333,7 +333,9 @@ int tracefs_hist_add_key(struct tracefs_hist *hist, const char *key,
>>                           enum tracefs_hist_key_type type);
>>   int tracefs_hist_add_value(struct tracefs_hist *hist, const char *value);
>>   int tracefs_hist_add_sort_key(struct tracefs_hist *hist,
>> -                             const char *sort_key, ...);
>> +                             const char *sort_key);
>> +int tracefs_hist_reset_sort_key(struct tracefs_hist *hist,
>> +                               const char *sort_key, ...);
> 
> The new name of the function, mentioned in the patch description, is
>      tracefs_hist_set_sort_key()
> I like the name with "set" instead of "reset". The behaviour of the
> function should be described in the function comments - to stress that
> the old keys are overwritten.
> 

Steven suggested to name it 'reset' in the review of the first version.
Both 'set' and 'reset' are OK for me.

Thanks!
Yordan


>>   int tracefs_hist_sort_key_direction(struct tracefs_hist *hist,
>>                                      const char *sort_key,
>>                                      enum tracefs_hist_sort_direction dir);
>> diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
>> index ea12204..a7c20de 100644
>> --- a/src/tracefs-hist.c
>> +++ b/src/tracefs-hist.c
>> @@ -437,16 +437,43 @@ add_sort_key(struct tracefs_hist *hist, const char *sort_key, char **list)
>>   /**
>>    * tracefs_hist_add_sort_key - add a key for sorting the histogram
>>    * @hist: The histogram to add the sort key to
>> + * @sort_key: The key to sort
>> + *
>> + * Call the function to add to the list of sort keys of the histohram in
>> + * the order of priority that the keys would be sorted on output.
>> + *
>> + * Returns 0 on success, -1 on error.
>> + */
>> +int tracefs_hist_add_sort_key(struct tracefs_hist *hist,
>> +                             const char *sort_key)
>> +{
>> +       char **list = hist->sort;
>> +
>> +       if (!hist || !sort_key)
>> +               return -1;
>> +
>> +       list = add_sort_key(hist, sort_key, hist->sort);
>> +       if (!list)
>> +               return -1;
>> +
>> +       hist->sort = list;
>> +
>> +       return 0;
>> +}
>> +
>> +/**
>> + * tracefs_hist_reset_sort_key - set a key for sorting the histogram
>> + * @hist: The histogram to set the sort key to
>>    * @sort_key: The key to sort (and the strings after it)
>>    *  Last one must be NULL.
>>    *
>> - * Add a list of sort keys in the order of priority that the
>> + * Set a list of sort keys in the order of priority that the
>>    * keys would be sorted on output. Keys must be added first.
>>    *
>>    * Returns 0 on success, -1 on error.
>>    */
>> -int tracefs_hist_add_sort_key(struct tracefs_hist *hist,
>> -                             const char *sort_key, ...)
>> +int tracefs_hist_reset_sort_key(struct tracefs_hist *hist,
>> +                               const char *sort_key, ...)
>>   {
>>          char **list = NULL;
>>          char **tmp;
>> --
>> 2.30.2
>>
> 
> 
> --
> Tzvetomir (Ceco) Stoyanov
> VMware Open Source Technology Center
> 

[RFC] Modifications of some 'hist' APIs

[Yordan Karadzhov]

Hi Steven et al.

I would like to suggest some further modifications of the current APIs of libtracefs. The problem that I am trying to 
solve with this modifications is the unnecessary and potentially confusing variety of patterns used when implementing 
the APIs for dealing with 'instances', 'kprobes', 'histigrams' and 'synthetic events'. I believe that 
unifying/templating the structure of those APIs will be beneficial and will make the usage fare more intuitive. I am 
posting below a pseudo code for one possible implementation of such template APIs that can be used as a starting point 
for a discussion.

/**
  * tracefs_XXX_alloc - allocate a new tracefs_XXX descriptor. This only
  * initializes the descriptor, it does not introduce any changes on the
  * system.
  *
  * @arguments: Some arguments specific for the type of the object ...
  *
  * Returns a newly allocated tracefs_XXX descriptor objext, or NULL in case
  * of an error. The returned instance must be freed by tracefs_XXX_free().
  */
struct tracefs_XXX *tracefs_XXX_alloc(arguments);

/**
  * tracefs_XXX_create - creates new tracefs_XXX object on the system.
  * This method calls tracefs_XXX_alloc() internally.
  *
  * @arguments: Some arguments specific for the type of the object ...
  * This function should not take an instance as argument. Otherwise the
  * same instance will have to be passed to tracefs_XXX_destroy(), which
  * can be a problem in some more sophisticated use-cases.
  *
  * Returns 0 on succes and -1 on error. On error, errno is set to:
  * EBUSY - the object already exists on the system.
  * ENOMEM - memory allocation failure.
  * ENIVAL - a parameter is passed as NULL or value that should not be or
  * a problem writing into the system.
  */
struct tracefs_XXX *tracefs_XXX_create(arguments);

/**
  * tracefs_XXX_find - find a tracefs_XXX object that already exists on the
  * system. This method calls tracefs_XXX_alloc() internally.
  *
  * @arguments: Some arguments specific for the type of the object ...
  * This function should not take an instance as argument. Otherwise the same
  * instance will have to be passed to tracefs_XXX_destroy(), which can be a
  * problem in some more sophisticated use-cases.
  *
  * Returns tracefs_XXX descriptor on succes and NULL if the object do not
  * exist on the system or in the case of an error. On error, errno is set to:
  * ENOMEM - memory allocation failure.
  * ENIVAL - a parameter is passed as NULL or value that should not be or
  * a problem writing into the system.
  */
struct tracefs_XXX *tracefs_XXX_find(arguments);

/**
  * tracefs_instance_destroy - Remove/clears a tracefs_XXX objext from the
  * system. tracefs_XXX_free() must be called in order to free the memory used
  * by the descriptor itself.
  *
  * @obj: Pointer to the tracefs_XXX descriptor of the objext to be destroyed.
  * @force: If false the object is not guaranteed to be destroyed. If @force
  * is true, all tangled objects that prevent the destruction of the object
  * will be destroyed as well.
  *
  * This function should not take any other arguments.
  *
  * Returns -1 in case of an error or if the object failed to destroy.
  * 0 otherwise.
  */
int tracefs_XXX_destroy(struct tracefs_XXX *obj, bool force);

/**
  * tracefs_XXX_free - Free a tracefs_XXX descriptor, previously allocated
  * by tracefs_XXX_alloc, tracefs_XXX_create() or tracefs_XXX_find().
  *
  * @obj: Pointer to the tracefs_XXX descriptor objext to be freed.
  *
  * This function should not take any other arguments.
  */
void tracefs_XXX_free(struct tracefs_XXX *obj);

/**
  * tracefs_XXX_YYY - Method implementing the user action 'YYY' (can be
  * enable/disable, start/stop/clear, ... etc.)
  *
  * @obj: Pointer to the tracefs_XXX objext for witch the user action will
  * apply.
  * @instance: Ftrace instance, can be NULL for top tracing instance.
  * @more_arguments: Some additional arguments specific for the type XXX and
  * the action YYY.
  */
void/int tracefs_XXX_YYY(struct tracefs_XXX *obj,
			 struct tracefs_instance *inst,
			 more_arguments);


Thanks a lot!
Yordan

Re: [RFC] Modifications of some 'hist' APIs

[Steven Rostedt]

On Mon, 11 Oct 2021 16:55:29 +0300
Yordan Karadzhov <y.karadz@gmail.com> wrote:

> Hi Steven et al.
> 
> I would like to suggest some further modifications of the current APIs of libtracefs. The problem that I am trying to 
> solve with this modifications is the unnecessary and potentially confusing variety of patterns used when implementing 
> the APIs for dealing with 'instances', 'kprobes', 'histigrams' and 'synthetic events'. I believe that 
> unifying/templating the structure of those APIs will be beneficial and will make the usage fare more intuitive. I am 
> posting below a pseudo code for one possible implementation of such template APIs that can be used as a starting point 
> for a discussion.

As we discussed. I think instances are a different beast than kprobes,
histograms and sythetic events, as instances is a platform for events,
where as the others are just events or part of events. So it's fine if
instances had a different interface.

> 
> /**
>   * tracefs_XXX_alloc - allocate a new tracefs_XXX descriptor. This only
>   * initializes the descriptor, it does not introduce any changes on the
>   * system.

This is fine, for all (including instances).

>   *
>   * @arguments: Some arguments specific for the type of the object ...
>   *
>   * Returns a newly allocated tracefs_XXX descriptor objext, or NULL in case
>   * of an error. The returned instance must be freed by tracefs_XXX_free().
>   */
> struct tracefs_XXX *tracefs_XXX_alloc(arguments);
> 
> /**
>   * tracefs_XXX_create - creates new tracefs_XXX object on the system.
>   * This method calls tracefs_XXX_alloc() internally.

This is fine too.

>   *
>   * @arguments: Some arguments specific for the type of the object ...
>   * This function should not take an instance as argument. Otherwise the
>   * same instance will have to be passed to tracefs_XXX_destroy(), which
>   * can be a problem in some more sophisticated use-cases.
>   *
>   * Returns 0 on succes and -1 on error. On error, errno is set to:
>   * EBUSY - the object already exists on the system.

As we discussed, this is is fine for kprobes, histograms and synthetic
events, but for instances, it can still succeed, but the user can check
if it was created or not.

We could add: tracefs_instance_create_unique() that will fail if the
instance already exists.

>   * ENOMEM - memory allocation failure.
>   * ENIVAL - a parameter is passed as NULL or value that should not be or
>   * a problem writing into the system.
>   */
> struct tracefs_XXX *tracefs_XXX_create(arguments);
> 
> /**
>   * tracefs_XXX_find - find a tracefs_XXX object that already exists on the
>   * system. This method calls tracefs_XXX_alloc() internally.

I'm not sure we need this, as it only really makes sense for instances,
and above we already talked about that.

If we were to have this interface for the other types, I would suggest
calling it "open" and not "find". But creating kprobes / histograms and
synthetic events may not be as trivial, as building it would require
knowing the details of how they are used. Kprobes and histograms may be
possible, but the synthetic events will prove to be more difficult.

>   *
>   * @arguments: Some arguments specific for the type of the object ...
>   * This function should not take an instance as argument. Otherwise the same
>   * instance will have to be passed to tracefs_XXX_destroy(), which can be a
>   * problem in some more sophisticated use-cases.
>   *
>   * Returns tracefs_XXX descriptor on succes and NULL if the object do not
>   * exist on the system or in the case of an error. On error, errno is set to:
>   * ENOMEM - memory allocation failure.
>   * ENIVAL - a parameter is passed as NULL or value that should not be or
>   * a problem writing into the system.
>   */
> struct tracefs_XXX *tracefs_XXX_find(arguments);
> 
> /**
>   * tracefs_instance_destroy - Remove/clears a tracefs_XXX objext from the
>   * system. tracefs_XXX_free() must be called in order to free the memory used
>   * by the descriptor itself.
>   *
>   * @obj: Pointer to the tracefs_XXX descriptor of the objext to be destroyed.
>   * @force: If false the object is not guaranteed to be destroyed. If @force
>   * is true, all tangled objects that prevent the destruction of the object
>   * will be destroyed as well.

As stated before, the force may still be a best effort attempt.

>   *
>   * This function should not take any other arguments.
>   *
>   * Returns -1 in case of an error or if the object failed to destroy.
>   * 0 otherwise.
>   */
> int tracefs_XXX_destroy(struct tracefs_XXX *obj, bool force);
> 
> /**
>   * tracefs_XXX_free - Free a tracefs_XXX descriptor, previously allocated
>   * by tracefs_XXX_alloc, tracefs_XXX_create() or tracefs_XXX_find().
>   *
>   * @obj: Pointer to the tracefs_XXX descriptor objext to be freed.
>   *
>   * This function should not take any other arguments.
>   */
> void tracefs_XXX_free(struct tracefs_XXX *obj);
> 
> /**
>   * tracefs_XXX_YYY - Method implementing the user action 'YYY' (can be
>   * enable/disable, start/stop/clear, ... etc.)
>   *
>   * @obj: Pointer to the tracefs_XXX objext for witch the user action will
>   * apply.
>   * @instance: Ftrace instance, can be NULL for top tracing instance.
>   * @more_arguments: Some additional arguments specific for the type XXX and
>   * the action YYY.

Sounds fine to me.

Thanks for explicitly specifying the details of the interface. 

-- Steve


>   */
> void/int tracefs_XXX_YYY(struct tracefs_XXX *obj,
> 			 struct tracefs_instance *inst,
> 			 more_arguments);
> 
> 
> Thanks a lot!
> Yordan
> 

Re: [PATCH v2 2/4] libtracefs: Transform tracefs_hist_add_sort_key()

[Steven Rostedt]

On Fri, 24 Sep 2021 19:56:41 +0300
Yordan Karadzhov <y.karadz@gmail.com> wrote:

> > The new name of the function, mentioned in the patch description, is
> >      tracefs_hist_set_sort_key()
> > I like the name with "set" instead of "reset". The behaviour of the
> > function should be described in the function comments - to stress that
> > the old keys are overwritten.
> >   
> 
> Steven suggested to name it 'reset' in the review of the first version.
> Both 'set' and 'reset' are OK for me.

Looking at this now, I think "set" is a better name.

I'll modify it.

-- Steve