Re: [LTP] [PATCH 1/3] mm/ksm: set the merge_across_nodes knob before testing

[Zhouping Liu <zliu@redhat.com>]

Hi Wanlong,

----- Original Message -----
> From: "Wanlong Gao" <gaowanlong@cn.fujitsu.com>
> To: "Zhouping Liu" <zliu@redhat.com>
> Cc: "LTP List" <ltp-list@lists.sourceforge.net>
> Sent: Friday, May 10, 2013 5:43:52 PM
> Subject: Re: [LTP] [PATCH 1/3] mm/ksm: set the merge_across_nodes knob before testing
> 
> On 05/10/2013 05:13 PM, Zhouping Liu wrote:
> > This kernel commit 90bd6fd31c809(ksm: allow trees per NUMA node)
> > introduced a new KSM sysfs knob /sys/kernel/mm/ksm/merge_across_nodes,
> > when it is set to zero, only pages from the same node are merged,
> > which is different with the previous behavior, and ksm test cases
> > sometimes will fail in NUMA system.
> > 
> > Signed-off-by: Zhouping Liu <zliu@redhat.com>
> 
> While how about wrap the set and reset "merge_across_nodes" value to
> functions,
> so that we can reduce the dup code and dup comments?

yeah, it sounds good, but... look at the codes again:

...
 void cleanup(void)
 {
+       if (access(PATH_KSM "merge_across_nodes", F_OK) == 0)
+               /* recover the old value */
+               SAFE_FILE_PRINTF(NULL, PATH_KSM "merge_across_nodes",
+                                "%d", merge_across_nodes);

resetting "merge_across_nodes" only needs one statement.

+
        umount_mem(CPATH, CPATH_NEW);
        TEST_CLEANUP;
 }
@@ -136,6 +143,16 @@ void setup(void)
        if (access(PATH_KSM, F_OK) == -1)
                tst_brkm(TCONF, NULL, "KSM configuration is not enabled");

+       if (access(PATH_KSM "merge_across_nodes", F_OK) == 0) {
+               /*
+                * Save the current value of merge_across_nodes knob,
+                * and make it perform as the default behavior.
+                */
+               SAFE_FILE_SCANF(NULL, PATH_KSM "merge_across_nodes",
+                               "%d", &merge_across_nodes);
+               SAFE_FILE_PRINTF(NULL, PATH_KSM "merge_across_nodes", "1");

and setting the value only needs three statements, and it looks easy to understand,
I can remove the comments from ksm02.c ksm03.c ksm04.c to reduce the duplicated comments.

also "merge_across_nodes" is a long string, I can't think of a better function name to
implement set/unset merge_across_nodes, so... IMO, the current codes is good, do you think so?

+       }
+
...
 
-- 
Thanks,
Zhouping

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and 
their applications. This 200-page book is written by three acclaimed 
leaders in the field. The early access version is available now. 
Download your free book today! http://p.sf.net/sfu/neotech_d2d_may
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list