[PATCH v1 1/2] perf tests stat+csv_output: Switch CSV separator to @

[PATCH v1 1/2] perf tests stat+csv_output: Switch CSV separator to @

[Ian Rogers]

Commas may appear in events like:
cpu/INT_MISC.RECOVERY_CYCLES,cmask=1,edge/
which causes the commachecker to see more fields than expected. Use @
as the CSV separator to avoid this.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/stat+csv_output.sh | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/tools/perf/tests/shell/stat+csv_output.sh b/tools/perf/tests/shell/stat+csv_output.sh
index b7f050aa6210..324fc9e6edd7 100755
--- a/tools/perf/tests/shell/stat+csv_output.sh
+++ b/tools/perf/tests/shell/stat+csv_output.sh
@@ -7,6 +7,7 @@
 set -e
 
 skip_test=0
+csv_sep=@
 
 function commachecker()
 {
@@ -34,7 +35,7 @@ function commachecker()
 		[ "$x" = "Failed" ] && continue
 
 		# Count the number of commas
-		x=$(echo $line | tr -d -c ',')
+		x=$(echo $line | tr -d -c $csv_sep)
 		cnt="${#x}"
 		# echo $line $cnt
 		[[ ! "$cnt" =~ $exp ]] && {
@@ -54,7 +55,7 @@ function ParanoidAndNotRoot()
 check_no_args()
 {
 	echo -n "Checking CSV output: no args "
-	perf stat -x, true 2>&1 | commachecker --no-args
+	perf stat -x$csv_sep true 2>&1 | commachecker --no-args
 	echo "[Success]"
 }
 
@@ -66,7 +67,7 @@ check_system_wide()
 		echo "[Skip] paranoid and not root"
 		return
 	fi
-	perf stat -x, -a true 2>&1 | commachecker --system-wide
+	perf stat -x$csv_sep -a true 2>&1 | commachecker --system-wide
 	echo "[Success]"
 }
 
@@ -79,14 +80,14 @@ check_system_wide_no_aggr()
 		return
 	fi
 	echo -n "Checking CSV output: system wide no aggregation "
-	perf stat -x, -A -a --no-merge true 2>&1 | commachecker --system-wide-no-aggr
+	perf stat -x$csv_sep -A -a --no-merge true 2>&1 | commachecker --system-wide-no-aggr
 	echo "[Success]"
 }
 
 check_interval()
 {
 	echo -n "Checking CSV output: interval "
-	perf stat -x, -I 1000 true 2>&1 | commachecker --interval
+	perf stat -x$csv_sep -I 1000 true 2>&1 | commachecker --interval
 	echo "[Success]"
 }
 
@@ -94,7 +95,7 @@ check_interval()
 check_event()
 {
 	echo -n "Checking CSV output: event "
-	perf stat -x, -e cpu-clock true 2>&1 | commachecker --event
+	perf stat -x$csv_sep -e cpu-clock true 2>&1 | commachecker --event
 	echo "[Success]"
 }
 
@@ -106,7 +107,7 @@ check_per_core()
 		echo "[Skip] paranoid and not root"
 		return
 	fi
-	perf stat -x, --per-core -a true 2>&1 | commachecker --per-core
+	perf stat -x$csv_sep --per-core -a true 2>&1 | commachecker --per-core
 	echo "[Success]"
 }
 
@@ -118,7 +119,7 @@ check_per_thread()
 		echo "[Skip] paranoid and not root"
 		return
 	fi
-	perf stat -x, --per-thread -a true 2>&1 | commachecker --per-thread
+	perf stat -x$csv_sep --per-thread -a true 2>&1 | commachecker --per-thread
 	echo "[Success]"
 }
 
@@ -130,7 +131,7 @@ check_per_die()
 		echo "[Skip] paranoid and not root"
 		return
 	fi
-	perf stat -x, --per-die -a true 2>&1 | commachecker --per-die
+	perf stat -x$csv_sep --per-die -a true 2>&1 | commachecker --per-die
 	echo "[Success]"
 }
 
@@ -142,7 +143,7 @@ check_per_node()
 		echo "[Skip] paranoid and not root"
 		return
 	fi
-	perf stat -x, --per-node -a true 2>&1 | commachecker --per-node
+	perf stat -x$csv_sep --per-node -a true 2>&1 | commachecker --per-node
 	echo "[Success]"
 }
 
@@ -154,7 +155,7 @@ check_per_socket()
 		echo "[Skip] paranoid and not root"
 		return
 	fi
-	perf stat -x, --per-socket -a true 2>&1 | commachecker --per-socket
+	perf stat -x$csv_sep --per-socket -a true 2>&1 | commachecker --per-socket
 	echo "[Success]"
 }
 
-- 
2.39.2.637.g21b0678d19-goog

[PATCH v1 2/2] perf test: Avoid counting commas in json linter

[Ian Rogers]

Commas may appear in events like:
cpu/INT_MISC.RECOVERY_CYCLES,cmask=1,edge/
which causes the count of commas to see more items than
expected. Switch to counting the entries in the dictionary, which is 1
more than the number of commas.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 .../tests/shell/lib/perf_json_output_lint.py  | 29 +++++++++----------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/tools/perf/tests/shell/lib/perf_json_output_lint.py b/tools/perf/tests/shell/lib/perf_json_output_lint.py
index d90f8d102eb9..97598d14e532 100644
--- a/tools/perf/tests/shell/lib/perf_json_output_lint.py
+++ b/tools/perf/tests/shell/lib/perf_json_output_lint.py
@@ -40,19 +40,6 @@ def is_counter_value(num):
   return isfloat(num) or num == '<not counted>' or num == '<not supported>'
 
 def check_json_output(expected_items):
-  if expected_items != -1:
-    for line in Lines:
-      if 'failed' not in line:
-        count = 0
-        count = line.count(',')
-        if count != expected_items and count >= 1 and count <= 3 and 'metric-value' in line:
-          # Events that generate >1 metric may have isolated metric
-          # values and possibly other prefixes like interval, core and
-          # aggregate-number.
-          continue
-        if count != expected_items:
-          raise RuntimeError(f'wrong number of fields. counted {count} expected {expected_items}'
-                             f' in \'{line}\'')
   checks = {
       'aggregate-number': lambda x: isfloat(x),
       'core': lambda x: True,
@@ -73,6 +60,16 @@ def check_json_output(expected_items):
   }
   input = '[\n' + ','.join(Lines) + '\n]'
   for item in json.loads(input):
+    if expected_items != -1:
+      count = len(item)
+      if count != expected_items and count >= 1 and count <= 4 and 'metric-value' in item:
+        # Events that generate >1 metric may have isolated metric
+        # values and possibly other prefixes like interval, core and
+        # aggregate-number.
+        pass
+      elif count != expected_items:
+        raise RuntimeError(f'wrong number of fields. counted {count} expected {expected_items}'
+                           f' in \'{item}\'')
     for key, value in item.items():
       if key not in checks:
         raise RuntimeError(f'Unexpected key: key={key} value={value}')
@@ -82,11 +79,11 @@ def check_json_output(expected_items):
 
 try:
   if args.no_args or args.system_wide or args.event:
-    expected_items = 6
-  elif args.interval or args.per_thread or args.system_wide_no_aggr:
     expected_items = 7
-  elif args.per_core or args.per_socket or args.per_node or args.per_die:
+  elif args.interval or args.per_thread or args.system_wide_no_aggr:
     expected_items = 8
+  elif args.per_core or args.per_socket or args.per_node or args.per_die:
+    expected_items = 9
   else:
     # If no option is specified, don't check the number of items.
     expected_items = -1
-- 
2.39.2.637.g21b0678d19-goog

Re: [PATCH v1 1/2] perf tests stat+csv_output: Switch CSV separator to @

[Arnaldo Carvalho de Melo]

Em Wed, Feb 22, 2023 at 11:18:17PM -0800, Ian Rogers escreveu:
> Commas may appear in events like:
> cpu/INT_MISC.RECOVERY_CYCLES,cmask=1,edge/
> which causes the commachecker to see more fields than expected. Use @
> as the CSV separator to avoid this.

Thanks, applied both patches.

- Arnaldo

 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/tests/shell/stat+csv_output.sh | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/perf/tests/shell/stat+csv_output.sh b/tools/perf/tests/shell/stat+csv_output.sh
> index b7f050aa6210..324fc9e6edd7 100755
> --- a/tools/perf/tests/shell/stat+csv_output.sh
> +++ b/tools/perf/tests/shell/stat+csv_output.sh
> @@ -7,6 +7,7 @@
>  set -e
>  
>  skip_test=0
> +csv_sep=@
>  
>  function commachecker()
>  {
> @@ -34,7 +35,7 @@ function commachecker()
>  		[ "$x" = "Failed" ] && continue
>  
>  		# Count the number of commas
> -		x=$(echo $line | tr -d -c ',')
> +		x=$(echo $line | tr -d -c $csv_sep)
>  		cnt="${#x}"
>  		# echo $line $cnt
>  		[[ ! "$cnt" =~ $exp ]] && {
> @@ -54,7 +55,7 @@ function ParanoidAndNotRoot()
>  check_no_args()
>  {
>  	echo -n "Checking CSV output: no args "
> -	perf stat -x, true 2>&1 | commachecker --no-args
> +	perf stat -x$csv_sep true 2>&1 | commachecker --no-args
>  	echo "[Success]"
>  }
>  
> @@ -66,7 +67,7 @@ check_system_wide()
>  		echo "[Skip] paranoid and not root"
>  		return
>  	fi
> -	perf stat -x, -a true 2>&1 | commachecker --system-wide
> +	perf stat -x$csv_sep -a true 2>&1 | commachecker --system-wide
>  	echo "[Success]"
>  }
>  
> @@ -79,14 +80,14 @@ check_system_wide_no_aggr()
>  		return
>  	fi
>  	echo -n "Checking CSV output: system wide no aggregation "
> -	perf stat -x, -A -a --no-merge true 2>&1 | commachecker --system-wide-no-aggr
> +	perf stat -x$csv_sep -A -a --no-merge true 2>&1 | commachecker --system-wide-no-aggr
>  	echo "[Success]"
>  }
>  
>  check_interval()
>  {
>  	echo -n "Checking CSV output: interval "
> -	perf stat -x, -I 1000 true 2>&1 | commachecker --interval
> +	perf stat -x$csv_sep -I 1000 true 2>&1 | commachecker --interval
>  	echo "[Success]"
>  }
>  
> @@ -94,7 +95,7 @@ check_interval()
>  check_event()
>  {
>  	echo -n "Checking CSV output: event "
> -	perf stat -x, -e cpu-clock true 2>&1 | commachecker --event
> +	perf stat -x$csv_sep -e cpu-clock true 2>&1 | commachecker --event
>  	echo "[Success]"
>  }
>  
> @@ -106,7 +107,7 @@ check_per_core()
>  		echo "[Skip] paranoid and not root"
>  		return
>  	fi
> -	perf stat -x, --per-core -a true 2>&1 | commachecker --per-core
> +	perf stat -x$csv_sep --per-core -a true 2>&1 | commachecker --per-core
>  	echo "[Success]"
>  }
>  
> @@ -118,7 +119,7 @@ check_per_thread()
>  		echo "[Skip] paranoid and not root"
>  		return
>  	fi
> -	perf stat -x, --per-thread -a true 2>&1 | commachecker --per-thread
> +	perf stat -x$csv_sep --per-thread -a true 2>&1 | commachecker --per-thread
>  	echo "[Success]"
>  }
>  
> @@ -130,7 +131,7 @@ check_per_die()
>  		echo "[Skip] paranoid and not root"
>  		return
>  	fi
> -	perf stat -x, --per-die -a true 2>&1 | commachecker --per-die
> +	perf stat -x$csv_sep --per-die -a true 2>&1 | commachecker --per-die
>  	echo "[Success]"
>  }
>  
> @@ -142,7 +143,7 @@ check_per_node()
>  		echo "[Skip] paranoid and not root"
>  		return
>  	fi
> -	perf stat -x, --per-node -a true 2>&1 | commachecker --per-node
> +	perf stat -x$csv_sep --per-node -a true 2>&1 | commachecker --per-node
>  	echo "[Success]"
>  }
>  
> @@ -154,7 +155,7 @@ check_per_socket()
>  		echo "[Skip] paranoid and not root"
>  		return
>  	fi
> -	perf stat -x, --per-socket -a true 2>&1 | commachecker --per-socket
> +	perf stat -x$csv_sep --per-socket -a true 2>&1 | commachecker --per-socket
>  	echo "[Success]"
>  }
>  
> -- 
> 2.39.2.637.g21b0678d19-goog
> 

-- 

- Arnaldo

Re: [PATCH v1 1/2] perf tests stat+csv_output: Switch CSV separator to @

[Ian Rogers]

On Thu, Feb 23, 2023 at 5:32 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Wed, Feb 22, 2023 at 11:18:17PM -0800, Ian Rogers escreveu:
> > Commas may appear in events like:
> > cpu/INT_MISC.RECOVERY_CYCLES,cmask=1,edge/
> > which causes the commachecker to see more fields than expected. Use @
> > as the CSV separator to avoid this.
>
> Thanks, applied both patches.

Thanks Arnaldo, I don't see the patches in the git branches so perhaps
something went wrong?

Ian

> - Arnaldo
>
>
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/tests/shell/stat+csv_output.sh | 23 ++++++++++++-----------
> >  1 file changed, 12 insertions(+), 11 deletions(-)
> >
> > diff --git a/tools/perf/tests/shell/stat+csv_output.sh b/tools/perf/tests/shell/stat+csv_output.sh
> > index b7f050aa6210..324fc9e6edd7 100755
> > --- a/tools/perf/tests/shell/stat+csv_output.sh
> > +++ b/tools/perf/tests/shell/stat+csv_output.sh
> > @@ -7,6 +7,7 @@
> >  set -e
> >
> >  skip_test=0
> > +csv_sep=@
> >
> >  function commachecker()
> >  {
> > @@ -34,7 +35,7 @@ function commachecker()
> >               [ "$x" = "Failed" ] && continue
> >
> >               # Count the number of commas
> > -             x=$(echo $line | tr -d -c ',')
> > +             x=$(echo $line | tr -d -c $csv_sep)
> >               cnt="${#x}"
> >               # echo $line $cnt
> >               [[ ! "$cnt" =~ $exp ]] && {
> > @@ -54,7 +55,7 @@ function ParanoidAndNotRoot()
> >  check_no_args()
> >  {
> >       echo -n "Checking CSV output: no args "
> > -     perf stat -x, true 2>&1 | commachecker --no-args
> > +     perf stat -x$csv_sep true 2>&1 | commachecker --no-args
> >       echo "[Success]"
> >  }
> >
> > @@ -66,7 +67,7 @@ check_system_wide()
> >               echo "[Skip] paranoid and not root"
> >               return
> >       fi
> > -     perf stat -x, -a true 2>&1 | commachecker --system-wide
> > +     perf stat -x$csv_sep -a true 2>&1 | commachecker --system-wide
> >       echo "[Success]"
> >  }
> >
> > @@ -79,14 +80,14 @@ check_system_wide_no_aggr()
> >               return
> >       fi
> >       echo -n "Checking CSV output: system wide no aggregation "
> > -     perf stat -x, -A -a --no-merge true 2>&1 | commachecker --system-wide-no-aggr
> > +     perf stat -x$csv_sep -A -a --no-merge true 2>&1 | commachecker --system-wide-no-aggr
> >       echo "[Success]"
> >  }
> >
> >  check_interval()
> >  {
> >       echo -n "Checking CSV output: interval "
> > -     perf stat -x, -I 1000 true 2>&1 | commachecker --interval
> > +     perf stat -x$csv_sep -I 1000 true 2>&1 | commachecker --interval
> >       echo "[Success]"
> >  }
> >
> > @@ -94,7 +95,7 @@ check_interval()
> >  check_event()
> >  {
> >       echo -n "Checking CSV output: event "
> > -     perf stat -x, -e cpu-clock true 2>&1 | commachecker --event
> > +     perf stat -x$csv_sep -e cpu-clock true 2>&1 | commachecker --event
> >       echo "[Success]"
> >  }
> >
> > @@ -106,7 +107,7 @@ check_per_core()
> >               echo "[Skip] paranoid and not root"
> >               return
> >       fi
> > -     perf stat -x, --per-core -a true 2>&1 | commachecker --per-core
> > +     perf stat -x$csv_sep --per-core -a true 2>&1 | commachecker --per-core
> >       echo "[Success]"
> >  }
> >
> > @@ -118,7 +119,7 @@ check_per_thread()
> >               echo "[Skip] paranoid and not root"
> >               return
> >       fi
> > -     perf stat -x, --per-thread -a true 2>&1 | commachecker --per-thread
> > +     perf stat -x$csv_sep --per-thread -a true 2>&1 | commachecker --per-thread
> >       echo "[Success]"
> >  }
> >
> > @@ -130,7 +131,7 @@ check_per_die()
> >               echo "[Skip] paranoid and not root"
> >               return
> >       fi
> > -     perf stat -x, --per-die -a true 2>&1 | commachecker --per-die
> > +     perf stat -x$csv_sep --per-die -a true 2>&1 | commachecker --per-die
> >       echo "[Success]"
> >  }
> >
> > @@ -142,7 +143,7 @@ check_per_node()
> >               echo "[Skip] paranoid and not root"
> >               return
> >       fi
> > -     perf stat -x, --per-node -a true 2>&1 | commachecker --per-node
> > +     perf stat -x$csv_sep --per-node -a true 2>&1 | commachecker --per-node
> >       echo "[Success]"
> >  }
> >
> > @@ -154,7 +155,7 @@ check_per_socket()
> >               echo "[Skip] paranoid and not root"
> >               return
> >       fi
> > -     perf stat -x, --per-socket -a true 2>&1 | commachecker --per-socket
> > +     perf stat -x$csv_sep --per-socket -a true 2>&1 | commachecker --per-socket
> >       echo "[Success]"
> >  }
> >
> > --
> > 2.39.2.637.g21b0678d19-goog
> >
>
> --
>
> - Arnaldo

Re: [PATCH v1 1/2] perf tests stat+csv_output: Switch CSV separator to @

[Arnaldo Carvalho de Melo]

Em Thu, Mar 02, 2023 at 11:30:36AM -0800, Ian Rogers escreveu:
> On Thu, Feb 23, 2023 at 5:32 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Wed, Feb 22, 2023 at 11:18:17PM -0800, Ian Rogers escreveu:
> > > Commas may appear in events like:
> > > cpu/INT_MISC.RECOVERY_CYCLES,cmask=1,edge/
> > > which causes the commachecker to see more fields than expected. Use @
> > > as the CSV separator to avoid this.
> >
> > Thanks, applied both patches.
> 
> Thanks Arnaldo, I don't see the patches in the git branches so perhaps
> something went wrong?

Its in my local branch, I'll push it.
 
> Ian
> 
> > - Arnaldo
> >
> >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > >  tools/perf/tests/shell/stat+csv_output.sh | 23 ++++++++++++-----------
> > >  1 file changed, 12 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/tools/perf/tests/shell/stat+csv_output.sh b/tools/perf/tests/shell/stat+csv_output.sh
> > > index b7f050aa6210..324fc9e6edd7 100755
> > > --- a/tools/perf/tests/shell/stat+csv_output.sh
> > > +++ b/tools/perf/tests/shell/stat+csv_output.sh
> > > @@ -7,6 +7,7 @@
> > >  set -e
> > >
> > >  skip_test=0
> > > +csv_sep=@
> > >
> > >  function commachecker()
> > >  {
> > > @@ -34,7 +35,7 @@ function commachecker()
> > >               [ "$x" = "Failed" ] && continue
> > >
> > >               # Count the number of commas
> > > -             x=$(echo $line | tr -d -c ',')
> > > +             x=$(echo $line | tr -d -c $csv_sep)
> > >               cnt="${#x}"
> > >               # echo $line $cnt
> > >               [[ ! "$cnt" =~ $exp ]] && {
> > > @@ -54,7 +55,7 @@ function ParanoidAndNotRoot()
> > >  check_no_args()
> > >  {
> > >       echo -n "Checking CSV output: no args "
> > > -     perf stat -x, true 2>&1 | commachecker --no-args
> > > +     perf stat -x$csv_sep true 2>&1 | commachecker --no-args
> > >       echo "[Success]"
> > >  }
> > >
> > > @@ -66,7 +67,7 @@ check_system_wide()
> > >               echo "[Skip] paranoid and not root"
> > >               return
> > >       fi
> > > -     perf stat -x, -a true 2>&1 | commachecker --system-wide
> > > +     perf stat -x$csv_sep -a true 2>&1 | commachecker --system-wide
> > >       echo "[Success]"
> > >  }
> > >
> > > @@ -79,14 +80,14 @@ check_system_wide_no_aggr()
> > >               return
> > >       fi
> > >       echo -n "Checking CSV output: system wide no aggregation "
> > > -     perf stat -x, -A -a --no-merge true 2>&1 | commachecker --system-wide-no-aggr
> > > +     perf stat -x$csv_sep -A -a --no-merge true 2>&1 | commachecker --system-wide-no-aggr
> > >       echo "[Success]"
> > >  }
> > >
> > >  check_interval()
> > >  {
> > >       echo -n "Checking CSV output: interval "
> > > -     perf stat -x, -I 1000 true 2>&1 | commachecker --interval
> > > +     perf stat -x$csv_sep -I 1000 true 2>&1 | commachecker --interval
> > >       echo "[Success]"
> > >  }
> > >
> > > @@ -94,7 +95,7 @@ check_interval()
> > >  check_event()
> > >  {
> > >       echo -n "Checking CSV output: event "
> > > -     perf stat -x, -e cpu-clock true 2>&1 | commachecker --event
> > > +     perf stat -x$csv_sep -e cpu-clock true 2>&1 | commachecker --event
> > >       echo "[Success]"
> > >  }
> > >
> > > @@ -106,7 +107,7 @@ check_per_core()
> > >               echo "[Skip] paranoid and not root"
> > >               return
> > >       fi
> > > -     perf stat -x, --per-core -a true 2>&1 | commachecker --per-core
> > > +     perf stat -x$csv_sep --per-core -a true 2>&1 | commachecker --per-core
> > >       echo "[Success]"
> > >  }
> > >
> > > @@ -118,7 +119,7 @@ check_per_thread()
> > >               echo "[Skip] paranoid and not root"
> > >               return
> > >       fi
> > > -     perf stat -x, --per-thread -a true 2>&1 | commachecker --per-thread
> > > +     perf stat -x$csv_sep --per-thread -a true 2>&1 | commachecker --per-thread
> > >       echo "[Success]"
> > >  }
> > >
> > > @@ -130,7 +131,7 @@ check_per_die()
> > >               echo "[Skip] paranoid and not root"
> > >               return
> > >       fi
> > > -     perf stat -x, --per-die -a true 2>&1 | commachecker --per-die
> > > +     perf stat -x$csv_sep --per-die -a true 2>&1 | commachecker --per-die
> > >       echo "[Success]"
> > >  }
> > >
> > > @@ -142,7 +143,7 @@ check_per_node()
> > >               echo "[Skip] paranoid and not root"
> > >               return
> > >       fi
> > > -     perf stat -x, --per-node -a true 2>&1 | commachecker --per-node
> > > +     perf stat -x$csv_sep --per-node -a true 2>&1 | commachecker --per-node
> > >       echo "[Success]"
> > >  }
> > >
> > > @@ -154,7 +155,7 @@ check_per_socket()
> > >               echo "[Skip] paranoid and not root"
> > >               return
> > >       fi
> > > -     perf stat -x, --per-socket -a true 2>&1 | commachecker --per-socket
> > > +     perf stat -x$csv_sep --per-socket -a true 2>&1 | commachecker --per-socket
> > >       echo "[Success]"
> > >  }
> > >
> > > --
> > > 2.39.2.637.g21b0678d19-goog
> > >
> >
> > --
> >
> > - Arnaldo

-- 

- Arnaldo

Re: [PATCH v1 1/2] perf tests stat+csv_output: Switch CSV separator to @

[Ian Rogers]

On Thu, Mar 2, 2023 at 12:40 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Thu, Mar 02, 2023 at 11:30:36AM -0800, Ian Rogers escreveu:
> > On Thu, Feb 23, 2023 at 5:32 AM Arnaldo Carvalho de Melo
> > <acme@kernel.org> wrote:
> > >
> > > Em Wed, Feb 22, 2023 at 11:18:17PM -0800, Ian Rogers escreveu:
> > > > Commas may appear in events like:
> > > > cpu/INT_MISC.RECOVERY_CYCLES,cmask=1,edge/
> > > > which causes the commachecker to see more fields than expected. Use @
> > > > as the CSV separator to avoid this.
> > >
> > > Thanks, applied both patches.
> >
> > Thanks Arnaldo, I don't see the patches in the git branches so perhaps
> > something went wrong?
>
> Its in my local branch, I'll push it.

Thanks Arnaldo! I see the change in perf-tools and perf/urgent which
means they should appear in Linux 6.3. How do these things get merged
into perf-tools-next? I'm building upon them for changes targeting
Linux 6.4.

Thanks,
Ian

> > Ian
> >
> > > - Arnaldo
> > >
> > >
> > > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > > ---
> > > >  tools/perf/tests/shell/stat+csv_output.sh | 23 ++++++++++++-----------
> > > >  1 file changed, 12 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/tools/perf/tests/shell/stat+csv_output.sh b/tools/perf/tests/shell/stat+csv_output.sh
> > > > index b7f050aa6210..324fc9e6edd7 100755
> > > > --- a/tools/perf/tests/shell/stat+csv_output.sh
> > > > +++ b/tools/perf/tests/shell/stat+csv_output.sh
> > > > @@ -7,6 +7,7 @@
> > > >  set -e
> > > >
> > > >  skip_test=0
> > > > +csv_sep=@
> > > >
> > > >  function commachecker()
> > > >  {
> > > > @@ -34,7 +35,7 @@ function commachecker()
> > > >               [ "$x" = "Failed" ] && continue
> > > >
> > > >               # Count the number of commas
> > > > -             x=$(echo $line | tr -d -c ',')
> > > > +             x=$(echo $line | tr -d -c $csv_sep)
> > > >               cnt="${#x}"
> > > >               # echo $line $cnt
> > > >               [[ ! "$cnt" =~ $exp ]] && {
> > > > @@ -54,7 +55,7 @@ function ParanoidAndNotRoot()
> > > >  check_no_args()
> > > >  {
> > > >       echo -n "Checking CSV output: no args "
> > > > -     perf stat -x, true 2>&1 | commachecker --no-args
> > > > +     perf stat -x$csv_sep true 2>&1 | commachecker --no-args
> > > >       echo "[Success]"
> > > >  }
> > > >
> > > > @@ -66,7 +67,7 @@ check_system_wide()
> > > >               echo "[Skip] paranoid and not root"
> > > >               return
> > > >       fi
> > > > -     perf stat -x, -a true 2>&1 | commachecker --system-wide
> > > > +     perf stat -x$csv_sep -a true 2>&1 | commachecker --system-wide
> > > >       echo "[Success]"
> > > >  }
> > > >
> > > > @@ -79,14 +80,14 @@ check_system_wide_no_aggr()
> > > >               return
> > > >       fi
> > > >       echo -n "Checking CSV output: system wide no aggregation "
> > > > -     perf stat -x, -A -a --no-merge true 2>&1 | commachecker --system-wide-no-aggr
> > > > +     perf stat -x$csv_sep -A -a --no-merge true 2>&1 | commachecker --system-wide-no-aggr
> > > >       echo "[Success]"
> > > >  }
> > > >
> > > >  check_interval()
> > > >  {
> > > >       echo -n "Checking CSV output: interval "
> > > > -     perf stat -x, -I 1000 true 2>&1 | commachecker --interval
> > > > +     perf stat -x$csv_sep -I 1000 true 2>&1 | commachecker --interval
> > > >       echo "[Success]"
> > > >  }
> > > >
> > > > @@ -94,7 +95,7 @@ check_interval()
> > > >  check_event()
> > > >  {
> > > >       echo -n "Checking CSV output: event "
> > > > -     perf stat -x, -e cpu-clock true 2>&1 | commachecker --event
> > > > +     perf stat -x$csv_sep -e cpu-clock true 2>&1 | commachecker --event
> > > >       echo "[Success]"
> > > >  }
> > > >
> > > > @@ -106,7 +107,7 @@ check_per_core()
> > > >               echo "[Skip] paranoid and not root"
> > > >               return
> > > >       fi
> > > > -     perf stat -x, --per-core -a true 2>&1 | commachecker --per-core
> > > > +     perf stat -x$csv_sep --per-core -a true 2>&1 | commachecker --per-core
> > > >       echo "[Success]"
> > > >  }
> > > >
> > > > @@ -118,7 +119,7 @@ check_per_thread()
> > > >               echo "[Skip] paranoid and not root"
> > > >               return
> > > >       fi
> > > > -     perf stat -x, --per-thread -a true 2>&1 | commachecker --per-thread
> > > > +     perf stat -x$csv_sep --per-thread -a true 2>&1 | commachecker --per-thread
> > > >       echo "[Success]"
> > > >  }
> > > >
> > > > @@ -130,7 +131,7 @@ check_per_die()
> > > >               echo "[Skip] paranoid and not root"
> > > >               return
> > > >       fi
> > > > -     perf stat -x, --per-die -a true 2>&1 | commachecker --per-die
> > > > +     perf stat -x$csv_sep --per-die -a true 2>&1 | commachecker --per-die
> > > >       echo "[Success]"
> > > >  }
> > > >
> > > > @@ -142,7 +143,7 @@ check_per_node()
> > > >               echo "[Skip] paranoid and not root"
> > > >               return
> > > >       fi
> > > > -     perf stat -x, --per-node -a true 2>&1 | commachecker --per-node
> > > > +     perf stat -x$csv_sep --per-node -a true 2>&1 | commachecker --per-node
> > > >       echo "[Success]"
> > > >  }
> > > >
> > > > @@ -154,7 +155,7 @@ check_per_socket()
> > > >               echo "[Skip] paranoid and not root"
> > > >               return
> > > >       fi
> > > > -     perf stat -x, --per-socket -a true 2>&1 | commachecker --per-socket
> > > > +     perf stat -x$csv_sep --per-socket -a true 2>&1 | commachecker --per-socket
> > > >       echo "[Success]"
> > > >  }
> > > >
> > > > --
> > > > 2.39.2.637.g21b0678d19-goog
> > > >
> > >
> > > --
> > >
> > > - Arnaldo
>
> --
>
> - Arnaldo

Re: [PATCH v1 1/2] perf tests stat+csv_output: Switch CSV separator to @

[Arnaldo Carvalho de Melo]

Em Fri, Mar 03, 2023 at 04:15:17PM -0800, Ian Rogers escreveu:
> On Thu, Mar 2, 2023 at 12:40 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > Em Thu, Mar 02, 2023 at 11:30:36AM -0800, Ian Rogers escreveu:
> > > On Thu, Feb 23, 2023 at 5:32 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > > Em Wed, Feb 22, 2023 at 11:18:17PM -0800, Ian Rogers escreveu:
> > > > > Commas may appear in events like:
> > > > > cpu/INT_MISC.RECOVERY_CYCLES,cmask=1,edge/
> > > > > which causes the commachecker to see more fields than expected. Use @
> > > > > as the CSV separator to avoid this.
> > > >
> > > > Thanks, applied both patches.
> > >
> > > Thanks Arnaldo, I don't see the patches in the git branches so perhaps
> > > something went wrong?
> >
> > Its in my local branch, I'll push it.
> 
> Thanks Arnaldo! I see the change in perf-tools and perf/urgent which
> means they should appear in Linux 6.3. How do these things get merged
> into perf-tools-next? I'm building upon them for changes targeting
> Linux 6.4.

I'll merge perf-tools into perf-tools-next as soon as Linus merges it.

Till then you can either to the merge yourself and continue working
while the merge upstream happens or perhaps cherry-pick just that
specific cset, then when a merge happens later, it will be noticed as
already applied and skipped.

I'll push some more changes I have to perf-tools (the old perf/urgent),
namely syncing the kernel headers with the copies in tools/ and then
push to Linus after it sits a few days on linux-next/pending-fixes.

- Arnaldo