<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, Dec 5, 2019 at 7:57 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>
<span class="gmail_default" style="font-size:small">...</span><br>
> +2.2.34 Reproducing race-conditions<br>
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^<br>
<br>
Can we please avoid renumbering the paragraphs, i.e. can you add the<br>
hugepages docs after this?<br></blockquote><div> </div><div><div class="gmail_default" style="font-size:small">Yes, sure.</div></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">
<span class="gmail_default" style="font-size:small">...</span><br>
> +/*<br>
> + * If system does not support hugetlb or failed to reserve the expected<br>
> + * number of hugepage, then this value will be set to 1.<br>
> + */<br>
> +extern unsigned int tst_no_hugepage;<br>
<br>
Hmm, this needs a bit more thoughts.<br>
<br>
So far all the needs_foo flags require the foo to be present on the<br>
system, i.e. are hard requirements. In this case hugepages are soft<br>
requirement, i.e. only part of the test will need that.<br>
<br>
We probably need a better name for this than needs_hugepages in<br>
tst_test. We may as well need a generic way how to describe that some of<br>
the requirements are hard and some of them are soft. I will give it some<br>
thoughts.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Your consideration is valuable. '.needs_hugepages' might fail to reserve the expected page numbers on any kind of platform which with fragmental memory, we'd better give an elegant way to handle that situation to make our test can work well on the remaining part.</div></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>
At least we should just call it request_hugepages in the tst_test<br>
structure instead.<br></blockquote><div> </div><div><div class="gmail_default" style="font-size:small">Ok, it's fine. Unless we can come up with a better.</div></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">
<span class="gmail_default" style="font-size:small">...</span><br>
> +<br>
> +int tst_request_hugepages(int hpages)<br>
> +{<br>
> +     int val;<br>
> +     long mem_avail, max_hpages;<br>
> +<br>
> +     if (access(PATH_HUGEPAGES, F_OK)) {<br>
> +             tst_res(TCONF, "Huge page is not supported.");<br>
> +             tst_no_hugepage = 1;<br>
<br>
I guess that it will make more sense to call it tst_hugepages and set it<br>
to a number of hugepages that we were able to reserve. I.e. set it to 0<br>
here and to hpages at the end.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Agree, that sounds more useful in many situations. </div></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">
<span class="gmail_default" style="font-size:small">...</span><br>
> +     if (tst_test->needs_hugepages) {<br>
> +             tst_sys_conf_save("?/proc/sys/vm/nr_hugepages");<br>
<br>
Can we call this from the tst_hugepage.c instead and only if we actually<br>
written to that file?<br></blockquote><div> </div><div><div class="gmail_default" style="font-size:small">I will have a try then.</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>
> +             tst_request_hugepages(tst_test->needs_hugepages);<br>
> +     }<br>
> +<br>
>       setup_ipc();<br>
>  <br>
>       if (tst_test->bufs)<br>
> @@ -1006,7 +1011,8 @@ static void do_cleanup(void)<br>
>               tst_rmdir();<br>
>       }<br>
>  <br>
> -     if (tst_test->save_restore)<br>
> +     if (tst_test->save_restore ||<br>
> +             (tst_test->needs_hugepages && !tst_no_hugepage))<br>
>               tst_sys_conf_restore(0);<br>
<br>
We can call the tst_sys_conf_restore() unconditionally here, if nothing<br>
has been saved it's no-op.<br></blockquote><div> </div><div><span class="gmail_default" style="font-size:small">Right!</span></div><div><span class="gmail_default" style="font-size:small"></span> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
>       if (tst_test->restore_wallclock)<br>
<br>
Otherwise it looks good.<br>
</blockquote><div><br></div><div class="gmail_default" style="font-size:small">Thanks for these useful advises.</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>