* [LTP] [PATCH] fzsync: limit sampling time @ 2018-11-29 11:09 Jan Stancek 2018-12-01 8:37 ` Li Wang 2018-12-01 9:12 ` [LTP] [PATCH v2] " Jan Stancek 0 siblings, 2 replies; 9+ messages in thread From: Jan Stancek @ 2018-11-29 11:09 UTC (permalink / raw) To: ltp Fixes: #429 Sampling can take considerably longer time on single CPU and very slow systems. This patch limits sampling time to 1/2 of fuzzing runtime (0.25 of test time). If we don't have enough samples by that time, stop sampling and use stats we gathered so far. Signed-off-by: Jan Stancek <jstancek@redhat.com> --- include/tst_fuzzy_sync.h | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h index 03f69b78bc82..0ca292c141c3 100644 --- a/include/tst_fuzzy_sync.h +++ b/include/tst_fuzzy_sync.h @@ -162,6 +162,7 @@ struct tst_fzsync_pair { * * Defaults to 0.5 (~150 seconds with default timeout). */ + float max_sampling_p; float exec_time_p; /** Internal; The test time remaining on tst_fzsync_pair_reset() */ float exec_time_start; @@ -199,6 +200,7 @@ static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair) CHK(avg_alpha, 0, 1, 0.25); CHK(min_samples, 20, INT_MAX, 1024); CHK(max_dev_ratio, 0, 1, 0.1); + CHK(max_sampling_p, 0, 1, 0.25); CHK(exec_time_p, 0, 1, 0.5); CHK(exec_loops, 20, INT_MAX, 3000000); } @@ -582,9 +584,18 @@ static inline void tst_fzsync_wait_b(struct tst_fzsync_pair *pair) static inline int tst_fzsync_run_a(struct tst_fzsync_pair *pair) { int exit = 0; + float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start; + + /* Limit amount of time spent on sampling */ + if ((pair->max_sampling_p < rem_p) + && (pair->sampling > 0)) { + tst_res(TINFO, "stopping sampling at %d samples", + pair->sampling); + pair->sampling = 0; + tst_fzsync_pair_info(pair); + } - if (pair->exec_time_p - < 1 - tst_timeout_remaining() / pair->exec_time_start) { + if (pair->exec_time_p < rem_p) { tst_res(TINFO, "Exceeded execution time, requesting exit"); exit = 1; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [LTP] [PATCH] fzsync: limit sampling time 2018-11-29 11:09 [LTP] [PATCH] fzsync: limit sampling time Jan Stancek @ 2018-12-01 8:37 ` Li Wang 2018-12-01 8:58 ` Jan Stancek 2018-12-01 9:12 ` [LTP] [PATCH v2] " Jan Stancek 1 sibling, 1 reply; 9+ messages in thread From: Li Wang @ 2018-12-01 8:37 UTC (permalink / raw) To: ltp On Thu, Nov 29, 2018 at 7:09 PM Jan Stancek <jstancek@redhat.com> wrote: > > Fixes: #429 > > Sampling can take considerably longer time on single CPU > and very slow systems. This patch limits sampling time to > 1/2 of fuzzing runtime (0.25 of test time). If we don't > have enough samples by that time, stop sampling and use > stats we gathered so far. This patch makes sense. > > Signed-off-by: Jan Stancek <jstancek@redhat.com> > --- > include/tst_fuzzy_sync.h | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h > index 03f69b78bc82..0ca292c141c3 100644 > --- a/include/tst_fuzzy_sync.h > +++ b/include/tst_fuzzy_sync.h > @@ -162,6 +162,7 @@ struct tst_fzsync_pair { > * > * Defaults to 0.5 (~150 seconds with default timeout). > */ > + float max_sampling_p; > float exec_time_p; > /** Internal; The test time remaining on tst_fzsync_pair_reset() */ > float exec_time_start; > @@ -199,6 +200,7 @@ static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair) > CHK(avg_alpha, 0, 1, 0.25); > CHK(min_samples, 20, INT_MAX, 1024); > CHK(max_dev_ratio, 0, 1, 0.1); > + CHK(max_sampling_p, 0, 1, 0.25); > CHK(exec_time_p, 0, 1, 0.5); > CHK(exec_loops, 20, INT_MAX, 3000000); > } > @@ -582,9 +584,18 @@ static inline void tst_fzsync_wait_b(struct tst_fzsync_pair *pair) > static inline int tst_fzsync_run_a(struct tst_fzsync_pair *pair) > { > int exit = 0; > + float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start; > + > + /* Limit amount of time spent on sampling */ > + if ((pair->max_sampling_p < rem_p) > + && (pair->sampling > 0)) { > + tst_res(TINFO, "stopping sampling at %d samples", > + pair->sampling); pair->sampling is the number of samples left, so I think the prints might be more precise if calculate the 'pair->min_samples - pair->sampling'(or pair->exec_loop)? > + pair->sampling = 0; > + tst_fzsync_pair_info(pair); > + } > > - if (pair->exec_time_p > - < 1 - tst_timeout_remaining() / pair->exec_time_start) { > + if (pair->exec_time_p < rem_p) { > tst_res(TINFO, > "Exceeded execution time, requesting exit"); > exit = 1; After involving max_sampling_p, here the warning prints will no longer be useful because it will never get 'pair->sampling > 0' when exec_time_p is exhausted. Base on your patch I suggest that: --- a/include/tst_fuzzy_sync.h +++ b/include/tst_fuzzy_sync.h @@ -590,7 +590,7 @@ static inline int tst_fzsync_run_a(struct tst_fzsync_pair *pair) if ((pair->max_sampling_p < rem_p) && (pair->sampling > 0)) { tst_res(TINFO, "stopping sampling at %d samples", - pair->sampling); + pair->exec_loop); pair->sampling = 0; tst_fzsync_pair_info(pair); } @@ -599,11 +599,6 @@ static inline int tst_fzsync_run_a(struct tst_fzsync_pair *pair) tst_res(TINFO, "Exceeded execution time, requesting exit"); exit = 1; - - if (pair->sampling > 0) { - tst_res(TWARN, - "Still sampling, consider increasing LTP_TIMEOUT_MUL"); - } } if (++pair->exec_loop > pair->exec_loops) { ======= Snip ======== Additionally, If we go this way to limit the sampling time, seems 'pair->min_samples' will become less meaning for fuzzy sync library, because we can just do sampling in a limited time and why we need the minimal samples? So what about using limited-time to replace min_samples? Maybe like: --- a/include/tst_fuzzy_sync.h +++ b/include/tst_fuzzy_sync.h @@ -123,19 +123,14 @@ struct tst_fzsync_pair { */ int delay; /** - * Internal; The number of samples left or the sampling state. + * Internal; The number of samples in the sampling phase. * - * A positive value is the number of remaining mandatory + * A positive value is the number of past mandatory * samples. Zero or a negative indicate some other state. */ int sampling; - /** - * The Minimum number of statistical samples which must be collected. - * - * The minimum number of iterations which must be performed before a - * random delay can be calculated. Defaults to 1024. - */ - int min_samples; + /** Internal; Exit sampling phase if this value is 1 */ + int sampling_exit; /** * The maximum allowed proportional average deviation. * @@ -198,7 +193,6 @@ struct tst_fzsync_pair { static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair) { CHK(avg_alpha, 0, 1, 0.25); - CHK(min_samples, 20, INT_MAX, 1024); CHK(max_dev_ratio, 0, 1, 0.1); CHK(max_sampling_p, 0, 1, 0.25); CHK(exec_time_p, 0, 1, 0.5); @@ -262,7 +256,8 @@ static void tst_fzsync_pair_reset(struct tst_fzsync_pair *pair, tst_init_stat(&pair->diff_ab); tst_init_stat(&pair->spins_avg); pair->delay = 0; - pair->sampling = pair->min_samples; + pair->sampling = 0; + pair->sampling_exit = 0; pair->exec_loop = 0; @@ -295,7 +290,6 @@ static inline void tst_fzsync_stat_info(struct tst_fzsync_stat stat, */ static void tst_fzsync_pair_info(struct tst_fzsync_pair *pair) { - tst_res(TINFO, "loop = %d", pair->exec_loop); tst_fzsync_stat_info(pair->diff_ss, "ns", "start_a - start_b"); tst_fzsync_stat_info(pair->diff_sa, "ns", "end_a - start_a"); tst_fzsync_stat_info(pair->diff_sb, "ns", "end_b - start_b"); @@ -450,6 +444,9 @@ static void tst_fzsync_pair_update(struct tst_fzsync_pair *pair) float alpha = pair->avg_alpha; float per_spin_time, time_delay, dev_ratio; + if (!pair->sampling_exit) + ++pair->sampling; + dev_ratio = (pair->diff_sa.dev_ratio + pair->diff_sb.dev_ratio + pair->diff_ab.dev_ratio @@ -465,11 +462,13 @@ static void tst_fzsync_pair_update(struct tst_fzsync_pair *pair) tst_upd_diff_stat(&pair->diff_ab, alpha, pair->a_end, pair->b_end); tst_upd_stat(&pair->spins_avg, alpha, pair->spins); - if (pair->sampling > 0 && --pair->sampling == 0) { + if (pair->sampling_exit && pair->sampling > 0) { tst_res(TINFO, - "Minimum sampling period ended, deviation ratio = %.2f", - dev_ratio); + "Sampling period ended, total samples = %d," + "deviation ratio = %.2f", + pair->sampling, dev_ratio); tst_fzsync_pair_info(pair); + pair->sampling = 0; } } 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; @@ -587,23 +586,14 @@ static inline int tst_fzsync_run_a(struct tst_fzsync_pair *pair) float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start; /* Limit amount of time spent on sampling */ - if ((pair->max_sampling_p < rem_p) - && (pair->sampling > 0)) { - tst_res(TINFO, "stopping sampling at %d samples", - pair->sampling); - pair->sampling = 0; - tst_fzsync_pair_info(pair); - } + if ((pair->max_sampling_p < rem_p && pair->sampling > 0) + || pair->sampling >= 0.25 * pair->exec_loops) + pair->sampling_exit = 1; if (pair->exec_time_p < rem_p) { tst_res(TINFO, "Exceeded execution time, requesting exit"); exit = 1; - - if (pair->sampling > 0) { - tst_res(TWARN, - "Still sampling, consider increasing LTP_TIMEOUT_MUL"); - } } if (++pair->exec_loop > pair->exec_loops) { -- Regards, Li Wang -------------- next part -------------- A non-text attachment was scrubbed... Name: tst_fuzzy_sync.h Type: text/x-chdr Size: 21482 bytes Desc: not available URL: <http://lists.linux.it/pipermail/ltp/attachments/20181201/bba985b0/attachment-0001.h> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH] fzsync: limit sampling time 2018-12-01 8:37 ` Li Wang @ 2018-12-01 8:58 ` Jan Stancek 2018-12-01 9:48 ` Li Wang 2018-12-01 9:57 ` Li Wang 0 siblings, 2 replies; 9+ messages in thread From: Jan Stancek @ 2018-12-01 8:58 UTC (permalink / raw) To: ltp ----- Original Message ----- > On Thu, Nov 29, 2018 at 7:09 PM Jan Stancek <jstancek@redhat.com> wrote: > > > > Fixes: #429 > > > > Sampling can take considerably longer time on single CPU > > and very slow systems. This patch limits sampling time to > > 1/2 of fuzzing runtime (0.25 of test time). If we don't > > have enough samples by that time, stop sampling and use > > stats we gathered so far. > > This patch makes sense. > > > > > Signed-off-by: Jan Stancek <jstancek@redhat.com> > > --- > > include/tst_fuzzy_sync.h | 15 +++++++++++++-- > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h > > index 03f69b78bc82..0ca292c141c3 100644 > > --- a/include/tst_fuzzy_sync.h > > +++ b/include/tst_fuzzy_sync.h > > @@ -162,6 +162,7 @@ struct tst_fzsync_pair { > > * > > * Defaults to 0.5 (~150 seconds with default timeout). > > */ > > + float max_sampling_p; > > float exec_time_p; > > /** Internal; The test time remaining on tst_fzsync_pair_reset() */ > > float exec_time_start; > > @@ -199,6 +200,7 @@ static void tst_fzsync_pair_init(struct tst_fzsync_pair > > *pair) > > CHK(avg_alpha, 0, 1, 0.25); > > CHK(min_samples, 20, INT_MAX, 1024); > > CHK(max_dev_ratio, 0, 1, 0.1); > > + CHK(max_sampling_p, 0, 1, 0.25); > > CHK(exec_time_p, 0, 1, 0.5); > > CHK(exec_loops, 20, INT_MAX, 3000000); > > } > > @@ -582,9 +584,18 @@ static inline void tst_fzsync_wait_b(struct > > tst_fzsync_pair *pair) > > static inline int tst_fzsync_run_a(struct tst_fzsync_pair *pair) > > { > > int exit = 0; > > + float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start; > > + > > + /* Limit amount of time spent on sampling */ > > + if ((pair->max_sampling_p < rem_p) > > + && (pair->sampling > 0)) { > > + tst_res(TINFO, "stopping sampling at %d samples", > > + pair->sampling); > > pair->sampling is the number of samples left, so I think the prints > might be more precise if calculate the 'pair->min_samples - > pair->sampling'(or pair->exec_loop)? Yes, I was lazy. > > > + pair->sampling = 0; > > + tst_fzsync_pair_info(pair); > > + } > > > > - if (pair->exec_time_p > > - < 1 - tst_timeout_remaining() / pair->exec_time_start) { > > + if (pair->exec_time_p < rem_p) { > > tst_res(TINFO, > > "Exceeded execution time, requesting exit"); > > exit = 1; > > After involving max_sampling_p, here the warning prints will no longer > be useful because it will never get 'pair->sampling > 0' when > exec_time_p is exhausted. You're right, "still sampling" warning can't be reached now. In my first iteration I had it limited only to single CPU systems, so the warning was still possible on SMP. I'll send v2. > > Base on your patch I suggest that: > > --- a/include/tst_fuzzy_sync.h > +++ b/include/tst_fuzzy_sync.h > @@ -590,7 +590,7 @@ static inline int tst_fzsync_run_a(struct > tst_fzsync_pair *pair) > if ((pair->max_sampling_p < rem_p) > && (pair->sampling > 0)) { > tst_res(TINFO, "stopping sampling at %d samples", > - pair->sampling); > + pair->exec_loop); > pair->sampling = 0; > tst_fzsync_pair_info(pair); > } > @@ -599,11 +599,6 @@ static inline int tst_fzsync_run_a(struct > tst_fzsync_pair *pair) > tst_res(TINFO, > "Exceeded execution time, requesting exit"); > exit = 1; > - > - if (pair->sampling > 0) { > - tst_res(TWARN, > - "Still sampling, consider increasing > LTP_TIMEOUT_MUL"); > - } > } > > if (++pair->exec_loop > pair->exec_loops) { > > > ======= Snip ======== > > Additionally, If we go this way to limit the sampling time, seems > 'pair->min_samples' will become less meaning for fuzzy sync library, True, but this applies only when sampling goes very slowly. > because we can just do sampling in a limited time and why we need the > minimal samples? Wouldn't we spend too much time sampling on fast systems without much gain? My impression was we want to get out of sampling phase ASAP, so library can start inserting random delays. Regards, Jan > > So what about using limited-time to replace min_samples? Maybe like: > > --- a/include/tst_fuzzy_sync.h > +++ b/include/tst_fuzzy_sync.h > @@ -123,19 +123,14 @@ struct tst_fzsync_pair { > */ > int delay; > /** > - * Internal; The number of samples left or the sampling state. > + * Internal; The number of samples in the sampling phase. > * > - * A positive value is the number of remaining mandatory > + * A positive value is the number of past mandatory > * samples. Zero or a negative indicate some other state. > */ > int sampling; > - /** > - * The Minimum number of statistical samples which must be collected. > - * > - * The minimum number of iterations which must be performed before a > - * random delay can be calculated. Defaults to 1024. > - */ > - int min_samples; > + /** Internal; Exit sampling phase if this value is 1 */ > + int sampling_exit; > /** > * The maximum allowed proportional average deviation. > * > @@ -198,7 +193,6 @@ struct tst_fzsync_pair { > static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair) > { > CHK(avg_alpha, 0, 1, 0.25); > - CHK(min_samples, 20, INT_MAX, 1024); > CHK(max_dev_ratio, 0, 1, 0.1); > CHK(max_sampling_p, 0, 1, 0.25); > CHK(exec_time_p, 0, 1, 0.5); > @@ -262,7 +256,8 @@ static void tst_fzsync_pair_reset(struct > tst_fzsync_pair *pair, > tst_init_stat(&pair->diff_ab); > tst_init_stat(&pair->spins_avg); > pair->delay = 0; > - pair->sampling = pair->min_samples; > + pair->sampling = 0; > + pair->sampling_exit = 0; > > pair->exec_loop = 0; > > @@ -295,7 +290,6 @@ static inline void tst_fzsync_stat_info(struct > tst_fzsync_stat stat, > */ > static void tst_fzsync_pair_info(struct tst_fzsync_pair *pair) > { > - tst_res(TINFO, "loop = %d", pair->exec_loop); > tst_fzsync_stat_info(pair->diff_ss, "ns", "start_a - start_b"); > tst_fzsync_stat_info(pair->diff_sa, "ns", "end_a - start_a"); > tst_fzsync_stat_info(pair->diff_sb, "ns", "end_b - start_b"); > @@ -450,6 +444,9 @@ static void tst_fzsync_pair_update(struct > tst_fzsync_pair *pair) > float alpha = pair->avg_alpha; > float per_spin_time, time_delay, dev_ratio; > > + if (!pair->sampling_exit) > + ++pair->sampling; > + > dev_ratio = (pair->diff_sa.dev_ratio > + pair->diff_sb.dev_ratio > + pair->diff_ab.dev_ratio > @@ -465,11 +462,13 @@ static void tst_fzsync_pair_update(struct > tst_fzsync_pair *pair) > tst_upd_diff_stat(&pair->diff_ab, alpha, > pair->a_end, pair->b_end); > tst_upd_stat(&pair->spins_avg, alpha, pair->spins); > - if (pair->sampling > 0 && --pair->sampling == 0) { > + if (pair->sampling_exit && pair->sampling > 0) { > tst_res(TINFO, > - "Minimum sampling period ended, > deviation ratio = %.2f", > - dev_ratio); > + "Sampling period ended, total samples = %d," > + "deviation ratio = %.2f", > + pair->sampling, dev_ratio); > tst_fzsync_pair_info(pair); > + pair->sampling = 0; > } > } 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; > @@ -587,23 +586,14 @@ static inline int tst_fzsync_run_a(struct > tst_fzsync_pair *pair) > float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start; > > /* Limit amount of time spent on sampling */ > - if ((pair->max_sampling_p < rem_p) > - && (pair->sampling > 0)) { > - tst_res(TINFO, "stopping sampling at %d samples", > - pair->sampling); > - pair->sampling = 0; > - tst_fzsync_pair_info(pair); > - } > + if ((pair->max_sampling_p < rem_p && pair->sampling > 0) > + || pair->sampling >= 0.25 * pair->exec_loops) > + pair->sampling_exit = 1; > > if (pair->exec_time_p < rem_p) { > tst_res(TINFO, > "Exceeded execution time, requesting exit"); > exit = 1; > - > - if (pair->sampling > 0) { > - tst_res(TWARN, > - "Still sampling, consider increasing > LTP_TIMEOUT_MUL"); > - } > } > > if (++pair->exec_loop > pair->exec_loops) { > > -- > Regards, > Li Wang > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH] fzsync: limit sampling time 2018-12-01 8:58 ` Jan Stancek @ 2018-12-01 9:48 ` Li Wang 2018-12-01 9:57 ` Li Wang 1 sibling, 0 replies; 9+ messages in thread From: Li Wang @ 2018-12-01 9:48 UTC (permalink / raw) To: ltp On Sat, Dec 1, 2018 at 4:58 PM Jan Stancek <jstancek@redhat.com> wrote: > > > > ----- Original Message ----- > > On Thu, Nov 29, 2018 at 7:09 PM Jan Stancek <jstancek@redhat.com> wrote: > > > > > > Fixes: #429 > > > > > > Sampling can take considerably longer time on single CPU > > > and very slow systems. This patch limits sampling time to > > > 1/2 of fuzzing runtime (0.25 of test time). If we don't > > > have enough samples by that time, stop sampling and use > > > stats we gathered so far. > > > > This patch makes sense. > > > > > > > > Signed-off-by: Jan Stancek <jstancek@redhat.com> > > > --- > > > include/tst_fuzzy_sync.h | 15 +++++++++++++-- > > > 1 file changed, 13 insertions(+), 2 deletions(-) > > > > > > diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h > > > index 03f69b78bc82..0ca292c141c3 100644 > > > --- a/include/tst_fuzzy_sync.h > > > +++ b/include/tst_fuzzy_sync.h > > > @@ -162,6 +162,7 @@ struct tst_fzsync_pair { > > > * > > > * Defaults to 0.5 (~150 seconds with default timeout). > > > */ > > > + float max_sampling_p; > > > float exec_time_p; > > > /** Internal; The test time remaining on tst_fzsync_pair_reset() */ > > > float exec_time_start; > > > @@ -199,6 +200,7 @@ static void tst_fzsync_pair_init(struct tst_fzsync_pair > > > *pair) > > > CHK(avg_alpha, 0, 1, 0.25); > > > CHK(min_samples, 20, INT_MAX, 1024); > > > CHK(max_dev_ratio, 0, 1, 0.1); > > > + CHK(max_sampling_p, 0, 1, 0.25); > > > CHK(exec_time_p, 0, 1, 0.5); > > > CHK(exec_loops, 20, INT_MAX, 3000000); > > > } > > > @@ -582,9 +584,18 @@ static inline void tst_fzsync_wait_b(struct > > > tst_fzsync_pair *pair) > > > static inline int tst_fzsync_run_a(struct tst_fzsync_pair *pair) > > > { > > > int exit = 0; > > > + float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start; > > > + > > > + /* Limit amount of time spent on sampling */ > > > + if ((pair->max_sampling_p < rem_p) > > > + && (pair->sampling > 0)) { > > > + tst_res(TINFO, "stopping sampling at %d samples", > > > + pair->sampling); > > > > pair->sampling is the number of samples left, so I think the prints > > might be more precise if calculate the 'pair->min_samples - > > pair->sampling'(or pair->exec_loop)? > > Yes, I was lazy. > > > > > > + pair->sampling = 0; > > > + tst_fzsync_pair_info(pair); > > > + } > > > > > > - if (pair->exec_time_p > > > - < 1 - tst_timeout_remaining() / pair->exec_time_start) { > > > + if (pair->exec_time_p < rem_p) { > > > tst_res(TINFO, > > > "Exceeded execution time, requesting exit"); > > > exit = 1; > > > > After involving max_sampling_p, here the warning prints will no longer > > be useful because it will never get 'pair->sampling > 0' when > > exec_time_p is exhausted. > > You're right, "still sampling" warning can't be reached now. > > In my first iteration I had it limited only to single CPU systems, > so the warning was still possible on SMP. > > I'll send v2. > > > > > Base on your patch I suggest that: > > > > --- a/include/tst_fuzzy_sync.h > > +++ b/include/tst_fuzzy_sync.h > > @@ -590,7 +590,7 @@ static inline int tst_fzsync_run_a(struct > > tst_fzsync_pair *pair) > > if ((pair->max_sampling_p < rem_p) > > && (pair->sampling > 0)) { > > tst_res(TINFO, "stopping sampling at %d samples", > > - pair->sampling); > > + pair->exec_loop); > > pair->sampling = 0; > > tst_fzsync_pair_info(pair); > > } > > @@ -599,11 +599,6 @@ static inline int tst_fzsync_run_a(struct > > tst_fzsync_pair *pair) > > tst_res(TINFO, > > "Exceeded execution time, requesting exit"); > > exit = 1; > > - > > - if (pair->sampling > 0) { > > - tst_res(TWARN, > > - "Still sampling, consider increasing > > LTP_TIMEOUT_MUL"); > > - } > > } > > > > if (++pair->exec_loop > pair->exec_loops) { > > > > > > ======= Snip ======== > > > > Additionally, If we go this way to limit the sampling time, seems > > 'pair->min_samples' will become less meaning for fuzzy sync library, > > True, but this applies only when sampling goes very slowly. > > > because we can just do sampling in a limited time and why we need the > > minimal samples? > > Wouldn't we spend too much time sampling on fast systems > without much gain? > > My impression was we want to get out of sampling phase ASAP, > so library can start inserting random delays. > > Regards, > Jan > > > > > So what about using limited-time to replace min_samples? Maybe like: > > > > --- a/include/tst_fuzzy_sync.h > > +++ b/include/tst_fuzzy_sync.h > > @@ -123,19 +123,14 @@ struct tst_fzsync_pair { > > */ > > int delay; > > /** > > - * Internal; The number of samples left or the sampling state. > > + * Internal; The number of samples in the sampling phase. > > * > > - * A positive value is the number of remaining mandatory > > + * A positive value is the number of past mandatory > > * samples. Zero or a negative indicate some other state. > > */ > > int sampling; > > - /** > > - * The Minimum number of statistical samples which must be collected. > > - * > > - * The minimum number of iterations which must be performed before a > > - * random delay can be calculated. Defaults to 1024. > > - */ > > - int min_samples; > > + /** Internal; Exit sampling phase if this value is 1 */ > > + int sampling_exit; > > /** > > * The maximum allowed proportional average deviation. > > * > > @@ -198,7 +193,6 @@ struct tst_fzsync_pair { > > static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair) > > { > > CHK(avg_alpha, 0, 1, 0.25); > > - CHK(min_samples, 20, INT_MAX, 1024); > > CHK(max_dev_ratio, 0, 1, 0.1); > > CHK(max_sampling_p, 0, 1, 0.25); > > CHK(exec_time_p, 0, 1, 0.5); > > @@ -262,7 +256,8 @@ static void tst_fzsync_pair_reset(struct > > tst_fzsync_pair *pair, > > tst_init_stat(&pair->diff_ab); > > tst_init_stat(&pair->spins_avg); > > pair->delay = 0; > > - pair->sampling = pair->min_samples; > > + pair->sampling = 0; > > + pair->sampling_exit = 0; > > > > pair->exec_loop = 0; > > > > @@ -295,7 +290,6 @@ static inline void tst_fzsync_stat_info(struct > > tst_fzsync_stat stat, > > */ > > static void tst_fzsync_pair_info(struct tst_fzsync_pair *pair) > > { > > - tst_res(TINFO, "loop = %d", pair->exec_loop); > > tst_fzsync_stat_info(pair->diff_ss, "ns", "start_a - start_b"); > > tst_fzsync_stat_info(pair->diff_sa, "ns", "end_a - start_a"); > > tst_fzsync_stat_info(pair->diff_sb, "ns", "end_b - start_b"); > > @@ -450,6 +444,9 @@ static void tst_fzsync_pair_update(struct > > tst_fzsync_pair *pair) > > float alpha = pair->avg_alpha; > > float per_spin_time, time_delay, dev_ratio; > > > > + if (!pair->sampling_exit) > > + ++pair->sampling; > > + > > dev_ratio = (pair->diff_sa.dev_ratio > > + pair->diff_sb.dev_ratio > > + pair->diff_ab.dev_ratio > > @@ -465,11 +462,13 @@ static void tst_fzsync_pair_update(struct > > tst_fzsync_pair *pair) > > tst_upd_diff_stat(&pair->diff_ab, alpha, > > pair->a_end, pair->b_end); > > tst_upd_stat(&pair->spins_avg, alpha, pair->spins); > > - if (pair->sampling > 0 && --pair->sampling == 0) { > > + if (pair->sampling_exit && pair->sampling > 0) { > > tst_res(TINFO, > > - "Minimum sampling period ended, > > deviation ratio = %.2f", > > - dev_ratio); > > + "Sampling period ended, total samples = %d," > > + "deviation ratio = %.2f", > > + pair->sampling, dev_ratio); > > tst_fzsync_pair_info(pair); > > + pair->sampling = 0; > > } > > } 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; > > @@ -587,23 +586,14 @@ static inline int tst_fzsync_run_a(struct > > tst_fzsync_pair *pair) > > float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start; > > > > /* Limit amount of time spent on sampling */ > > - if ((pair->max_sampling_p < rem_p) > > - && (pair->sampling > 0)) { > > - tst_res(TINFO, "stopping sampling at %d samples", > > - pair->sampling); > > - pair->sampling = 0; > > - tst_fzsync_pair_info(pair); > > - } > > + if ((pair->max_sampling_p < rem_p && pair->sampling > 0) > > + || pair->sampling >= 0.25 * pair->exec_loops) > > + pair->sampling_exit = 1; > > > > if (pair->exec_time_p < rem_p) { > > tst_res(TINFO, > > "Exceeded execution time, requesting exit"); > > exit = 1; > > - > > - if (pair->sampling > 0) { > > - tst_res(TWARN, > > - "Still sampling, consider increasing > > LTP_TIMEOUT_MUL"); > > - } > > } > > > > if (++pair->exec_loop > pair->exec_loops) { > > > > -- > > Regards, > > Li Wang > > -- Regards, Li Wang ^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH] fzsync: limit sampling time 2018-12-01 8:58 ` Jan Stancek 2018-12-01 9:48 ` Li Wang @ 2018-12-01 9:57 ` Li Wang 1 sibling, 0 replies; 9+ messages in thread From: Li Wang @ 2018-12-01 9:57 UTC (permalink / raw) To: ltp Oops, I click send button before input anything in last email. Sorry about that. On Sat, Dec 1, 2018 at 4:58 PM Jan Stancek <jstancek@redhat.com> wrote: > Wouldn't we spend too much time sampling on fast systems > without much gain? Well, seems there is no need to do much sampling on faster system. > > My impression was we want to get out of sampling phase ASAP, > so library can start inserting random delays. Ok, after thinking over, to drop min_samples maybe doesn't make much sense for library. Now I'd keep the patch as the V2. -- Regards, Li Wang ^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH v2] fzsync: limit sampling time 2018-11-29 11:09 [LTP] [PATCH] fzsync: limit sampling time Jan Stancek 2018-12-01 8:37 ` Li Wang @ 2018-12-01 9:12 ` Jan Stancek 2018-12-03 8:58 ` Li Wang ` (2 more replies) 1 sibling, 3 replies; 9+ messages in thread From: Jan Stancek @ 2018-12-01 9:12 UTC (permalink / raw) To: ltp Fixes: #429 Sampling can take considerably longer time on single CPU and very slow systems. This patch limits sampling time to 1/2 of fuzzing runtime (0.25 of test time). If we don't have enough samples by that time, stop sampling and use stats we gathered so far. Signed-off-by: Jan Stancek <jstancek@redhat.com> --- include/tst_fuzzy_sync.h | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) Changes in v2: - drop 'still sampling' warning - use exec_loop in 'stopping sampling at' message diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h index 03f69b78bc82..d572461c24b6 100644 --- a/include/tst_fuzzy_sync.h +++ b/include/tst_fuzzy_sync.h @@ -162,6 +162,7 @@ struct tst_fzsync_pair { * * Defaults to 0.5 (~150 seconds with default timeout). */ + float max_sampling_p; float exec_time_p; /** Internal; The test time remaining on tst_fzsync_pair_reset() */ float exec_time_start; @@ -199,6 +200,7 @@ static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair) CHK(avg_alpha, 0, 1, 0.25); CHK(min_samples, 20, INT_MAX, 1024); CHK(max_dev_ratio, 0, 1, 0.1); + CHK(max_sampling_p, 0, 1, 0.25); CHK(exec_time_p, 0, 1, 0.5); CHK(exec_loops, 20, INT_MAX, 3000000); } @@ -582,17 +584,21 @@ static inline void tst_fzsync_wait_b(struct tst_fzsync_pair *pair) static inline int tst_fzsync_run_a(struct tst_fzsync_pair *pair) { int exit = 0; + float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start; + + /* Limit amount of time spent on sampling */ + if ((pair->max_sampling_p < rem_p) + && (pair->sampling > 0)) { + tst_res(TINFO, "stopping sampling at %d samples", + pair->exec_loop); + pair->sampling = 0; + tst_fzsync_pair_info(pair); + } - if (pair->exec_time_p - < 1 - tst_timeout_remaining() / pair->exec_time_start) { + if (pair->exec_time_p < rem_p) { tst_res(TINFO, "Exceeded execution time, requesting exit"); exit = 1; - - if (pair->sampling > 0) { - tst_res(TWARN, - "Still sampling, consider increasing LTP_TIMEOUT_MUL"); - } } if (++pair->exec_loop > pair->exec_loops) { -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [LTP] [PATCH v2] fzsync: limit sampling time 2018-12-01 9:12 ` [LTP] [PATCH v2] " Jan Stancek @ 2018-12-03 8:58 ` Li Wang 2018-12-03 9:18 ` Richard Palethorpe 2018-12-03 12:47 ` Cyril Hrubis 2 siblings, 0 replies; 9+ messages in thread From: Li Wang @ 2018-12-03 8:58 UTC (permalink / raw) To: ltp On Sat, Dec 1, 2018 at 5:12 PM Jan Stancek <jstancek@redhat.com> wrote: > > Fixes: #429 > > Sampling can take considerably longer time on single CPU > and very slow systems. This patch limits sampling time to > 1/2 of fuzzing runtime (0.25 of test time). If we don't > have enough samples by that time, stop sampling and use > stats we gathered so far. Patch v2 looks good to me, and I'd suggest to reserve more time before pushing, in case Richard has some extra comments. > > Signed-off-by: Jan Stancek <jstancek@redhat.com> Reviewed-by: Li Wang <liwang@redhat.com> -- Regards, Li Wang ^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH v2] fzsync: limit sampling time 2018-12-01 9:12 ` [LTP] [PATCH v2] " Jan Stancek 2018-12-03 8:58 ` Li Wang @ 2018-12-03 9:18 ` Richard Palethorpe 2018-12-03 12:47 ` Cyril Hrubis 2 siblings, 0 replies; 9+ messages in thread From: Richard Palethorpe @ 2018-12-03 9:18 UTC (permalink / raw) To: ltp Hello Jan, Jan Stancek <jstancek@redhat.com> writes: > Fixes: #429 > > Sampling can take considerably longer time on single CPU > and very slow systems. This patch limits sampling time to > 1/2 of fuzzing runtime (0.25 of test time). If we don't > have enough samples by that time, stop sampling and use > stats we gathered so far. I suppose this improves the worst case where the race condition will never trigger without a random delay. As even in cases where the delay makes the chances worse, we still get 50% of the time without a delay (the new worst case). > > Signed-off-by: Jan Stancek <jstancek@redhat.com> > --- > include/tst_fuzzy_sync.h | 20 +++++++++++++------- > 1 file changed, 13 insertions(+), 7 deletions(-) > > Changes in v2: > - drop 'still sampling' warning > - use exec_loop in 'stopping sampling at' message > > diff --git a/include/tst_fuzzy_sync.h b/include/tst_fuzzy_sync.h > index 03f69b78bc82..d572461c24b6 100644 > --- a/include/tst_fuzzy_sync.h > +++ b/include/tst_fuzzy_sync.h > @@ -162,6 +162,7 @@ struct tst_fzsync_pair { > * > * Defaults to 0.5 (~150 seconds with default timeout). > */ > + float max_sampling_p; > float exec_time_p; > /** Internal; The test time remaining on tst_fzsync_pair_reset() */ > float exec_time_start; > @@ -199,6 +200,7 @@ static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair) > CHK(avg_alpha, 0, 1, 0.25); > CHK(min_samples, 20, INT_MAX, 1024); > CHK(max_dev_ratio, 0, 1, 0.1); > + CHK(max_sampling_p, 0, 1, 0.25); > CHK(exec_time_p, 0, 1, 0.5); > CHK(exec_loops, 20, INT_MAX, 3000000); > } > @@ -582,17 +584,21 @@ static inline void tst_fzsync_wait_b(struct tst_fzsync_pair *pair) > static inline int tst_fzsync_run_a(struct tst_fzsync_pair *pair) > { > int exit = 0; > + float rem_p = 1 - tst_timeout_remaining() / pair->exec_time_start; > + > + /* Limit amount of time spent on sampling */ > + if ((pair->max_sampling_p < rem_p) > + && (pair->sampling > 0)) { > + tst_res(TINFO, "stopping sampling at %d samples", > + pair->exec_loop); > + pair->sampling = 0; > + tst_fzsync_pair_info(pair); > + } > > - if (pair->exec_time_p > - < 1 - tst_timeout_remaining() / pair->exec_time_start) { > + if (pair->exec_time_p < rem_p) { > tst_res(TINFO, > "Exceeded execution time, requesting exit"); > exit = 1; > - > - if (pair->sampling > 0) { > - tst_res(TWARN, > - "Still sampling, consider increasing LTP_TIMEOUT_MUL"); > - } I worry a little bit about removing this warning and replacing it with an info message. However assuming a benchmarking module is introduced to set/check LTP_TIMEOUT_MUL then it would be better not to print a warning here. So LGTM. > } > > if (++pair->exec_loop > pair->exec_loops) { -- Thank you, Richard. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [LTP] [PATCH v2] fzsync: limit sampling time 2018-12-01 9:12 ` [LTP] [PATCH v2] " Jan Stancek 2018-12-03 8:58 ` Li Wang 2018-12-03 9:18 ` Richard Palethorpe @ 2018-12-03 12:47 ` Cyril Hrubis 2 siblings, 0 replies; 9+ messages in thread From: Cyril Hrubis @ 2018-12-03 12:47 UTC (permalink / raw) To: ltp Hi! > Sampling can take considerably longer time on single CPU > and very slow systems. This patch limits sampling time to > 1/2 of fuzzing runtime (0.25 of test time). If we don't > have enough samples by that time, stop sampling and use > stats we gathered so far. I do wonder if it would be better to really base the proportional sampling time based on the actual test runtime (i.e. hardcoding it to be exec_time_p/2) because that would mean we would have to tune only one paramter when adjusting the test runtime. But I guess only a few tests will adjust the test runtime anyway. -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-12-03 12:47 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-29 11:09 [LTP] [PATCH] fzsync: limit sampling time Jan Stancek 2018-12-01 8:37 ` Li Wang 2018-12-01 8:58 ` Jan Stancek 2018-12-01 9:48 ` Li Wang 2018-12-01 9:57 ` Li Wang 2018-12-01 9:12 ` [LTP] [PATCH v2] " Jan Stancek 2018-12-03 8:58 ` Li Wang 2018-12-03 9:18 ` Richard Palethorpe 2018-12-03 12:47 ` Cyril Hrubis
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.