[LTP] [PATCH] Fix short of nodemask array.

[gmail <lans.zhang2008@gm...>]

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.
In addition, some test cases in current implement doesn't consider
if a node number is greater than sizeof(unsigned long) * 8 - 1.

Signed-off-by: Lans Zhang <lans.zhang2008@...>
---
  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)));

  	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

Re: [LTP] [PATCH] Fix short of nodemask array.

[Wanlong Gao <gaowanlong@cn...>]

On 03/04/2013 12:49 PM, gmail wrote:
> 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.
> In addition, some test cases in current implement doesn't consider
> if a node number is greater than sizeof(unsigned long) * 8 - 1.

Hi Zhouping, Jan,

Could you review this patch? It already flies for a long time.

Thanks,
Wanlong Gao

> 
> Signed-off-by: Lans Zhang <lans.zhang2008@...>
> ---
>   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)));
> 
>   	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
> 

Re: [LTP] [PATCH] Fix short of nodemask array.

[Zhouping Liu <zliu@re...>]

Hi Lans,

----- Original Message -----
> From: "gmail" <lans.zhang2008@...>
> To: ltp-list@...
> 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.

> 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 <lans.zhang2008@...>
> ---
>   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)))

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@...
> https://lists.sourceforge.net/lists/listinfo/ltp-list
> 

-- 
Thanks,
Zhouping

Re: [LTP] [PATCH] Fix short of nodemask array.

[Jan Stancek <jstancek@re...>]

----- Original Message -----
> From: "Zhouping Liu" <zliu@...>
> To: "gmail" <lans.zhang2008@...>
> Cc: ltp-list@..., "Wanlong Gao" <gaowanlong@...>, "Jan Stancek" <jstancek@...>
> Sent: Tuesday, 16 April, 2013 6:34:53 AM
> Subject: Re: [LTP] [PATCH]  Fix short of nodemask array.
> 
> Hi Lans,
> 
> ----- Original Message -----
> > From: "gmail" <lans.zhang2008@...>
> > To: ltp-list@...
> > 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 <lans.zhang2008@...>
> > ---
> >   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@...
> > https://lists.sourceforge.net/lists/listinfo/ltp-list
> > 
> 
> --
> Thanks,
> Zhouping
> 

Re: [LTP] [PATCH] Fix short of nodemask array.

[Lans Zhang <lans.zhang2008@gm...>]

On 04/16/2013 02:58 PM, Jan Stancek wrote:
>
>
>
>
> ----- Original Message -----
>> From: "Zhouping Liu"<zliu@...>
>> To: "gmail"<lans.zhang2008@...>
>> Cc: ltp-list@..., "Wanlong Gao"<gaowanlong@...>, "Jan Stancek"<jstancek@...>
>> Sent: Tuesday, 16 April, 2013 6:34:53 AM
>> Subject: Re: [LTP] [PATCH]  Fix short of nodemask array.
>>
>> Hi Lans,
>>
>> ----- Original Message -----
>>> From: "gmail"<lans.zhang2008@...>
>>> To: ltp-list@...
>>> 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.

Correct.

>
>>
>>> 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<lans.zhang2008@...>
>>> ---
>>>    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);

Looks better. I will use this style in V2.

Thanks,
lz

>
> 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@...
>>> https://lists.sourceforge.net/lists/listinfo/ltp-list
>>>
>>
>> --
>> Thanks,
>> Zhouping
>>
>

Re: [LTP] [PATCH] Fix short of nodemask array.

[Wanlong Gao <gaowanlong@cn...>]

On 04/17/2013 11:23 AM, Lans Zhang wrote:

Please CC the ltp-list when reply.

Thanks,
Wanlong Gao

> On 04/16/2013 12:34 PM, Zhouping Liu wrote:
>> Hi Lans,
>>
>> ----- Original Message -----
>>> From: "gmail"<lans.zhang2008@...>
>>> To: ltp-list@...
>>> 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.
> 
> kernel requires the bits in nmask being longer than MAX_NUMNODES defined in kernel
> must be cleared:
> 
> /* Copy a node mask from user space. */
> static int get_nodes(nodemask_t *nodes, const unsigned long __user *nmask,
>                      unsigned long maxnode)
> {
>         unsigned long k;
>         unsigned long nlongs;
>         unsigned long endmask;
> 
>         --maxnode;
>         nodes_clear(*nodes);
>         if (maxnode == 0 || !nmask)
>                 return 0;
>         if (maxnode > PAGE_SIZE*BITS_PER_BYTE)
>                 return -EINVAL;
> 
>         nlongs = BITS_TO_LONGS(maxnode);
>         if ((maxnode % BITS_PER_LONG) == 0)
>                 endmask = ~0UL;
>         else
>                 endmask = (1UL << (maxnode % BITS_PER_LONG)) - 1;
> 
>         /* When the user specified more nodes than supported just check
>            if the non supported part is all zero. */
>         if (nlongs > BITS_TO_LONGS(MAX_NUMNODES)) {
>                 if (nlongs > PAGE_SIZE/sizeof(long))
>                         return -EINVAL;
>                 for (k = BITS_TO_LONGS(MAX_NUMNODES); k < nlongs; k++) {
>                         unsigned long t;
>                         if (get_user(t, nmask + k))
>                                 return -EFAULT;
>                         if (k == nlongs - 1) {
>                                 if (t & endmask)
>                                         return -EINVAL;
>                         } else if (t)   <--- failed here.
>                                 return -EINVAL;
>                 }
>                 nlongs = BITS_TO_LONGS(MAX_NUMNODES);
>                 endmask = ~0UL;
>         }
> 
> As what you concern, the patch header should be read more clear. I would
> use Jan Stancek's suggestion:
> 
> 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<lans.zhang2008@...>
>>> ---
>>>    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)))
>>
>> 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.
> 
> OK.
> 
> Thanks,
> lz
> 
>>
>> 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@...
>>> https://lists.sourceforge.net/lists/listinfo/ltp-list
>>>
>>
> 
> 

[LTP] [PATCH V2] Fix short of nodemask array.

[Lans Zhang <lans.zhang2008@gm...>]

Changes since V1:
1. Enhance patch header.
2. Use BITS_PER_LONG and set_node() to simplify the logic.

In kernel, if user specified more nodes, e.g, 512 nodes, than the
supported, e.g, 4 nodes, kernel will check if the non supported
part, e.g, 508 nodes, is all zeroed in node bitmap. Currently, we
are overrunning "nmask", whose length is shorter than MAXNODES,
and where the unsupported bits should be cleared to pass the check.

Signed-off-by: Lans Zhang <lans.zhang2008@...>
---
  testcases/kernel/mem/cpuset/cpuset01.c |   11 +++++++----
  testcases/kernel/mem/include/mem.h     |    7 +++++++
  testcases/kernel/mem/ksm/ksm02.c       |    6 +++---
  testcases/kernel/mem/ksm/ksm04.c       |    6 +++---
  testcases/kernel/mem/lib/mem.c         |    9 +++++----
  5 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/testcases/kernel/mem/cpuset/cpuset01.c b/testcases/kernel/mem/cpuset/cpuset01.c
index 1069335..99fbb9c 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 / BITS_PER_LONG] = { 0 };
  	char mems[BUFSIZ], buf[BUFSIZ];

  	read_cpuset_files(CPATH, "cpus", buf);
@@ -107,9 +107,12 @@ static void testcpuset(void)
  	case -1:
  		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)
+		for (i = 0; i < nnodes; i++) {
+			if (nodes[i] >= MAXNODES)
+				continue;
+			set_node(nmask, nodes[i]);
+		}
+		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/include/mem.h b/testcases/kernel/mem/include/mem.h
index fdf558e..f54aa17 100644
--- a/testcases/kernel/mem/include/mem.h
+++ b/testcases/kernel/mem/include/mem.h
@@ -14,6 +14,13 @@
  #define PATH_SYS_SYSTEM		"/sys/devices/system"
  #define PATH_SYSVM		"/proc/sys/vm/"
  #define PATH_MEMINFO		"/proc/meminfo"
+#define BITS_PER_LONG           (8 * sizeof(long))
+
+static inline void set_node(unsigned long *array, unsigned int node,
+                            unsigned int v)
+{
+	array[node / BITS_PER_LONG] |= 1UL << (node % BITS_PER_LONG);
+}

  /* OOM */

diff --git a/testcases/kernel/mem/ksm/ksm02.c b/testcases/kernel/mem/ksm/ksm02.c
index 412b52e..6c96c74 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 / BITS_PER_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;
+	set_node(nmask, node);

  	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 e164b29..ed5e0b3 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 / BITS_PER_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;
+	set_node(nmask, node);

  	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 c9525e5..69ea046 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -95,7 +95,7 @@ void testoom(int mempolicy, int lite)
  {
  #if HAVE_NUMA_H && HAVE_LINUX_MEMPOLICY_H && HAVE_NUMAIF_H \
  	&& HAVE_MPOL_CONSTANTS
-	unsigned long nmask = 0;
+	unsigned long nmask[MAXNODES / BITS_PER_LONG] = { 0 };
  	unsigned int num_nodes, *nodes;
  	int ret;

@@ -112,7 +112,7 @@ void testoom(int mempolicy, int lite)
  		switch(mempolicy) {
  		case MPOL_BIND:
  			/* bind the second node */
-			nmask = 1 << nodes[1];
+			set_node(nmask, nodes[1]);
  			break;
  		case MPOL_INTERLEAVE:
  		case MPOL_PREFERRED:
@@ -123,13 +123,14 @@ void testoom(int mempolicy, int lite)
  				return;
  			} else {
  				/* Using the 2nd,3rd node */
-				nmask = (1 << nodes[1]) | (1 << nodes[2]);
+				set_node(nmask, nodes[1]);
+				set_node(nmask, nodes[2]);
  			}
  			break;
  		default:
  			tst_brkm(TBROK|TERRNO, cleanup, "Bad mempolicy mode");
  		}
-		if (set_mempolicy(mempolicy, &nmask, MAXNODES) == -1)
+		if (set_mempolicy(mempolicy, nmask, MAXNODES) == -1)
  			tst_brkm(TBROK|TERRNO, cleanup, "set_mempolicy");
  	}
  #endif
-- 
1.7.8.110.g4cb5d

Re: [LTP] [PATCH V2] Fix short of nodemask array.

[Jan Stancek <jstancek@re...>]

----- Original Message -----
> From: "Lans Zhang" <lans.zhang2008@...>
> To: "Zhouping Liu" <zliu@...>, "Jan Stancek" <jstancek@...>
> Cc: "Wanlong Gao" <gaowanlong@...>, ltp-list@...
> Sent: Wednesday, 17 April, 2013 7:09:00 AM
> Subject: [LTP][PATCH V2] Fix short of nodemask array.
> 
> Changes since V1:
> 1. Enhance patch header.
> 2. Use BITS_PER_LONG and set_node() to simplify the logic.
> 
> In kernel, if user specified more nodes, e.g, 512 nodes, than the
> supported, e.g, 4 nodes, kernel will check if the non supported
> part, e.g, 508 nodes, is all zeroed in node bitmap. Currently, we
> are overrunning "nmask", whose length is shorter than MAXNODES,
> and where the unsupported bits should be cleared to pass the check.
> 
> Signed-off-by: Lans Zhang <lans.zhang2008@...>

Hi,

'v' should be probably dropped here, since it's not used and rest
of patch uses 2 parameters anyway:

> +static inline void set_node(unsigned long *array, unsigned int node,
> +                            unsigned int v)
> +{
> +	array[node / BITS_PER_LONG] |= 1UL << (node % BITS_PER_LONG);
> +}
> 

Other than that, it looks good to me.

Regards,
Jan

Re: [LTP] [PATCH V2] Fix short of nodemask array.

[Lans Zhang <lans.zhang2008@gm...>]

On 04/17/2013 03:37 PM, Jan Stancek wrote:
>
>
>
>
> ----- Original Message -----
>> From: "Lans Zhang"<lans.zhang2008@...>
>> To: "Zhouping Liu"<zliu@...>, "Jan Stancek"<jstancek@...>
>> Cc: "Wanlong Gao"<gaowanlong@...>, ltp-list@...
>> Sent: Wednesday, 17 April, 2013 7:09:00 AM
>> Subject: [LTP][PATCH V2] Fix short of nodemask array.
>>
>> Changes since V1:
>> 1. Enhance patch header.
>> 2. Use BITS_PER_LONG and set_node() to simplify the logic.
>>
>> In kernel, if user specified more nodes, e.g, 512 nodes, than the
>> supported, e.g, 4 nodes, kernel will check if the non supported
>> part, e.g, 508 nodes, is all zeroed in node bitmap. Currently, we
>> are overrunning "nmask", whose length is shorter than MAXNODES,
>> and where the unsupported bits should be cleared to pass the check.
>>
>> Signed-off-by: Lans Zhang<lans.zhang2008@...>
>
> Hi,
>
> 'v' should be probably dropped here, since it's not used and rest
> of patch uses 2 parameters anyway:
>
>> +static inline void set_node(unsigned long *array, unsigned int node,
>> +                            unsigned int v)
>> +{
>> +	array[node / BITS_PER_LONG] |= 1UL<<  (node % BITS_PER_LONG);
>> +}
>>
>
> Other than that, it looks good to me.

I will send a V3 if Zhouping Liu also gatekeep V2 except this minor
change.

Thanks,
lz

>
> Regards,
> Jan
>

Re: [LTP] [PATCH V2] Fix short of nodemask array.

[Zhouping Liu <zliu@re...>]

----- Original Message -----
> From: "Lans Zhang" <lans.zhang2008@...>
> To: "Jan Stancek" <jstancek@...>
> Cc: "Zhouping Liu" <zliu@...>, "Wanlong Gao" <gaowanlong@...>, ltp-list@...
> Sent: Wednesday, April 17, 2013 4:35:36 PM
> Subject: Re: [LTP][PATCH V2] Fix short of nodemask array.
> 
> On 04/17/2013 03:37 PM, Jan Stancek wrote:
> >
> >
> >
> >
> > ----- Original Message -----
> >> From: "Lans Zhang"<lans.zhang2008@...>
> >> To: "Zhouping Liu"<zliu@...>, "Jan Stancek"<jstancek@...>
> >> Cc: "Wanlong Gao"<gaowanlong@...>,
> >> ltp-list@...
> >> Sent: Wednesday, 17 April, 2013 7:09:00 AM
> >> Subject: [LTP][PATCH V2] Fix short of nodemask array.
> >>
> >> Changes since V1:
> >> 1. Enhance patch header.
> >> 2. Use BITS_PER_LONG and set_node() to simplify the logic.
> >>
> >> In kernel, if user specified more nodes, e.g, 512 nodes, than the
> >> supported, e.g, 4 nodes, kernel will check if the non supported
> >> part, e.g, 508 nodes, is all zeroed in node bitmap. Currently, we
> >> are overrunning "nmask", whose length is shorter than MAXNODES,
> >> and where the unsupported bits should be cleared to pass the check.
> >>
> >> Signed-off-by: Lans Zhang<lans.zhang2008@...>
> >
> > Hi,
> >
> > 'v' should be probably dropped here, since it's not used and rest
> > of patch uses 2 parameters anyway:
> >
> >> +static inline void set_node(unsigned long *array, unsigned int node,
> >> +                            unsigned int v)
> >> +{
> >> +	array[node / BITS_PER_LONG] |= 1UL<<  (node % BITS_PER_LONG);
> >> +}
> >>
> >
> > Other than that, it looks good to me.
> 
> I will send a V3 if Zhouping Liu also gatekeep V2 except this minor
> change.

I'm also fine for the rest.
just one reminder: commit 0b3419364 modified lib/mem.c, which caused conflicts
with your patch, please prepare a new patch based on the latest tree, in order
to push it easily for maintainer.

-- 
Thanks,
Zhouping

Re: [LTP] [PATCH V2] Fix short of nodemask array.

[Lans Zhang <lans.zhang2008@gm...>]

On 04/17/2013 04:56 PM, Zhouping Liu wrote:
>
>
> ----- Original Message -----
>> From: "Lans Zhang"<lans.zhang2008@...>
>> To: "Jan Stancek"<jstancek@...>
>> Cc: "Zhouping Liu"<zliu@...>, "Wanlong Gao"<gaowanlong@...>, ltp-list@...
>> Sent: Wednesday, April 17, 2013 4:35:36 PM
>> Subject: Re: [LTP][PATCH V2] Fix short of nodemask array.
>>
>> On 04/17/2013 03:37 PM, Jan Stancek wrote:
>>>
>>>
>>>
>>>
>>> ----- Original Message -----
>>>> From: "Lans Zhang"<lans.zhang2008@...>
>>>> To: "Zhouping Liu"<zliu@...>, "Jan Stancek"<jstancek@...>
>>>> Cc: "Wanlong Gao"<gaowanlong@...>,
>>>> ltp-list@...
>>>> Sent: Wednesday, 17 April, 2013 7:09:00 AM
>>>> Subject: [LTP][PATCH V2] Fix short of nodemask array.
>>>>
>>>> Changes since V1:
>>>> 1. Enhance patch header.
>>>> 2. Use BITS_PER_LONG and set_node() to simplify the logic.
>>>>
>>>> In kernel, if user specified more nodes, e.g, 512 nodes, than the
>>>> supported, e.g, 4 nodes, kernel will check if the non supported
>>>> part, e.g, 508 nodes, is all zeroed in node bitmap. Currently, we
>>>> are overrunning "nmask", whose length is shorter than MAXNODES,
>>>> and where the unsupported bits should be cleared to pass the check.
>>>>
>>>> Signed-off-by: Lans Zhang<lans.zhang2008@...>
>>>
>>> Hi,
>>>
>>> 'v' should be probably dropped here, since it's not used and rest
>>> of patch uses 2 parameters anyway:
>>>
>>>> +static inline void set_node(unsigned long *array, unsigned int node,
>>>> +                            unsigned int v)
>>>> +{
>>>> +	array[node / BITS_PER_LONG] |= 1UL<<   (node % BITS_PER_LONG);
>>>> +}
>>>>
>>>
>>> Other than that, it looks good to me.
>>
>> I will send a V3 if Zhouping Liu also gatekeep V2 except this minor
>> change.
>
> I'm also fine for the rest.
> just one reminder: commit 0b3419364 modified lib/mem.c, which caused conflicts
> with your patch, please prepare a new patch based on the latest tree, in order
> to push it easily for maintainer.

NP. V3 will be based on the latest git tree.

Thanks,
lz

>

[LTP] [PATCH V3] Fix short of nodemask array.

[Lans Zhang <lans.zhang2008@gm...>]

Changes since V2:
remove unused parameter in set_node()

In kernel, if user specified more nodes, e.g, 512 nodes, than the
supported, e.g, 4 nodes, kernel will check if the non supported
part, e.g, 508 nodes, is all zeroed in node bitmap. Currently, we
are overrunning "nmask", whose length is shorter than MAXNODES,
and where the unsupported bits should be cleared to pass the check.

Signed-off-by: Lans Zhang <lans.zhang2008@...>
---
   testcases/kernel/mem/cpuset/cpuset01.c |   11 +++++++----
   testcases/kernel/mem/include/mem.h     |    6 ++++++
   testcases/kernel/mem/ksm/ksm02.c       |    6 +++---
   testcases/kernel/mem/ksm/ksm04.c       |    6 +++---
   testcases/kernel/mem/lib/mem.c         |    9 +++++----
   5 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/testcases/kernel/mem/cpuset/cpuset01.c b/testcases/kernel/mem/cpuset/cpuset01.c
index 1069335..99fbb9c 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 / BITS_PER_LONG] = { 0 };
   	char mems[BUFSIZ], buf[BUFSIZ];

   	read_cpuset_files(CPATH, "cpus", buf);
@@ -107,9 +107,12 @@ static void testcpuset(void)
   	case -1:
   		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)
+		for (i = 0; i < nnodes; i++) {
+			if (nodes[i] >= MAXNODES)
+				continue;
+			set_node(nmask, nodes[i]);
+		}
+		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/include/mem.h b/testcases/kernel/mem/include/mem.h
index c9e0540..9e96b31 100644
--- a/testcases/kernel/mem/include/mem.h
+++ b/testcases/kernel/mem/include/mem.h
@@ -14,6 +14,12 @@
   #define PATH_SYS_SYSTEM		"/sys/devices/system"
   #define PATH_SYSVM		"/proc/sys/vm/"
   #define PATH_MEMINFO		"/proc/meminfo"
+#define BITS_PER_LONG           (8 * sizeof(long))
+
+static inline void set_node(unsigned long *array, unsigned int node)
+{
+	array[node / BITS_PER_LONG] |= 1UL << (node % BITS_PER_LONG);
+}

   /* OOM */

diff --git a/testcases/kernel/mem/ksm/ksm02.c b/testcases/kernel/mem/ksm/ksm02.c
index 412b52e..6c96c74 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 / BITS_PER_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;
+	set_node(nmask, node);

   	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 e164b29..ed5e0b3 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 / BITS_PER_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;
+	set_node(nmask, node);

   	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 f6aa351..da98552 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -95,7 +95,7 @@ static void set_global_mempolicy(int mempolicy)
   {
   #if HAVE_NUMA_H && HAVE_LINUX_MEMPOLICY_H && HAVE_NUMAIF_H \
   	&& HAVE_MPOL_CONSTANTS
-	unsigned long nmask = 0;
+	unsigned long nmask[MAXNODES / BITS_PER_LONG] = { 0 };
   	unsigned int num_nodes, *nodes;
   	int ret;

@@ -112,7 +112,7 @@ static void set_global_mempolicy(int mempolicy)
   		switch(mempolicy) {
   		case MPOL_BIND:
   			/* bind the second node */
-			nmask = 1 << nodes[1];
+			set_node(nmask, nodes[1]);
   			break;
   		case MPOL_INTERLEAVE:
   		case MPOL_PREFERRED:
@@ -123,13 +123,14 @@ static void set_global_mempolicy(int mempolicy)
   				return;
   			} else {
   				/* Using the 2nd,3rd node */
-				nmask = (1 << nodes[1]) | (1 << nodes[2]);
+				set_node(nmask, nodes[1]);
+				set_node(nmask, nodes[2]);
   			}
   			break;
   		default:
   			tst_brkm(TBROK|TERRNO, cleanup, "Bad mempolicy mode");
   		}
-		if (set_mempolicy(mempolicy, &nmask, MAXNODES) == -1)
+		if (set_mempolicy(mempolicy, nmask, MAXNODES) == -1)
   			tst_brkm(TBROK|TERRNO, cleanup, "set_mempolicy");
   	}
   #endif
-- 
1.7.8.110.g4cb5d

Re: [LTP] [PATCH V3] Fix short of nodemask array.

[Jan Stancek <jstancek@re...>]

----- Original Message -----
> From: "Lans Zhang" <lans.zhang2008@...>
> To: "Jan Stancek" <jstancek@...>, "Zhouping Liu" <zliu@...>
> Cc: "Wanlong Gao" <gaowanlong@...>, ltp-list@...
> Sent: Wednesday, 17 April, 2013 11:18:37 AM
> Subject: [PATCH V3] [LTP] Fix short of nodemask array.
> 
> Changes since V2:
> remove unused parameter in set_node()
> 
> In kernel, if user specified more nodes, e.g, 512 nodes, than the
> supported, e.g, 4 nodes, kernel will check if the non supported
> part, e.g, 508 nodes, is all zeroed in node bitmap. Currently, we
> are overrunning "nmask", whose length is shorter than MAXNODES,
> and where the unsupported bits should be cleared to pass the check.
> 
> Signed-off-by: Lans Zhang <lans.zhang2008@...>

Reviewed-by: Jan Stancek <jstancek@...>

Looks good to me, regards,
Jan

Re: [LTP] [PATCH V3] Fix short of nodemask array.

[Wanlong Gao <gaowanlong@cn...>]

On 04/17/2013 05:22 PM, Jan Stancek wrote:
> 
> 
> 
> 
> ----- Original Message -----
>> From: "Lans Zhang" <lans.zhang2008@...>
>> To: "Jan Stancek" <jstancek@...>, "Zhouping Liu" <zliu@...>
>> Cc: "Wanlong Gao" <gaowanlong@...>, ltp-list@...
>> Sent: Wednesday, 17 April, 2013 11:18:37 AM
>> Subject: [PATCH V3] [LTP] Fix short of nodemask array.
>>
>> Changes since V2:
>> remove unused parameter in set_node()
>>
>> In kernel, if user specified more nodes, e.g, 512 nodes, than the
>> supported, e.g, 4 nodes, kernel will check if the non supported
>> part, e.g, 508 nodes, is all zeroed in node bitmap. Currently, we
>> are overrunning "nmask", whose length is shorter than MAXNODES,
>> and where the unsupported bits should be cleared to pass the check.
>>
>> Signed-off-by: Lans Zhang <lans.zhang2008@...>
> 
> Reviewed-by: Jan Stancek <jstancek@...>

Reviewed-by: Wanlong Gao <gaowanlong@...>

> 
> Looks good to me, regards,
> Jan
> 

Re: [LTP] [PATCH V3] Fix short of nodemask array.

[Zhouping Liu <zliu@re...>]

----- Original Message -----
> From: "Lans Zhang" <lans.zhang2008@...>
> To: "Jan Stancek" <jstancek@...>, "Zhouping Liu" <zliu@...>
> Cc: "Wanlong Gao" <gaowanlong@...>, ltp-list@...
> Sent: Wednesday, April 17, 2013 5:18:37 PM
> Subject: [PATCH V3] [LTP] Fix short of nodemask array.
> 
> Changes since V2:
> remove unused parameter in set_node()
> 
> In kernel, if user specified more nodes, e.g, 512 nodes, than the
> supported, e.g, 4 nodes, kernel will check if the non supported
> part, e.g, 508 nodes, is all zeroed in node bitmap. Currently, we
> are overrunning "nmask", whose length is shorter than MAXNODES,
> and where the unsupported bits should be cleared to pass the check.
> 
> Signed-off-by: Lans Zhang <lans.zhang2008@...>

Reviewed-by: Zhouping Liu <zliu@...>

-- 
Thanks,
Zhouping

Re: [LTP] [PATCH V3] Fix short of nodemask array.

[Wanlong Gao <gaowanlong@cn...>]

On 04/17/2013 05:18 PM, Lans Zhang wrote:
> Changes since V2:
> remove unused parameter in set_node()
> 
> In kernel, if user specified more nodes, e.g, 512 nodes, than the
> supported, e.g, 4 nodes, kernel will check if the non supported
> part, e.g, 508 nodes, is all zeroed in node bitmap. Currently, we
> are overrunning "nmask", whose length is shorter than MAXNODES,
> and where the unsupported bits should be cleared to pass the check.
> 
> Signed-off-by: Lans Zhang <lans.zhang2008@...>

Applied, thank you all involved.

Wanlong Gao