On 05/10/2013 06:18 PM, Zhouping Liu wrote: > Hi Wanlong, > > ----- Original Message ----- >> From: "Wanlong Gao" <[email protected]> >> To: "Zhouping Liu" <[email protected]> >> Cc: "LTP List" <[email protected]> >> 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 <[email protected]> >> >> 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?
OK to me. Thanks, Wanlong Gao > > + } > + > ... > > ------------------------------------------------------------------------------ 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 [email protected] https://lists.sourceforge.net/lists/listinfo/ltp-list
