<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small">Hi Richard,</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Sorry for late reply. This patch V2 is good, not sure if below comments are useful to you, so just FYI.</div><div class="gmail_extra"><br><div class="gmail_quote">Richard Palethorpe <span dir="ltr"><<a href="mailto:rpalethorpe@suse.com" target="_blank">rpalethorpe@suse.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Make it so the test writer simply has to mark the beginning and end of a race<br>
in each thread. Previously they must choose where to call a delay and where to<br>
call a timestamp function which is used to calculate the delay as well as the<br>
update function.<br><span class="gmail_default" style="font-size:small">...</span><br>
+ /**<br>
+ * Internal; Number of spins to use in the delay.<br>
+ *<br>
+ * A negative value delays thread A and a positive delays thread B.<br>
+ */<br>
+ int delay;<br>
+ /**<br>
+ * Internal; The number of samples left or the sampling state.<br>
+ *<br>
+ * A positive value is the number of remaining mandatory<br>
+ * samples. Zero or a negative indicate ssome other state.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">s/ssome/some/, a typo here?</div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+ /** Internal; Atomic counter used by fzsync_pair_wait() */<br>
int a_cntr;<br>
+ /** Internal; Atomic counter used by fzsync_pair_wait() */<br>
int b_cntr;<br>
+ /** Internal; Used by tst_fzsync_pair_exit() and fzsync_pair_wait() */<br>
int exit;<br>
+ /**<br>
+ * The maximum desired execution time as a proportion of the timeout<br>
+ *<br>
+ * A value x so that 0 < x < 1 which decides how long the test should<br>
+ * be run for (assuming the loop limit is not exceeded first).<br>
+ *<br>
+ * Defaults to 0.1 (~30 seconds with default timeout).<br>
+ */<br>
+ float exec_time_p;<br>
+ /** Internal; The test time remaining on tst_fzsync_pair_reset() */<br>
+ float exec_time_start;<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">I think maybe call it as 'exec_time_remain' or 'exec_time_total' is better? because this value is the remain part of the whole timeout time before next resetting.</div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<span class="gmail_default" style="font-size:small"></span><br>
-#define TST_FZSYNC_PAIR_INIT { \<br>
- .avg_alpha = 0.25, \<br>
- .delay_inc = 1, \<br>
- .update_gap = 0xF, \<br>
- .info_gap = 0x7FFFF \<br>
+static void tst_fzsync_pair_init(struct tst_fzsync_pair *pair)<br>
+{<br>
+ CHK(avg_alpha, 0, 1, 0.25);<br>
+ CHK(min_samples, 20, INT_MAX, 1024);<br>
+ CHK(max_dev_ratio, 0, 1, 0.1);<br>
+ CHK(exec_time_p, 0, 1, 0.2);<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">The code comment says the default is 0.1, but why here gives 0.2?</div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+static void tst_fzsync_pair_reset(struct tst_fzsync_pair *pair,<br>
+ void *(*run_b)(void *))<br>
+{<br>
+ tst_fzsync_pair_cleanup(pair)<wbr>;<br>
+<br>
+ tst_init_stat(&pair->diff_ss)<wbr>;<br>
+ tst_init_stat(&pair->diff_sa)<wbr>;<br>
+ tst_init_stat(&pair->diff_sb)<wbr>;<br>
+ tst_init_stat(&pair->diff_ab)<wbr>;<br>
+ tst_init_stat(&pair->spins_av<wbr>g);<br>
+ pair->delay = 0;<br>
+ pair->sampling = pair->min_samples;<br>
+<br>
+ pair->exec_loop = 0;<br>
+<br>
+ pair->a_cntr = 0;<br>
+ pair->b_cntr = 0;<br>
+ tst_atomic_store(0, &pair->exit);<br>
+ if (run_b)<br>
+ SAFE_PTHREAD_CREATE(&pair->th<wbr>read_b, 0, run_b, 0);<br>
+<br>
+ pair->exec_time_start = (float)tst_timeout_remaining()<wbr>;</blockquote><div><br></div><div class="gmail_default" style="font-size:small">Here we start to get the whole remain time before reaching timeout aborting, and base on that to comparing in tst_fzsync_run_a() to guarantee test exit when execute time exceed. That's fine, but it is too early to get the time here I guess, because samples collection will also cost some of the limitation_time(~30 seconds), and we don't know what proportion the sampling occupied in exec_time_p, if it's too much, then there only has very little time on race execution. </div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">To avoid this, maybe we should make the limitation time all spend on race execution but any on samplings. </div><div class="gmail_default" style="font-size:small">So, I suggest to get pair->exec_time_start value after samples data collection in the end of tst_fzsync_pair_update() function.</div><div class="gmail_default" style="font-size:small"></div></div>-- <br><div class="gmail-m_-8999978948160852626m_3200052602969549604m_826276637791501812gmail_signature"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div>
</div></div></div></div></div></div></div>