[LTP] [PATCH] fzsync: don't compare spins_avg (float) against 1 (int)

All of lore.kernel.org
 help / color / mirror / Atom feed

* [LTP] [PATCH] fzsync: don't compare spins_avg (float) against 1 (int)
@ 2019-09-04 14:20 Jan Stancek
  2019-09-05  7:23 ` Richard Palethorpe
  2019-09-05 12:08 ` [LTP] [PATCH v2] fzsync: drop pair->spins_avg.avg check Jan Stancek
  0 siblings, 2 replies; 6+ messages in thread
From: Jan Stancek @ 2019-09-04 14:20 UTC (permalink / raw)
  To: ltp

tst_fzsync_pair.spins is increased at least by one on every iteration.
If during all calibration loops, both A and B manage to complete at
nearly same time, then spins is increased only by one.

spins_avg starts as 0, and tst_exp_moving_avg() will move it closer
and closer to 1, but it will never be 1:
        float f = 0; int i;
        for (i = 0; i < 4096; i++)
                f = tst_exp_moving_avg(0.25f, 1, f);
        printf("%.15f %d\n", f, f >= 1);
	$ 0.999999880790710 0

which on rare occasion can cause: "Can't calculate random delay".

Compare against float slightly smaller than 1.
Also print stats one more time when we hit the warning.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 include/tst_fuzzy_sync.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h
index de0402c9bbe9..2c0389c5d63e 100644
--- a/include/tst_fuzzy_sync.h
+++ b/include/tst_fuzzy_sync.h
@@ -73,6 +73,8 @@
 /* how much of exec time is sampling allowed to take */
 #define SAMPLING_SLICE 0.5f
 
+#define EPSILON 0.999f
+
 /** Some statistics for a variable */
 struct tst_fzsync_stat {
 	float avg;
@@ -477,7 +479,7 @@ static void tst_fzsync_pair_update(struct tst_fzsync_pair *pair)
 			tst_res(TINFO, "Minimum sampling period ended");
 			tst_fzsync_pair_info(pair);
 		}
-	} else if (fabsf(pair->diff_ab.avg) >= 1 && pair->spins_avg.avg >= 1) {
+	} else if (fabsf(pair->diff_ab.avg) > EPSILON && pair->spins_avg.avg > EPSILON) {
 		per_spin_time = fabsf(pair->diff_ab.avg) / pair->spins_avg.avg;
 		time_delay = drand48() * (pair->diff_sa.avg + pair->diff_sb.avg)
 			- pair->diff_sb.avg;
@@ -495,6 +497,7 @@ static void tst_fzsync_pair_update(struct tst_fzsync_pair *pair)
 		}
 	} else if (!pair->sampling) {
 		tst_res(TWARN, "Can't calculate random delay");
+		tst_fzsync_pair_info(pair);
 		pair->sampling = -1;
 	}
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [LTP] [PATCH] fzsync: don't compare spins_avg (float) against 1 (int)
  2019-09-04 14:20 [LTP] [PATCH] fzsync: don't compare spins_avg (float) against 1 (int) Jan Stancek
@ 2019-09-05  7:23 ` Richard Palethorpe
  2019-09-05  7:39   ` Jan Stancek
  2019-09-05 12:08 ` [LTP] [PATCH v2] fzsync: drop pair->spins_avg.avg check Jan Stancek
  1 sibling, 1 reply; 6+ messages in thread
From: Richard Palethorpe @ 2019-09-05  7:23 UTC (permalink / raw)
  To: ltp

Hello,

Jan Stancek <jstancek@redhat.com> writes:

> tst_fzsync_pair.spins is increased at least by one on every iteration.
> If during all calibration loops, both A and B manage to complete at
> nearly same time, then spins is increased only by one.
>
> spins_avg starts as 0, and tst_exp_moving_avg() will move it closer
> and closer to 1, but it will never be 1:
>         float f = 0; int i;
>         for (i = 0; i < 4096; i++)
>                 f = tst_exp_moving_avg(0.25f, 1, f);
>         printf("%.15f %d\n", f, f >= 1);
> 	$ 0.999999880790710 0
>
> which on rare occasion can cause: "Can't calculate random delay".

I'm surprised that you have seen this. Out of interest, does this happen
on a particular setup, or just randomly?

>
> Compare against float slightly smaller than 1.
> Also print stats one more time when we hit the warning.
>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>  include/tst_fuzzy_sync.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h
> index de0402c9bbe9..2c0389c5d63e 100644
> --- a/include/tst_fuzzy_sync.h
> +++ b/include/tst_fuzzy_sync.h
> @@ -73,6 +73,8 @@
>  /* how much of exec time is sampling allowed to take */
>  #define SAMPLING_SLICE 0.5f
>
> +#define EPSILON 0.999f
> +
>  /** Some statistics for a variable */
>  struct tst_fzsync_stat {
>  	float avg;
> @@ -477,7 +479,7 @@ static void tst_fzsync_pair_update(struct tst_fzsync_pair *pair)
>  			tst_res(TINFO, "Minimum sampling period ended");
>  			tst_fzsync_pair_info(pair);
>  		}
> -	} else if (fabsf(pair->diff_ab.avg) >= 1 && pair->spins_avg.avg >= 1) {
> +	} else if (fabsf(pair->diff_ab.avg) > EPSILON && pair->spins_avg.avg > EPSILON) {
>  		per_spin_time = fabsf(pair->diff_ab.avg) / pair->spins_avg.avg;

I suppose you could also use MAX(pair->spins_avg.avg, 1f) and drop the
check. It might even be OK to only check that it is greater than
zero. I'm not sure if the number of delay spins will be sane either way,
but it probably can't do any harm.

>  		time_delay = drand48() * (pair->diff_sa.avg + pair->diff_sb.avg)
>  			- pair->diff_sb.avg;
> @@ -495,6 +497,7 @@ static void tst_fzsync_pair_update(struct tst_fzsync_pair *pair)
>  		}
>  	} else if (!pair->sampling) {
>  		tst_res(TWARN, "Can't calculate random delay");
> +		tst_fzsync_pair_info(pair);
>  		pair->sampling = -1;
>  	}


--
Thank you,
Richard.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [LTP] [PATCH] fzsync: don't compare spins_avg (float) against 1 (int)
  2019-09-05  7:23 ` Richard Palethorpe
@ 2019-09-05  7:39   ` Jan Stancek
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Stancek @ 2019-09-05  7:39 UTC (permalink / raw)
  To: ltp


----- Original Message -----
> Hello,
> 
> Jan Stancek <jstancek@redhat.com> writes:
> 
> > tst_fzsync_pair.spins is increased at least by one on every iteration.
> > If during all calibration loops, both A and B manage to complete at
> > nearly same time, then spins is increased only by one.
> >
> > spins_avg starts as 0, and tst_exp_moving_avg() will move it closer
> > and closer to 1, but it will never be 1:
> >         float f = 0; int i;
> >         for (i = 0; i < 4096; i++)
> >                 f = tst_exp_moving_avg(0.25f, 1, f);
> >         printf("%.15f %d\n", f, f >= 1);
> > 	$ 0.999999880790710 0
> >
> > which on rare occasion can cause: "Can't calculate random delay".
> 
> I'm surprised that you have seen this. Out of interest, does this happen
> on a particular setup, or just randomly?

It happens very rarely on power9 lpar running fedora 30 and cve-2017-2671,
where there's only single syscall in fuzzing section.

It takes couple hours to reproduce on demand.

> 
> >
> > Compare against float slightly smaller than 1.
> > Also print stats one more time when we hit the warning.
> >
> > Signed-off-by: Jan Stancek <jstancek@redhat.com>
> > ---
> >  include/tst_fuzzy_sync.h | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h
> > index de0402c9bbe9..2c0389c5d63e 100644
> > --- a/include/tst_fuzzy_sync.h
> > +++ b/include/tst_fuzzy_sync.h
> > @@ -73,6 +73,8 @@
> >  /* how much of exec time is sampling allowed to take */
> >  #define SAMPLING_SLICE 0.5f
> >
> > +#define EPSILON 0.999f
> > +
> >  /** Some statistics for a variable */
> >  struct tst_fzsync_stat {
> >  	float avg;
> > @@ -477,7 +479,7 @@ static void tst_fzsync_pair_update(struct
> > tst_fzsync_pair *pair)
> >  			tst_res(TINFO, "Minimum sampling period ended");
> >  			tst_fzsync_pair_info(pair);
> >  		}
> > -	} else if (fabsf(pair->diff_ab.avg) >= 1 && pair->spins_avg.avg >= 1) {
> > +	} else if (fabsf(pair->diff_ab.avg) > EPSILON && pair->spins_avg.avg >
> > EPSILON) {
> >  		per_spin_time = fabsf(pair->diff_ab.avg) / pair->spins_avg.avg;
> 
> I suppose you could also use MAX(pair->spins_avg.avg, 1f) and drop the
> check. It might even be OK to only check that it is greater than
> zero. I'm not sure if the number of delay spins will be sane either way,
> but it probably can't do any harm.

I'll send v2.

Thanks,
Jan

> 
> >  		time_delay = drand48() * (pair->diff_sa.avg + pair->diff_sb.avg)
> >  			- pair->diff_sb.avg;
> > @@ -495,6 +497,7 @@ static void tst_fzsync_pair_update(struct
> > tst_fzsync_pair *pair)
> >  		}
> >  	} else if (!pair->sampling) {
> >  		tst_res(TWARN, "Can't calculate random delay");
> > +		tst_fzsync_pair_info(pair);
> >  		pair->sampling = -1;
> >  	}
> 
> 
> --
> Thank you,
> Richard.
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [LTP] [PATCH v2] fzsync: drop pair->spins_avg.avg check
  2019-09-04 14:20 [LTP] [PATCH] fzsync: don't compare spins_avg (float) against 1 (int) Jan Stancek
  2019-09-05  7:23 ` Richard Palethorpe
@ 2019-09-05 12:08 ` Jan Stancek
  2019-09-05 12:29   ` Richard Palethorpe
  1 sibling, 1 reply; 6+ messages in thread
From: Jan Stancek @ 2019-09-05 12:08 UTC (permalink / raw)
  To: ltp

tst_fzsync_pair.spins is increased at least by one on every iteration.
If during all calibration loops, both A and B manage to complete at
nearly same time, then spins is increased only by one.

spins_avg starts as 0, and tst_exp_moving_avg() will move it closer
and closer to 1, but it will never be 1:
        float f = 0; int i;
        for (i = 0; i < 4096; i++)
                f = tst_exp_moving_avg(0.25f, 1, f);
        printf("%.15f %d\n", f, f >= 1);
	$ 0.999999880790710 0
which on rare occasion can cause: "Can't calculate random delay".

Drop check and use MAX(pair->spins_avg.avg, 1f) to calculate per_spin_time.
Also print stats one more time when we hit the warning.

Suggested-by: Richard Palethorpe <rpalethorpe@suse.com>
Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 include/tst_fuzzy_sync.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h
index de0402c9bbe9..f9a1947c7542 100644
--- a/include/tst_fuzzy_sync.h
+++ b/include/tst_fuzzy_sync.h
@@ -477,8 +477,8 @@ static void tst_fzsync_pair_update(struct tst_fzsync_pair *pair)
 			tst_res(TINFO, "Minimum sampling period ended");
 			tst_fzsync_pair_info(pair);
 		}
-	} else if (fabsf(pair->diff_ab.avg) >= 1 && pair->spins_avg.avg >= 1) {
-		per_spin_time = fabsf(pair->diff_ab.avg) / pair->spins_avg.avg;
+	} else if (fabsf(pair->diff_ab.avg) >= 1) {
+		per_spin_time = fabsf(pair->diff_ab.avg) / MAX(pair->spins_avg.avg, 1.0f);
 		time_delay = drand48() * (pair->diff_sa.avg + pair->diff_sb.avg)
 			- pair->diff_sb.avg;
 		pair->delay += (int)(time_delay / per_spin_time);
@@ -495,6 +495,7 @@ static void tst_fzsync_pair_update(struct tst_fzsync_pair *pair)
 		}
 	} else if (!pair->sampling) {
 		tst_res(TWARN, "Can't calculate random delay");
+		tst_fzsync_pair_info(pair);
 		pair->sampling = -1;
 	}
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [LTP] [PATCH v2] fzsync: drop pair->spins_avg.avg check
  2019-09-05 12:08 ` [LTP] [PATCH v2] fzsync: drop pair->spins_avg.avg check Jan Stancek
@ 2019-09-05 12:29   ` Richard Palethorpe
  2019-09-07  9:31     ` Jan Stancek
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Palethorpe @ 2019-09-05 12:29 UTC (permalink / raw)
  To: ltp

Hello,

Jan Stancek <jstancek@redhat.com> writes:

> tst_fzsync_pair.spins is increased at least by one on every iteration.
> If during all calibration loops, both A and B manage to complete at
> nearly same time, then spins is increased only by one.
>
> spins_avg starts as 0, and tst_exp_moving_avg() will move it closer
> and closer to 1, but it will never be 1:
>         float f = 0; int i;
>         for (i = 0; i < 4096; i++)
>                 f = tst_exp_moving_avg(0.25f, 1, f);
>         printf("%.15f %d\n", f, f >= 1);
> 	$ 0.999999880790710 0
> which on rare occasion can cause: "Can't calculate random delay".
>
> Drop check and use MAX(pair->spins_avg.avg, 1f) to calculate per_spin_time.
> Also print stats one more time when we hit the warning.
>
> Suggested-by: Richard Palethorpe <rpalethorpe@suse.com>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>  include/tst_fuzzy_sync.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h
> index de0402c9bbe9..f9a1947c7542 100644
> --- a/include/tst_fuzzy_sync.h
> +++ b/include/tst_fuzzy_sync.h
> @@ -477,8 +477,8 @@ static void tst_fzsync_pair_update(struct tst_fzsync_pair *pair)
>  			tst_res(TINFO, "Minimum sampling period ended");
>  			tst_fzsync_pair_info(pair);
>  		}
> -	} else if (fabsf(pair->diff_ab.avg) >= 1 && pair->spins_avg.avg >= 1) {
> -		per_spin_time = fabsf(pair->diff_ab.avg) / pair->spins_avg.avg;
> +	} else if (fabsf(pair->diff_ab.avg) >= 1) {
> +		per_spin_time = fabsf(pair->diff_ab.avg) / MAX(pair->spins_avg.avg, 1.0f);
>  		time_delay = drand48() * (pair->diff_sa.avg + pair->diff_sb.avg)
>  			- pair->diff_sb.avg;
>  		pair->delay += (int)(time_delay / per_spin_time);
> @@ -495,6 +495,7 @@ static void tst_fzsync_pair_update(struct tst_fzsync_pair *pair)
>  		}
>  	} else if (!pair->sampling) {
>  		tst_res(TWARN, "Can't calculate random delay");
> +		tst_fzsync_pair_info(pair);
>  		pair->sampling = -1;
>  	}

LGTM

-- 
Thank you,
Richard.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [LTP] [PATCH v2] fzsync: drop pair->spins_avg.avg check
  2019-09-05 12:29   ` Richard Palethorpe
@ 2019-09-07  9:31     ` Jan Stancek
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Stancek @ 2019-09-07  9:31 UTC (permalink / raw)
  To: ltp



----- Original Message -----
> Hello,
> 
> Jan Stancek <jstancek@redhat.com> writes:
> 
> > tst_fzsync_pair.spins is increased at least by one on every iteration.
> > If during all calibration loops, both A and B manage to complete at
> > nearly same time, then spins is increased only by one.
> >
> > spins_avg starts as 0, and tst_exp_moving_avg() will move it closer
> > and closer to 1, but it will never be 1:
> >         float f = 0; int i;
> >         for (i = 0; i < 4096; i++)
> >                 f = tst_exp_moving_avg(0.25f, 1, f);
> >         printf("%.15f %d\n", f, f >= 1);
> > 	$ 0.999999880790710 0
> > which on rare occasion can cause: "Can't calculate random delay".
> >
> > Drop check and use MAX(pair->spins_avg.avg, 1f) to calculate per_spin_time.
> > Also print stats one more time when we hit the warning.
> >
> > Suggested-by: Richard Palethorpe <rpalethorpe@suse.com>
> > Signed-off-by: Jan Stancek <jstancek@redhat.com>
> > ---
> >  include/tst_fuzzy_sync.h | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h
> > index de0402c9bbe9..f9a1947c7542 100644
> > --- a/include/tst_fuzzy_sync.h
> > +++ b/include/tst_fuzzy_sync.h
> > @@ -477,8 +477,8 @@ static void tst_fzsync_pair_update(struct
> > tst_fzsync_pair *pair)
> >  			tst_res(TINFO, "Minimum sampling period ended");
> >  			tst_fzsync_pair_info(pair);
> >  		}
> > -	} else if (fabsf(pair->diff_ab.avg) >= 1 && pair->spins_avg.avg >= 1) {
> > -		per_spin_time = fabsf(pair->diff_ab.avg) / pair->spins_avg.avg;
> > +	} else if (fabsf(pair->diff_ab.avg) >= 1) {
> > +		per_spin_time = fabsf(pair->diff_ab.avg) / MAX(pair->spins_avg.avg,
> > 1.0f);
> >  		time_delay = drand48() * (pair->diff_sa.avg + pair->diff_sb.avg)
> >  			- pair->diff_sb.avg;
> >  		pair->delay += (int)(time_delay / per_spin_time);
> > @@ -495,6 +495,7 @@ static void tst_fzsync_pair_update(struct
> > tst_fzsync_pair *pair)
> >  		}
> >  	} else if (!pair->sampling) {
> >  		tst_res(TWARN, "Can't calculate random delay");
> > +		tst_fzsync_pair_info(pair);
> >  		pair->sampling = -1;
> >  	}
> 
> LGTM

Pushed.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-09-07  9:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-04 14:20 [LTP] [PATCH] fzsync: don't compare spins_avg (float) against 1 (int) Jan Stancek
2019-09-05  7:23 ` Richard Palethorpe
2019-09-05  7:39   ` Jan Stancek
2019-09-05 12:08 ` [LTP] [PATCH v2] fzsync: drop pair->spins_avg.avg check Jan Stancek
2019-09-05 12:29   ` Richard Palethorpe
2019-09-07  9:31     ` Jan Stancek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.