<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 Tue, Oct 26, 2021 at 3:13 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>
> > Rarely there is a need to set the test runtime dynamically, the only<br>
> > tests in LTP that does this are the timer tests that can get two<br>
> > parameters, number of iterations and sleep time, and the test runtime is<br>
> > close to the multiplication of these two.<br>
> ><br>
> > It's still cleaner to set the runtime and let the test library figure<br>
> > out the timeout in this case.<br>
> ><br>
> <br>
> If so, should we consider to hinden the .timeout in struct tst_test<br>
> to prevent users from changing it?<br>
<br>
If we decide to apply this patchset that would be logical end result.<br>
There are only a few .timeout = foo left in the codebase after this<br>
patchset that either disable timeout for the few unpredictable cases or<br>
shorten it to make the test timeout faster if it gets stuck. We can deal<br>
with these by making the .max_runtime accept -1 and by shortening the<br>
default timeout considerably.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Yes, that should be great.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">After a quick reviewing the whole patchset, I feel that .timeout is</div><div class="gmail_default" style="font-size:small">redundant since .max_runtime can do more thing to totally replace</div><div class="gmail_default" style="font-size:small">it by the end.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">----------------</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Btw, it looks weird to simply double the runtime by plus MAX(10u, runtime)</div><div class="gmail_default" style="font-size:small">in the runtime_to_timeout, I guess you probably just wanna another</div><div class="gmail_default" style="font-size:small">10sec for some reclaiming work.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">And the .max_runtime is also maximal time per test iteration,</div><div class="gmail_default" style="font-size:small">but from the output below misleading me to think it is for the</div><div class="gmail_default" style="font-size:small">whole test time.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">See:</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small"># LTP_TIMEOUT_MUL=1 ./pty03 <br>tst_test.c:1376: TINFO: Test max runtime 360s<br>tst_test.c:1371: TINFO: Timeout per run is 0h 12m 00s<br></div><div class="gmail_default" style="font-size:small">....</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>
> IIRC, we currently have ".timeout == -1" to disable test timed<br>
> out in unsure situation, e.g some OOM tests. But in this patch,<br>
> I saw you remove that, but not handle it in tst_set_runtime.<br>
<br>
Ah, right, I've removed the timeout == -1 handling by mistake. I wanted<br>
to keep it working after this patchset as well until a follow up<br>
patchset deals with the rest of the tests that set the .timeout.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Sound good. </div></div><div> </div></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>