linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] libtracefs: Fixes for sqlhist
@ 2022-06-06 19:29 Steven Rostedt
  2022-06-06 19:29 ` [PATCH 1/6] libtracefs: Fix make sqlhist when built again Steven Rostedt
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Steven Rostedt @ 2022-06-06 19:29 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Harald Seiler, Steven Rostedt (Google)

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

At Kernel Recipes, I demonstrated sqlhist and how to use it. Along the way,
I discovered several bugs. This series addresses those bugs that were found.

Steven Rostedt (Google) (6):
  libtracefs: Fix make sqlhist when built again
  libtracefs: Add libtracefs.a to dependency of sqlhist
  libtracefs: Differentiate FROM and JOIN events if they are the same
    event
  libtracefs: Use unique names for sql field variables
  libtracefs: Differentiate WHERE clause when FROM and TO events are the
    same
  libtracefs sqlhist: Report errors executing the commands

 Documentation/libtracefs-sql.txt |  18 ++++--
 include/tracefs-local.h          |   4 ++
 samples/Makefile                 |   4 +-
 src/sqlhist-parse.h              |   3 +-
 src/tracefs-hist.c               | 105 ++++++++++++++++++++++++++++---
 src/tracefs-sqlhist.c            |  44 +++++++++----
 6 files changed, 150 insertions(+), 28 deletions(-)

-- 
2.35.1


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

* [PATCH 1/6] libtracefs: Fix make sqlhist when built again
  2022-06-06 19:29 [PATCH 0/6] libtracefs: Fixes for sqlhist Steven Rostedt
@ 2022-06-06 19:29 ` Steven Rostedt
  2022-06-06 19:29 ` [PATCH 2/6] libtracefs: Add libtracefs.a to dependency of sqlhist Steven Rostedt
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2022-06-06 19:29 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Harald Seiler, Steven Rostedt (Google)

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

"make sqlhist" currently only works after a "make clean". That is because
the sqlhist.c is extracted from the man page and left there after the
first build. On the second build, the sqlhist.c still exists and the
target for "sqlhist" runs the default build as it has nothing to build
with, which fails to build as the default does not work with it.

Add the sqlhist target to .PHONY to tell make that it is not a real target
and it should not try to build it using the default methods.

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

diff --git a/samples/Makefile b/samples/Makefile
index 97a570d6c956..c1392fbbb12b 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -67,3 +67,5 @@ $(bdir)/%.o: $(bdir)/%.c
 
 clean:
 	$(Q)$(call do_clean,$(sdir)/* $(bdir)/sqlhist.c $(bdir)/sqlhist.o)
+
+.PHONY: sqlhist
-- 
2.35.1


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

* [PATCH 2/6] libtracefs: Add libtracefs.a to dependency of sqlhist
  2022-06-06 19:29 [PATCH 0/6] libtracefs: Fixes for sqlhist Steven Rostedt
  2022-06-06 19:29 ` [PATCH 1/6] libtracefs: Fix make sqlhist when built again Steven Rostedt
@ 2022-06-06 19:29 ` Steven Rostedt
  2022-06-06 19:29 ` [PATCH 3/6] libtracefs: Differentiate FROM and JOIN events if they are the same event Steven Rostedt
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2022-06-06 19:29 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Harald Seiler, Steven Rostedt (Google)

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

As sqlhist is statically linked to the libtracefs.a file, it should be
rebuilt if the libtracefs.a is updated. Because sqlhist.c is extracted
from the man pages, it does not get the automatic dependency updates like
the rest of the source tree has.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 samples/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/Makefile b/samples/Makefile
index c1392fbbb12b..6628679e6227 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -46,7 +46,7 @@ sqlhist: $(sdir)/sqlhist
 $(TARGETS): $(sdir)
 
 # sqlhist is unique and stands on its own
-$(sdir)/sqlhist: $(bdir)/sqlhist.c
+$(sdir)/sqlhist: $(bdir)/sqlhist.c $(LIBTRACEFS_STATIC)
 	$(call do_sample_build,$@,$<)
 
 $(sdir)/%: $(bdir)/%.o
-- 
2.35.1


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

* [PATCH 3/6] libtracefs: Differentiate FROM and JOIN events if they are the same event
  2022-06-06 19:29 [PATCH 0/6] libtracefs: Fixes for sqlhist Steven Rostedt
  2022-06-06 19:29 ` [PATCH 1/6] libtracefs: Fix make sqlhist when built again Steven Rostedt
  2022-06-06 19:29 ` [PATCH 2/6] libtracefs: Add libtracefs.a to dependency of sqlhist Steven Rostedt
@ 2022-06-06 19:29 ` Steven Rostedt
  2022-06-06 19:29 ` [PATCH 4/6] libtracefs: Use unique names for sql field variables Steven Rostedt
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2022-06-06 19:29 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Harald Seiler, Steven Rostedt (Google)

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

If both the FROM and the JOIN event are the same, the fields get
confusing due to the way tracefs_sql() does not distinguish the events
when transforming the variable names, and it may process the JOIN fields
as the FROM fields, and the wrong field gets passed to the synthetic
event.

Add a new FIELD_TYPE enum and set the fields to FROM or TO to know which
event the field belongs to, and use that to avoid picking the field for
the wrong event.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 src/tracefs-sqlhist.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/tracefs-sqlhist.c b/src/tracefs-sqlhist.c
index caf927fd5e1c..d854f7b54344 100644
--- a/src/tracefs-sqlhist.c
+++ b/src/tracefs-sqlhist.c
@@ -30,6 +30,12 @@ enum alias_type {
 	ALIAS_FIELD,
 };
 
+enum field_type {
+	FIELD_NONE,
+	FIELD_FROM,
+	FIELD_TO,
+};
+
 #define for_each_field(expr, field, table) \
 	for (expr = (table)->fields; expr; expr = (field)->next)
 
@@ -42,6 +48,7 @@ struct field {
 	const char		*label;
 	const char		*field;
 	const char		*type;
+	enum field_type		ftype;
 };
 
 struct filter {
@@ -583,6 +590,7 @@ static int update_vars(struct tep_handle *tep,
 {
 	struct sqlhist_bison *sb = table->sb;
 	struct field *event_field = &expr->field;
+	enum field_type ftype = FIELD_NONE;
 	struct tep_event *event;
 	struct field *field;
 	const char *label;
@@ -592,6 +600,11 @@ static int update_vars(struct tep_handle *tep,
 	const char *p;
 	int label_len = 0, event_len, system_len;
 
+	if (expr == table->to)
+		ftype = FIELD_TO;
+	else if (expr == table->from)
+		ftype = FIELD_FROM;
+
 	p = strchr(raw, '.');
 	if (p) {
 		char *str;
@@ -673,6 +686,7 @@ static int update_vars(struct tep_handle *tep,
 		field->event_name = event_name;
 		field->event = event;
 		field->field = raw + len + 1;
+		field->ftype = ftype;
 
 		if (!strcmp(field->field, "TIMESTAMP"))
 			field->field = store_str(sb, TRACEFS_TIMESTAMP);
@@ -1452,7 +1466,8 @@ static struct tracefs_synth *build_synth(struct tep_handle *tep,
 		if (expr->type == EXPR_FIELD) {
 			ret = -1;
 			field = &expr->field;
-			if (field->system == start_system &&
+			if (field->ftype != FIELD_TO &&
+			    field->system == start_system &&
 			    field->event_name == start_event) {
 				int type;
 				type = verify_field_type(tep, table->sb, expr);
-- 
2.35.1


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

* [PATCH 4/6] libtracefs: Use unique names for sql field variables
  2022-06-06 19:29 [PATCH 0/6] libtracefs: Fixes for sqlhist Steven Rostedt
                   ` (2 preceding siblings ...)
  2022-06-06 19:29 ` [PATCH 3/6] libtracefs: Differentiate FROM and JOIN events if they are the same event Steven Rostedt
@ 2022-06-06 19:29 ` Steven Rostedt
  2022-06-06 19:29 ` [PATCH 5/6] libtracefs: Differentiate WHERE clause when FROM and TO events are the same Steven Rostedt
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2022-06-06 19:29 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Harald Seiler, Steven Rostedt (Google)

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

When creating two different synthetic events using tracefs_sql(), if
they share the same label, the second one will fail because
tracefs_sql() will use that label as a variable name. In the kernel,
each variable must be unique, and the second label will cause a conflict
with the variable named after the first label.

Use a unique identifier when creating the variables based on the label,
but still keep the label text in the variable name to let users still
know what label the variable is for.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 include/tracefs-local.h |   4 ++
 src/sqlhist-parse.h     |   3 +-
 src/tracefs-hist.c      | 105 ++++++++++++++++++++++++++++++++++++----
 src/tracefs-sqlhist.c   |   2 +-
 4 files changed, 102 insertions(+), 12 deletions(-)

diff --git a/include/tracefs-local.h b/include/tracefs-local.h
index d0ed2abfe5e7..678b9870d03e 100644
--- a/include/tracefs-local.h
+++ b/include/tracefs-local.h
@@ -15,6 +15,8 @@
 #define BUILD_BUG_ON(cond)			\
 	do { if (!(1/!(cond))) { } } while (0)
 
+#define HASH_BITS 10
+
 struct tracefs_options_mask {
 	unsigned long long	mask;
 };
@@ -120,4 +122,6 @@ int trace_rescan_events(struct tep_handle *tep,
 struct tep_event *get_tep_event(struct tep_handle *tep,
 				const char *system, const char *name);
 
+unsigned int quick_hash(const char *str);
+
 #endif /* _TRACE_FS_LOCAL_H */
diff --git a/src/sqlhist-parse.h b/src/sqlhist-parse.h
index d5b86cacd8ce..7bd2a6350ece 100644
--- a/src/sqlhist-parse.h
+++ b/src/sqlhist-parse.h
@@ -4,8 +4,9 @@
 #include <stdarg.h>
 #include <tracefs.h>
 
+#include <tracefs-local.h>
+
 struct str_hash;
-#define HASH_BITS 10
 
 struct sql_table;
 
diff --git a/src/tracefs-hist.c b/src/tracefs-hist.c
index 0d113127def9..2f12cc471294 100644
--- a/src/tracefs-hist.c
+++ b/src/tracefs-hist.c
@@ -654,6 +654,12 @@ struct action {
 	char				*save;
 };
 
+struct name_hash {
+	struct name_hash	*next;
+	char			*name;
+	char			*hash;
+};
+
 /*
  * @name: name of the synthetic event
  * @start_system: system of the starting event
@@ -684,6 +690,7 @@ struct tracefs_synth {
 	struct action		*actions;
 	struct action		**next_action;
 	struct tracefs_dynevent	*dyn_event;
+	struct name_hash	*name_hash[1 << HASH_BITS];
 	char			*start_hist;
 	char			*end_hist;
 	char			*name;
@@ -724,6 +731,21 @@ static void action_free(struct action *action)
 	free(action);
 }
 
+static void free_name_hash(struct name_hash **hash)
+{
+	struct name_hash *item;
+	int i;
+
+	for (i = 0; i < 1 << HASH_BITS; i++) {
+		while ((item = hash[i])) {
+			hash[i] = item->next;
+			free(item->name);
+			free(item->hash);
+			free(item);
+		}
+	}
+}
+
 /**
  * tracefs_synth_free - free the resources alloced to a synth
  * @synth: The tracefs_synth descriptor
@@ -750,6 +772,7 @@ void tracefs_synth_free(struct tracefs_synth *synth)
 	tracefs_list_free(synth->end_keys);
 	tracefs_list_free(synth->start_vars);
 	tracefs_list_free(synth->end_vars);
+	free_name_hash(synth->name_hash);
 	free(synth->start_filter);
 	free(synth->end_filter);
 	free(synth->start_type);
@@ -1096,7 +1119,7 @@ static int add_synth_fields(struct tracefs_synth *synth,
 		return -1;
 	synth->synthetic_fields = list;
 
-	ret = asprintf(&str, "$%s", name);
+	ret = asprintf(&str, "$%s", var ? : name);
 	if (ret < 0) {
 		trace_list_pop(synth->synthetic_fields);
 		return -1;
@@ -1196,7 +1219,7 @@ static unsigned int make_rand(void)
 	return((unsigned)(seed/65536) % 32768);
 }
 
-static char *new_arg(struct tracefs_synth *synth)
+static char *new_name(struct tracefs_synth *synth, const char *name)
 {
 	int cnt = synth->arg_cnt + 1;
 	char *arg;
@@ -1205,9 +1228,9 @@ static char *new_arg(struct tracefs_synth *synth)
 	/* Create a unique argument name */
 	if (!synth->arg_name[0]) {
 		/* make_rand() returns at most 32768 (total 13 bytes in use) */
-		sprintf(synth->arg_name, "__arg_%u_", make_rand());
+		sprintf(synth->arg_name, "%u", make_rand());
 	}
-	ret = asprintf(&arg, "%s%d", synth->arg_name, cnt);
+	ret = asprintf(&arg, "__%s_%s_%d", name, synth->arg_name, cnt);
 	if (ret < 0)
 		return NULL;
 
@@ -1215,6 +1238,55 @@ static char *new_arg(struct tracefs_synth *synth)
 	return arg;
 }
 
+static struct name_hash *find_name(struct tracefs_synth *synth, const char *name)
+{
+	unsigned int key = quick_hash(name);
+	struct name_hash *hash = synth->name_hash[key];
+
+	for (; hash; hash = hash->next) {
+		if (!strcmp(hash->name, name))
+			return hash;
+	}
+	return NULL;
+}
+
+static const char *hash_name(struct tracefs_synth *synth, const char *name)
+{
+	struct name_hash *hash;
+	int key;
+
+	hash = find_name(synth, name);
+	if (hash)
+		return hash->hash;
+
+	hash = malloc(sizeof(*hash));
+	if (!hash)
+		return name;
+
+	hash->hash = new_name(synth, name);
+	if (!hash->hash) {
+		free(hash);
+		return name;
+	}
+
+	key = quick_hash(name);
+	hash->next = synth->name_hash[key];
+	synth->name_hash[key] = hash;
+
+	hash->name = strdup(name);
+	if (!hash->name) {
+		free(hash->hash);
+		free(hash);
+		return name;
+	}
+	return hash->hash;
+}
+
+static char *new_arg(struct tracefs_synth *synth)
+{
+	return new_name(synth, "arg");
+}
+
 /**
  * tracefs_synth_add_compare_field - add a comparison between start and end
  * @synth: The tracefs_synth descriptor
@@ -1245,6 +1317,7 @@ int tracefs_synth_add_compare_field(struct tracefs_synth *synth,
 				    const char *name)
 {
 	const struct tep_format_field *start_field;
+	const char *hname;
 	char *start_arg;
 	char *compare;
 	int ret;
@@ -1296,11 +1369,12 @@ int tracefs_synth_add_compare_field(struct tracefs_synth *synth,
 	if (ret < 0)
 		return -1;
 
-	ret = add_var(&synth->end_vars, name, compare, false);
+	hname = hash_name(synth, name);
+	ret = add_var(&synth->end_vars, hname, compare, false);
 	if (ret < 0)
 		goto out_free;
 
-	ret = add_synth_fields(synth, start_field, name, NULL);
+	ret = add_synth_fields(synth, start_field, name, hname);
 	if (ret < 0)
 		goto out_free;
 
@@ -1316,6 +1390,7 @@ __hidden int synth_add_start_field(struct tracefs_synth *synth,
 				   enum tracefs_hist_key_type type)
 {
 	const struct tep_format_field *field;
+	const char *var;
 	char *start_arg;
 	char **tmp;
 	int *types;
@@ -1330,6 +1405,8 @@ __hidden int synth_add_start_field(struct tracefs_synth *synth,
 	if (!name)
 		name = start_field;
 
+	var = hash_name(synth, name);
+
 	if (!trace_verify_event_field(synth->start_event, start_field, &field))
 		return -1;
 
@@ -1341,11 +1418,11 @@ __hidden int synth_add_start_field(struct tracefs_synth *synth,
 	if (ret)
 		goto out_free;
 
-	ret = add_var(&synth->end_vars, name, start_arg, true);
+	ret = add_var(&synth->end_vars, var, start_arg, true);
 	if (ret)
 		goto out_free;
 
-	ret = add_synth_fields(synth, field, name, NULL);
+	ret = add_synth_fields(synth, field, name, var);
 	if (ret)
 		goto out_free;
 
@@ -1417,6 +1494,7 @@ int tracefs_synth_add_end_field(struct tracefs_synth *synth,
 				const char *name)
 {
 	const struct tep_format_field *field;
+	const char *hname = NULL;
 	char *tmp_var = NULL;
 	int ret;
 
@@ -1425,17 +1503,24 @@ int tracefs_synth_add_end_field(struct tracefs_synth *synth,
 		return -1;
 	}
 
+	if (name) {
+		if (strncmp(name, "__arg", 5) != 0)
+			hname = hash_name(synth, name);
+		else
+			hname = name;
+	}
+
 	if (!name)
 		tmp_var = new_arg(synth);
 
 	if (!trace_verify_event_field(synth->end_event, end_field, &field))
 		return -1;
 
-	ret = add_var(&synth->end_vars, name ? : tmp_var, end_field, false);
+	ret = add_var(&synth->end_vars, name ? hname : tmp_var, end_field, false);
 	if (ret)
 		goto out;
 
-	ret = add_synth_fields(synth, field, name, tmp_var);
+	ret = add_synth_fields(synth, field, name, hname ? : tmp_var);
 	free(tmp_var);
  out:
 	return ret;
diff --git a/src/tracefs-sqlhist.c b/src/tracefs-sqlhist.c
index d854f7b54344..22e4b652e48b 100644
--- a/src/tracefs-sqlhist.c
+++ b/src/tracefs-sqlhist.c
@@ -174,7 +174,7 @@ static void parse_error(struct sqlhist_bison *sb, const char *text,
 	va_end(ap);
 }
 
-static inline unsigned int quick_hash(const char *str)
+__hidden unsigned int quick_hash(const char *str)
 {
 	unsigned int val = 0;
 	int len = strlen(str);
-- 
2.35.1


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

* [PATCH 5/6] libtracefs: Differentiate WHERE clause when FROM and TO events are the same
  2022-06-06 19:29 [PATCH 0/6] libtracefs: Fixes for sqlhist Steven Rostedt
                   ` (3 preceding siblings ...)
  2022-06-06 19:29 ` [PATCH 4/6] libtracefs: Use unique names for sql field variables Steven Rostedt
@ 2022-06-06 19:29 ` Steven Rostedt
  2022-06-06 19:29 ` [PATCH 6/6] libtracefs sqlhist: Report errors executing the commands Steven Rostedt
  2022-06-07 10:01 ` [PATCH 0/6] libtracefs: Fixes for sqlhist Harald Seiler
  6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2022-06-06 19:29 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Harald Seiler, Steven Rostedt (Google)

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

With "FROM sched_switch as start JOIN sched_switch AS end" and then
filtering with "WHERE start.prev_state = 2 && end.next_prio < 100" the
parser gets confused and uses the "start.next_prio" instead of the
"end.next_prio", making the filter only on the start event and not on the
end event as expected.

Fix it by testing the field type (FIELD_TO/FIELD_FROM) as well as the
system and event of the field when selecting which event the field is a
part of.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 src/tracefs-sqlhist.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/src/tracefs-sqlhist.c b/src/tracefs-sqlhist.c
index 22e4b652e48b..5205356356a5 100644
--- a/src/tracefs-sqlhist.c
+++ b/src/tracefs-sqlhist.c
@@ -904,20 +904,21 @@ static int verify_filter_error(struct sqlhist_bison *sb, struct expr *expr,
 }
 
 static int do_verify_filter(struct sqlhist_bison *sb, struct filter *filter,
-			    const char **system, const char **event)
+			    const char **system, const char **event,
+			    enum field_type *ftype)
 {
 	int ret;
 
 	if (filter->type == FILTER_OR ||
 	    filter->type == FILTER_AND) {
-		ret = do_verify_filter(sb, &filter->lval->filter, system, event);
+		ret = do_verify_filter(sb, &filter->lval->filter, system, event, ftype);
 		if (ret)
 			return ret;
-		return do_verify_filter(sb, &filter->rval->filter, system, event);
+		return do_verify_filter(sb, &filter->rval->filter, system, event, ftype);
 	}
 	if (filter->type == FILTER_GROUP ||
 	    filter->type == FILTER_NOT_GROUP) {
-		return do_verify_filter(sb, &filter->lval->filter, system, event);
+		return do_verify_filter(sb, &filter->lval->filter, system, event, ftype);
 	}
 
 	/*
@@ -927,6 +928,7 @@ static int do_verify_filter(struct sqlhist_bison *sb, struct filter *filter,
 	if (!*system && !*event) {
 		*system = filter->lval->field.system;
 		*event = filter->lval->field.event_name;
+		*ftype = filter->lval->field.ftype;
 		return 0;
 	}
 
@@ -938,7 +940,8 @@ static int do_verify_filter(struct sqlhist_bison *sb, struct filter *filter,
 }
 
 static int verify_filter(struct sqlhist_bison *sb, struct filter *filter,
-			 const char **system, const char **event)
+			 const char **system, const char **event,
+			 enum field_type *ftype)
 {
 	int ret;
 
@@ -949,17 +952,17 @@ static int verify_filter(struct sqlhist_bison *sb, struct filter *filter,
 	case FILTER_NOT_GROUP:
 		break;
 	default:
-		return do_verify_filter(sb, filter, system, event);
+		return do_verify_filter(sb, filter, system, event, ftype);
 	}
 
-	ret = do_verify_filter(sb, &filter->lval->filter, system, event);
+	ret = do_verify_filter(sb, &filter->lval->filter, system, event, ftype);
 	if (ret)
 		return ret;
 
 	switch (filter->type) {
 	case FILTER_OR:
 	case FILTER_AND:
-		return do_verify_filter(sb, &filter->rval->filter, system, event);
+		return do_verify_filter(sb, &filter->rval->filter, system, event, ftype);
 	default:
 		return 0;
 	}
@@ -1516,16 +1519,18 @@ static struct tracefs_synth *build_synth(struct tep_handle *tep,
 	for (expr = table->where; expr; expr = expr->next) {
 		const char *filter_system = NULL;
 		const char *filter_event = NULL;
+		enum field_type ftype = FIELD_NONE;
 		bool *started;
 		bool start;
 
 		ret = verify_filter(table->sb, &expr->filter, &filter_system,
-				    &filter_event);
+				    &filter_event, &ftype);
 		if (ret < 0)
 			goto free;
 
 		start = filter_system == start_system &&
-			filter_event == start_event;
+			filter_event == start_event &&
+			ftype != FIELD_TO;
 
 		if (start)
 			started = &started_start;
-- 
2.35.1


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

* [PATCH 6/6] libtracefs sqlhist: Report errors executing the commands
  2022-06-06 19:29 [PATCH 0/6] libtracefs: Fixes for sqlhist Steven Rostedt
                   ` (4 preceding siblings ...)
  2022-06-06 19:29 ` [PATCH 5/6] libtracefs: Differentiate WHERE clause when FROM and TO events are the same Steven Rostedt
@ 2022-06-06 19:29 ` Steven Rostedt
  2022-06-07 10:01 ` [PATCH 0/6] libtracefs: Fixes for sqlhist Harald Seiler
  6 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2022-06-06 19:29 UTC (permalink / raw)
  To: linux-trace-devel; +Cc: Harald Seiler, Steven Rostedt (Google)

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

If the -e option is set and the parsing succeeds but the executing does
not, sqlhist exits with success. It should instead check the return of the
creation of the histogram or synthetic event and return the error message
on failure and not succeed.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 Documentation/libtracefs-sql.txt | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/Documentation/libtracefs-sql.txt b/Documentation/libtracefs-sql.txt
index 5e48baa4da39..6d606dbcaf88 100644
--- a/Documentation/libtracefs-sql.txt
+++ b/Documentation/libtracefs-sql.txt
@@ -407,8 +407,13 @@ static int do_sql(const char *instance_name,
 			}
 		}
 		tracefs_synth_echo_cmd(&seq, synth);
-		if (execute)
-			tracefs_synth_create(synth);
+		if (execute) {
+			ret = tracefs_synth_create(synth);
+			if (ret < 0) {
+				fprintf(stderr, "%s\n", tracefs_error_last(NULL));
+				exit(-1);
+			}
+		}
 	} else {
 		struct tracefs_instance *instance = NULL;
 		struct tracefs_hist *hist;
@@ -430,8 +435,13 @@ static int do_sql(const char *instance_name,
 			}
 		}
 		tracefs_hist_echo_cmd(&seq, instance, hist, 0);
-		if (execute)
-			tracefs_hist_start(instance, hist);
+		if (execute) {
+			ret = tracefs_hist_start(instance, hist);
+			if (ret < 0) {
+				fprintf(stderr, "%s\n", tracefs_error_last(instance));
+				exit(-1);
+			}
+		}
 	}
 
 	tracefs_synth_free(synth);
-- 
2.35.1


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

* Re: [PATCH 0/6] libtracefs: Fixes for sqlhist
  2022-06-06 19:29 [PATCH 0/6] libtracefs: Fixes for sqlhist Steven Rostedt
                   ` (5 preceding siblings ...)
  2022-06-06 19:29 ` [PATCH 6/6] libtracefs sqlhist: Report errors executing the commands Steven Rostedt
@ 2022-06-07 10:01 ` Harald Seiler
  6 siblings, 0 replies; 8+ messages in thread
From: Harald Seiler @ 2022-06-07 10:01 UTC (permalink / raw)
  To: Steven Rostedt, linux-trace-devel

Hi,

On Mon, 2022-06-06 at 15:29 -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> 
> At Kernel Recipes, I demonstrated sqlhist and how to use it. Along the way,
> I discovered several bugs. This series addresses those bugs that were found.

For the whole series:

Tested-by: Harald Seiler <hws@denx.de>

As a sanity check I accumulated offcpu time for a process:

	sqlhist -e -n 'offcpu' 'SELECT start.prev_comm AS comm, (end.TIMESTAMP_USECS - start.TIMESTAMP_USECS) as lat FROM sched_switch as start JOIN sched_switch as end ON start.prev_pid == end.next_pid WHERE start.prev_comm == "ping"'
	echo "hist:keys=comm:vals=lat:sort=lat" >/sys/kernel/tracing/events/synthetic/offcpu/trigger

And then verified the collected time is consistent by comparing with
getrusage(2) values for the process (as returned by time(1)).

And indeed,

	wall_time == ru_utime + ru_stime + lat

So it is measuring exactly what I want.

Thanks again for getting this working!

> Steven Rostedt (Google) (6):
>   libtracefs: Fix make sqlhist when built again
>   libtracefs: Add libtracefs.a to dependency of sqlhist
>   libtracefs: Differentiate FROM and JOIN events if they are the same
>     event
>   libtracefs: Use unique names for sql field variables
>   libtracefs: Differentiate WHERE clause when FROM and TO events are the
>     same
>   libtracefs sqlhist: Report errors executing the commands
> 
>  Documentation/libtracefs-sql.txt |  18 ++++--
>  include/tracefs-local.h          |   4 ++
>  samples/Makefile                 |   4 +-
>  src/sqlhist-parse.h              |   3 +-
>  src/tracefs-hist.c               | 105 ++++++++++++++++++++++++++++---
>  src/tracefs-sqlhist.c            |  44 +++++++++----
>  6 files changed, 150 insertions(+), 28 deletions(-)
> 

-- 
Harald

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

end of thread, other threads:[~2022-06-07 10:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-06 19:29 [PATCH 0/6] libtracefs: Fixes for sqlhist Steven Rostedt
2022-06-06 19:29 ` [PATCH 1/6] libtracefs: Fix make sqlhist when built again Steven Rostedt
2022-06-06 19:29 ` [PATCH 2/6] libtracefs: Add libtracefs.a to dependency of sqlhist Steven Rostedt
2022-06-06 19:29 ` [PATCH 3/6] libtracefs: Differentiate FROM and JOIN events if they are the same event Steven Rostedt
2022-06-06 19:29 ` [PATCH 4/6] libtracefs: Use unique names for sql field variables Steven Rostedt
2022-06-06 19:29 ` [PATCH 5/6] libtracefs: Differentiate WHERE clause when FROM and TO events are the same Steven Rostedt
2022-06-06 19:29 ` [PATCH 6/6] libtracefs sqlhist: Report errors executing the commands Steven Rostedt
2022-06-07 10:01 ` [PATCH 0/6] libtracefs: Fixes for sqlhist Harald Seiler

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).