<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 22, 2016 at 9:42 PM, Li Wang <span dir="ltr"><<a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Cyril,<br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Tue, Mar 22, 2016 at 9:14 PM, Cyril Hrubis <span dir="ltr"><<a href="mailto:chrubis@suse.cz" target="_blank">chrubis@suse.cz</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"><span>> diff --git a/testcases/kernel/mem/include/mem.h b/testcases/kernel/mem/include/mem.h<br>
> index 43988fe..fbb11b2 100644<br>
> --- a/testcases/kernel/mem/include/mem.h<br>
> +++ b/testcases/kernel/mem/include/mem.h<br>
> @@ -43,6 +43,7 @@ void testoom(int mempolicy, int lite, int retcode, int allow_sigkill);<br>
>  /* KSM */<br>
><br>
>  #define PATH_KSM             "/sys/kernel/mm/ksm/"<br>
> +int max_page_sharing;<br>
<br>
</span>This variable is not used in the library. It does not make sense to<br>
declare it in the header file.<br></blockquote></span><div><br>no, it has been used in mem.c file.<br></div></div></div></div></blockquote><div><br></div><div>Oops! you are right, I didn't noticed that it just a string(not the value) in the mem.c codes. sorry for making noise here. I will re-write the patches.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br>see here:<br>------------<br>diff --git a/testcases/kernel/mem/lib/<div><span class="">mem.c b/testcases/kernel/mem/lib/mem.c<br>
index 7b5bb35..d37d6a4 100644<br>
--- a/testcases/kernel/mem/lib/mem.c<br>
+++ b/testcases/kernel/mem/lib/mem.c<br>
@@ -502,9 +502,12 @@ void create_same_memory(int size, int num, int unit)<br>
        stop_ksm_children(child, num);<br>
<br></span>
        tst_resm(TINFO, "KSM merging...");<span class=""><br>
+       if (access(PATH_KSM "max_page_sharing", F_OK) == 0)<br></span>
+               SAFE_FILE_PRINTF(cleanup, PATH_KSM "max_page_sharing",<span class=""><br>
+                               "%ld", size * pages * num);<br>
        SAFE_FILE_PRINTF(cleanup, PATH_KSM "run", "1");<br>
        SAFE_FILE_PRINTF(cleanup, PATH_KSM "pages_to_scan", "%ld",<br>
-                        size * pages *num);<br>
+                        size * pages * num);<br>
        SAFE_FILE_PRINTF(cleanup, PATH_KSM "sleep_millisecs", "0");<br>
<br>
        resume_ksm_children(child, num);<br>
@@ -595,6 +598,9 @@ void test_ksm_merge_across_nodes(unsigned long nr_pages)<br>
        SAFE_FILE_PRINTF(cleanup, PATH_KSM "sleep_millisecs", "0");<br>
        SAFE_FILE_PRINTF(cleanup, PATH_KSM "pages_to_scan", "%ld",<br>
                         nr_pages * num_nodes);<br></span><span class="">
+       if (access(PATH_KSM "max_page_sharing", F_OK) == 0)<br></span>
+               SAFE_FILE_PRINTF(cleanup, PATH_KSM "max_page_sharing",<span class=""><br>
+                       "%ld", nr_pages * num_nodes);<br><br></span></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><br><div><div class="h5">
>  void test_ksm_merge_across_nodes(unsigned long nr_pages);<br>
><br>
> diff --git a/testcases/kernel/mem/ksm/ksm01.c b/testcases/kernel/mem/ksm/ksm01.c<br>
> index b62df06..a352abd 100644<br>
> --- a/testcases/kernel/mem/ksm/ksm01.c<br>
> +++ b/testcases/kernel/mem/ksm/ksm01.c<br>
> @@ -105,6 +105,9 @@ void setup(void)<br>
>               tst_brkm(TCONF, NULL, "2.6.32 or greater kernel required");<br>
>       if (access(PATH_KSM, F_OK) == -1)<br>
>               tst_brkm(TCONF, NULL, "KSM configuration is not enabled");<br>
> +     if (access(PATH_KSM "max_page_sharing", F_OK) == 0)<br>
> +             SAFE_FILE_SCANF(NULL, PATH_KSM "max_page_sharing",<br>
> +                             "%d", &max_page_sharing);<br>
><br>
>       /*<br>
>        * kernel commit 90bd6fd introduced a new KSM sysfs knob<br>
> @@ -128,4 +131,8 @@ void cleanup(void)<br>
>       if (access(PATH_KSM "merge_across_nodes", F_OK) == 0)<br>
>               FILE_PRINTF(PATH_KSM "merge_across_nodes",<br>
>                                "%d", merge_across_nodes);<br>
> +<br>
> +     if (access(PATH_KSM "max_page_sharing", F_OK) == 0)<br>
> +             FILE_PRINTF(PATH_KSM "max_page_sharing",<br>
> +                              "%d", max_page_sharing);<br>
>  }<br>
<br>
</div></div></span><div><div class="h5">You are saving and restoring the file in each test. Why can't you just<br>
add two functions to save and restore it the library instead and call<br>
them from test setup and cleanup?<br></div></div></blockquote><div><br>ok, that's a better way.<br> </div><span class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
It's also quite confusing that the test saves and restores a value while<br>
it's not modifying it at all. It will be less confusing if it's all done<br>
in the library.<br>
<span><font color="#888888"><br>
--<br>
Cyril Hrubis<br>
<a href="mailto:chrubis@suse.cz" target="_blank">chrubis@suse.cz</a><br>
</font></span></blockquote></span></div><span class="HOEnZb"><font color="#888888"><br><br clear="all"><br>-- <br><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div>Regards,<br></div>Li Wang<br></div><div>Email: <a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a><br></div></div></div></div></div></div>
</font></span></div></div>
</blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div>Regards,<br></div>Li Wang<br></div><div>Email: <a href="mailto:liwang@redhat.com" target="_blank">liwang@redhat.com</a><br></div></div></div></div></div></div>
</div></div>