<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, May 19, 2022 at 5:03 PM Cyril Hrubis <<a href="mailto:chrubis@suse.cz">chrubis@suse.cz</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi!<br>
> Okay, my perspective is shortsighted as well.<br>
> <br>
> This solution is correct in the direction but overlooks the significant<br>
> global value 'tst_start_time'. If that value reflush within tcnt loop we<br>
> don't need to reset max_runtime again, actually the real work in my<br>
> previous patch is to invoke heartbeat() which touches tst_start_time.<br>
> <br>
> So I would suggest using heartbeat() instead of only sending SIGUSR1<br>
> to lib_pid. Or, do simply revision like:<br>
<br>
Ah, right, I guess that we should just call heartbeat() before each<br>
iteration of the test then.<br>
<br>
So basically:<br>
<br>
diff --git a/lib/tst_test.c b/lib/tst_test.c<br>
index dad8aad92..f3090217b 100644<br>
--- a/lib/tst_test.c<br>
+++ b/lib/tst_test.c<br>
@@ -1317,6 +1317,24 @@ static void do_cleanup(void)<br>
        cleanup_ipc();<br>
 }<br>
<br>
+static void heartbeat(void)<br>
+{<br>
+       if (tst_clock_gettime(CLOCK_MONOTONIC, &tst_start_time))<br>
+               tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");<br>
+<br>
+       if (getppid() == 1) {<br>
+               tst_res(TFAIL, "Main test process might have exit!");<br>
+               /*<br>
+                * We need kill the task group immediately since the<br>
+                * main process has exit.<br>
+                */<br>
+               kill(0, SIGKILL);<br>
+               exit(TBROK);<br>
+       }<br>
+<br>
+       kill(getppid(), SIGUSR1);<br>
+}<br>
+<br>
 static void run_tests(void)<br>
 {<br>
        unsigned int i;<br>
@@ -1324,6 +1342,7 @@ static void run_tests(void)<br>
<br>
        if (!tst_test->test) {<br>
                saved_results = *results;<br>
+               heartbeat();<br>
                tst_test->test_all();<br>
<br>
                if (getpid() != main_pid) {<br>
@@ -1339,6 +1358,7 @@ static void run_tests(void)<br>
<br>
        for (i = 0; i < tst_test->tcnt; i++) {<br>
                saved_results = *results;<br>
+               heartbeat();<br>
                tst_test->test(i);<br>
<br>
                if (getpid() != main_pid) {<br>
@@ -1349,6 +1369,8 @@ static void run_tests(void)<br>
<br>
                if (results_equal(&saved_results, results))<br>
                        tst_brk(TBROK, "Test %i haven't reported results!", i);<br>
+<br>
+               kill(getppid(), SIGUSR1);<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">As we already invoke heartbeat() at the beginning of each tcnt loop,</div><div class="gmail_default" style="font-size:small">why are we still sending SIGUSR1 here?</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Otherwise this version looks good to me.</div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
        }<br>
 }<br>
<br>
@@ -1379,24 +1401,6 @@ static void add_paths(void)<br>
        free(new_path);<br>
 }<br>
<br>
-static void heartbeat(void)<br>
-{<br>
-       if (tst_clock_gettime(CLOCK_MONOTONIC, &tst_start_time))<br>
-               tst_res(TWARN | TERRNO, "tst_clock_gettime() failed");<br>
-<br>
-       if (getppid() == 1) {<br>
-               tst_res(TFAIL, "Main test process might have exit!");<br>
-               /*<br>
-                * We need kill the task group immediately since the<br>
-                * main process has exit.<br>
-                */<br>
-               kill(0, SIGKILL);<br>
-               exit(TBROK);<br>
-       }<br>
-<br>
-       kill(getppid(), SIGUSR1);<br>
-}<br>
-<br>
 static void testrun(void)<br>
 {<br>
        unsigned int i = 0;<br>
@@ -1425,7 +1429,6 @@ static void testrun(void)<br>
                        break;<br>
<br>
                run_tests();<br>
-               heartbeat();<br>
        }<br>
<br>
        do_test_cleanup();<br>
<br>
<br>
I guess that this should go in a separate patch on the top of the<br>
"Introduce concept of max runtime".<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Agree.</div></div><div class="gmail_default" style="font-size:small"><br></div></div><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>