linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] libtracefs: Updates to event filtering
@ 2021-07-30  3:03 Steven Rostedt
  2021-07-30  3:03 ` [PATCH 1/6] libtracefs: Update to version 1.3-dev Steven Rostedt
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Steven Rostedt @ 2021-07-30  3:03 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (VMware)

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

First, move the version to 1.3-dev, as I found that installing this with a
newer stable (1.2.4), where this forked form stable at 1.2.1, kept code from
using this version.

Change "GR" and "NQ" to "GT" and "NE" to represent "greater than" and "not
equal" respectively.

Changed the way filters are built, to something that can easily be used, and
is less error prone.

Updated kerneldoc.

Added filtering APIs for normal events, and also a way to verify a filter.

Steven Rostedt (VMware) (6):
  libtracefs: Update to version 1.3-dev
  libtracefs: Rename GR and NQ to GT and NE respectively
  libtracefs: Append the synth filter with parens and conjunctions
  libtracefs: Add kerneldoc comments to the remaining synth functions
  libtracefs: Add filter creating and verify API
  libtracefs: Add man pages for tracefs_event_append_filter()

 Documentation/libtracefs-filter.txt | 329 ++++++++++++
 Documentation/libtracefs-synth.txt  |  78 +--
 Makefile                            |   4 +-
 include/tracefs-local.h             |  14 +
 include/tracefs.h                   |  42 +-
 src/Makefile                        |   1 +
 src/tracefs-filter.c                | 747 ++++++++++++++++++++++++++++
 src/tracefs-hist.c                  | 345 ++++++-------
 8 files changed, 1308 insertions(+), 252 deletions(-)
 create mode 100644 Documentation/libtracefs-filter.txt
 create mode 100644 src/tracefs-filter.c

-- 
2.30.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/6] libtracefs: Update to version 1.3-dev
  2021-07-30  3:03 [PATCH 0/6] libtracefs: Updates to event filtering Steven Rostedt
@ 2021-07-30  3:03 ` Steven Rostedt
  2021-07-30  3:03 ` [PATCH 2/6] libtracefs: Rename GR and NQ to GT and NE respectively Steven Rostedt
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2021-07-30  3:03 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (VMware)

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Since the development of 1.3 source has diverged from the stable 1.2 path,
it still has the old 1.2.1 as the version number. If this is added to a
system with 1.2.2 or above, it wont be used, as the symlinks point to the
highest version (1.2.2 or above).

Set this to 1.3.dev to make it the latest version to be used.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 395b20745f9d..96d7f5528a56 100644
--- a/Makefile
+++ b/Makefile
@@ -1,8 +1,8 @@
 # SPDX-License-Identifier: LGPL-2.1
 # libtracefs version
 TFS_VERSION = 1
-TFS_PATCHLEVEL = 2
-TFS_EXTRAVERSION = 1
+TFS_PATCHLEVEL = 3
+TFS_EXTRAVERSION = dev
 TRACEFS_VERSION = $(TFS_VERSION).$(TFS_PATCHLEVEL).$(TFS_EXTRAVERSION)
 
 export TFS_VERSION
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/6] libtracefs: Rename GR and NQ to GT and NE respectively
  2021-07-30  3:03 [PATCH 0/6] libtracefs: Updates to event filtering Steven Rostedt
  2021-07-30  3:03 ` [PATCH 1/6] libtracefs: Update to version 1.3-dev Steven Rostedt
@ 2021-07-30  3:03 ` Steven Rostedt
  2021-07-30  3:03 ` [PATCH 3/6] libtracefs: Append the synth filter with parens and conjunctions Steven Rostedt
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2021-07-30  3:03 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (VMware)

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

When writing the sqlhist interface, I found that calling ">" GT and "!="
NE were more sensible than calling them GR and NQ, as we already have LT
as "less than" why not use GT as "greater than". As well as EQ for
"equal", it seems more appropriate to have NE be "not equal".

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/libtracefs-synth.txt | 4 ++--
 include/tracefs.h                  | 4 ++--
 src/tracefs-hist.c                 | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/libtracefs-synth.txt b/Documentation/libtracefs-synth.txt
index 0e74e4f3aa0b..b20b4a7eb911 100644
--- a/Documentation/libtracefs-synth.txt
+++ b/Documentation/libtracefs-synth.txt
@@ -137,9 +137,9 @@ may be one of:
 
 *TRACEFS_COMPARE_EQ* - _field_ == _val_
 
-*TRACEFS_COMPARE_NQ* - _field_ != _val_
+*TRACEFS_COMPARE_NE* - _field_ != _val_
 
-*TRACEFS_COMPARE_GR* - _field_ > _val_
+*TRACEFS_COMPARE_GT* - _field_ > _val_
 
 *TRACEFS_COMPARE_GE* - _field_ >= _val_
 
diff --git a/include/tracefs.h b/include/tracefs.h
index 9cfd2577da2e..03a16bbffdc7 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -315,8 +315,8 @@ enum tracefs_synth_calc {
 
 enum tracefs_synth_compare {
 	TRACEFS_COMPARE_EQ,
-	TRACEFS_COMPARE_NQ,
-	TRACEFS_COMPARE_GR,
+	TRACEFS_COMPARE_NE,
+	TRACEFS_COMPARE_GT,
 	TRACEFS_COMPARE_GE,
 	TRACEFS_COMPARE_LT,
 	TRACEFS_COMPARE_LE,
diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
index 8b9078791ab2..11355e648692 100644
--- a/src/tracefs-hist.c
+++ b/src/tracefs-hist.c
@@ -1178,11 +1178,11 @@ static int add_synth_filter(char **filter, const char *field,
 		op = "==";
 		break;
 
-	case TRACEFS_COMPARE_NQ:
+	case TRACEFS_COMPARE_NE:
 		op = "!=";
 		break;
 
-	case TRACEFS_COMPARE_GR:
+	case TRACEFS_COMPARE_GT:
 		op = ">";
 		if (is_string)
 			goto inval;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/6] libtracefs: Append the synth filter with parens and conjunctions
  2021-07-30  3:03 [PATCH 0/6] libtracefs: Updates to event filtering Steven Rostedt
  2021-07-30  3:03 ` [PATCH 1/6] libtracefs: Update to version 1.3-dev Steven Rostedt
  2021-07-30  3:03 ` [PATCH 2/6] libtracefs: Rename GR and NQ to GT and NE respectively Steven Rostedt
@ 2021-07-30  3:03 ` Steven Rostedt
  2021-07-30  3:03 ` [PATCH 4/6] libtracefs: Add kerneldoc comments to the remaining synth functions Steven Rostedt
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2021-07-30  3:03 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (VMware)

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

While implementing the sqlhist, I found that the
tracefs_synth_add_start/end_filter() was not sufficient in adding the
possible filters that the kernel accepts. That is, we could not implement:

  A && (B || C) && D

as it only allowed appending operators, and the kernel sets && as a higher
precedence than ||. That is, if we try to do the above with just:

  A && B || C && D

The kernel will interpret it as:

  (A && B) || (C && D)

I tried to fix this with a precedence, showing different levels of
precedence between options and it too wasn't sufficient to handle all the
cases that the kernel can.

Instead, go with the KISS approach, and do it with just letting the user
append the parenthesis, nots and conjunctions. Keeping the state of the
last update makes sure that an invalid append does not go through.

For example, to produce:

  A || (!(B && !C) && !(D)

 tracefs_synth_append_start_field( TRACEFS_FILTER_COMPARE, A );
 tracefs_synth_append_start_field( TRACEFS_FILTER_OR, NULL );
 tracefs_synth_append_start_field( TRACEFS_FILTER_OPEN_PAREN, NULL );
 tracefs_synth_append_start_field( TRACEFS_FILTER_NOT, NULL );
 tracefs_synth_append_start_field( TRACEFS_FILTER_OPEN_PAREN, NULL );
 tracefs_synth_append_start_field( TRACEFS_FILTER_COMPARE, B );
 tracefs_synth_append_start_field( TRACEFS_FILTER_AND, NULL );
 tracefs_synth_append_start_field( TRACEFS_FILTER_NOT, NULL );
 tracefs_synth_append_start_field( TRACEFS_FILTER_COMPARE, C );
 tracefs_synth_append_start_field( TRACEFS_FILTER_CLOSE_PAREN, NULL );
 tracefs_synth_append_start_field( TRACEFS_FILTER_AND, NULL );
 tracefs_synth_append_start_field( TRACEFS_FILTER_NOT, NULL );
 tracefs_synth_append_start_field( TRACEFS_FILTER_OPEN_PAREN, NULL );
 tracefs_synth_append_start_field( TRACEFS_FILTER_COMPARE, D );

The number of left over open parenthesis is kept track of and on printing or
executing the synth, it will close all the parenthesis that are left open.

(note, the original code had "-" for negative, which was wrong)

Also, rename the parameter in tracefs from "or" to "or_conj" as "or" is a
keyword in C++ as reported by Yordan Karadzhov:

  Link: https://lore.kernel.org/linux-trace-devel/20210727135030.25914-1-y.karadz@gmail.com/

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/libtracefs-synth.txt |  74 +++---
 include/tracefs.h                  |  29 ++-
 src/tracefs-hist.c                 | 366 ++++++++++++++++++-----------
 3 files changed, 293 insertions(+), 176 deletions(-)

diff --git a/Documentation/libtracefs-synth.txt b/Documentation/libtracefs-synth.txt
index b20b4a7eb911..47e93bd42674 100644
--- a/Documentation/libtracefs-synth.txt
+++ b/Documentation/libtracefs-synth.txt
@@ -4,7 +4,7 @@ libtracefs(3)
 NAME
 ----
 tracefs_synth_init, tracefs_synth_add_match_field, tracefs_synth_add_compare_field, tracefs_synth_add_start_field,
-tracefs_synth_add_end_field, tracefs_synth_add_start_filter, tracefs_synth_add_end_filter, tracefs_synth_create,
+tracefs_synth_add_end_field, tracefs_synth_append_start_filter, tracefs_synth_append_end_filter, tracefs_synth_create,
 tracefs_synth_destroy, tracefs_synth_free, tracefs_synth_show - Creation of synthetic events
 
 SYNOPSIS
@@ -37,16 +37,16 @@ int tracefs_synth_add_start_field(struct tracefs_synth pass:[*]synth,
 int tracefs_synth_add_end_field(struct tracefs_synth pass:[*]synth,
 				const char pass:[*]end_field,
 				const char pass:[*]name);
-int tracefs_synth_add_start_filter(struct tracefs_synth pass:[*]synth,
-				   const char pass:[*]field,
-				   enum tracefs_synth_compare compare,
-				   const char pass:[*]val,
-				   bool neg, bool or);
-int tracefs_synth_add_end_filter(struct tracefs_synth pass:[*]synth,
-				 const char pass:[*]field,
-				 enum tracefs_synth_compare compare,
-				 const char pass:[*]val,
-				 bool neg, bool or);
+int tracefs_synth_append_start_filter(struct tracefs_synth pass:[*]synth,
+				      struct tracefs_filter type,
+				      const char pass:[*]field,
+				      enum tracefs_synth_compare compare,
+				      const char pass:[*]val);
+int tracefs_synth_append_end_filter(struct tracefs_synth pass:[*]synth,
+				    struct tracefs_filter type,
+				    const char pass:[*]field,
+				    enum tracefs_synth_compare compare,
+				    const char pass:[*]val);
 int tracefs_synth_create(struct tracefs_instance pass:[*]instance,
 			 struct tracefs_synth pass:[*]synth);
 int tracefs_synth_destroy(struct tracefs_instance pass:[*]instance,
@@ -127,13 +127,21 @@ will be the same as _start_field_.
 event as _name_ in the synthetic event. If _name_ is NULL, then the name used
 will be the same as _end_field_.
 
-*tracefs_synth_add_start_filter*() adds a filter to the starting event
-comparing the content of the starting event's _field_ to _val_ based
-on _compare_. If _neg_ is set, then the compare is wrapped in parenthesis
-and negated. The _or_ field only is used if more than one call to
-*tracefs_synth_add_start_filter*() is done, and if _or_ is set, the
-next compare is "or'd" (||), otherwise it is "and'd" (&&). _compare_
-may be one of:
+*tracefs_synth_append_start_filter*() creates a filter or appends to it for the
+starting event. Depending on _type_, it will build a string of tokens for
+parenthesis or logic statemens, or it may add a comparison of _field_
+to _val_ based on _compare_.
+
+If _type_ is:
+*TRACEFS_FILTER_COMPARE*     -  See below
+*TRACEFS_FILTER_AND*         -  Append "&&" to the filter
+*TRACEFS_FILTER_OR*          -  Append "||" to the filter
+*TRACEFS_FILTER_NOT*         -  Append "!" to the filter
+*TRACEFS_FILTER_OPEN_PAREN*  -  Append "(" to the filter
+*TRACEFS_FILTER_CLOSE_PAREN* -  Append ")" to the filter
+
+_field_, _compare_, and _val_ are ignored unless _type_ is equal to
+*TRACEFS_FILTER_COMPARE*, then _compare will be used for the following:
 
 *TRACEFS_COMPARE_EQ* - _field_ == _val_
 
@@ -151,7 +159,7 @@ may be one of:
 
 *TRACEFS_COMPARE_AND* - _field_ & _val_ : where _field_ is a flags field.
 
-*tracefs_synth_add_end_filter*() is the same as *tracefs_synth_add_start_filter* but
+*tracefs_synth_append_end_filter*() is the same as *tracefs_synth_append_start_filter* but
 filters on the ending event.
 
 *tracefs_synth_create*() creates the synthetic event in the system in the system
@@ -249,20 +257,28 @@ static void make_event(void)
 					TRACEFS_SYNTH_DELTA_END, "delta");
 
 	/* Only record if start event "prio" is less than 100 */
-	tracefs_synth_add_start_filter(synth, "prio",
-				       TRACEFS_COMPARE_LT, "100",
-				       false, false);
+	tracefs_synth_append_start_filter(synth, TRACEFS_FILTER_COMPARE,
+					  "prio", TRACEFS_COMPARE_LT, "100");
 
 	/*
 	 * Only record if end event "next_prio" is less than 50
-	 * or the previous task's prio was less than 100.
+	 * or the previous task's prio was not greater than or equal to 100.
+	 *   next_prio < 50 || !(prev_prio >= 100)
+	 */
+	tracefs_synth_append_end_filter(synth, TRACEFS_FILTER_COMPARE,
+					"next_prio", TRACEFS_COMPARE_LT, "50");
+	tracefs_synth_append_end_filter(synth, TRACEFS_FILTER_OR, NULL, 0, NULL);
+	tracefs_synth_append_end_filter(synth, TRACEFS_FILTER_NOT, NULL, 0, NULL);
+	tracefs_synth_append_end_filter(synth, TRACEFS_FILTER_OPEN_PAREN, NULL, 0, NULL);
+	tracefs_synth_append_end_filter(synth, TRACEFS_FILTER_COMPARE,
+					"prev_prio", TRACEFS_COMPARE_GE, "100");
+	/*
+	 * Note, the above only added: "next_prio < 50 || !(prev_prio >= 100"
+	 * That's because, when the synth is executed, the remaining close parenthesis
+	 * will be added. That is, the string will end up being:
+	 * "next_prio < 50 || !(prev_prio >= 100)" when one of tracefs_sync_create()
+	 * or tracefs_sync_show() is run.
 	 */
-	tracefs_synth_add_end_filter(synth, "next_prio",
-				       TRACEFS_COMPARE_LT, "50",
-				       false, false);
-	tracefs_synth_add_end_filter(synth, "prev_prio",
-				       TRACEFS_COMPARE_LT, "100",
-				       false, true);
 }
 
 /* Display how to create the synthetic event */
diff --git a/include/tracefs.h b/include/tracefs.h
index 03a16bbffdc7..386ad2c1678f 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -324,6 +324,15 @@ enum tracefs_synth_compare {
 	TRACEFS_COMPARE_AND,
 };
 
+enum tracefs_filter {
+	TRACEFS_FILTER_COMPARE,
+	TRACEFS_FILTER_AND,
+	TRACEFS_FILTER_OR,
+	TRACEFS_FILTER_NOT,
+	TRACEFS_FILTER_OPEN_PAREN,
+	TRACEFS_FILTER_CLOSE_PAREN,
+};
+
 #define TRACEFS_TIMESTAMP "common_timestamp"
 #define TRACEFS_TIMESTAMP_USECS "common_timestamp.usecs"
 
@@ -351,16 +360,16 @@ int tracefs_synth_add_start_field(struct tracefs_synth *synth,
 int tracefs_synth_add_end_field(struct tracefs_synth *synth,
 				const char *end_field,
 				const char *name);
-int tracefs_synth_add_start_filter(struct tracefs_synth *synth,
-				   const char *field,
-				   enum tracefs_synth_compare compare,
-				   const char *val,
-				   bool neg, bool or);
-int tracefs_synth_add_end_filter(struct tracefs_synth *synth,
-				 const char *field,
-				 enum tracefs_synth_compare compare,
-				 const char *val,
-				 bool neg, bool or);
+int tracefs_synth_append_start_filter(struct tracefs_synth *synth,
+				      enum tracefs_filter type,
+				      const char *field,
+				      enum tracefs_synth_compare compare,
+				      const char *val);
+int tracefs_synth_append_end_filter(struct tracefs_synth *synth,
+				    enum tracefs_filter type,
+				    const char *field,
+				    enum tracefs_synth_compare compare,
+				    const char *val);
 int tracefs_synth_create(struct tracefs_instance *instance,
 			 struct tracefs_synth *synth);
 int tracefs_synth_destroy(struct tracefs_instance *instance,
diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
index 11355e648692..d518ae77fe72 100644
--- a/src/tracefs-hist.c
+++ b/src/tracefs-hist.c
@@ -545,6 +545,8 @@ int tracefs_hist_sort_key_direction(struct tracefs_hist *hist,
  * @end_names: The fields in the end event to record
  * @start_filters: The fields in the end event to record
  * @end_filters: The fields in the end event to record
+ * @start_parens: Current parenthesis level for start event
+ * @end_parens: Current parenthesis level for end event
  */
 struct tracefs_synth {
 	struct tep_handle	*tep;
@@ -559,7 +561,10 @@ struct tracefs_synth {
 	char			**end_vars;
 	char			*start_filter;
 	char			*end_filter;
-
+	unsigned int		start_parens;
+	unsigned int		start_state;
+	unsigned int		end_parens;
+	unsigned int		end_state;
 	int			arg_cnt;
 };
 
@@ -1163,152 +1168,214 @@ int tracefs_synth_add_end_field(struct tracefs_synth *synth,
 	return ret;
 }
 
-static int add_synth_filter(char **filter, const char *field,
-			    enum tracefs_synth_compare compare,
-			    const char *val, bool is_string,
-			    bool neg, bool or)
-{
-	const char *minus = "";
-	const char *op;
-	char *str = NULL;
-	int ret;
-
-	switch (compare) {
-	case TRACEFS_COMPARE_EQ:
-		op = "==";
-		break;
+enum {
+	S_START,
+	S_COMPARE,
+	S_NOT,
+	S_CONJUNCTION,
+	S_OPEN_PAREN,
+	S_CLOSE_PAREN,
+};
 
-	case TRACEFS_COMPARE_NE:
-		op = "!=";
-		break;
+static int append_synth_filter(char **filter, unsigned int *state,
+			       unsigned int *open_parens,
+			       struct tep_event *event,
+			       enum tracefs_filter type,
+			       const char *field_name,
+			       enum tracefs_synth_compare compare,
+			       const char *val)
+{
+	const struct tep_format_field *field;
+	bool is_string;
+	char *conj = "||";
+	char *tmp;
 
-	case TRACEFS_COMPARE_GT:
-		op = ">";
-		if (is_string)
+	switch (type) {
+	case TRACEFS_FILTER_COMPARE:
+		switch (*state) {
+		case S_START:
+		case S_OPEN_PAREN:
+		case S_CONJUNCTION:
+		case S_NOT:
+			break;
+		default:
 			goto inval;
+		}
 		break;
 
-	case TRACEFS_COMPARE_GE:
-		op = ">=";
-		if (is_string)
+	case TRACEFS_FILTER_AND:
+		conj = "&&";
+		/* Fall through */
+	case TRACEFS_FILTER_OR:
+		switch (*state) {
+		case S_COMPARE:
+		case S_CLOSE_PAREN:
+			break;
+		default:
 			goto inval;
-		break;
+		}
+		/* Don't lose old filter on failure */
+		tmp = strdup(*filter);
+		if (!tmp)
+			return -1;
+		tmp = append_string(tmp, NULL, conj);
+		if (!tmp)
+			return -1;
+		free(*filter);
+		*filter = tmp;
+		*state = S_CONJUNCTION;
+		return 0;
 
-	case TRACEFS_COMPARE_LT:
-		op = "<";
-		if (is_string)
+	case TRACEFS_FILTER_NOT:
+		switch (*state) {
+		case S_START:
+		case S_OPEN_PAREN:
+		case S_CONJUNCTION:
+		case S_NOT:
+			break;
+		default:
 			goto inval;
-		break;
+		}
+		if (*filter) {
+			tmp = strdup(*filter);
+			tmp = append_string(tmp, NULL, "!");
+		} else {
+			tmp = strdup("!");
+		}
+		if (!tmp)
+			return -1;
+		free(*filter);
+		*filter = tmp;
+		*state = S_NOT;
+		return 0;
 
-	case TRACEFS_COMPARE_LE:
-		op = "<=";
-		if (is_string)
+	case TRACEFS_FILTER_OPEN_PAREN:
+		switch (*state) {
+		case S_START:
+		case S_OPEN_PAREN:
+		case S_NOT:
+		case S_CONJUNCTION:
+			break;
+		default:
 			goto inval;
-		break;
+		}
+		if (*filter) {
+			tmp = strdup(*filter);
+			tmp = append_string(tmp, NULL, "(");
+		} else {
+			tmp = strdup("(");
+		}
+		if (!tmp)
+			return -1;
+		free(*filter);
+		*filter = tmp;
+		*state = S_OPEN_PAREN;
+		(*open_parens)++;
+		return 0;
 
-	case TRACEFS_COMPARE_RE:
-		op = "~";
-		if (!is_string)
+	case TRACEFS_FILTER_CLOSE_PAREN:
+		switch (*state) {
+		case S_CLOSE_PAREN:
+		case S_COMPARE:
+			break;
+		default:
 			goto inval;
-		break;
-
-	case TRACEFS_COMPARE_AND:
-		op = "&";
-		if (is_string)
+		}
+		if (!*open_parens)
 			goto inval;
-		break;
-	}
 
-	if (neg)
-		minus = "-";
+		tmp = strdup(*filter);
+		if (!tmp)
+			return -1;
+		tmp = append_string(tmp, NULL, ")");
+		if (!tmp)
+			return -1;
+		free(*filter);
+		*filter = tmp;
+		*state = S_CLOSE_PAREN;
+		(*open_parens)--;
+		return 0;
+	}
 
-	if (is_string && val[0] != '"')
-		ret = asprintf(&str, "%s(%s %s \"%s\")",
-			       minus, field, op, val);
-	else
-		ret = asprintf(&str, "%s(%s %s %s)",
-			       minus, field, op, val);
+	if (!field_name || !val)
+		goto inval;
 
-	if (ret < 0)
+	if (!verify_event_fields(event, NULL, field_name, NULL, &field))
 		return -1;
 
-	if (*filter) {
-		char *new;
-		char *conjunction = or ? "||" : "&&";
+	is_string = field->flags & TEP_FIELD_IS_STRING;
 
-		ret = asprintf(&new, "%s %s %s", *filter,
-			       conjunction, str);
-		free(str);
-		if (ret < 0)
+	if (!is_string && (field->flags & TEP_FIELD_IS_ARRAY))
+		goto inval;
+
+	if (*filter) {
+		tmp = strdup(*filter);
+		if (!tmp)
 			return -1;
-		free(*filter);
-		*filter = new;
+		tmp = append_string(tmp, NULL, field_name);
 	} else {
-		*filter = str;
+		tmp = strdup(field_name);
 	}
 
-	return 0;
-inval:
-	errno = -EINVAL;
-	return -1;
-}
+	switch (compare) {
+	case TRACEFS_COMPARE_EQ: tmp = append_string(tmp, NULL, " == "); break;
+	case TRACEFS_COMPARE_NE: tmp = append_string(tmp, NULL, " != "); break;
+	case TRACEFS_COMPARE_RE:
+		if (!is_string)
+			goto inval;
+		tmp = append_string(tmp, NULL, "~");
+		break;
+	default:
+		if (is_string)
+			goto inval;
+	}
 
-int tracefs_synth_add_start_filter(struct tracefs_synth *synth,
-				   const char *field,
-				   enum tracefs_synth_compare compare,
-				   const char *val,
-				   bool neg, bool or)
-{
-	const struct tep_format_field *start_field;
-	bool is_string;
+	switch (compare) {
+	case TRACEFS_COMPARE_GT: tmp = append_string(tmp, NULL, " > "); break;
+	case TRACEFS_COMPARE_GE: tmp = append_string(tmp, NULL, " >= "); break;
+	case TRACEFS_COMPARE_LT: tmp = append_string(tmp, NULL, " < "); break;
+	case TRACEFS_COMPARE_LE: tmp = append_string(tmp, NULL, " <= "); break;
+	case TRACEFS_COMPARE_AND: tmp = append_string(tmp, NULL, " & "); break;
+	default: break;
+	}
 
-	if (!field || !val)
-		goto inval;
+	tmp = append_string(tmp, NULL, val);
 
-	if (!verify_event_fields(synth->start_event, NULL,
-				 field, NULL, &start_field))
+	if (!tmp)
 		return -1;
 
-	is_string = start_field->flags & TEP_FIELD_IS_STRING;
+	free(*filter);
+	*filter = tmp;
+	*state = S_COMPARE;
 
-	if (!is_string && (start_field->flags & TEP_FIELD_IS_ARRAY))
-		goto inval;
-
-	return add_synth_filter(&synth->start_filter,
-				field, compare, val, is_string,
-				neg, or);
+	return 0;
 inval:
-	errno = -EINVAL;
+	errno = EINVAL;
 	return -1;
 }
 
-int tracefs_synth_add_end_filter(struct tracefs_synth *synth,
-				 const char *field,
-				 enum tracefs_synth_compare compare,
-				 const char *val,
-				 bool neg, bool or)
+int tracefs_synth_append_start_filter(struct tracefs_synth *synth,
+				      enum tracefs_filter type,
+				      const char *field,
+				      enum tracefs_synth_compare compare,
+				      const char *val)
 {
-	const struct tep_format_field *end_field;
-	bool is_string;
-
-	if (!field || !val)
-		goto inval;
-
-	if (!verify_event_fields(synth->end_event, NULL,
-				 field, NULL, &end_field))
-		return -1;
-
-	is_string = end_field->flags & TEP_FIELD_IS_STRING;
-
-	if (!is_string && (end_field->flags & TEP_FIELD_IS_ARRAY))
-		goto inval;
+	return append_synth_filter(&synth->start_filter, &synth->start_state,
+				   &synth->start_parens,
+				   synth->start_event,
+				   type, field, compare, val);
+}
 
-	return add_synth_filter(&synth->end_filter,
-				field, compare, val, is_string,
-				neg, or);
-inval:
-	errno = -EINVAL;
-	return -1;
+int tracefs_synth_append_end_filter(struct tracefs_synth *synth,
+				    enum tracefs_filter type,
+				    const char *field,
+				    enum tracefs_synth_compare compare,
+				    const char *val)
+{
+	return append_synth_filter(&synth->end_filter, &synth->end_state,
+				   &synth->end_parens,
+				   synth->end_event,
+				   type, field, compare, val);
 }
 
 static char *create_synthetic_event(struct tracefs_synth *synth)
@@ -1414,6 +1481,41 @@ static char *create_end_hist(struct tracefs_synth *synth)
 	return append_string(end_hist, NULL, ")");
 }
 
+static char *append_filter(char *hist, char *filter, unsigned int parens)
+{
+	int i;
+
+	if (!filter)
+		return hist;
+
+	hist = append_string(hist, NULL, " if ");
+	hist = append_string(hist, NULL, filter);
+	for (i = 0; i < parens; i++)
+		hist = append_string(hist, NULL, ")");
+	return hist;
+}
+
+static int test_state(int state)
+{
+	switch (state) {
+	case S_START:
+	case S_CLOSE_PAREN:
+	case S_COMPARE:
+		return 0;
+	}
+
+	errno = EBADE;
+	return -1;
+}
+
+static int verify_state(struct tracefs_synth *synth)
+{
+	if (test_state(synth->start_state) < 0 ||
+	    test_state(synth->end_state) < 0)
+		return -1;
+	return 0;
+}
+
 /**
  * tracefs_synth_create - creates the synthetic event on the system
  * @instance: The instance to modify the start and end events
@@ -1441,6 +1543,9 @@ int tracefs_synth_create(struct tracefs_instance *instance,
 		return -1;
 	}
 
+	if (verify_state(synth) < 0)
+		return -1;
+
 	synthetic_event = create_synthetic_event(synth);
 	if (!synthetic_event)
 		return -1;
@@ -1451,18 +1556,14 @@ int tracefs_synth_create(struct tracefs_instance *instance,
 		goto free_synthetic;
 
 	start_hist = create_hist(synth->start_keys, synth->start_vars);
-	if (synth->start_filter) {
-		start_hist = append_string(start_hist, NULL, " if ");
-		start_hist = append_string(start_hist, NULL, synth->start_filter);
-	}
+	start_hist = append_filter(start_hist, synth->start_filter,
+				   synth->start_parens);
 	if (!start_hist)
 		goto remove_synthetic;
 
 	end_hist = create_end_hist(synth);
-	if (synth->end_filter) {
-		end_hist = append_string(end_hist, NULL, " if ");
-		end_hist = append_string(end_hist, NULL, synth->end_filter);
-	}
+	end_hist = append_filter(end_hist, synth->end_filter,
+				   synth->end_parens);
 	if (!end_hist)
 		goto remove_synthetic;
 
@@ -1528,20 +1629,16 @@ int tracefs_synth_destroy(struct tracefs_instance *instance,
 	tracefs_event_disable(instance, "synthetic", synth->name);
 
 	hist = create_end_hist(synth);
-	if (synth->end_filter) {
-		hist = append_string(hist, NULL, " if ");
-		hist = append_string(hist, NULL, synth->end_filter);
-	}
+	hist = append_filter(hist, synth->end_filter,
+			     synth->end_parens);
 	if (!hist)
 		return -1;
 	ret = remove_hist(instance, synth->end_event, hist);
 	free(hist);
 
 	hist = create_hist(synth->start_keys, synth->start_vars);
-	if (synth->start_filter) {
-		hist = append_string(hist, NULL, " if ");
-		hist = append_string(hist, NULL, synth->start_filter);
-	}
+	hist = append_filter(hist, synth->start_filter,
+			     synth->start_parens);
 	if (!hist)
 		return -1;
 
@@ -1599,10 +1696,8 @@ int tracefs_synth_show(struct trace_seq *seq,
 	path = tracefs_instance_get_dir(instance);
 
 	hist = create_hist(synth->start_keys, synth->start_vars);
-	if (synth->start_filter) {
-		hist = append_string(hist, NULL, " if ");
-		hist = append_string(hist, NULL, synth->start_filter);
-	}
+	hist = append_filter(hist, synth->start_filter,
+			     synth->start_parens);
 	if (!hist)
 		goto out_free;
 
@@ -1611,11 +1706,8 @@ int tracefs_synth_show(struct trace_seq *seq,
 			 synth->start_event->name);
 	free(hist);
 	hist = create_end_hist(synth);
-
-	if (synth->end_filter) {
-		hist = append_string(hist, NULL, " if ");
-		hist = append_string(hist, NULL, synth->end_filter);
-	}
+	hist = append_filter(hist, synth->end_filter,
+			     synth->end_parens);
 	if (!hist)
 		goto out_free;
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/6] libtracefs: Add kerneldoc comments to the remaining synth functions
  2021-07-30  3:03 [PATCH 0/6] libtracefs: Updates to event filtering Steven Rostedt
                   ` (2 preceding siblings ...)
  2021-07-30  3:03 ` [PATCH 3/6] libtracefs: Append the synth filter with parens and conjunctions Steven Rostedt
@ 2021-07-30  3:03 ` Steven Rostedt
  2021-07-30  3:03 ` [PATCH 5/6] libtracefs: Add filter creating and verify API Steven Rostedt
  2021-07-30  3:03 ` [PATCH 6/6] libtracefs: Add man pages for tracefs_event_append_filter() Steven Rostedt
  5 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2021-07-30  3:03 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (VMware)

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Added kerneldoc comments to:

  tracefs_synth_show()
  tracefs_synth_append_start_filter()
  tracefs_synth_append_end_filter();

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 src/tracefs-hist.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
index d518ae77fe72..8a0d43fafb7e 100644
--- a/src/tracefs-hist.c
+++ b/src/tracefs-hist.c
@@ -580,6 +580,15 @@ static const struct tep_format_field common_timestamp_usecs = {
 	.size			= 8,
 };
 
+/**
+ * tracefs_synth_free - free the resources alloced to a synth
+ * @synth: The tracefs_synth descriptor
+ *
+ * Frees the resources allocated for a @synth created with
+ * tracefs_synth_init(). It does not touch the system. That is,
+ * any synthetic event created, will not be destroyed by this
+ * function.
+ */
 void tracefs_synth_free(struct tracefs_synth *synth)
 {
 	if (!synth)
@@ -1354,6 +1363,55 @@ inval:
 	return -1;
 }
 
+/**
+ * tracefs_synth_append_start_filter - create or append a filter
+ * @synth: The tracefs_synth descriptor
+ * @type: The type of element to add to the filter
+ * @field: For @type == TRACEFS_FILTER_COMPARE, the field to compare
+ * @compare: For @type == TRACEFS_FILTER_COMPARE, how to compare @field to @val
+ * @val: For @type == TRACEFS_FILTER_COMPARE, what value @field is to be
+ *
+ * This will put together a filter string for the starting event
+ * of @synth. It check to make sure that what is added is correct compared
+ * to the filter that is already built.
+ *
+ * @type can be:
+ *     TRACEFS_FILTER_COMPARE:        See below
+ *     TRACEFS_FILTER_AND:            Append "&&" to the filter
+ *     TRACEFS_FILTER_OR:             Append "||" to the filter
+ *     TRACEFS_FILTER_NOT:            Append "!" to the filter
+ *     TRACEFS_FILTER_OPEN_PAREN:     Append "(" to the filter
+ *     TRACEFS_FILTER_CLOSE_PAREN:    Append ")" to the filter
+ *
+ * For all types except TRACEFS_FILTER_COMPARE, the @field, @compare,
+ * and @val are ignored.
+ *
+ * For @type == TRACEFS_FILTER_COMPARE.
+ *
+ *  @field is the name of the field for the start event to compare.
+ *         If it is not a field for the start event, this return an
+ *         error.
+ *
+ *  @compare can be one of:
+ *     TRACEFS_COMPARE_EQ:       Test @field == @val
+ *     TRACEFS_COMPARE_NE:       Test @field != @val
+ *     TRACEFS_COMPARE_GT:       Test @field > @val
+ *     TRACEFS_COMPARE_GE:       Test @field >= @val
+ *     TRACEFS_COMPARE_LT:       Test @field < @val
+ *     TRACEFS_COMPARE_LE:       Test @field <= @val
+ *     TRACEFS_COMPARE_RE:       Test @field ~ @val
+ *     TRACEFS_COMPARE_AND:      Test @field & @val
+ *
+ * If the @field is of type string, and @compare is not
+ *   TRACEFS_COMPARE_EQ, TRACEFS_COMPARE_NE or TRACEFS_COMPARE_RE,
+ *   then this will return an error.
+ *
+ * Various other checks are made, for instance, if more CLOSE_PARENs
+ * are added than existing OPEN_PARENs. Or if AND is added after an
+ * OPEN_PAREN or another AND or an OR or a NOT.
+ *
+ * Returns 0 on success and -1 on failure.
+ */
 int tracefs_synth_append_start_filter(struct tracefs_synth *synth,
 				      enum tracefs_filter type,
 				      const char *field,
@@ -1366,6 +1424,17 @@ int tracefs_synth_append_start_filter(struct tracefs_synth *synth,
 				   type, field, compare, val);
 }
 
+/**
+ * tracefs_synth_append_end_filter - create or append a filter
+ * @synth: The tracefs_synth descriptor
+ * @type: The type of element to add to the filter
+ * @field: For @type == TRACEFS_FILTER_COMPARE, the field to compare
+ * @compare: For @type == TRACEFS_FILTER_COMPARE, how to compare @field to @val
+ * @val: For @type == TRACEFS_FILTER_COMPARE, what value @field is to be
+ *
+ * Performs the same thing as tracefs_synth_append_start_filter() but
+ * for the @synth end event.
+ */
 int tracefs_synth_append_end_filter(struct tracefs_synth *synth,
 				    enum tracefs_filter type,
 				    const char *field,
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/6] libtracefs: Add filter creating and verify API
  2021-07-30  3:03 [PATCH 0/6] libtracefs: Updates to event filtering Steven Rostedt
                   ` (3 preceding siblings ...)
  2021-07-30  3:03 ` [PATCH 4/6] libtracefs: Add kerneldoc comments to the remaining synth functions Steven Rostedt
@ 2021-07-30  3:03 ` Steven Rostedt
  2021-07-30  3:03 ` [PATCH 6/6] libtracefs: Add man pages for tracefs_event_append_filter() Steven Rostedt
  5 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2021-07-30  3:03 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (VMware), Yordan Karadzhov

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Add the following API:

  tracefs_event_append_filter()
  tracefs_event_verify_filter()

Pull out the filter logic from trace-hist.c and place it into its own
file: tracefs-filter.c, that allows users to use these filters directly
for individual events.

Suggested-by: Yordan Karadzhov <y.karadz@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/tracefs-local.h |  14 +
 include/tracefs.h       |  13 +-
 src/Makefile            |   1 +
 src/tracefs-filter.c    | 747 ++++++++++++++++++++++++++++++++++++++++
 src/tracefs-hist.c      | 258 +-------------
 5 files changed, 787 insertions(+), 246 deletions(-)
 create mode 100644 src/tracefs-filter.c

diff --git a/include/tracefs-local.h b/include/tracefs-local.h
index 2324dec9d076..41fbcc0faa95 100644
--- a/include/tracefs-local.h
+++ b/include/tracefs-local.h
@@ -71,4 +71,18 @@ struct tracefs_options_mask *
 enabled_opts_mask(struct tracefs_instance *instance);
 
 char **trace_list_create_empty(void);
+
+char *append_string(char *str, const char *delim, const char *add);
+int trace_test_state(int state);
+bool trace_verify_event_field(struct tep_event *event,
+			      const char *field_name,
+			      const struct tep_format_field **ptr_field);
+int trace_append_filter(char **filter, unsigned int *state,
+			unsigned int *open_parens,
+			struct tep_event *event,
+			enum tracefs_filter type,
+			const char *field_name,
+			enum tracefs_compare compare,
+			 const char *val);
+
 #endif /* _TRACE_FS_LOCAL_H */
diff --git a/include/tracefs.h b/include/tracefs.h
index 386ad2c1678f..246647f6496d 100644
--- a/include/tracefs.h
+++ b/include/tracefs.h
@@ -313,7 +313,7 @@ enum tracefs_synth_calc {
 	TRACEFS_SYNTH_ADD,
 };
 
-enum tracefs_synth_compare {
+enum tracefs_compare {
 	TRACEFS_COMPARE_EQ,
 	TRACEFS_COMPARE_NE,
 	TRACEFS_COMPARE_GT,
@@ -333,6 +333,13 @@ enum tracefs_filter {
 	TRACEFS_FILTER_CLOSE_PAREN,
 };
 
+int tracefs_event_append_filter(struct tep_event *event, char **filter,
+				enum tracefs_filter type,
+				const char *field, enum tracefs_compare compare,
+				const char *val);
+int tracefs_event_verify_filter(struct tep_event *event, const char *filter,
+				char **err);
+
 #define TRACEFS_TIMESTAMP "common_timestamp"
 #define TRACEFS_TIMESTAMP_USECS "common_timestamp.usecs"
 
@@ -363,12 +370,12 @@ int tracefs_synth_add_end_field(struct tracefs_synth *synth,
 int tracefs_synth_append_start_filter(struct tracefs_synth *synth,
 				      enum tracefs_filter type,
 				      const char *field,
-				      enum tracefs_synth_compare compare,
+				      enum tracefs_compare compare,
 				      const char *val);
 int tracefs_synth_append_end_filter(struct tracefs_synth *synth,
 				    enum tracefs_filter type,
 				    const char *field,
-				    enum tracefs_synth_compare compare,
+				    enum tracefs_compare compare,
 				    const char *val);
 int tracefs_synth_create(struct tracefs_instance *instance,
 			 struct tracefs_synth *synth);
diff --git a/src/Makefile b/src/Makefile
index c7f7c1cc1680..767af49034ee 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -10,6 +10,7 @@ OBJS += tracefs-tools.o
 OBJS += tracefs-marker.o
 OBJS += tracefs-kprobes.o
 OBJS += tracefs-hist.o
+OBJS += tracefs-filter.o
 
 OBJS := $(OBJS:%.o=$(bdir)/%.o)
 DEPS := $(OBJS:$(bdir)/%.o=$(bdir)/.%.d)
diff --git a/src/tracefs-filter.c b/src/tracefs-filter.c
new file mode 100644
index 000000000000..def8f68cb52a
--- /dev/null
+++ b/src/tracefs-filter.c
@@ -0,0 +1,747 @@
+// SPDX-License-Identifier: LGPL-2.1
+/*
+ * Copyright (C) 2021 VMware Inc, Steven Rostedt <rostedt@goodmis.org>
+ *
+ * Updates:
+ * Copyright (C) 2021, VMware, Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
+ *
+ */
+#include <trace-seq.h>
+#include <stdlib.h>
+#include <ctype.h>
+#include <errno.h>
+
+#include "tracefs.h"
+#include "tracefs-local.h"
+
+enum {
+	S_START,
+	S_COMPARE,
+	S_NOT,
+	S_CONJUNCTION,
+	S_OPEN_PAREN,
+	S_CLOSE_PAREN,
+};
+
+static const struct tep_format_field common_timestamp = {
+	.type			= "u64",
+	.name			= "common_timestamp",
+	.size			= 8,
+};
+
+static const struct tep_format_field common_timestamp_usecs = {
+	.type			= "u64",
+	.name			= "common_timestamp.usecs",
+	.size			= 8,
+};
+
+/*
+ * This also must be able to accept fields that are OK via the histograms,
+ * such as common_timestamp.
+ */
+static const struct tep_format_field *get_event_field(struct tep_event *event,
+					 const char *field_name)
+{
+	if (!strcmp(field_name, TRACEFS_TIMESTAMP))
+		return &common_timestamp;
+
+	if (!strcmp(field_name, TRACEFS_TIMESTAMP_USECS))
+		return &common_timestamp_usecs;
+
+	return tep_find_any_field(event, field_name);
+}
+
+__hidden bool
+trace_verify_event_field(struct tep_event *event,
+			 const char *field_name,
+			 const struct tep_format_field **ptr_field)
+{
+	const struct tep_format_field *field;
+
+	field = get_event_field(event, field_name);
+	if (!field) {
+		errno = ENODEV;
+		return false;
+	}
+
+	if (ptr_field)
+		*ptr_field = field;
+
+	return true;
+}
+
+__hidden int trace_test_state(int state)
+{
+	switch (state) {
+	case S_START:
+	case S_CLOSE_PAREN:
+	case S_COMPARE:
+		return 0;
+	}
+
+	errno = EBADE;
+	return -1;
+}
+
+static int append_filter(char **filter, unsigned int *state,
+			 unsigned int *open_parens,
+			 struct tep_event *event,
+			 enum tracefs_filter type,
+			 const char *field_name,
+			 enum tracefs_compare compare,
+			 const char *val)
+{
+	const struct tep_format_field *field;
+	bool is_string;
+	char *conj = "||";
+	char *tmp;
+
+	switch (type) {
+	case TRACEFS_FILTER_COMPARE:
+		switch (*state) {
+		case S_START:
+		case S_OPEN_PAREN:
+		case S_CONJUNCTION:
+		case S_NOT:
+			break;
+		default:
+			goto inval;
+		}
+		break;
+
+	case TRACEFS_FILTER_AND:
+		conj = "&&";
+		/* Fall through */
+	case TRACEFS_FILTER_OR:
+		switch (*state) {
+		case S_COMPARE:
+		case S_CLOSE_PAREN:
+			break;
+		default:
+			goto inval;
+		}
+		/* Don't lose old filter on failure */
+		tmp = strdup(*filter);
+		if (!tmp)
+			return -1;
+		tmp = append_string(tmp, NULL, conj);
+		if (!tmp)
+			return -1;
+		free(*filter);
+		*filter = tmp;
+		*state = S_CONJUNCTION;
+		return 0;
+
+	case TRACEFS_FILTER_NOT:
+		switch (*state) {
+		case S_START:
+		case S_OPEN_PAREN:
+		case S_CONJUNCTION:
+		case S_NOT:
+			break;
+		default:
+			goto inval;
+		}
+		if (*filter) {
+			tmp = strdup(*filter);
+			tmp = append_string(tmp, NULL, "!");
+		} else {
+			tmp = strdup("!");
+		}
+		if (!tmp)
+			return -1;
+		free(*filter);
+		*filter = tmp;
+		*state = S_NOT;
+		return 0;
+
+	case TRACEFS_FILTER_OPEN_PAREN:
+		switch (*state) {
+		case S_START:
+		case S_OPEN_PAREN:
+		case S_NOT:
+		case S_CONJUNCTION:
+			break;
+		default:
+			goto inval;
+		}
+		if (*filter) {
+			tmp = strdup(*filter);
+			tmp = append_string(tmp, NULL, "(");
+		} else {
+			tmp = strdup("(");
+		}
+		if (!tmp)
+			return -1;
+		free(*filter);
+		*filter = tmp;
+		*state = S_OPEN_PAREN;
+		(*open_parens)++;
+		return 0;
+
+	case TRACEFS_FILTER_CLOSE_PAREN:
+		switch (*state) {
+		case S_CLOSE_PAREN:
+		case S_COMPARE:
+			break;
+		default:
+			goto inval;
+		}
+		if (!*open_parens)
+			goto inval;
+
+		tmp = strdup(*filter);
+		if (!tmp)
+			return -1;
+		tmp = append_string(tmp, NULL, ")");
+		if (!tmp)
+			return -1;
+		free(*filter);
+		*filter = tmp;
+		*state = S_CLOSE_PAREN;
+		(*open_parens)--;
+		return 0;
+	}
+
+	if (!field_name || !val)
+		goto inval;
+
+	if (!trace_verify_event_field(event, field_name, &field))
+		return -1;
+
+	is_string = field->flags & TEP_FIELD_IS_STRING;
+
+	if (!is_string && (field->flags & TEP_FIELD_IS_ARRAY))
+		goto inval;
+
+	if (*filter) {
+		tmp = strdup(*filter);
+		if (!tmp)
+			return -1;
+		tmp = append_string(tmp, NULL, field_name);
+	} else {
+		tmp = strdup(field_name);
+	}
+
+	switch (compare) {
+	case TRACEFS_COMPARE_EQ: tmp = append_string(tmp, NULL, " == "); break;
+	case TRACEFS_COMPARE_NE: tmp = append_string(tmp, NULL, " != "); break;
+	case TRACEFS_COMPARE_RE:
+		if (!is_string)
+			goto inval;
+		tmp = append_string(tmp, NULL, "~");
+		break;
+	default:
+		if (is_string)
+			goto inval;
+	}
+
+	switch (compare) {
+	case TRACEFS_COMPARE_GT: tmp = append_string(tmp, NULL, " > "); break;
+	case TRACEFS_COMPARE_GE: tmp = append_string(tmp, NULL, " >= "); break;
+	case TRACEFS_COMPARE_LT: tmp = append_string(tmp, NULL, " < "); break;
+	case TRACEFS_COMPARE_LE: tmp = append_string(tmp, NULL, " <= "); break;
+	case TRACEFS_COMPARE_AND: tmp = append_string(tmp, NULL, " & "); break;
+	default: break;
+	}
+
+	tmp = append_string(tmp, NULL, val);
+
+	if (!tmp)
+		return -1;
+
+	free(*filter);
+	*filter = tmp;
+	*state = S_COMPARE;
+
+	return 0;
+inval:
+	errno = EINVAL;
+	return -1;
+}
+
+static int count_parens(char *filter, unsigned int *state)
+{
+	bool backslash = false;
+	int quote = 0;
+	int open = 0;
+	int i;
+
+	if (!filter)
+		return 0;
+
+	for (i = 0; filter[i]; i++) {
+		if (quote) {
+			if (backslash)
+				backslash = false;
+			else if (filter[i] == '\\')
+				backslash = true;
+			else if (quote == filter[i])
+				quote = 0;
+			continue;
+		}
+
+		switch (filter[i]) {
+		case '(':
+			*state = S_OPEN_PAREN;
+			open++;
+			break;
+		case ')':
+			*state = S_CLOSE_PAREN;
+			open--;
+			break;
+		case '\'':
+		case '"':
+			*state = S_COMPARE;
+			quote = filter[i];
+			break;
+		case '!':
+			switch (filter[i+1]) {
+			case '=':
+			case '~':
+				*state = S_COMPARE;
+				i++;
+				break;
+			default:
+				*state = S_NOT;
+			}
+			break;
+		case '&':
+		case '|':
+			if (filter[i] == filter[i+1]) {
+				*state = S_CONJUNCTION;
+				i++;
+				break;
+			}
+			/* Fall through */
+		case '0' ... '9':
+		case 'a' ... 'z':
+		case 'A' ... 'Z':
+		case '_': case '+': case '-': case '*': case '/':
+			*state = S_COMPARE;
+			break;
+		}
+	}
+	return open;
+}
+
+__hidden int trace_append_filter(char **filter, unsigned int *state,
+			 unsigned int *open_parens,
+			 struct tep_event *event,
+			 enum tracefs_filter type,
+			 const char *field_name,
+			 enum tracefs_compare compare,
+			 const char *val)
+{
+	return append_filter(filter, state, open_parens, event, type,
+			     field_name, compare, val);
+}
+
+/**
+ * tracefs_event_append_filter - create or append a filter for an event
+ * @event: tep_event to create / append a filter for
+ * @filter: Pointer to string to append to (pointer to NULL to create)
+ * @type: The type of element to add to the filter
+ * @field: For @type == TRACEFS_FILTER_COMPARE, the field to compare
+ * @compare: For @type == TRACEFS_FILTER_COMPARE, how to compare @field to @val
+ * @val: For @type == TRACEFS_FILTER_COMPARE, what value @field is to be
+ *
+ * This will put together a filter string for the starting event
+ * of @synth. It check to make sure that what is added is correct compared
+ * to the filter that is already built.
+ *
+ * @type can be:
+ *     TRACEFS_FILTER_COMPARE:        See below
+ *     TRACEFS_FILTER_AND:            Append "&&" to the filter
+ *     TRACEFS_FILTER_OR:             Append "||" to the filter
+ *     TRACEFS_FILTER_NOT:            Append "!" to the filter
+ *     TRACEFS_FILTER_OPEN_PAREN:     Append "(" to the filter
+ *     TRACEFS_FILTER_CLOSE_PAREN:    Append ")" to the filter
+ *
+ * For all types except TRACEFS_FILTER_COMPARE, the @field, @compare,
+ * and @val are ignored.
+ *
+ * For @type == TRACEFS_FILTER_COMPARE.
+ *
+ *  @field is the name of the field for the start event to compare.
+ *         If it is not a field for the start event, this return an
+ *         error.
+ *
+ *  @compare can be one of:
+ *     TRACEFS_COMPARE_EQ:       Test @field == @val
+ *     TRACEFS_COMPARE_NE:       Test @field != @val
+ *     TRACEFS_COMPARE_GT:       Test @field > @val
+ *     TRACEFS_COMPARE_GE:       Test @field >= @val
+ *     TRACEFS_COMPARE_LT:       Test @field < @val
+ *     TRACEFS_COMPARE_LE:       Test @field <= @val
+ *     TRACEFS_COMPARE_RE:       Test @field ~ @val
+ *     TRACEFS_COMPARE_AND:      Test @field & @val
+ *
+ * If the @field is of type string, and @compare is not
+ *   TRACEFS_COMPARE_EQ, TRACEFS_COMPARE_NE or TRACEFS_COMPARE_RE,
+ *   then this will return an error.
+ *
+ * Various other checks are made, for instance, if more CLOSE_PARENs
+ * are added than existing OPEN_PARENs. Or if AND is added after an
+ * OPEN_PAREN or another AND or an OR or a NOT.
+ *
+ * Returns 0 on success and -1 on failure.
+ */
+int tracefs_event_append_filter(struct tep_event *event, char **filter,
+				enum tracefs_filter type,
+				const char *field, enum tracefs_compare compare,
+				const char *val)
+{
+	unsigned int open_parens;
+	unsigned int state = 0;
+	char *str = NULL;
+	int open;
+	int ret;
+
+	if (!filter) {
+		errno = EINVAL;
+		return -1;
+	}
+
+	open = count_parens(*filter, &state);
+	if (open < 0) {
+		errno = EINVAL;
+		return -1;
+	}
+
+	if (*filter) {
+		/* append_filter() will free filter on error */
+		str = strdup(*filter);
+		if (!str)
+			return -1;
+	}
+	open_parens = open;
+
+	ret = append_filter(&str, &state, &open_parens,
+			    event, type, field, compare, val);
+	if (!ret) {
+		free(*filter);
+		*filter = str;
+	}
+	return 0;
+}
+
+static int error_msg(char **err, char *str,
+		     const char *filter, int i, const char *msg)
+{
+	char ws[i+2];
+	char *errmsg;
+
+	free(str);
+
+	/* msg is NULL for parsing append_filter failing */
+	if (!msg) {
+		switch(errno) {
+		case ENODEV:
+			msg = "field not valid";
+			break;
+		default:
+			msg = "Invalid filter";
+
+		}
+	} else
+		errno = EINVAL;
+
+	if (!err)
+		return -1;
+
+	if (!filter) {
+		*err = strdup(msg);
+		return -1;
+	}
+
+	memset(ws, ' ', i);
+	ws[i] = '^';
+	ws[i+1] = '\0';
+
+	errmsg = strdup(filter);
+	errmsg = append_string(errmsg, "\n", ws);
+	errmsg = append_string(errmsg, "\n", msg);
+	errmsg = append_string(errmsg, NULL, "\n");
+
+	*err = errmsg;
+	return -1;
+}
+
+static int get_field_end(const char *filter, int i, int *end)
+{
+	int start_i = i;
+
+	for (; filter[i]; i++) {
+		switch(filter[i]) {
+		case '0' ... '9':
+			if (i == start_i)
+				return 0;
+			/* Fall through */
+		case 'a' ... 'z':
+		case 'A' ... 'Z':
+		case '_':
+			continue;
+		default:
+			*end = i;
+			return i - start_i;
+		}
+	}
+	*end = i;
+	return i - start_i;
+}
+
+static int get_compare(const char *filter, int i, enum tracefs_compare *cmp)
+{
+	int start_i = i;
+
+	for (; filter[i]; i++) {
+		if (!isspace(filter[i]))
+			break;
+	}
+
+	switch(filter[i]) {
+	case '=':
+		if (filter[i+1] != '=')
+			goto err;
+		*cmp = TRACEFS_COMPARE_EQ;
+		i++;
+		break;
+	case '!':
+		if (filter[i+1] == '=') {
+			*cmp = TRACEFS_COMPARE_NE;
+			i++;
+			break;
+		}
+		if (filter[i+1] == '~') {
+			/* todo! */
+		}
+		goto err;
+	case '>':
+		if (filter[i+1] == '=') {
+			*cmp = TRACEFS_COMPARE_GE;
+			i++;
+			break;
+		}
+		*cmp = TRACEFS_COMPARE_GT;
+		break;
+	case '<':
+		if (filter[i+1] == '=') {
+			*cmp = TRACEFS_COMPARE_LE;
+			i++;
+			break;
+		}
+		*cmp = TRACEFS_COMPARE_LT;
+		break;
+	case '~':
+		*cmp = TRACEFS_COMPARE_RE;
+		break;
+	case '&':
+		*cmp = TRACEFS_COMPARE_AND;
+		break;
+	default:
+		goto err;
+	}
+	i++;
+
+	for (; filter[i]; i++) {
+		if (!isspace(filter[i]))
+			break;
+	}
+	return i - start_i;
+ err:
+	return start_i - i; /* negative or zero */
+}
+
+static int get_val_end(const char *filter, int i, int *end)
+{
+	bool backslash = false;
+	int start_i = i;
+	int quote;
+
+	switch (filter[i]) {
+	case '0':
+		i++;
+		if (tolower(filter[i+1]) != 'x' &&
+		    !isdigit(filter[i+1]))
+			break;
+		/* fall through */
+	case '1' ... '9':
+		switch (tolower(filter[i])) {
+		case 'x':
+			for (i++; filter[i]; i++) {
+				if (!isxdigit(filter[i]))
+					break;
+			}
+			break;
+		case '0':
+			for (i++; filter[i]; i++) {
+				if (filter[i] < '0' ||
+				    filter[i] > '7')
+					break;
+			}
+			break;
+		default:
+			for (i++; filter[i]; i++) {
+				if (!isdigit(filter[i]))
+					break;
+			}
+			break;
+		}
+		break;
+	case '"':
+	case '\'':
+		quote = filter[i];
+		for (i++; filter[i]; i++) {
+			if (backslash) {
+				backslash = false;
+				continue;
+			}
+			switch (filter[i]) {
+			case '\\':
+				backslash = true;
+				continue;
+			case '"':
+			case '\'':
+				if (filter[i] == quote)
+					break;
+				continue;
+			default:
+				continue;
+			}
+			break;
+		}
+		if (filter[i])
+			i++;
+		break;
+	default:
+		break;
+	}
+
+	*end = i;
+	return i - start_i;
+}
+
+/**
+ * tracefs_event_verify_filter - verify a given filter works for an event
+ * @event: The event to test the given filter for
+ * @filter: The filter to test
+ * @err: Error message for syntax errors (NULL to ignore)
+ *
+ * Parse the @filter to verify that it is valid for the given @event.
+ *
+ * Returns 0 on succes and -1 on error, and except for memory allocation
+ * errors, @err will be allocated with an error message. It must
+ * be freed with free().
+ */
+int tracefs_event_verify_filter(struct tep_event *event, const char *filter,
+				char **err)
+{
+	enum tracefs_filter filter_type;
+	enum tracefs_compare compare;
+	char *str = NULL;
+	char buf[(filter ? strlen(filter) : 0) + 1];
+	char *field;
+	char *val;
+	unsigned int state = 0;
+	unsigned int open = 0;
+	int len;
+	int end;
+	int n;
+	int i;
+
+	if (!filter)
+		return error_msg(err, str, NULL, 0, "No filter given");
+
+	len = strlen(filter);
+
+	for (i = 0; i < len; i++) {
+		field = NULL;
+		val = NULL;
+		compare = 0;
+
+		switch (filter[i]) {
+		case '(':
+			filter_type = TRACEFS_FILTER_OPEN_PAREN;
+			break;
+		case ')':
+			filter_type = TRACEFS_FILTER_CLOSE_PAREN;
+			break;
+		case '!':
+			filter_type = TRACEFS_FILTER_NOT;
+			break;
+		case '&':
+		case '|':
+
+			if (filter[i] == filter[i+1]) {
+				i++;
+				if (filter[i] == '&')
+					filter_type = TRACEFS_FILTER_AND;
+				else
+					filter_type = TRACEFS_FILTER_OR;
+				break;
+			}
+			if (filter[i] == '|')
+				return error_msg(err, str, filter, i,
+						 "Invalid op");
+
+			return error_msg(err, str, filter, i,
+					 "Invalid location for '&'");
+		default:
+			if (isspace(filter[i]))
+				continue;
+
+			field = buf;
+
+			n = get_field_end(filter, i, &end);
+			if (!n)
+				return error_msg(err, str, filter, i,
+						 "Invalid field name");
+
+			strncpy(field, filter+i, n);
+
+			i += n;
+			field[n++] = '\0';
+
+			val = field + n;
+
+			n = get_compare(filter, i, &compare);
+			if (n <= 0)
+				return error_msg(err, str, filter, i - n,
+						 "Invalid compare");
+
+			i += n;
+			get_val_end(filter, i, &end);
+			n = end - i;
+			if (!n)
+				return error_msg(err, str, filter, i,
+						 "Invalid value");
+			strncpy(val, filter + i, n);
+			val[n] = '\0';
+			i += n - 1;
+
+			filter_type = TRACEFS_FILTER_COMPARE;
+			break;
+		}
+		n = append_filter(&str, &state, &open,
+				    event, filter_type, field, compare, val);
+
+		if (n < 0)
+			return error_msg(err, str, filter, i, NULL);
+	}
+
+	if (open)
+		return error_msg(err, str, filter, i,
+				 "Not enough closed parenthesis");
+	switch (state) {
+	case S_COMPARE:
+	case S_CLOSE_PAREN:
+		break;
+	default:
+		return error_msg(err, str, filter, i,
+				 "Unfinished filter");
+	}
+
+	free(str);
+	return 0;
+}
diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
index 8a0d43fafb7e..7292f89457c9 100644
--- a/src/tracefs-hist.c
+++ b/src/tracefs-hist.c
@@ -568,18 +568,6 @@ struct tracefs_synth {
 	int			arg_cnt;
 };
 
-static const struct tep_format_field common_timestamp = {
-	.type			= "u64",
-	.name			= "common_timestamp",
-	.size			= 8,
-};
-
-static const struct tep_format_field common_timestamp_usecs = {
-	.type			= "u64",
-	.name			= "common_timestamp.usecs",
-	.size			= 8,
-};
-
 /**
  * tracefs_synth_free - free the resources alloced to a synth
  * @synth: The tracefs_synth descriptor
@@ -609,18 +597,6 @@ void tracefs_synth_free(struct tracefs_synth *synth)
 	free(synth);
 }
 
-static const struct tep_format_field *get_event_field(struct tep_event *event,
-					 const char *field_name)
-{
-	if (!strcmp(field_name, TRACEFS_TIMESTAMP))
-		return &common_timestamp;
-
-	if (!strcmp(field_name, TRACEFS_TIMESTAMP_USECS))
-		return &common_timestamp_usecs;
-
-	return tep_find_any_field(event, field_name);
-}
-
 static bool verify_event_fields(struct tep_event *start_event,
 				struct tep_event *end_event,
 				const char *start_field_name,
@@ -630,14 +606,14 @@ static bool verify_event_fields(struct tep_event *start_event,
 	const struct tep_format_field *start_field;
 	const struct tep_format_field *end_field;
 
-	start_field = get_event_field(start_event, start_field_name);
-	if (!start_field)
-		goto nodev;
+	if (!trace_verify_event_field(start_event, start_field_name,
+				      &start_field))
+		return false;
 
 	if (end_event) {
-		end_field = get_event_field(end_event, end_field_name);
-		if (!start_field)
-			goto nodev;
+		if (!trace_verify_event_field(end_event, end_field_name,
+					      &end_field))
+			return false;
 
 		if (start_field->flags != end_field->flags ||
 		    start_field->size != end_field->size) {
@@ -650,12 +626,9 @@ static bool verify_event_fields(struct tep_event *start_event,
 		*ptr_start_field = start_field;
 
 	return true;
- nodev:
-	errno = ENODEV;
-	return false;
 }
 
-static char *append_string(char *str, const char *space, const char *add)
+__hidden char *append_string(char *str, const char *space, const char *add)
 {
 	char *new;
 	int len;
@@ -1110,8 +1083,7 @@ int tracefs_synth_add_start_field(struct tracefs_synth *synth,
 	if (!name)
 		name = start_field;
 
-	if (!verify_event_fields(synth->start_event, NULL,
-				 start_field, NULL, &field))
+	if (!trace_verify_event_field(synth->start_event, start_field, &field))
 		return -1;
 
 	start_arg = new_arg(synth);
@@ -1163,8 +1135,7 @@ int tracefs_synth_add_end_field(struct tracefs_synth *synth,
 	if (!name)
 		name = end_field;
 
-	if (!verify_event_fields(synth->end_event, NULL,
-				 end_field, NULL, &field))
+	if (!trace_verify_event_field(synth->end_event, end_field, &field))
 		return -1;
 
 	ret = add_var(&synth->end_vars, name, end_field, false);
@@ -1177,192 +1148,6 @@ int tracefs_synth_add_end_field(struct tracefs_synth *synth,
 	return ret;
 }
 
-enum {
-	S_START,
-	S_COMPARE,
-	S_NOT,
-	S_CONJUNCTION,
-	S_OPEN_PAREN,
-	S_CLOSE_PAREN,
-};
-
-static int append_synth_filter(char **filter, unsigned int *state,
-			       unsigned int *open_parens,
-			       struct tep_event *event,
-			       enum tracefs_filter type,
-			       const char *field_name,
-			       enum tracefs_synth_compare compare,
-			       const char *val)
-{
-	const struct tep_format_field *field;
-	bool is_string;
-	char *conj = "||";
-	char *tmp;
-
-	switch (type) {
-	case TRACEFS_FILTER_COMPARE:
-		switch (*state) {
-		case S_START:
-		case S_OPEN_PAREN:
-		case S_CONJUNCTION:
-		case S_NOT:
-			break;
-		default:
-			goto inval;
-		}
-		break;
-
-	case TRACEFS_FILTER_AND:
-		conj = "&&";
-		/* Fall through */
-	case TRACEFS_FILTER_OR:
-		switch (*state) {
-		case S_COMPARE:
-		case S_CLOSE_PAREN:
-			break;
-		default:
-			goto inval;
-		}
-		/* Don't lose old filter on failure */
-		tmp = strdup(*filter);
-		if (!tmp)
-			return -1;
-		tmp = append_string(tmp, NULL, conj);
-		if (!tmp)
-			return -1;
-		free(*filter);
-		*filter = tmp;
-		*state = S_CONJUNCTION;
-		return 0;
-
-	case TRACEFS_FILTER_NOT:
-		switch (*state) {
-		case S_START:
-		case S_OPEN_PAREN:
-		case S_CONJUNCTION:
-		case S_NOT:
-			break;
-		default:
-			goto inval;
-		}
-		if (*filter) {
-			tmp = strdup(*filter);
-			tmp = append_string(tmp, NULL, "!");
-		} else {
-			tmp = strdup("!");
-		}
-		if (!tmp)
-			return -1;
-		free(*filter);
-		*filter = tmp;
-		*state = S_NOT;
-		return 0;
-
-	case TRACEFS_FILTER_OPEN_PAREN:
-		switch (*state) {
-		case S_START:
-		case S_OPEN_PAREN:
-		case S_NOT:
-		case S_CONJUNCTION:
-			break;
-		default:
-			goto inval;
-		}
-		if (*filter) {
-			tmp = strdup(*filter);
-			tmp = append_string(tmp, NULL, "(");
-		} else {
-			tmp = strdup("(");
-		}
-		if (!tmp)
-			return -1;
-		free(*filter);
-		*filter = tmp;
-		*state = S_OPEN_PAREN;
-		(*open_parens)++;
-		return 0;
-
-	case TRACEFS_FILTER_CLOSE_PAREN:
-		switch (*state) {
-		case S_CLOSE_PAREN:
-		case S_COMPARE:
-			break;
-		default:
-			goto inval;
-		}
-		if (!*open_parens)
-			goto inval;
-
-		tmp = strdup(*filter);
-		if (!tmp)
-			return -1;
-		tmp = append_string(tmp, NULL, ")");
-		if (!tmp)
-			return -1;
-		free(*filter);
-		*filter = tmp;
-		*state = S_CLOSE_PAREN;
-		(*open_parens)--;
-		return 0;
-	}
-
-	if (!field_name || !val)
-		goto inval;
-
-	if (!verify_event_fields(event, NULL, field_name, NULL, &field))
-		return -1;
-
-	is_string = field->flags & TEP_FIELD_IS_STRING;
-
-	if (!is_string && (field->flags & TEP_FIELD_IS_ARRAY))
-		goto inval;
-
-	if (*filter) {
-		tmp = strdup(*filter);
-		if (!tmp)
-			return -1;
-		tmp = append_string(tmp, NULL, field_name);
-	} else {
-		tmp = strdup(field_name);
-	}
-
-	switch (compare) {
-	case TRACEFS_COMPARE_EQ: tmp = append_string(tmp, NULL, " == "); break;
-	case TRACEFS_COMPARE_NE: tmp = append_string(tmp, NULL, " != "); break;
-	case TRACEFS_COMPARE_RE:
-		if (!is_string)
-			goto inval;
-		tmp = append_string(tmp, NULL, "~");
-		break;
-	default:
-		if (is_string)
-			goto inval;
-	}
-
-	switch (compare) {
-	case TRACEFS_COMPARE_GT: tmp = append_string(tmp, NULL, " > "); break;
-	case TRACEFS_COMPARE_GE: tmp = append_string(tmp, NULL, " >= "); break;
-	case TRACEFS_COMPARE_LT: tmp = append_string(tmp, NULL, " < "); break;
-	case TRACEFS_COMPARE_LE: tmp = append_string(tmp, NULL, " <= "); break;
-	case TRACEFS_COMPARE_AND: tmp = append_string(tmp, NULL, " & "); break;
-	default: break;
-	}
-
-	tmp = append_string(tmp, NULL, val);
-
-	if (!tmp)
-		return -1;
-
-	free(*filter);
-	*filter = tmp;
-	*state = S_COMPARE;
-
-	return 0;
-inval:
-	errno = EINVAL;
-	return -1;
-}
-
 /**
  * tracefs_synth_append_start_filter - create or append a filter
  * @synth: The tracefs_synth descriptor
@@ -1415,10 +1200,10 @@ inval:
 int tracefs_synth_append_start_filter(struct tracefs_synth *synth,
 				      enum tracefs_filter type,
 				      const char *field,
-				      enum tracefs_synth_compare compare,
+				      enum tracefs_compare compare,
 				      const char *val)
 {
-	return append_synth_filter(&synth->start_filter, &synth->start_state,
+	return trace_append_filter(&synth->start_filter, &synth->start_state,
 				   &synth->start_parens,
 				   synth->start_event,
 				   type, field, compare, val);
@@ -1438,10 +1223,10 @@ int tracefs_synth_append_start_filter(struct tracefs_synth *synth,
 int tracefs_synth_append_end_filter(struct tracefs_synth *synth,
 				    enum tracefs_filter type,
 				    const char *field,
-				    enum tracefs_synth_compare compare,
+				    enum tracefs_compare compare,
 				    const char *val)
 {
-	return append_synth_filter(&synth->end_filter, &synth->end_state,
+	return trace_append_filter(&synth->end_filter, &synth->end_state,
 				   &synth->end_parens,
 				   synth->end_event,
 				   type, field, compare, val);
@@ -1564,23 +1349,10 @@ static char *append_filter(char *hist, char *filter, unsigned int parens)
 	return hist;
 }
 
-static int test_state(int state)
-{
-	switch (state) {
-	case S_START:
-	case S_CLOSE_PAREN:
-	case S_COMPARE:
-		return 0;
-	}
-
-	errno = EBADE;
-	return -1;
-}
-
 static int verify_state(struct tracefs_synth *synth)
 {
-	if (test_state(synth->start_state) < 0 ||
-	    test_state(synth->end_state) < 0)
+	if (trace_test_state(synth->start_state) < 0 ||
+	    trace_test_state(synth->end_state) < 0)
 		return -1;
 	return 0;
 }
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 6/6] libtracefs: Add man pages for tracefs_event_append_filter()
  2021-07-30  3:03 [PATCH 0/6] libtracefs: Updates to event filtering Steven Rostedt
                   ` (4 preceding siblings ...)
  2021-07-30  3:03 ` [PATCH 5/6] libtracefs: Add filter creating and verify API Steven Rostedt
@ 2021-07-30  3:03 ` Steven Rostedt
  5 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2021-07-30  3:03 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Steven Rostedt (VMware)

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Add man pages for tracefs_event_append_filter() and
tracefs_event_verify_filter()

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/libtracefs-filter.txt | 329 ++++++++++++++++++++++++++++
 1 file changed, 329 insertions(+)
 create mode 100644 Documentation/libtracefs-filter.txt

diff --git a/Documentation/libtracefs-filter.txt b/Documentation/libtracefs-filter.txt
new file mode 100644
index 000000000000..7e167bc8f34a
--- /dev/null
+++ b/Documentation/libtracefs-filter.txt
@@ -0,0 +1,329 @@
+libtracefs(3)
+=============
+
+NAME
+----
+tracefs_event_append_filter, tracefs_event_verify_filter - Add and verify event filters
+
+SYNOPSIS
+--------
+[verse]
+--
+*#include <tracefs.h>*
+
+int tracefs_event_append_filter(struct tep_event pass:[*]event, char pass:[**] filter,
+				 struct tracefs_filter type, const char pass:[*]field,
+				 enum tracefs_synth_compare compare, const char pass:[*]val);
+int tracefs_event_verify_filter(struct tep_event pass:[*]event, const char pass:[*]filter, char pass:[**]err);
+
+--
+
+DESCRIPTION
+-----------
+*tracefs_event_append_filter*() is a way to create and verify event filters for
+a given event. It will verify that the _field_ belongs to the event and that
+the _compare_ option that is used is valid for the type of the field, as well
+as _val_. For the _type_ that is not of *TRACEFS_FILTER_COMPARE*, it will build
+the logical string and also make sure that the syntax is correct. For example,
+there are no more close parenthesis than open parenthesis. An AND (&&) or OR
+(||) is not misplaced, etc.
+
+*tracefs_synth_append_start_filter*() creates a filter or appends to it for the
+starting event. Depending on _type_, it will build a string of tokens for
+parenthesis or logic statemens, or it may add a comparison of _field_
+to _val_ based on _compare_.
+
+If _type_ is:
+*TRACEFS_FILTER_COMPARE*     -  See below
+*TRACEFS_FILTER_AND*         -  Append "&&" to the filter
+*TRACEFS_FILTER_OR*          -  Append "||" to the filter
+*TRACEFS_FILTER_NOT*         -  Append "!" to the filter
+*TRACEFS_FILTER_OPEN_PAREN*  -  Append "(" to the filter
+*TRACEFS_FILTER_CLOSE_PAREN* -  Append ")" to the filter
+
+_field_, _compare_, and _val_ are ignored unless _type_ is equal to
+*TRACEFS_FILTER_COMPARE*, then _compare will be used for the following:
+
+*TRACEFS_COMPARE_EQ* - _field_ == _val_
+
+*TRACEFS_COMPARE_NE* - _field_ != _val_
+
+*TRACEFS_COMPARE_GT* - _field_ > _val_
+
+*TRACEFS_COMPARE_GE* - _field_ >= _val_
+
+*TRACEFS_COMPARE_LT* - _field_ < _val_
+
+*TRACEFS_COMPARE_LE* - _field_ <pass:[=] _val_
+
+*TRACEFS_COMPARE_RE* - _field_ ~ "_val_" : where _field_ is a string.
+
+*TRACEFS_COMPARE_AND* - _field_ & _val_ : where _field_ is a flags field.
+
+*tracefs_event_verify_filter*() will parse _filter_ to make sure that the
+fields are for the _event_, and that the syntax is correct. If there's an
+error in the syntax, and _err_ is not NULL, then it will be allocated with an
+error message stating what was found wrong with the filter. _err_ must be freed
+with *free*().
+
+RETURN VALUE
+------------
+*tracefs_event_append_filter*() returns 0 on success and -1 on error.
+
+*tracefs_event_verify_filter*() returns 0 on success and -1 on error. if there
+is an error, and _errno_ is not *ENOMEM*, then _err_ is allocated and will
+contain a string describing what was found wrong with _filter_. _err_ must be
+freed with *free*().
+
+EXAMPLE
+-------
+[source,c]
+--
+#include <stdlib.h>
+#include <stdio.h>
+#include <ctype.h>
+#include <tracefs.h>
+
+static void usage(char **argv)
+{
+	fprintf(stderr, "usage: %s [system] event filter\n", argv[0]);
+	exit(-1);
+}
+
+int main (int argc, char **argv)
+{
+	struct tep_handle *tep;
+	struct tep_event *event;
+	const char *system = NULL;
+	const char *event_name;
+	const char *filter;
+	char *new_filter = NULL;
+	char *err = NULL;
+	int i;
+
+	if (argc < 3)
+		usage(argv);
+
+	if (argc < 4) {
+		event_name = argv[1];
+		filter = argv[2];
+	} else {
+		system = argv[1];
+		event_name = argv[2];
+		filter = argv[3];
+	}
+
+	/* Load all events from the system */
+	tep = tracefs_local_events(NULL);
+	if (!tep) {
+		perror("tep");
+		exit(-1);
+	}
+
+	event = tep_find_event_by_name(tep, system, event_name);
+	if (!event) {
+		fprintf(stderr, "Event %s%s%s not found\n",
+			system ? system : "" , system ? " " : "",
+			event_name);
+		exit(-1);
+	}
+
+	if (tracefs_event_verify_filter(event, filter, &err) < 0) {
+		perror("tracecfs_event_verify_filter");
+		if (err)
+			fprintf(stderr, "%s", err);
+		free(err);
+		exit(-1);
+	}
+
+	for (i = 0; filter[i]; i++) {
+		char buf[strlen(filter)];
+		char *field = NULL;
+		char *val = NULL;
+		enum tracefs_filter type;
+		enum tracefs_compare compare = 0;
+		int start_i, n;
+		int quote;
+		bool backslash;
+
+		while (isspace(filter[i]))
+			i++;
+
+		switch(filter[i]) {
+		case '(':
+			type = TRACEFS_FILTER_OPEN_PAREN;
+			break;
+		case ')':
+			type = TRACEFS_FILTER_CLOSE_PAREN;
+			break;
+		case '!':
+			type = TRACEFS_FILTER_NOT;
+			break;
+		case '&':
+			type = TRACEFS_FILTER_AND;
+			i++;
+			break;
+		case '|':
+			type = TRACEFS_FILTER_OR;
+			i++;
+			break;
+		default:
+			type = TRACEFS_FILTER_COMPARE;
+
+			while (isspace(filter[i]))
+				i++;
+
+			start_i = i;
+			for (; filter[i]; i++) {
+				switch(filter[i]) {
+				case 'a' ... 'z':
+				case 'A' ... 'Z':
+				case '0' ... '9':
+				case '_':
+					continue;
+				}
+				break;
+			}
+
+			n = i - start_i;
+			field = buf;
+			strncpy(field, filter + start_i, n);
+			field[n++] = '\0';
+
+			val = buf + n;
+
+			while (isspace(filter[i]))
+				i++;
+
+			start_i = i;
+			switch(filter[i++]) {
+			case '>':
+				compare = TRACEFS_COMPARE_GT;
+				if (filter[i] == '=') {
+					i++;
+					compare = TRACEFS_COMPARE_GE;
+				}
+				break;
+			case '<':
+				compare = TRACEFS_COMPARE_LT;
+				if (filter[i] == '=') {
+					i++;
+					compare = TRACEFS_COMPARE_LE;
+				}
+				break;
+			case '=':
+				compare = TRACEFS_COMPARE_EQ;
+				i++;
+				break;
+			case '!':
+				compare = TRACEFS_COMPARE_NE;
+				i++;
+				break;
+			case '~':
+				compare = TRACEFS_COMPARE_RE;
+				break;
+			case '&':
+				compare = TRACEFS_COMPARE_AND;
+				break;
+			}
+
+			while (isspace(filter[i]))
+				i++;
+
+			quote = 0;
+			backslash = false;
+			start_i = i;
+			for (; filter[i]; i++) {
+				if (quote) {
+					if (backslash)
+						backslash = false;
+					else if (filter[i] == '\\')
+						backslash = true;
+					else if (filter[i] == quote)
+						quote = 0;
+					continue;
+				}
+				switch(filter[i]) {
+				case '"': case '\'':
+					quote = filter[i];
+					continue;
+				case 'a' ... 'z':
+				case 'A' ... 'Z':
+				case '0' ... '9':
+				case '_':
+					continue;
+				}
+				break;
+			}
+			n = i - start_i;
+			strncpy(val, filter + start_i, n);
+			val[n] = '\0';
+			break;
+		}
+		n = tracefs_event_append_filter(event, &new_filter, type,
+						field, compare, val);
+		if (n < 0) {
+			fprintf(stderr, "Failed making new filter:\n'%s'\n",
+				new_filter ? new_filter : "(null)");
+			exit(-1);
+		}
+	}
+
+	tep_free(tep);
+
+	printf("Created new filter: '%s'\n", new_filter);
+	free(new_filter);
+
+	exit(0);
+}
+--
+
+FILES
+-----
+[verse]
+--
+*tracefs.h*
+	Header file to include in order to have access to the library APIs.
+*-ltracefs*
+	Linker switch to add when building a program that uses the library.
+--
+
+SEE ALSO
+--------
+_libtracefs(3)_,
+_libtraceevent(3)_,
+_trace-cmd(1)_,
+_tracefs_hist_alloc(3)_,
+_tracefs_hist_free(3)_,
+_tracefs_hist_add_key(3)_,
+_tracefs_hist_add_value(3)_,
+_tracefs_hist_add_name(3)_,
+_tracefs_hist_start(3)_,
+_tracefs_hist_destory(3)_,
+_tracefs_hist_add_sort_key(3)_,
+_tracefs_hist_sort_key_direction(3)_
+
+AUTHOR
+------
+[verse]
+--
+*Steven Rostedt* <rostedt@goodmis.org>
+*Tzvetomir Stoyanov* <tz.stoyanov@gmail.com>
+*sameeruddin shaik* <sameeruddin.shaik8@gmail.com>
+--
+REPORTING BUGS
+--------------
+Report bugs to  <linux-trace-devel@vger.kernel.org>
+
+LICENSE
+-------
+libtracefs is Free Software licensed under the GNU LGPL 2.1
+
+RESOURCES
+---------
+https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/
+
+COPYING
+-------
+Copyright \(C) 2020 VMware, Inc. Free use of this software is granted under
+the terms of the GNU Public License (GPL).
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-07-30  3:03 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-30  3:03 [PATCH 0/6] libtracefs: Updates to event filtering Steven Rostedt
2021-07-30  3:03 ` [PATCH 1/6] libtracefs: Update to version 1.3-dev Steven Rostedt
2021-07-30  3:03 ` [PATCH 2/6] libtracefs: Rename GR and NQ to GT and NE respectively Steven Rostedt
2021-07-30  3:03 ` [PATCH 3/6] libtracefs: Append the synth filter with parens and conjunctions Steven Rostedt
2021-07-30  3:03 ` [PATCH 4/6] libtracefs: Add kerneldoc comments to the remaining synth functions Steven Rostedt
2021-07-30  3:03 ` [PATCH 5/6] libtracefs: Add filter creating and verify API Steven Rostedt
2021-07-30  3:03 ` [PATCH 6/6] libtracefs: Add man pages for tracefs_event_append_filter() Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).