From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from sog-mx-4.v43.ch3.sourceforge.com ([172.29.43.194] helo=mx.sourceforge.net) by sfs-ml-4.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1URzqx-0005fk-BW for ltp-list@lists.sourceforge.net; Tue, 16 Apr 2013 06:58:55 +0000 Received: from mx3-phx2.redhat.com ([209.132.183.24]) by sog-mx-4.v43.ch3.sourceforge.com with esmtp (Exim 4.76) id 1URzqv-0002Lk-BK for ltp-list@lists.sourceforge.net; Tue, 16 Apr 2013 06:58:55 +0000 Date: Tue, 16 Apr 2013 02:58:38 -0400 (EDT) From: Jan Stancek Message-ID: <2046993339.742574.1366095518611.JavaMail.root@redhat.com> In-Reply-To: <414861962.834600.1366086893127.JavaMail.root@redhat.com> References: <513427E9.3000807@gmail.com> <414861962.834600.1366086893127.JavaMail.root@redhat.com> MIME-Version: 1.0 Subject: Re: [LTP] [PATCH] Fix short of nodemask array. List-Id: Linux Test Project General Discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ltp-list-bounces@lists.sourceforge.net To: Zhouping Liu Cc: ltp-list@lists.sourceforge.net, gmail ----- Original Message ----- > From: "Zhouping Liu" > To: "gmail" > Cc: ltp-list@lists.sourceforge.net, "Wanlong Gao" , "Jan Stancek" > Sent: Tuesday, 16 April, 2013 6:34:53 AM > Subject: Re: [LTP] [PATCH] Fix short of nodemask array. > > Hi Lans, > > ----- Original Message ----- > > From: "gmail" > > To: ltp-list@lists.sourceforge.net > > Sent: Monday, March 4, 2013 12:49:45 PM > > Subject: [LTP] [PATCH] Fix short of nodemask array. > > > > In kernel, when the user specified more nodes, e.g, 512 nodes, than > > supported, e.g, 4 nodes, just check if the non supported part is all > > zero. If user space just specified the nodemask array with only one > > element, which makes the check in kernel actually become invalid. > > I think the comments here is not clear, you should explain more details > about the 'one element' or the reason why the checking in kernel will > be invalid. I think what it says is that we are overrunning "nmask", whose length is shorter than MAXNODES. > > > In addition, some test cases in current implement doesn't consider > > if a node number is greater than sizeof(unsigned long) * 8 - 1. > > yes, you are right, we don't care the system which has more than 512 numa > nodes. > and the current code only support 64 numa nodes, only one unsigned long node > mask. > > I checked kernel code in RHEL6, CONFIG_NODES_SHIFT=9, so 512 is the biggest > value to support in RHEL6 for x86_64, and I didn't meet any large NUMA system > which have 512+ numa nodes so far. > > > > > Signed-off-by: Lans Zhang > > --- > > testcases/kernel/mem/cpuset/cpuset01.c | 6 +++--- > > testcases/kernel/mem/ksm/ksm02.c | 6 +++--- > > testcases/kernel/mem/ksm/ksm04.c | 6 +++--- > > testcases/kernel/mem/lib/mem.c | 6 +++--- > > 4 files changed, 12 insertions(+), 12 deletions(-) > > > > diff --git a/testcases/kernel/mem/cpuset/cpuset01.c > > b/testcases/kernel/mem/cpuset/cpuset01.c > > index b473e45..e459306 100644 > > --- a/testcases/kernel/mem/cpuset/cpuset01.c > > +++ b/testcases/kernel/mem/cpuset/cpuset01.c > > @@ -93,7 +93,7 @@ static void testcpuset(void) > > { > > int lc; > > int child, i, status; > > - unsigned long nmask = 0; > > + unsigned long nmask[MAXNODES / (8*sizeof(unsigned long))] = { 0 }; > > char mems[BUFSIZ], buf[BUFSIZ]; > > > > read_cpuset_files(CPATH, "cpus", buf); > > @@ -108,8 +108,8 @@ static void testcpuset(void) > > tst_brkm(TBROK | TERRNO, cleanup, "fork"); > > case 0: > > for (i = 0; i < nnodes; i++) > > - nmask += 1 << nodes[i]; > > - if (set_mempolicy(MPOL_BIND, &nmask, MAXNODES) == -1) > > + nmask[i / (8*sizeof(unsigned long))] |= 1 << (i % (8*sizeof(unsigned > > long))); > > + if (set_mempolicy(MPOL_BIND, nmask, MAXNODES) == -1) > > tst_brkm(TBROK | TERRNO, cleanup, "set_mempolicy"); > > exit(mem_hog_cpuset(ncpus > 1 ? ncpus : 1)); > > } > > diff --git a/testcases/kernel/mem/ksm/ksm02.c > > b/testcases/kernel/mem/ksm/ksm02.c > > index be9ff96..9d2d142 100644 > > --- a/testcases/kernel/mem/ksm/ksm02.c > > +++ b/testcases/kernel/mem/ksm/ksm02.c > > @@ -87,7 +87,7 @@ int main(int argc, char *argv[]) > > int lc; > > char *msg; > > int size = 128, num = 3, unit = 1; > > - unsigned long nmask = 0; > > + unsigned long nmask[MAXNODES / (8*sizeof(unsigned long))] = { 0 }; > > unsigned int node; > > > > msg = parse_opts(argc, argv, ksm_options, ksm_usage); > > @@ -95,7 +95,7 @@ int main(int argc, char *argv[]) > > tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg); > > > > node = get_a_numa_node(tst_exit); > > - nmask = 1 << node; > > + nmask[node / (8*sizeof(unsigned long))] = 1 << (node % (8*sizeof(unsigned > > long))); > > I found '1 << (node % (8*sizeof(unsigned long)))' is used frequently, I > suggest we > can create a macro, such as: > # define GET_NODE_MASK(nid) 1 << (nid % (8*sizeof(unsigned long))) I agree, also 'nmask[i / (8*sizeof(unsigned long))]' is there often. How about something like: #define bitsperlong (8 * sizeof(unsigned long)) void set_bit(unsigned long *b, unsigned int n, unsigned int v) { if (v) b[n / bitsperlong] |= 1UL << (n % bitsperlong); else b[n / bitsperlong] &= ~(1UL << (n % bitsperlong)); } set_bit(nmask, node, 1); Regards, Jan > > also please update the patch, as there's some updates before your patch in > lib/mem.c, > which makes some conflict with the latest tree. > > Thanks, > Zhouping > > > > > setup(); > > > > @@ -103,7 +103,7 @@ int main(int argc, char *argv[]) > > Tst_count = 0; > > check_ksm_options(&size, &num, &unit); > > > > - if (set_mempolicy(MPOL_BIND, &nmask, MAXNODES) == -1) { > > + if (set_mempolicy(MPOL_BIND, nmask, MAXNODES) == -1) { > > if (errno != ENOSYS) > > tst_brkm(TBROK | TERRNO, cleanup, > > "set_mempolicy"); > > diff --git a/testcases/kernel/mem/ksm/ksm04.c > > b/testcases/kernel/mem/ksm/ksm04.c > > index e4aa417..7829afd 100644 > > --- a/testcases/kernel/mem/ksm/ksm04.c > > +++ b/testcases/kernel/mem/ksm/ksm04.c > > @@ -87,7 +87,7 @@ int main(int argc, char *argv[]) > > int lc; > > char *msg; > > int size = 128, num = 3, unit = 1; > > - unsigned long nmask = 0; > > + unsigned long nmask[MAXNODES / (8*sizeof(unsigned long))] = { 0 }; > > unsigned int node; > > > > msg = parse_opts(argc, argv, ksm_options, ksm_usage); > > @@ -95,7 +95,7 @@ int main(int argc, char *argv[]) > > tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg); > > > > node = get_a_numa_node(tst_exit); > > - nmask = 1 << node; > > + nmask[node / (8*sizeof(unsigned long))] = 1 << (node % (8*sizeof(unsigned > > long))); > > > > setup(); > > > > @@ -105,7 +105,7 @@ int main(int argc, char *argv[]) > > > > write_memcg(); > > > > - if (set_mempolicy(MPOL_BIND, &nmask, MAXNODES) == -1) { > > + if (set_mempolicy(MPOL_BIND, nmask, MAXNODES) == -1) { > > if (errno != ENOSYS) > > tst_brkm(TBROK | TERRNO, cleanup, > > "set_mempolicy"); > > diff --git a/testcases/kernel/mem/lib/mem.c > > b/testcases/kernel/mem/lib/mem.c > > index f095fe1..abe29fd 100644 > > --- a/testcases/kernel/mem/lib/mem.c > > +++ b/testcases/kernel/mem/lib/mem.c > > @@ -66,12 +66,12 @@ void oom(int testcase, int mempolicy, int lite) > > int status; > > #if HAVE_NUMA_H && HAVE_LINUX_MEMPOLICY_H && HAVE_NUMAIF_H \ > > && HAVE_MPOL_CONSTANTS > > - unsigned long nmask = 0; > > + unsigned long nmask[MAXNODES / (8*sizeof(unsigned long))] = { 0 }; > > unsigned int node; > > > > if (mempolicy) > > node = get_a_numa_node(cleanup); > > - nmask += 1 << node; > > + nmask[node / (8*sizeof(unsigned long))] = 1 << (node % (8*sizeof(unsigned > > long))); > > #endif > > > > switch (pid = fork()) { > > @@ -81,7 +81,7 @@ void oom(int testcase, int mempolicy, int lite) > > #if HAVE_NUMA_H && HAVE_LINUX_MEMPOLICY_H && HAVE_NUMAIF_H \ > > && HAVE_MPOL_CONSTANTS > > if (mempolicy) > > - if (set_mempolicy(MPOL_BIND, &nmask, MAXNODES) == -1) > > + if (set_mempolicy(MPOL_BIND, nmask, MAXNODES) == -1) > > tst_brkm(TBROK | TERRNO, cleanup, > > "set_mempolicy"); > > #endif > > -- > > 1.7.8.110.g4cb5d > > > > ------------------------------------------------------------------------------ > > Minimize network downtime and maximize team effectiveness. > > Reduce network management and security costs.Learn how to hire > > the most talented Cisco Certified professionals. Visit the > > Employer Resources Portal > > http://www.cisco.com/web/learning/employer_resources/index.html > > _______________________________________________ > > Ltp-list mailing list > > Ltp-list@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/ltp-list > > > > -- > Thanks, > Zhouping > ------------------------------------------------------------------------------ Precog is a next-generation analytics platform capable of advanced analytics on semi-structured data. The platform includes APIs for building apps and a phenomenal toolset for data science. Developers can use our toolset for easy data analysis & visualization. Get a free account! http://www2.precog.com/precogplatform/slashdotnewsletter _______________________________________________ Ltp-list mailing list Ltp-list@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ltp-list