All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zhouping Liu <zliu@redhat.com>
To: Caspar Zhang <caspar@casparzhang.com>
Cc: LTP List <ltp-list@lists.sourceforge.net>
Subject: Re: [LTP] [PATCH 1/3] mm/ksm: set the merge_across_nodes knob before testing
Date: Mon, 13 May 2013 04:40:20 -0400 (EDT)	[thread overview]
Message-ID: <1284641591.353244.1368434420898.JavaMail.root@redhat.com> (raw)
In-Reply-To: <51909CD7.9050408@casparzhang.com>



----- Original Message -----
> From: "Caspar Zhang" <caspar@casparzhang.com>
> To: "Zhouping Liu" <zliu@redhat.com>
> Cc: "LTP List" <ltp-list@lists.sourceforge.net>
> Sent: Monday, May 13, 2013 3:57:11 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:
> > +		/*
> > +		 * Save the current value of merge_across_nodes knob,
> > +		 * and make it perform as the default behavior.
> > +		 */
> 
> This comment is unnecessary. it's not difficult to know it's a
> save/recover operation. I think you can remove it or give a short
> comment of where does this new sysfile come from (e.g. kernel ver/commit
> id, etc)

OK, as merge_across_nodes is new for us, I think we should give some useful
comments, so I updated the comment like below:

+       /*
+        * kernel commit 90bd6fd introduced a new KSM sysfs knob
+        * /sys/kernel/mm/ksm/merge_across_nodes, setting it to '0'
+        * will prevent KSM pages being merged across numa nodes,
+        * which will cause the case fail, so we need to make sure
+        * it is enabled before testing.
+        */ 
+       if (access(PATH_KSM "merge_across_nodes", F_OK) == 0) {
+               SAFE_FILE_SCANF(NULL, PATH_KSM "merge_across_nodes",
+                               "%d", &merge_across_nodes);
+               SAFE_FILE_PRINTF(NULL, PATH_KSM "merge_across_nodes", "1");
+       }
+

and the comment message only appear once in ksm01.c, removed it from ksm0[2|3|4].c.

please review V2.

-- 
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

      reply	other threads:[~2013-05-13  8:40 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-10  9:13 [LTP] [PATCH 1/3] mm/ksm: set the merge_across_nodes knob before testing Zhouping Liu
2013-05-10  9:13 ` [LTP] [PATCH 2/3] mem: introduce clean_node() func Zhouping Liu
2013-05-10  9:13 ` [LTP] [PATCH 3/3] mem/ksm06: add a new case to test merge_across_nodes sysfs knob Zhouping Liu
2013-05-10  9:43 ` [LTP] [PATCH 1/3] mm/ksm: set the merge_across_nodes knob before testing Wanlong Gao
2013-05-10 10:18   ` Zhouping Liu
2013-05-11 11:28     ` Wanlong Gao
2013-05-13  7:57 ` Caspar Zhang
2013-05-13  8:40   ` Zhouping Liu [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1284641591.353244.1368434420898.JavaMail.root@redhat.com \
    --to=zliu@redhat.com \
    --cc=caspar@casparzhang.com \
    --cc=ltp-list@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.