[v2,8/9] kernel-shark-qt: Add Advanced filter to the session context.

Commit Message

Yordan Karadzhov June 28, 2018, 4:30 p.m. UTC

This patch adds to the KernelShark session's context instrumentation
for sophisticated filtering based on the content of the trace event.

Signed-off-by: Yordan Karadzhov (VMware) <[e-mail]>
---
 kernel-shark-qt/src/libkshark.c | 36 ++++++++++++++++++++++++++++++---
 kernel-shark-qt/src/libkshark.h | 11 +++++++++-
 2 files changed, 43 insertions(+), 4 deletions(-)

Comments

Steven Rostedt June 28, 2018, 11:37 p.m. UTC | #1

On Thu, 28 Jun 2018 19:30:11 +0300
"Yordan Karadzhov (VMware)" <[e-mail]> wrote:

> @@ -306,9 +315,23 @@ void kshark_filter_entries(struct kshark_context *kshark_ctx,
>  			   size_t n_entries)
>  {
>  	int i;

Also add a space here.

> +	if (kshark_ctx->advanced_event_filter->filters) {

Can this be called with kshark_ctx->advanced_event_filter == NULL?

> +		/*
> +		 * The advanced filter is set but applying it requires
> +		 * access to prevent_record, hence the data has to be
> +		 * reloaded.
> +		 */

I'm a little confused by the comment above. So if this is called with
advanced filters, then it is an error?

> +		fprintf(stderr,
> +			"Failed to filter!\n");
> +		fprintf(stderr,
> +			"Reset the Advanced filter or reload the data.\n");
> +		return;
> +	}
> +
>  	if (!kshark_filter_is_set(kshark_ctx))
>  		return;
>  
> +	/* No need to reload the data. Apply the Id filters. */

Why is this comment added here? Would it make sense to add it in the
original patch? Does advanced filters make this different?

-- Steve

>  	for (i = 0; i < n_entries; ++i) {
>  		/* Start with and entry which is visible everywhere. */
>  		data[i]->visible = 0xFF;

Patch

diff --git a/kernel-shark-qt/src/libkshark.c b/kernel-shark-qt/src/libkshark.c
index 22f82a9..7d70142 100644
--- a/kernel-shark-qt/src/libkshark.c
+++ b/kernel-shark-qt/src/libkshark.c
@@ -118,6 +118,9 @@  bool kshark_open(struct kshark_context *kshark_ctx, const char *file)
 	kshark_ctx->handle = handle;
 	kshark_ctx->pevent = tracecmd_get_pevent(handle);
 
+	kshark_ctx->advanced_event_filter =
+		pevent_filter_alloc(kshark_ctx->pevent);
+
 	/*
 	 * Turn off function trace indent and turn on show parent
 	 * if possible.
@@ -134,7 +137,7 @@  void kshark_close(struct kshark_context *kshark_ctx)
 		return;
 
 	/*
-	 * All Id filters are file specific. Make sure that the Pids and Event Ids
+	 * All filters are file specific. Make sure that the Pids and Event Ids
 	 * from this file are not going to be used with another file.
 	 */
 	tracecmd_filter_id_clear(kshark_ctx->show_task_filter);
@@ -142,6 +145,12 @@  void kshark_close(struct kshark_context *kshark_ctx)
 	tracecmd_filter_id_clear(kshark_ctx->show_event_filter);
 	tracecmd_filter_id_clear(kshark_ctx->hide_event_filter);
 
+	if (kshark_ctx->advanced_event_filter) {
+		pevent_filter_reset(kshark_ctx->advanced_event_filter);
+		pevent_filter_free(kshark_ctx->advanced_event_filter);
+		kshark_ctx->advanced_event_filter = NULL;
+	}
+
 	tracecmd_close(kshark_ctx->handle);
 	kshark_ctx->handle = NULL;
 	kshark_ctx->pevent = NULL;
@@ -306,9 +315,23 @@  void kshark_filter_entries(struct kshark_context *kshark_ctx,
 			   size_t n_entries)
 {
 	int i;
+	if (kshark_ctx->advanced_event_filter->filters) {
+		/*
+		 * The advanced filter is set but applying it requires
+		 * access to prevent_record, hence the data has to be
+		 * reloaded.
+		 */
+		fprintf(stderr,
+			"Failed to filter!\n");
+		fprintf(stderr,
+			"Reset the Advanced filter or reload the data.\n");
+		return;
+	}
+
 	if (!kshark_filter_is_set(kshark_ctx))
 		return;
 
+	/* No need to reload the data. Apply the Id filters. */
 	for (i = 0; i < n_entries; ++i) {
 		/* Start with and entry which is visible everywhere. */
 		data[i]->visible = 0xFF;
@@ -353,7 +376,7 @@  size_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
 				struct kshark_entry ***data_rows)
 {
 	int n_cpus = tracecmd_cpus(kshark_ctx->handle);
-	int cpu, next_cpu;
+	int cpu, next_cpu, ret;
 	size_t count, total = 0;
 	uint64_t ts;
 	
@@ -361,6 +384,7 @@  size_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
 	struct kshark_entry *entry, **next;
 	struct kshark_entry **cpu_list, **rows;
 	struct kshark_task_list *task;
+	struct event_filter *adv_filter = kshark_ctx->advanced_event_filter;
 
 	if (*data_rows)
 		free(*data_rows);
@@ -384,8 +408,14 @@  size_t kshark_load_data_entries(struct kshark_context *kshark_ctx,
 				goto fail;
 
 			/* Apply event filtering. */
-			if (!kshark_show_event(kshark_ctx, entry->event_id))
+			ret = FILTER_NONE;
+			if (adv_filter->filters)
+				ret = pevent_filter_match(adv_filter, rec);
+
+			if (!kshark_show_event(kshark_ctx, entry->event_id) ||
+			    ret != FILTER_MATCH) {
 				unset_event_filter_flag(kshark_ctx, entry);
+			}
 
 			/* Apply task filtering. */
 			if (!kshark_show_task(kshark_ctx, entry->pid)) {
diff --git a/kernel-shark-qt/src/libkshark.h b/kernel-shark-qt/src/libkshark.h
index 2b79bc8..2ec8450 100644
--- a/kernel-shark-qt/src/libkshark.h
+++ b/kernel-shark-qt/src/libkshark.h
@@ -103,6 +103,12 @@  struct kshark_context {
 	 * have this bit unset in their "visible" fields.
 	 */
 	uint8_t				filter_mask;
+
+	/**
+	 * Filter allowing sophisticated filtering based on the content of
+	 * the event.
+	 */
+	struct event_filter		*advanced_event_filter;
 };
 
 /**
@@ -245,9 +251,12 @@  void kshark_filter_clear(struct kshark_context *kshark_ctx, int filter_id);
 /**
  * @brief This function loops over the array of entries specified by "data"
  * and "n_entries" and sets the "visible" fields of each entry according to
- * the criteria provided by the filters of the session's context. The field
+ * the criteria provided by the Id filters of the session's context. The field
  * "filter_mask" of the session's context is used to control the level of
  * visibility/invisibility of the entries which are filtered-out.
+ * WARNING: Do not use this function if the advanced filter is set. Applying
+ * the advanced filter requires access to prevent_record, hence the data has
+ * to be reloaded using kshark_load_data_entries().
  * @param kshark_ctx: kshark_ctx: Input location for the session context
  * pointer.
  * @param data: Input location for the trace data to be filtered.