2020-01-23 17:29:36

by Paul E. McKenney

Subject: [PATCH RFC locktorture] Print ratio of acquisitions, not failures

The __torture_print_stats() function in locktorture.c carefully
initializes local variable "min" to statp[0].n_lock_acquired, but
then compares it to statp[i].n_lock_fail. Given that the .n_lock_fail
field should normally be zero, and given the initialization, it seems
reasonable to display the maximum and minimum number acquisitions
instead of miscomputing the maximum and minimum number of failures.
This commit therefore switches from failures to acquisitions.

Reported-by: Will Deacon <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
Cc: Davidlohr Bueso <[email protected]>
Cc: Josh Triplett <[email protected]>
Cc: Peter Zijlstra <[email protected]>

diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 99475a6..687c1d8 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -696,10 +696,10 @@ static void __torture_print_stats(char *page,
if (statp[i].n_lock_fail)
fail = true;
sum += statp[i].n_lock_acquired;
- if (max < statp[i].n_lock_fail)
- max = statp[i].n_lock_fail;
- if (min > statp[i].n_lock_fail)
- min = statp[i].n_lock_fail;
+ if (max < statp[i].n_lock_acquired)
+ max = statp[i].n_lock_acquired;
+ if (min > statp[i].n_lock_acquired)
+ min = statp[i].n_lock_acquired;
}
page += sprintf(page,
"%s: Total: %lld Max/Min: %ld/%ld %s Fail: %d %s\n",

2020-01-31 13:16:08

by Will Deacon

Subject: Re: [PATCH RFC locktorture] Print ratio of acquisitions, not failures

On Thu, Jan 23, 2020 at 09:27:07AM -0800, Paul E. McKenney wrote:
> diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
> index 99475a6..687c1d8 100644
> --- a/kernel/locking/locktorture.c
> +++ b/kernel/locking/locktorture.c
> @@ -696,10 +696,10 @@ static void __torture_print_stats(char *page,
> if (statp[i].n_lock_fail)
> fail = true;
> sum += statp[i].n_lock_acquired;
> - if (max < statp[i].n_lock_fail)
> - max = statp[i].n_lock_fail;
> - if (min > statp[i].n_lock_fail)
> - min = statp[i].n_lock_fail;
> + if (max < statp[i].n_lock_acquired)
> + max = statp[i].n_lock_acquired;
> + if (min > statp[i].n_lock_acquired)
> + min = statp[i].n_lock_acquired;
> }
> page += sprintf(page,
> "%s: Total: %lld Max/Min: %ld/%ld %s Fail: %d %s\n",

Acked-by: Will Deacon <[email protected]>

Will

2020-01-31 17:52:20

by Davidlohr Bueso

Subject: Re: [PATCH RFC locktorture] Print ratio of acquisitions, not failures

On Thu, 23 Jan 2020, Paul E. McKenney wrote:

>The __torture_print_stats() function in locktorture.c carefully
>initializes local variable "min" to statp[0].n_lock_acquired, but
>then compares it to statp[i].n_lock_fail. Given that the .n_lock_fail
>field should normally be zero, and given the initialization, it seems
>reasonable to display the maximum and minimum number acquisitions
>instead of miscomputing the maximum and minimum number of failures.
>This commit therefore switches from failures to acquisitions.

This makes sense, thanks!

>
>Reported-by: Will Deacon <[email protected]>
>Signed-off-by: Paul E. McKenney <[email protected]>
>Cc: Davidlohr Bueso <[email protected]>
>Cc: Josh Triplett <[email protected]>
>Cc: Peter Zijlstra <[email protected]>

Acked-by: Davidlohr Bueso <[email protected]>