On Wed, Oct 27, 2021 at 2:58 PM Masami Hiramatsu mhiramat@kernel.org wrote:

Hi Kalesh,

On Wed, 27 Oct 2021 13:59:09 -0700 Kalesh Singh kaleshsingh@google.com wrote:

...

If an event trigger references a variable defined in another trigger, it has to be removed before the trigger that defines the variable is removed.

Signed-off-by: Kalesh Singh kaleshsingh@google.com

tools/testing/selftests/ftrace/test.d/functions | 9 +++++++++ 1 file changed, 9 insertions(+)

diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions index 000fd05e84b1..bd9e85f4d626 100644 --- a/tools/testing/selftests/ftrace/test.d/functions +++ b/tools/testing/selftests/ftrace/test.d/functions @@ -22,6 +22,15 @@ reset_trigger_file() { file=`echo $line | cut -f1 -d:` echo "!$cmd" >> $file done

• # remove triggers with references next
• grep -H '$' $@ |
• while read line; do

•    cmd=`echo $line | cut -f2- -d: | cut -f1 -d"["`
  

• file=`echo $line | cut -f1 -d:`
  

• echo "!$cmd" >> $file
  

• done

Why don't you use 'tac'? I love that idea :) Did you find any issue?

Hi Masami,

Thanks for the reviews. As with the first set of patches using tac gives a regression here, though I'm not sure why it doesn't work -- I also thought reversing the order would handle any dependencies correctly.

- Kalesh

I think the function which cleaning up the tracing file should use the 'tac' rollback method, because it is natural, simple and robust. Then the first loop for removing action triggers is not needed anymore.

Thank you,

...

 grep -Hv ^# $@ |
 while read line; do
     cmd=`echo $line | cut -f2- -d: | cut -f1 -d"["`

-- 2.33.0.1079.g6e70778dc9-goog

-- Masami Hiramatsu mhiramat@kernel.org

On Wed, 27 Oct 2021 13:59:10 -0700 Kalesh Singh kaleshsingh@google.com wrote:

Add tests for the parsing of hist trigger expressions; and to validate expression evaluation.

Signed-off-by: Kalesh Singh kaleshsingh@google.com

This looks good to me :)

Acked-by: Masami Hiramatsu mhiramat@kernel.org

Thanks!

---

Changes in v5:

• Add README pattern to requires tag, per Masami

Changes in v3:

• Remove .sym-offset error check tests

Changes in v2:

• Add Namhyung's Reviewed-by
• Update comment to clarify err_pos in "Too many subexpressions" test

.../trigger/trigger-hist-expressions.tc | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-expressions.tc

diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-expressions.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-expressions.tc new file mode 100644 index 000000000000..9690f9f809e7 --- /dev/null +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-expressions.tc @@ -0,0 +1,72 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# description: event trigger - test histogram expression parsing +# requires: set_event events/sched/sched_process_fork/trigger events/sched/sched_process_fork/hist error_log "<var1>=<field|var_ref|numeric_literal>":README

+fail() { #msg

• echo $1
• exit_fail

+}

+get_hist_var() { #var_name hist_path

• hist_output=`grep -m1 "$1: " $2`
• hitcount=`echo $hist_output | awk '{ for (i=1; i<=NF; ++i) { if ($i ~ "hitcount:") print $(i+1)} }'`
• var_sum=`echo $hist_output | awk '{ for (i=1; i<=NF; ++i) { if ($i ~ "'$1':") print $(i+1)} }'`
• var_val=$(( var_sum / hitcount ))
• echo $var_val

+}

+test_hist_expr() { # test_name expression expected_val

• reset_trigger
• echo "Test hist trigger expressions - $1"
• echo "hist:keys=common_pid:x=$2" > events/sched/sched_process_fork/trigger
• echo 'hist:keys=common_pid:vals=$x' >> events/sched/sched_process_fork/trigger
• for i in `seq 1 10` ; do ( echo "forked" > /dev/null); done
• actual=`get_hist_var x events/sched/sched_process_fork/hist`
• if [ $actual != $3 ]; then

•    fail "Failed hist trigger expression evaluation: Expression: $2 Expected: $3, Actual: $actual"
  

• fi
• reset_trigger

+}

+check_error() { # test_name command-with-error-pos-by-^

• reset_trigger
• echo "Test hist trigger expressions - $1"
• ftrace_errlog_check 'hist:sched:sched_process_fork' "$2" 'events/sched/sched_process_fork/trigger'
• reset_trigger

+}

+test_hist_expr "Variable assignment" "123" "123"

+test_hist_expr "Subtraction not associative" "16-8-4-2" "2"

+test_hist_expr "Division not associative" "64/8/4/2" "1"

+test_hist_expr "Same precedence operators (+,-) evaluated left to right" "16-8+4+2" "14"

+test_hist_expr "Same precedence operators (*,/) evaluated left to right" "4*3/2*2" "12"

+test_hist_expr "Multiplication evaluated before addition/subtraction" "4+3*2-2" "8"

+test_hist_expr "Division evaluated before addition/subtraction" "4+6/2-2" "5"

+# Division by zero returns -1 +test_hist_expr "Handles division by zero" "3/0" "-1"

+# err pos for "too many subexpressions" is dependent on where +# the last subexpression was detected. This can vary depending +# on how the expression tree was generated. +check_error "Too many subexpressions" 'hist:keys=common_pid:x=32+^10*3/20-4' +check_error "Too many subexpressions" 'hist:keys=common_pid:x=^1+2+3+4+5'

+check_error "Unary minus not supported in subexpression" 'hist:keys=common_pid:x=-(^1)+2'

+exit 0

2.33.0.1079.g6e70778dc9-goog