Skip to contentSkip to conversation

tracing: Add event filter logic for !(...)

PATCH In review Steven Rostedt 3 participants 17 messages Dec 3, 2014 – Dec 4, 2014

Subsystems (2)
Kselftest Tracing
lore
Share & tools
lore

Steven Rostedt

author maintainer
#0 lore raw

cover tracing: Add event filter logic for !(...)

There's no changes to the first patch that you can see from here:

http://lkml.kernel.org/r/[email protected]

But I added two more patches:

1) adds the logic to do ! with the AND and OR cases. Like:

  !(prev_pid == 0 && next_pid == 1)

2) I added ftracetests selftest to test a bunch of filter cases. I can probably add more, but this is a start.

Steven Rostedt (Red Hat) (3):
tracing: Add NOT to filtering logic
tracing: Allow NOT to filter AND and OR clauses
ftracetests: Add test to test event filter logic

----

 kernel/trace/trace_events_filter.c                 |  29 ++-
 .../selftests/ftrace/test.d/ftrace/filter.tc       | 200 +++++++++++++++++++++
 2 files changed, 223 insertions(+), 6 deletions(-)
author maintainer
#1 lore raw

PATCH 1/3 tracing: Add NOT to filtering logic

From: "Steven Rostedt (Red Hat)" <[email protected]>

Ted noticed that he could not filter on an event for a bit being cleared. That's because the filtering logic only tests event fields with a limited number of comparisons which, for bit logic, only include "&", which can test if a bit is set, but there's no good way to see if a bit is clear.

This adds a way to do: !(field & 2048)

Which returns true if the bit is not set, and false otherwise.

Note, currently !(field1 == 10 && field2 == 15) is not supported. That is, the 'not' only works for direct comparisons, not for the AND and OR logic.

Link http://lkml.kernel.org/r/[email protected] open here
Link http://lkml.kernel.org/r/[email protected] open here
Suggested-by "Theodore Ts'o" <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
---
 kernel/trace/trace_events_filter.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)
kernel/trace/trace_events_filter.c +18 -2
index 7a8c1528e141..e6a33db83856 100644
@@ -45,6 +45,7 @@ enum filter_op_ids
 	OP_GT,
 	OP_GE,
 	OP_BAND,
+	OP_NOT,
 	OP_NONE,
 	OP_OPEN_PAREN,
 };

@@ -67,6 +68,7 @@ static struct filter_op filter_ops[] = {
 	{ OP_GT,	">",		5 },
 	{ OP_GE,	">=",		5 },
 	{ OP_BAND,	"&",		6 },
+	{ OP_NOT,	"!",		6 },
 	{ OP_NONE,	"OP_NONE",	0 },
 	{ OP_OPEN_PAREN, "(",		0 },
 };

@@ -85,6 +87,7 @@ enum {
 	FILT_ERR_MISSING_FIELD,
 	FILT_ERR_INVALID_FILTER,
 	FILT_ERR_IP_FIELD_ONLY,
+	FILT_ERR_ILLEGAL_NOT_OP,
 };
 
 static char *err_text[] = {

@@ -101,6 +104,7 @@ static char *err_text[] = {
 	"Missing field name and/or value",
 	"Meaningless filter expression",
 	"Only 'ip' field is supported for function trace",
+	"Illegal use of '!'",
 };
 
 struct opstack_op {

@@ -139,6 +143,7 @@ struct pred_stack {
 	int			index;
 };
 
+/* If not of not match is equal to not of not, then it is a match */
 #define DEFINE_COMPARISON_PRED(type)					\
 static int filter_pred_##type(struct filter_pred *pred, void *event)	\
 {									\

@@ -166,7 +171,7 @@ static int filter_pred_##type(struct filter_pred *pred, void *event)	\
 		break;							\
 	}								\
 									\
-	return match;							\
+	return !!match == !pred->not;					\
 }
 
 #define DEFINE_EQUALITY_PRED(size)					\

@@ -1028,7 +1033,7 @@ static int init_pred(struct filter_parse_state *ps,
 	}
 
 	if (pred->op == OP_NE)
-		pred->not = 1;
+		pred->not ^= 1;
 
 	pred->fn = fn;
 	return 0;

@@ -1590,6 +1595,17 @@ static int replace_preds(struct ftrace_event_call *call,
 			continue;
 		}
 
+		if (elt->op == OP_NOT) {
+			if (!n_preds || operand1 || operand2) {
+				parse_error(ps, FILT_ERR_ILLEGAL_NOT_OP, 0);
+				err = -EINVAL;
+				goto fail;
+			}
+			if (!dry_run)
+				filter->preds[n_preds - 1].not ^= 1;
+			continue;
+		}
+
 		if (WARN_ON(n_preds++ == MAX_FILTER_PRED)) {
 			parse_error(ps, FILT_ERR_TOO_MANY_PREDS, 0);
 			err = -ENOSPC;
Signature · 1 line
2.1.3
author maintainer
#2 lore raw

PATCH 2/3 tracing: Allow NOT to filter AND and OR clauses

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

Add support to allow not "!" for and (&&) and (||). That is:

!(field1 == X && field2 == Y)

Where the value of the full clause will be notted.

Signed-off-by Steven Rostedt <[email protected]>
---
 kernel/trace/trace_events_filter.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
kernel/trace/trace_events_filter.c +5 -4
index e6a33db83856..ced69da0ff55 100644
@@ -489,9 +489,10 @@ static int process_ops(struct filter_pred *preds,
 		if (!WARN_ON_ONCE(!pred->fn))
 			match = pred->fn(pred, rec);
 		if (!!match == type)
-			return match;
+			break;
 	}
-	return match;
+	/* If not of not match is equal to not of not, then it is a match */
+	return !!match == !op->not;
 }
 
 struct filter_match_preds_data {

@@ -740,10 +741,10 @@ static int filter_set_pred(struct event_filter *filter,
 		 * then this op can be folded.
 		 */
 		if (left->index & FILTER_PRED_FOLD &&
-		    (left->op == dest->op ||
+		    ((left->op == dest->op && !left->not) ||
 		     left->left == FILTER_PRED_INVALID) &&
 		    right->index & FILTER_PRED_FOLD &&
-		    (right->op == dest->op ||
+		    ((right->op == dest->op && !right->not) ||
 		     right->left == FILTER_PRED_INVALID))
 			dest->index |= FILTER_PRED_FOLD;
Signature · 1 line
2.1.3
author maintainer
#3 lore raw

PATCH 3/3 ftracetests: Add test to test event filter logic

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

Add a test to test the event filter logic. It currently tests the following filters against sched:sched_switch event.

   ( prev_pid != 0 )
   ( prev_pid == 0 )
   ( prev_pid < 100 )
   ( prev_pid <= $$ )
   ( prev_pid > 100 )
   ( prev_pid >= $$ )
  ! ( prev_pid != 0 )
  ! ( prev_pid == 0 )
  ! ( prev_pid < 100 )
  ! ( prev_pid <= $$ )
  ! ( prev_pid > 100 )
  ! ( prev_pid >= $$ )
   ( prev_pid != 0 && next_pid > 10 )
   ( prev_pid != 0 || next_pid > 10 )
  ! ( prev_pid != 0 && next_pid > 10 )
  ! ( prev_pid != 0 || next_pid > 10 )
   ( prev_pid & 1 )
   ( prev_pid & 2 )
   ( prev_pid & 4 )
   ( prev_pid & 8 )
   ( prev_pid & 16 )
  ! ( prev_pid & 1 )
  ! ( prev_pid & 2 )
  ! ( prev_pid & 4 )
  ! ( prev_pid & 8 )
  ! ( prev_pid & 16 )
   ( next_comm ~ "ftrace-test-fil" )
   ( next_comm != "ftrace-test-fil" )
  ! ( next_comm ~ "ftrace-test-fil" )
  ! ( next_comm != "ftrace-test-fil" )
Signed-off-by Steven Rostedt <[email protected]>
---
 .../selftests/ftrace/test.d/ftrace/filter.tc       | 200 +++++++++++++++++++++
 1 file changed, 200 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/filter.tc
tools/testing/selftests/ftrace/test.d/ftrace/filter.tc new file +200 -0
new file mode 100644
index 000000000000..de4c5ce57099
@@ -0,0 +1,200 @@
+#!/bin/sh
+# description: ftrace - test filter logic
+
+EVENT=events/sched/sched_switch
+FILTER=$EVENT/filter
+ENABLE=$EVENT/enable
+
+FIELD1=prev_pid
+FIELD2=next_pid
+FIELD3=next_comm
+
+ITER=100
+
+enable_event() {
+    echo 1 > $ENABLE
+}
+
+disable_event() {
+    echo 0 > $ENABLE
+}
+
+clear_filter() {
+    echo 0 > $FILTER
+}
+
+do_reset() {
+    enable_tracing
+    disable_event
+    clear_filter
+    clear_trace
+}
+
+fail() { # msg
+    do_reset
+    echo $1
+    exit -1
+}
+
+run_code() {
+    # make lots of pids to filter on
+    for i in `seq $ITER`; do
+	ls /usr > /dev/null
+    done
+}
+
+run_event() {
+    enable_event
+    run_code
+    disable_event
+}
+
+disable_tracing
+clear_trace
+enable_tracing
+
+test_cmp() {
+    cmp=$1
+    rval=$2
+    filter=$3
+    not=$4
+
+    echo "$not ( $FIELD1 $filter )"
+    echo "$not ( $FIELD1 $filter )" > $FILTER
+
+    run_event
+
+    cat trace | sed -ne "s/.*$FIELD1=\([^ ]*\).*/\1/p" |
+    while read pid; do
+	if [ "$pid" $cmp $rval ]; then
+	    if [ "$not" == '!' ]; then
+		fail "$pid matched filter: $filter; but should not not have for $not"
+	    fi
+	elif [ "$not" != '!' ]; then 
+	    fail "$pid does not match filter: $filter"
+	fi
+    done
+    clear_trace
+}
+
+test_cmp2() {
+    cmp1=$1
+    rval1=$2
+    conj=$3
+    cmp2=$4
+    rval2=$5
+    filter1=$6
+    filter2=$7
+    fconj=$8
+    not=$9
+
+    echo "$not ( $FIELD1 $filter1 $fconj $FIELD2 $filter2 )"
+    echo "$not ( $FIELD1 $filter1 $fconj $FIELD2 $filter2 )" > $FILTER
+
+    run_event
+
+    cat trace | sed -ne "s/.*$FIELD1=\([^ ]*\).*$FIELD2=\([^ ]*\).*/\1 \2/p" |
+    while read pid1 pid2; do
+	if [ "$pid1" $cmp1 $rval1 $conj "$pid2" $cmp2 $rval2 ]; then
+	    if [ "$not" == '!' ]; then
+		fail "$pid1 and $pid2 match not filter: $filter1 $fconj $filter2"
+	    fi
+	elif [ "$not" != '!' ]; then
+	    fail "$pid1 and $pid2 does not match filter: $filter1 $fconj $filter2"
+	fi
+    done
+    clear_trace
+}
+
+# first simple compares
+test_cmp -ne 0 "!= 0" ""
+test_cmp -eq 0 "== 0" ""
+test_cmp -lt 100 "< 100" ""
+test_cmp -le $$ "<= $$" ""
+test_cmp -gt 100 "> 100" ""
+test_cmp -ge $$ ">= $$" ""
+
+# Now test the not case
+test_cmp -ne 0 "!= 0" '!'
+test_cmp -eq 0 "== 0" '!'
+test_cmp -lt 100 "< 100" '!'
+test_cmp -le $$ "<= $$" '!'
+test_cmp -gt 100 "> 100" '!'
+test_cmp -ge $$ ">= $$" '!'
+
+# Test more complex compares (&& and !!)
+test_cmp2 -ne 0 -a -gt 10 "!= 0" "> 10" "&&" ''
+test_cmp2 -ne 0 -o -gt 10 "!= 0" "> 10" "||" ''
+
+test_cmp2 -ne 0 -a -gt 10 "!= 0" "> 10" "&&" '!'
+test_cmp2 -ne 0 -o -gt 10 "!= 0" "> 10" "||" '!'
+
+# test bitmask
+
+bit_test() {
+    not=$1
+
+    let x=1
+    for i in `seq 5`; do
+	echo "$not ( $FIELD1 & $x )"
+	echo "$not ( $FIELD1 & $x )" > $FILTER
+	run_event
+
+	cat trace | sed -ne "s/.*$FIELD1=\([^ ]*\).*/\1/p" |
+	while read pid; do
+
+	    # do not exit if val becomes zero
+	    set +e
+	    let val="$pid & $x"
+	    set -e
+
+	    if [ $val -eq 0 ]; then
+		if [ "$not" != '!' ]; then
+		    fail "$pid did not match bitmask $x"
+		fi
+	    elif [ "$not" == '!' ]; then
+		fail "$pid did not match not bitmask $x"
+	    fi
+	done
+	clear_trace
+	let x="$x << 1"
+    done
+}		
+
+bit_test ''
+bit_test '!'
+
+test_string() {
+    cmp=$1
+    filter=$2
+    not=$3
+
+    comm=`cat /proc/$$/comm`
+
+    echo "$not ( $FIELD3 $filter \"$comm\" )"
+    echo "$not ( $FIELD3 $filter \"$comm\" )" > $FILTER
+
+    run_event
+
+    cat trace | sed -ne "s/.*$FIELD3=\([^ ]*\).*/\1/p" |
+    while read com; do
+	if [ "$com" $cmp "$comm" ]; then
+	    if [ "$not" == '!' ]; then
+		fail "$com matched filter: $filter; but should not not have for $not"
+	    fi
+	elif [ "$not" != '!' ]; then 
+	    fail "$com does not match filter: $filter"
+	fi
+    done
+    clear_trace
+}
+
+test_string "==" "~" ''
+test_string "!=" "!=" ''
+
+test_string "==" "~" '!'
+test_string "!=" "!=" '!'
+
+do_reset
+
+exit 0
Signature · 1 line
2.1.3
#4 lore raw

Re: [PATCH 3/3] ftracetests: Add test to test event filter logic

Quoted 27 lines · Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

Add a test to test the event filter logic. It currently tests the following filters against sched:sched_switch event.

   ( prev_pid != 0 )
   ( prev_pid == 0 )
   ( prev_pid < 100 )
   ( prev_pid <= $$ )
   ( prev_pid > 100 )
   ( prev_pid >= $$ )
  ! ( prev_pid != 0 )
  ! ( prev_pid == 0 )
  ! ( prev_pid < 100 )
  ! ( prev_pid <= $$ )
  ! ( prev_pid > 100 )
  ! ( prev_pid >= $$ )
   ( prev_pid != 0 && next_pid > 10 )
   ( prev_pid != 0 || next_pid > 10 )
  ! ( prev_pid != 0 && next_pid > 10 )
  ! ( prev_pid != 0 || next_pid > 10 )
   ( prev_pid & 1 )
   ( prev_pid & 2 )
   ( prev_pid & 4 )
   ( prev_pid & 8 )
   ( prev_pid & 16 )
  ! ( prev_pid & 1 )
Quoted 8 lines
  ! ( prev_pid & 2 )
  ! ( prev_pid & 4 )
  ! ( prev_pid & 8 )
  ! ( prev_pid & 16 )
   ( next_comm ~ "ftrace-test-fil" )
   ( next_comm != "ftrace-test-fil" )
  ! ( next_comm ~ "ftrace-test-fil" )
  ! ( next_comm != "ftrace-test-fil" )

thanks for the tests!
Since you're adding full support for 'not',
I think would be good to have few more tests where ! is not a top node. Like:
(prev_pid != 0 && !(next_pid & 2))
and another one with ! at multiple levels, like:
(prev_pid != 0 && !(next_pid != 2 && !(prev_pid > 3))) ... or reject them during parsing.

author maintainer
#5 lore raw

Re: [PATCH 3/3] ftracetests: Add test to test event filter logic

Quoted 37 lines · On Tue, 2 Dec 2014 22:22:16 -0800 Alexei Starovoitov <[email protected]> wrote:
Quoted 27 lines · Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

Add a test to test the event filter logic. It currently tests the following filters against sched:sched_switch event.

   ( prev_pid != 0 )
   ( prev_pid == 0 )
   ( prev_pid < 100 )
   ( prev_pid <= $$ )
   ( prev_pid > 100 )
   ( prev_pid >= $$ )
  ! ( prev_pid != 0 )
  ! ( prev_pid == 0 )
  ! ( prev_pid < 100 )
  ! ( prev_pid <= $$ )
  ! ( prev_pid > 100 )
  ! ( prev_pid >= $$ )
   ( prev_pid != 0 && next_pid > 10 )
   ( prev_pid != 0 || next_pid > 10 )
  ! ( prev_pid != 0 && next_pid > 10 )
  ! ( prev_pid != 0 || next_pid > 10 )
   ( prev_pid & 1 )
   ( prev_pid & 2 )
   ( prev_pid & 4 )
   ( prev_pid & 8 )
   ( prev_pid & 16 )
  ! ( prev_pid & 1 )
Quoted 8 lines
  ! ( prev_pid & 2 )
  ! ( prev_pid & 4 )
  ! ( prev_pid & 8 )
  ! ( prev_pid & 16 )
   ( next_comm ~ "ftrace-test-fil" )
   ( next_comm != "ftrace-test-fil" )
  ! ( next_comm ~ "ftrace-test-fil" )
  ! ( next_comm != "ftrace-test-fil" )
Quoted 8 lines

thanks for the tests!
Since you're adding full support for 'not',
I think would be good to have few more tests where ! is not a top node. Like:
(prev_pid != 0 && !(next_pid & 2))
and another one with ! at multiple levels, like:
(prev_pid != 0 && !(next_pid != 2 && !(prev_pid > 3))) ... or reject them during parsing.

Sure, would you like to add them :-)

I spent more time on this than I should have. I have other things to work on and this little project has already put me behind on my other tasks.

-- Steve

#6 lore raw

Re: [PATCH 3/3] ftracetests: Add test to test event filter logic

Quoted 49 lines · Steven Rostedt
Quoted 37 lines · On Tue, 2 Dec 2014 22:22:16 -0800 Alexei Starovoitov <[email protected]> wrote:
Quoted 27 lines · Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

Add a test to test the event filter logic. It currently tests the following filters against sched:sched_switch event.

   ( prev_pid != 0 )
   ( prev_pid == 0 )
   ( prev_pid < 100 )
   ( prev_pid <= $$ )
   ( prev_pid > 100 )
   ( prev_pid >= $$ )
  ! ( prev_pid != 0 )
  ! ( prev_pid == 0 )
  ! ( prev_pid < 100 )
  ! ( prev_pid <= $$ )
  ! ( prev_pid > 100 )
  ! ( prev_pid >= $$ )
   ( prev_pid != 0 && next_pid > 10 )
   ( prev_pid != 0 || next_pid > 10 )
  ! ( prev_pid != 0 && next_pid > 10 )
  ! ( prev_pid != 0 || next_pid > 10 )
   ( prev_pid & 1 )
   ( prev_pid & 2 )
   ( prev_pid & 4 )
   ( prev_pid & 8 )
   ( prev_pid & 16 )
  ! ( prev_pid & 1 )
Quoted 8 lines
  ! ( prev_pid & 2 )
  ! ( prev_pid & 4 )
  ! ( prev_pid & 8 )
  ! ( prev_pid & 16 )
   ( next_comm ~ "ftrace-test-fil" )
   ( next_comm != "ftrace-test-fil" )
  ! ( next_comm ~ "ftrace-test-fil" )
  ! ( next_comm != "ftrace-test-fil" )
Quoted 8 lines

thanks for the tests!
Since you're adding full support for 'not',
I think would be good to have few more tests where ! is not a top node. Like:
(prev_pid != 0 && !(next_pid & 2))
and another one with ! at multiple levels, like:
(prev_pid != 0 && !(next_pid != 2 && !(prev_pid > 3))) ... or reject them during parsing.

Quoted 5 lines

Sure, would you like to add them :-)

I spent more time on this than I should have. I have other things to work on and this little project has already put me behind on my other tasks.

aren't we all starting to hack things only to realize that the scope of work is too large? ;)
Anyway, will try to add these tests when your patches land.

author maintainer
#7 lore raw

Re: [PATCH 3/3] ftracetests: Add test to test event filter logic

Quoted 2 lines · On Wed, 3 Dec 2014 09:53:18 -0800 Alexei Starovoitov <[email protected]> wrote:

aren't we all starting to hack things only to realize that the scope of work is too large? ;)

Yep. Although I did say I would only spend a few hours on a solution, and would give up if it too longer. The patches themselves took much shorter than I expected. It was writing the tests that brought me over the brink. Although, I did a lot of manual tests to make sure things did what was expected. But those manual tests need to become automatic ones.

Quoted 1 line

Anyway, will try to add these tests when your patches land.

I'm placing the code changes in my 3.19 queue, and the test cases in my 3.20 queue. This is probably a good idea anyway, as the test cases will fail if the code wasn't implemented, and the test goes in a separate branch, where the code isn't implemented :-) I did a merge to do the test cases.

I also need to solve the busybox limitation. I don't know how that shell can do bit logic, which is needed to verify that the filters really did work.

-- Steve

#8 lore raw

Re: [PATCH 3/3] ftracetests: Add test to test event filter logic

In reply to Steven Rostedt
Quoted 27 lines · Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

Add a test to test the event filter logic. It currently tests the following filters against sched:sched_switch event.

   ( prev_pid != 0 )
   ( prev_pid == 0 )
   ( prev_pid < 100 )
   ( prev_pid <= $$ )
   ( prev_pid > 100 )
   ( prev_pid >= $$ )
  ! ( prev_pid != 0 )
  ! ( prev_pid == 0 )
  ! ( prev_pid < 100 )
  ! ( prev_pid <= $$ )
  ! ( prev_pid > 100 )
  ! ( prev_pid >= $$ )
   ( prev_pid != 0 && next_pid > 10 )
   ( prev_pid != 0 || next_pid > 10 )
  ! ( prev_pid != 0 && next_pid > 10 )
  ! ( prev_pid != 0 || next_pid > 10 )
   ( prev_pid & 1 )
   ( prev_pid & 2 )
   ( prev_pid & 4 )
   ( prev_pid & 8 )
   ( prev_pid & 16 )
  ! ( prev_pid & 1 )
Quoted 9 lines
  ! ( prev_pid & 2 )
  ! ( prev_pid & 4 )
  ! ( prev_pid & 8 )
  ! ( prev_pid & 16 )
   ( next_comm ~ "ftrace-test-fil" )
   ( next_comm != "ftrace-test-fil" )
  ! ( next_comm ~ "ftrace-test-fil" )
  ! ( next_comm != "ftrace-test-fil" )

Hmm, this uses some bash-only syntax, here is the result of checkbashisms.

# checkbashisms tools/testing/selftests/ftrace/test.d/ftrace/filter.tc
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 70 (should be 'b = a'):
            if [ "$not" == '!' ]; then
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 99 (should be 'b = a'):
            if [ "$not" == '!' ]; then
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 137 (let ...):
    let x=1
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 148 (let ...):
            let val="$pid & $x"
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 155 (should be 'b = a'):
            elif [ "$not" == '!' ]; then
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 160 (let ...):
        let x="$x << 1"
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 182 (should be 'b = a'):
            if [ "$not" == '!' ]; then

to allow run this on busybox or dash, we'd better clean it.

Quoted 2 lines
Signed-off-by Steven Rostedt <[email protected]>

---

[...]

tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 127

Quoted 3 lines
tools/testing/selftests/ftrace/test.d/ftrace/filter.tc new file +3 -0
+# Test more complex compares (&& and !!)
+test_cmp2 -ne 0 -a -gt 10 "!= 0" "> 10" "&&" ''
+test_cmp2 -ne 0 -o -gt 10 "!= 0" "> 10" "||" ''

This might better be
test_cmp2 -ne 0 -a -gt 10 "!= 0" "&&" "> 10" ''
test_cmp2 -ne 0 -o -gt 10 "!= 0" "||" "> 10" ''

:-)

tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 130

Quoted 3 lines
tools/testing/selftests/ftrace/test.d/ftrace/filter.tc new file +3 -0
+
+test_cmp2 -ne 0 -a -gt 10 "!= 0" "> 10" "&&" '!'
+test_cmp2 -ne 0 -o -gt 10 "!= 0" "> 10" "||" '!'

Thank you,

Signature · 4 lines
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
author maintainer
#9 lore raw

Re: [PATCH 3/3] ftracetests: Add test to test event filter logic

Quoted 56 lines · On Wed, 03 Dec 2014 18:26:43 +0900 Masami Hiramatsu…
Quoted 27 lines · Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

Add a test to test the event filter logic. It currently tests the following filters against sched:sched_switch event.

   ( prev_pid != 0 )
   ( prev_pid == 0 )
   ( prev_pid < 100 )
   ( prev_pid <= $$ )
   ( prev_pid > 100 )
   ( prev_pid >= $$ )
  ! ( prev_pid != 0 )
  ! ( prev_pid == 0 )
  ! ( prev_pid < 100 )
  ! ( prev_pid <= $$ )
  ! ( prev_pid > 100 )
  ! ( prev_pid >= $$ )
   ( prev_pid != 0 && next_pid > 10 )
   ( prev_pid != 0 || next_pid > 10 )
  ! ( prev_pid != 0 && next_pid > 10 )
  ! ( prev_pid != 0 || next_pid > 10 )
   ( prev_pid & 1 )
   ( prev_pid & 2 )
   ( prev_pid & 4 )
   ( prev_pid & 8 )
   ( prev_pid & 16 )
  ! ( prev_pid & 1 )
Quoted 9 lines
  ! ( prev_pid & 2 )
  ! ( prev_pid & 4 )
  ! ( prev_pid & 8 )
  ! ( prev_pid & 16 )
   ( next_comm ~ "ftrace-test-fil" )
   ( next_comm != "ftrace-test-fil" )
  ! ( next_comm ~ "ftrace-test-fil" )
  ! ( next_comm != "ftrace-test-fil" )

Hmm, this uses some bash-only syntax, here is the result of checkbashisms.

# checkbashisms tools/testing/selftests/ftrace/test.d/ftrace/filter.tc
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 70 (should be 'b = a'):
            if [ "$not" == '!' ]; then
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 99 (should be 'b = a'):
            if [ "$not" == '!' ]; then
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 137 (let ...):
    let x=1
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 148 (let ...):
            let val="$pid & $x"
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 155 (should be 'b = a'):
            elif [ "$not" == '!' ]; then
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 160 (let ...):
        let x="$x << 1"
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 182 (should be 'b = a'):
            if [ "$not" == '!' ]; then
Quoted 1 line

to allow run this on busybox or dash, we'd better clean it.

Do you know how to fix this?

Quoted 12 lines
Quoted 3 lines
Signed-off-by Steven Rostedt <[email protected]>

---

[...]

tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 127

Quoted 3 lines
tools/testing/selftests/ftrace/test.d/ftrace/filter.tc new file +3 -0
+# Test more complex compares (&& and !!)
+test_cmp2 -ne 0 -a -gt 10 "!= 0" "> 10" "&&" ''
+test_cmp2 -ne 0 -o -gt 10 "!= 0" "> 10" "||" ''

This might better be
test_cmp2 -ne 0 -a -gt 10 "!= 0" "&&" "> 10" ''
test_cmp2 -ne 0 -o -gt 10 "!= 0" "||" "> 10" ''

Quoted 1 line

:-)

Sure.

OK, I'll hold off on sending this patch then till 3.20.

I'll still add the update to the kernel for 3.19, but the testing for it needs work. It passes my test suite, but I don't know how to handle the busybox limitations.

-- Steve

Quoted 8 lines

tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 130

Quoted 4 lines
tools/testing/selftests/ftrace/test.d/ftrace/filter.tc new file +3 -0
+
+test_cmp2 -ne 0 -a -gt 10 "!= 0" "> 10" "&&" '!'
+test_cmp2 -ne 0 -o -gt 10 "!= 0" "> 10" "||" '!'

Thank you,

author maintainer
#10 lore raw

Re: [PATCH 3/3] ftracetests: Add test to test event filter logic

Quoted 2 lines · On Wed, 3 Dec 2014 04:34:23 -0500 Steven Rostedt <[email protected]> wrote:

OK, I'll hold off on sending this patch then till 3.20.

BTW, can we add another option that just shows the echo output. The -d enables -x which floods the screen with lots of useless information and it's hard to see what is going on.

Thanks,

-- Steve

#11 lore raw

Re: [PATCH 3/3] ftracetests: Add test to test event filter logic

Quoted 9 lines · Steven Rostedt
Quoted 2 lines · On Wed, 3 Dec 2014 04:34:23 -0500 Steven Rostedt <[email protected]> wrote:

OK, I'll hold off on sending this patch then till 3.20.

BTW, can we add another option that just shows the echo output. The -d enables -x which floods the screen with lots of useless information and it's hard to see what is going on.

I see, actually -d is not for debugging each test case, but just for the ftracetest itself.
I'll add --verbose to show echos in testcases.

Thank you,

Signature · 4 lines
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
#12 lore raw

Re: [PATCH 3/3] ftracetests: Add test to test event filter logic

In reply to Steven Rostedt
Quoted 59 lines · Steven Rostedt
Quoted 56 lines · On Wed, 03 Dec 2014 18:26:43 +0900 Masami Hiramatsu…
Quoted 27 lines · Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

Add a test to test the event filter logic. It currently tests the following filters against sched:sched_switch event.

   ( prev_pid != 0 )
   ( prev_pid == 0 )
   ( prev_pid < 100 )
   ( prev_pid <= $$ )
   ( prev_pid > 100 )
   ( prev_pid >= $$ )
  ! ( prev_pid != 0 )
  ! ( prev_pid == 0 )
  ! ( prev_pid < 100 )
  ! ( prev_pid <= $$ )
  ! ( prev_pid > 100 )
  ! ( prev_pid >= $$ )
   ( prev_pid != 0 && next_pid > 10 )
   ( prev_pid != 0 || next_pid > 10 )
  ! ( prev_pid != 0 && next_pid > 10 )
  ! ( prev_pid != 0 || next_pid > 10 )
   ( prev_pid & 1 )
   ( prev_pid & 2 )
   ( prev_pid & 4 )
   ( prev_pid & 8 )
   ( prev_pid & 16 )
  ! ( prev_pid & 1 )
Quoted 9 lines
  ! ( prev_pid & 2 )
  ! ( prev_pid & 4 )
  ! ( prev_pid & 8 )
  ! ( prev_pid & 16 )
   ( next_comm ~ "ftrace-test-fil" )
   ( next_comm != "ftrace-test-fil" )
  ! ( next_comm ~ "ftrace-test-fil" )
  ! ( next_comm != "ftrace-test-fil" )

Hmm, this uses some bash-only syntax, here is the result of checkbashisms.

# checkbashisms tools/testing/selftests/ftrace/test.d/ftrace/filter.tc
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 70 (should be 'b = a'):
            if [ "$not" == '!' ]; then
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 99 (should be 'b = a'):
            if [ "$not" == '!' ]; then
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 137 (let ...):
    let x=1
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 148 (let ...):
            let val="$pid & $x"
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 155 (should be 'b = a'):
            elif [ "$not" == '!' ]; then
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 160 (let ...):
        let x="$x << 1"
possible bashism in tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 182 (should be 'b = a'):
            if [ "$not" == '!' ]; then
Quoted 3 lines
Quoted 1 line

to allow run this on busybox or dash, we'd better clean it.

Do you know how to fix this?

Yes, it's easy :).
As the tool output, [ a == b ] can be changed to [ a = b ], and "let ..." can be changed to "$((...))"

Quoted 13 lines
Quoted 13 lines
Quoted 4 lines
Signed-off-by Steven Rostedt <[email protected]>

---

[...]

tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 127

Quoted 3 lines
tools/testing/selftests/ftrace/test.d/ftrace/filter.tc new file +3 -0
+# Test more complex compares (&& and !!)
+test_cmp2 -ne 0 -a -gt 10 "!= 0" "> 10" "&&" ''
+test_cmp2 -ne 0 -o -gt 10 "!= 0" "> 10" "||" ''

This might better be
test_cmp2 -ne 0 -a -gt 10 "!= 0" "&&" "> 10" ''
test_cmp2 -ne 0 -o -gt 10 "!= 0" "||" "> 10" ''

Quoted 5 lines
Quoted 1 line

:-)

Sure.

OK, I'll hold off on sending this patch then till 3.20.

OK.

Quoted 4 lines

I'll still add the update to the kernel for 3.19, but the testing for it needs work. It passes my test suite, but I don't know how to handle the busybox limitations.

If you are using Fedora, it provides dash and busybox packages too.

Thank you,

Signature · 4 lines
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
author maintainer
#13 lore raw

Re: [PATCH 3/3] ftracetests: Add test to test event filter logic

Quoted 7 lines · On Thu, 04 Dec 2014 08:14:01 +0900 Masami Hiramatsu…
Quoted 3 lines
Quoted 1 line

to allow run this on busybox or dash, we'd better clean it.

Do you know how to fix this?

Yes, it's easy :).
As the tool output, [ a == b ] can be changed to [ a = b ], and "let ..." can be changed to "$((...))"

OK, thanks.

Quoted 20 lines
Quoted 14 lines
Quoted 14 lines
Quoted 5 lines
Signed-off-by Steven Rostedt <[email protected]>

---

[...]

tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 127

Quoted 3 lines
tools/testing/selftests/ftrace/test.d/ftrace/filter.tc new file +3 -0
+# Test more complex compares (&& and !!)
+test_cmp2 -ne 0 -a -gt 10 "!= 0" "> 10" "&&" ''
+test_cmp2 -ne 0 -o -gt 10 "!= 0" "> 10" "||" ''

This might better be
test_cmp2 -ne 0 -a -gt 10 "!= 0" "&&" "> 10" ''
test_cmp2 -ne 0 -o -gt 10 "!= 0" "||" "> 10" ''

Quoted 5 lines
Quoted 1 line

:-)

Sure.

OK, I'll hold off on sending this patch then till 3.20.

Quoted 8 lines

OK.

Quoted 4 lines

I'll still add the update to the kernel for 3.19, but the testing for it needs work. It passes my test suite, but I don't know how to handle the busybox limitations.

If you are using Fedora, it provides dash and busybox packages too.

Yep, and so does Debian. In fact, I notice that my Debian boxes all have /bin/sh linked to dash not bash (which means they were not as susceptible to the bash bug).

-- Steve

author maintainer
#14 lore raw

PATCH v2 1/1 ftracetests: Add test to test event filter logic

In reply to Masami Hiramatsu

Add a test to test the event filter logic. It currently tests the following filters against sched:sched_switch event.

   ( prev_pid != 0 )
   ( prev_pid == 0 )
   ( prev_pid < 100 )
   ( prev_pid <= $$ )
   ( prev_pid > 100 )
   ( prev_pid >= $$ )
  ! ( prev_pid != 0 )
  ! ( prev_pid == 0 )
  ! ( prev_pid < 100 )
  ! ( prev_pid <= $$ )
  ! ( prev_pid > 100 )
  ! ( prev_pid >= $$ )
   ( prev_pid != 0 && next_pid > 10 )
   ( prev_pid != 0 || next_pid > 10 )
  ! ( prev_pid != 0 && next_pid > 10 )
  ! ( prev_pid != 0 || next_pid > 10 )
   ( prev_pid & 1 )
   ( prev_pid & 2 )
   ( prev_pid & 4 )
   ( prev_pid & 8 )
   ( prev_pid & 16 )
  ! ( prev_pid & 1 )
  ! ( prev_pid & 2 )
  ! ( prev_pid & 4 )
  ! ( prev_pid & 8 )
  ! ( prev_pid & 16 )
   ( next_comm ~ "ftrace-test-fil" )
   ( next_comm != "ftrace-test-fil" )
  ! ( next_comm ~ "ftrace-test-fil" )
  ! ( next_comm != "ftrace-test-fil" )
Signed-off-by Steven Rostedt <[email protected]>
---
 .../selftests/ftrace/test.d/ftrace/filter.tc       | 197 +++++++++++++++++++++
 1 file changed, 197 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/ftrace/filter.tc
tools/testing/selftests/ftrace/test.d/ftrace/filter.tc new file +197 -0
new file mode 100644
index 000000000000..eef7cc6320f2
@@ -0,0 +1,197 @@
+#!/bin/sh
+# description: ftrace - test filter logic
+
+EVENT=events/sched/sched_switch
+FILTER=$EVENT/filter
+ENABLE=$EVENT/enable
+
+FIELD1=prev_pid
+FIELD2=next_pid
+FIELD3=next_comm
+
+ITER=100
+
+enable_event() {
+    echo 1 > $ENABLE
+}
+
+disable_event() {
+    echo 0 > $ENABLE
+}
+
+clear_filter() {
+    echo 0 > $FILTER
+}
+
+do_reset() {
+    enable_tracing
+    disable_event
+    clear_filter
+    clear_trace
+}
+
+fail() { # msg
+    do_reset
+    echo $1
+    exit -1
+}
+
+run_code() {
+    # make lots of pids to filter on
+    for i in `seq $ITER`; do
+	ls /usr > /dev/null
+    done
+}
+
+run_event() {
+    enable_event
+    run_code
+    disable_event
+}
+
+disable_tracing
+clear_trace
+enable_tracing
+
+test_cmp() {
+    cmp=$1
+    rval=$2
+    filter=$3
+    not=$4
+
+    echo "$not ( $FIELD1 $filter )"
+    echo "$not ( $FIELD1 $filter )" > $FILTER
+
+    run_event
+
+    cat trace | sed -ne "s/.*$FIELD1=\([^ ]*\).*/\1/p" |
+    while read pid; do
+	if [ "$pid" $cmp $rval ]; then
+	    if [ "$not" = '!' ]; then
+		fail "$pid matched filter: $filter; but should not not have for $not"
+	    fi
+	elif [ "$not" != '!' ]; then 
+	    fail "$pid does not match filter: $filter"
+	fi
+    done
+    clear_trace
+}
+
+test_cmp2() {
+    cmp1=$1
+    rval1=$2
+    conj=$3
+    cmp2=$4
+    rval2=$5
+    filter1=$6
+    filter2=$7
+    fconj=$8
+    not=$9
+
+    echo "$not ( $FIELD1 $filter1 $fconj $FIELD2 $filter2 )"
+    echo "$not ( $FIELD1 $filter1 $fconj $FIELD2 $filter2 )" > $FILTER
+
+    run_event
+
+    cat trace | sed -ne "s/.*$FIELD1=\([^ ]*\).*$FIELD2=\([^ ]*\).*/\1 \2/p" |
+    while read pid1 pid2; do
+	if [ "$pid1" $cmp1 $rval1 $conj "$pid2" $cmp2 $rval2 ]; then
+	    if [ "$not" = '!' ]; then
+		fail "$pid1 and $pid2 match not filter: $filter1 $fconj $filter2"
+	    fi
+	elif [ "$not" != '!' ]; then
+	    fail "$pid1 and $pid2 does not match filter: $filter1 $fconj $filter2"
+	fi
+    done
+    clear_trace
+}
+
+# first simple compares
+test_cmp -ne 0 "!= 0" ""
+test_cmp -eq 0 "== 0" ""
+test_cmp -lt 100 "< 100" ""
+test_cmp -le $$ "<= $$" ""
+test_cmp -gt 100 "> 100" ""
+test_cmp -ge $$ ">= $$" ""
+
+# Now test the not case
+test_cmp -ne 0 "!= 0" '!'
+test_cmp -eq 0 "== 0" '!'
+test_cmp -lt 100 "< 100" '!'
+test_cmp -le $$ "<= $$" '!'
+test_cmp -gt 100 "> 100" '!'
+test_cmp -ge $$ ">= $$" '!'
+
+# Test more complex compares (&& and !!)
+test_cmp2 -ne 0 -a -gt 10 "!= 0" "> 10" "&&" ''
+test_cmp2 -ne 0 -o -gt 10 "!= 0" "> 10" "||" ''
+
+test_cmp2 -ne 0 -a -gt 10 "!= 0" "> 10" "&&" '!'
+test_cmp2 -ne 0 -o -gt 10 "!= 0" "> 10" "||" '!'
+
+# test bitmask
+
+bit_test() {
+    not=$1
+
+    x=1
+    for i in `seq 5`; do
+	echo "$not ( $FIELD1 & $x )"
+	echo "$not ( $FIELD1 & $x )" > $FILTER
+	run_event
+
+	cat trace | sed -ne "s/.*$FIELD1=\([^ ]*\).*/\1/p" |
+	while read pid; do
+
+	    val=$(($pid & $x))
+
+	    if [ $val -eq 0 ]; then
+		if [ "$not" != '!' ]; then
+		    fail "$pid did not match bitmask $x"
+		fi
+	    elif [ "$not" = '!' ]; then
+		fail "$pid did not match not bitmask $x"
+	    fi
+	done
+	clear_trace
+	x=$(($x << 1))
+    done
+}		
+
+bit_test ''
+bit_test '!'
+
+test_string() {
+    cmp=$1
+    filter=$2
+    not=$3
+
+    comm=`cat /proc/$$/comm`
+
+    echo "$not ( $FIELD3 $filter \"$comm\" )"
+    echo "$not ( $FIELD3 $filter \"$comm\" )" > $FILTER
+
+    run_event
+
+    cat trace | sed -ne "s/.*$FIELD3=\([^ ]*\).*/\1/p" |
+    while read com; do
+	if [ "$com" $cmp "$comm" ]; then
+	    if [ "$not" = '!' ]; then
+		fail "$com matched filter: $filter; but should not not have for $not"
+	    fi
+	elif [ "$not" != '!' ]; then 
+	    fail "$com does not match filter: $filter"
+	fi
+    done
+    clear_trace
+}
+
+test_string "=" "~" ''
+test_string "!=" "!=" ''
+
+test_string "=" "~" '!'
+test_string "!=" "!=" '!'
+
+do_reset
+
+exit 0
Signature · 1 line
1.8.1.4
#15 lore raw

Re: [PATCH v2] ftracetests: Add test to test event filter logic

Hi Steven,

I have just one comment.

tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 130

Quoted 6 lines · Steven Rostedt
tools/testing/selftests/ftrace/test.d/ftrace/filter.tc new file +6 -0
+# Test more complex compares (&& and !!)
+test_cmp2 -ne 0 -a -gt 10 "!= 0" "> 10" "&&" ''
+test_cmp2 -ne 0 -o -gt 10 "!= 0" "> 10" "||" ''
+
+test_cmp2 -ne 0 -a -gt 10 "!= 0" "> 10" "&&" '!'
+test_cmp2 -ne 0 -o -gt 10 "!= 0" "> 10" "||" '!'

These are not fixed :(

tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 88

Quoted 3 lines
tools/testing/selftests/ftrace/test.d/ftrace/filter.tc new file +3 -0
+    filter1=$6
+    filter2=$7
+    fconj=$8

are better to be

Review

Quoted 3 lines
(quoted diff) +3 -0
+    filter1=$6
+    fconj=$7
+    filter2=$8

so that we can easily review parameters.

Others are OK for me.

Thank you,

Signature · 4 lines
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
author maintainer
#16 lore raw

Re: [PATCH v2] ftracetests: Add test to test event filter logic

Quoted 5 lines · On Thu, 04 Dec 2014 19:06:37 +0900 Masami Hiramatsu…

Hi Steven,

I have just one comment.

(2014/12/04 12:18), Steven Rostedt wrote:

Quoted 8 lines

tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 130

Quoted 6 lines
tools/testing/selftests/ftrace/test.d/ftrace/filter.tc new file +6 -0
+# Test more complex compares (&& and !!)
+test_cmp2 -ne 0 -a -gt 10 "!= 0" "> 10" "&&" ''
+test_cmp2 -ne 0 -o -gt 10 "!= 0" "> 10" "||" ''
+
+test_cmp2 -ne 0 -a -gt 10 "!= 0" "> 10" "&&" '!'
+test_cmp2 -ne 0 -o -gt 10 "!= 0" "> 10" "||" '!'

These are not fixed :(

Oops, forgot about this. I was so focused on the dash commands I forgot about the easy stuff.

Quoted 7 lines

tools/testing/selftests/ftrace/test.d/ftrace/filter.tc line 88

Quoted 4 lines
tools/testing/selftests/ftrace/test.d/ftrace/filter.tc new file +3 -0
+    filter1=$6
+    filter2=$7
+    fconj=$8

are better to be

Quoted 8 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+    filter1=$6
+    fconj=$7
+    filter2=$8

so that we can easily review parameters.

Others are OK for me.

Yep, I'll make a v3 soon.

-- Steve