[PATCH 1/2] perf test lock_contention.sh: Skip test if the number of CPUs is low

[PATCH 1/2] perf test lock_contention.sh: Skip test if the number of CPUs is low

[vmolnaro]

From: Veronika Molnarova <vmolnaro@redhat.com>

Machines with less then 4 CPUs weren't consistently triggering lock
events required for the test.

Skip the test on those machines. The limit of 4 CPUs is set as it
generates around 100 lock events for a test.
---
 tools/perf/tests/shell/lock_contention.sh | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/tests/shell/lock_contention.sh b/tools/perf/tests/shell/lock_contention.sh
index f2cc187b618..1d3598ff6a0 100755
--- a/tools/perf/tests/shell/lock_contention.sh
+++ b/tools/perf/tests/shell/lock_contention.sh
@@ -31,6 +31,12 @@ check() {
 		echo "[Skip] No lock contention tracepoints"
 		err=2
 		exit
+	fi 
+
+	if [ `nproc` -lt 4 ]; then
+		echo "[Skip] Low number of CPUs (`nproc`), lock event cannot be triggered certainly"
+		err=2
+		exit
 	fi
 }
 
-- 
2.41.0

[PATCH 2/2] perf test stat+shadow_stat.sh: Add threshold for rounding errors

[vmolnaro]

From: Veronika Molnarova <vmolnaro@redhat.com>

The test was failing in specific scenarios due to imperfection of FP
arithmetics. The `bc` command wasn't correctly rounding the result
of division causing the failure.

Replace the `bc` with `awk` which should work with more decimal
places and add a threshold to catch any possible rounding errors.
The acceptable rounding error is set to 0.01 when the test
passes with a warning message.
---
 tools/perf/tests/shell/stat+shadow_stat.sh | 30 +++++++++++++++++-----
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/tools/perf/tests/shell/stat+shadow_stat.sh b/tools/perf/tests/shell/stat+shadow_stat.sh
index 0e9cba84e75..73c990fe5ee 100755
--- a/tools/perf/tests/shell/stat+shadow_stat.sh
+++ b/tools/perf/tests/shell/stat+shadow_stat.sh
@@ -4,6 +4,8 @@
 
 set -e
 
+TRESHOLD=0.015
+
 # skip if system-wide mode is forbidden
 perf stat -a true > /dev/null 2>&1 || exit 2
 
@@ -33,10 +35,18 @@ test_global_aggr()
 		fi
 
 		# use printf for rounding and a leading zero
-		res=`printf "%.2f" "$(echo "scale=6; $num / $cyc" | bc -q)"`
+		res=`echo $num $cyc | awk '{printf "%.2f", $1 / $2}'`
 		if [ "$ipc" != "$res" ]; then
-			echo "IPC is different: $res != $ipc  ($num / $cyc)"
-			exit 1
+			# check the difference from the real result for FP imperfections
+			diff=`echo $ipc $res $TRESHOLD | \
+			awk '{x = ($1 - $2) < 0 ? ($2 - $1) : ($1 - $2); print (x > $3)}'`
+
+			if [ $diff -eq 1 ]; then
+				echo "IPC is different: $res != $ipc  ($num / $cyc)"
+				exit 1
+			fi
+
+			echo "Warning: Difference of IPC is under the treshold"
 		fi
 	done
 }
@@ -67,10 +77,18 @@ test_no_aggr()
 		fi
 
 		# use printf for rounding and a leading zero
-		res=`printf "%.2f" "$(echo "scale=6; $num / $cyc" | bc -q)"`
+		res=`echo $num $cyc | awk '{printf "%.2f", $1 / $2}'`
 		if [ "$ipc" != "$res" ]; then
-			echo "IPC is different for $cpu: $res != $ipc  ($num / $cyc)"
-			exit 1
+			# check difference from the real result for FP imperfections
+			diff=`echo $ipc $res $TRESHOLD | \
+			awk '{x = ($1 - $2) < 0 ? ($2 - $1) : ($1 - $2); print (x > $3)}'`
+
+			if [ $diff -eq 1 ]; then
+				echo "IPC is different: $res != $ipc  ($num / $cyc)"
+				exit 1
+			fi
+
+			echo "Warning: Difference of IPC is under the treshold"
 		fi
 	done
 }
-- 
2.41.0

Re: [PATCH 2/2] perf test stat+shadow_stat.sh: Add threshold for rounding errors

[Michael Petlan]

On Tue, 12 Sep 2023, vmolnaro@redhat.com wrote:
> From: Veronika Molnarova <vmolnaro@redhat.com>
> 
> The test was failing in specific scenarios due to imperfection of FP
> arithmetics. The `bc` command wasn't correctly rounding the result
> of division causing the failure.
> 
> Replace the `bc` with `awk` which should work with more decimal
> places and add a threshold to catch any possible rounding errors.
> The acceptable rounding error is set to 0.01 when the test
> passes with a warning message.

Acked-by: Michael Petlan <mpetlan@redhat.com>
> ---
>  tools/perf/tests/shell/stat+shadow_stat.sh | 30 +++++++++++++++++-----
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/tests/shell/stat+shadow_stat.sh b/tools/perf/tests/shell/stat+shadow_stat.sh
> index 0e9cba84e75..73c990fe5ee 100755
> --- a/tools/perf/tests/shell/stat+shadow_stat.sh
> +++ b/tools/perf/tests/shell/stat+shadow_stat.sh
> @@ -4,6 +4,8 @@
>  
>  set -e
>  
> +TRESHOLD=0.015
> +
>  # skip if system-wide mode is forbidden
>  perf stat -a true > /dev/null 2>&1 || exit 2
>  
> @@ -33,10 +35,18 @@ test_global_aggr()
>  		fi
>  
>  		# use printf for rounding and a leading zero
> -		res=`printf "%.2f" "$(echo "scale=6; $num / $cyc" | bc -q)"`
> +		res=`echo $num $cyc | awk '{printf "%.2f", $1 / $2}'`
>  		if [ "$ipc" != "$res" ]; then
> -			echo "IPC is different: $res != $ipc  ($num / $cyc)"
> -			exit 1
> +			# check the difference from the real result for FP imperfections
> +			diff=`echo $ipc $res $TRESHOLD | \
> +			awk '{x = ($1 - $2) < 0 ? ($2 - $1) : ($1 - $2); print (x > $3)}'`
> +
> +			if [ $diff -eq 1 ]; then
> +				echo "IPC is different: $res != $ipc  ($num / $cyc)"
> +				exit 1
> +			fi
> +
> +			echo "Warning: Difference of IPC is under the treshold"
>  		fi
>  	done
>  }
> @@ -67,10 +77,18 @@ test_no_aggr()
>  		fi
>  
>  		# use printf for rounding and a leading zero
> -		res=`printf "%.2f" "$(echo "scale=6; $num / $cyc" | bc -q)"`
> +		res=`echo $num $cyc | awk '{printf "%.2f", $1 / $2}'`
>  		if [ "$ipc" != "$res" ]; then
> -			echo "IPC is different for $cpu: $res != $ipc  ($num / $cyc)"
> -			exit 1
> +			# check difference from the real result for FP imperfections
> +			diff=`echo $ipc $res $TRESHOLD | \
> +			awk '{x = ($1 - $2) < 0 ? ($2 - $1) : ($1 - $2); print (x > $3)}'`
> +
> +			if [ $diff -eq 1 ]; then
> +				echo "IPC is different: $res != $ipc  ($num / $cyc)"
> +				exit 1
> +			fi
> +
> +			echo "Warning: Difference of IPC is under the treshold"
>  		fi
>  	done
>  }
> -- 
> 2.41.0
> 
> 

Re: [PATCH 2/2] perf test stat+shadow_stat.sh: Add threshold for rounding errors

[Arnaldo Carvalho de Melo]

Em Tue, Sep 12, 2023 at 02:19:26PM +0200, vmolnaro@redhat.com escreveu:
> From: Veronika Molnarova <vmolnaro@redhat.com>
> 
> The test was failing in specific scenarios due to imperfection of FP
> arithmetics. The `bc` command wasn't correctly rounding the result
> of division causing the failure.
> 
> Replace the `bc` with `awk` which should work with more decimal
> places and add a threshold to catch any possible rounding errors.
> The acceptable rounding error is set to 0.01 when the test
> passes with a warning message.
> ---
>  tools/perf/tests/shell/stat+shadow_stat.sh | 30 +++++++++++++++++-----
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/tests/shell/stat+shadow_stat.sh b/tools/perf/tests/shell/stat+shadow_stat.sh
> index 0e9cba84e75..73c990fe5ee 100755
> --- a/tools/perf/tests/shell/stat+shadow_stat.sh
> +++ b/tools/perf/tests/shell/stat+shadow_stat.sh
> @@ -4,6 +4,8 @@
>  
>  set -e
>  
> +TRESHOLD=0.015

You got it right on the patch description, I fixed it here as:

⬢[acme@toolbox perf-tools-next]$ git diff
diff --git a/tools/perf/tests/shell/stat+shadow_stat.sh b/tools/perf/tests/shell/stat+shadow_stat.sh
index 626d669158d2d089..a7e4a8f2ad186458 100755
--- a/tools/perf/tests/shell/stat+shadow_stat.sh
+++ b/tools/perf/tests/shell/stat+shadow_stat.sh
@@ -4,7 +4,7 @@

 set -e

-TRESHOLD=0.015
+THRESHOLD=0.015

 # skip if system-wide mode is forbidden
 perf stat -a true > /dev/null 2>&1 || exit 2
@@ -38,7 +38,7 @@ test_global_aggr()
                res=`echo $num $cyc | awk '{printf "%.2f", $1 / $2}'`
                if [ "$ipc" != "$res" ]; then
                        # check the difference from the real result for FP imperfections
-                       diff=`echo $ipc $res $TRESHOLD | \
+                       diff=`echo $ipc $res $THRESHOLD | \
                        awk '{x = ($1 - $2) < 0 ? ($2 - $1) : ($1 - $2); print (x > $3)}'`

                        if [ $diff -eq 1 ]; then
@@ -80,7 +80,7 @@ test_no_aggr()
                res=`echo $num $cyc | awk '{printf "%.2f", $1 / $2}'`
                if [ "$ipc" != "$res" ]; then
                        # check difference from the real result for FP imperfections
-                       diff=`echo $ipc $res $TRESHOLD | \
+                       diff=`echo $ipc $res $THRESHOLD | \
                        awk '{x = ($1 - $2) < 0 ? ($2 - $1) : ($1 - $2); print (x > $3)}'`

                        if [ $diff -eq 1 ]; then
⬢[acme@toolbox perf-tools-next]$

Re: [PATCH 1/2] perf test lock_contention.sh: Skip test if the number of CPUs is low

[Michael Petlan]

On Tue, 12 Sep 2023, vmolnaro@redhat.com wrote:
> From: Veronika Molnarova <vmolnaro@redhat.com>
> 
> Machines with less then 4 CPUs weren't consistently triggering lock
> events required for the test.
> 
> Skip the test on those machines. The limit of 4 CPUs is set as it
> generates around 100 lock events for a test.

Acked-by: Michael Petlan <mpetlan@redhat.com>
> ---
>  tools/perf/tests/shell/lock_contention.sh | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tools/perf/tests/shell/lock_contention.sh b/tools/perf/tests/shell/lock_contention.sh
> index f2cc187b618..1d3598ff6a0 100755
> --- a/tools/perf/tests/shell/lock_contention.sh
> +++ b/tools/perf/tests/shell/lock_contention.sh
> @@ -31,6 +31,12 @@ check() {
>  		echo "[Skip] No lock contention tracepoints"
>  		err=2
>  		exit
> +	fi 
> +
> +	if [ `nproc` -lt 4 ]; then
> +		echo "[Skip] Low number of CPUs (`nproc`), lock event cannot be triggered certainly"
> +		err=2
> +		exit
>  	fi
>  }
>  
> -- 
> 2.41.0
> 
> 

Re: [PATCH 1/2] perf test lock_contention.sh: Skip test if the number of CPUs is low

[Arnaldo Carvalho de Melo]

Em Tue, Sep 12, 2023 at 02:19:25PM +0200, vmolnaro@redhat.com escreveu:
> From: Veronika Molnarova <vmolnaro@redhat.com>
> 
> Machines with less then 4 CPUs weren't consistently triggering lock
> events required for the test.
> 
> Skip the test on those machines. The limit of 4 CPUs is set as it
> generates around 100 lock events for a test.

Better, but still some issues, fixing manually.

⬢[acme@toolbox perf-tools-next]$ b4 am -ctsl --cc-trailers 20230912121926.12111-1-vmolnaro@redhat.com
Grabbing thread from lore.kernel.org/all/20230912121926.12111-1-vmolnaro%40redhat.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 4 messages in the thread
Checking attestation on all messages, may take a moment...
---
  ✓ [PATCH 1/2] perf test lock_contention.sh: Skip test if the number of CPUs is low
    + Acked-by: Michael Petlan <mpetlan@redhat.com> (✓ DKIM/redhat.com)
    + Link: https://lore.kernel.org/r/20230912121926.12111-1-vmolnaro@redhat.com
    + Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
  ✓ [PATCH 2/2] perf test stat+shadow_stat.sh: Add threshold for rounding errors
    + Acked-by: Michael Petlan <mpetlan@redhat.com> (✓ DKIM/redhat.com)
    + Link: https://lore.kernel.org/r/20230912121926.12111-2-vmolnaro@redhat.com
    + Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
  ---
  ✓ Signed: DKIM/redhat.com
---
Total patches: 2
---
 Link: https://lore.kernel.org/r/20230912121926.12111-1-vmolnaro@redhat.com
 Base: not specified
       git am ./20230912_vmolnaro_perf_test_lock_contention_sh_skip_test_if_the_number_of_cpus_is_low.mbx
⬢[acme@toolbox perf-tools-next]$ git diff
⬢[acme@toolbox perf-tools-next]$        git am ./20230912_vmolnaro_perf_test_lock_contention_sh_skip_test_if_the_number_of_cpus_is_low.mbx
Applying: perf test lock_contention.sh: Skip test if the number of CPUs is low
.git/rebase-apply/patch:13: trailing whitespace.
	fi 
warning: 1 line adds whitespace errors.
Applying: perf test stat+shadow_stat.sh: Add threshold for rounding errors
⬢[acme@toolbox perf-tools-next]$ 


> ---
>  tools/perf/tests/shell/lock_contention.sh | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tools/perf/tests/shell/lock_contention.sh b/tools/perf/tests/shell/lock_contention.sh
> index f2cc187b618..1d3598ff6a0 100755
> --- a/tools/perf/tests/shell/lock_contention.sh
> +++ b/tools/perf/tests/shell/lock_contention.sh
> @@ -31,6 +31,12 @@ check() {
>  		echo "[Skip] No lock contention tracepoints"
>  		err=2
>  		exit
> +	fi 
> +
> +	if [ `nproc` -lt 4 ]; then
> +		echo "[Skip] Low number of CPUs (`nproc`), lock event cannot be triggered certainly"
> +		err=2
> +		exit
>  	fi
>  }
>  
> -- 
> 2.41.0
> 

-- 

- Arnaldo

[PATCH 1/2] perf test lock_contention.sh: Skip test if the number of CPUs is low

[vmolnaro]

From: notValord <v.m.veverka@gmail.com>

Machines with less then 4 CPUs weren't consistently triggering lock
events required for the test.

Skip the test on those machines. The limit of 4 CPUs is set as it
generates around 100 lock events for a test.
---
 tools/perf/tests/shell/lock_contention.sh | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/tests/shell/lock_contention.sh b/tools/perf/tests/shell/lock_contention.sh
index f2cc187b618..1d3598ff6a0 100755
--- a/tools/perf/tests/shell/lock_contention.sh
+++ b/tools/perf/tests/shell/lock_contention.sh
@@ -31,6 +31,12 @@ check() {
 		echo "[Skip] No lock contention tracepoints"
 		err=2
 		exit
+	fi 
+
+	if [ `nproc` -lt 4 ]; then
+		echo "[Skip] Low number of CPUs (`nproc`), lock event cannot be triggered certainly"
+		err=2
+		exit
 	fi
 }
 
-- 
2.41.0

Re: [PATCH 1/2] perf test lock_contention.sh: Skip test if the number of CPUs is low

[Michael Petlan]

On Mon, 21 Aug 2023, vmolnaro@redhat.com wrote:
> From: notValord <v.m.veverka@gmail.com>
> 
> Machines with less then 4 CPUs weren't consistently triggering lock
> events required for the test.
> 
> Skip the test on those machines. The limit of 4 CPUs is set as it
> generates around 100 lock events for a test.

Acked-by: Michael Petlan <mpetlan@redhat.com>

> ---
>  tools/perf/tests/shell/lock_contention.sh | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tools/perf/tests/shell/lock_contention.sh b/tools/perf/tests/shell/lock_contention.sh
> index f2cc187b618..1d3598ff6a0 100755
> --- a/tools/perf/tests/shell/lock_contention.sh
> +++ b/tools/perf/tests/shell/lock_contention.sh
> @@ -31,6 +31,12 @@ check() {
>  		echo "[Skip] No lock contention tracepoints"
>  		err=2
>  		exit
> +	fi 
> +
> +	if [ `nproc` -lt 4 ]; then
> +		echo "[Skip] Low number of CPUs (`nproc`), lock event cannot be triggered certainly"
> +		err=2
> +		exit
>  	fi
>  }
>  
> -- 
> 2.41.0
> 
>