Fix short of nodemask array.

PATCH Superseded Lans Zhang 3 participants 5 messages Apr 17, 2013

lore

Share & tools

lore

Lans Zhang

author

2013-04-17 05:04 UTC #0 lore raw

PATCH v2 1/1 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 <[email protected]>

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

testcases/kernel/mem/cpuset/cpuset01.c +7 -4

index 1069335..99fbb9c 100644
@@ -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));
 	}

testcases/kernel/mem/include/mem.h +7 -0

index fdf558e..f54aa17 100644
@@ -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 */

testcases/kernel/mem/ksm/ksm02.c +3 -3

index 412b52e..6c96c74 100644
@@ -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");

testcases/kernel/mem/ksm/ksm04.c +3 -3

index e164b29..ed5e0b3 100644
@@ -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");

testcases/kernel/mem/lib/mem.c +5 -4

index c9525e5..69ea046 100644
@@ -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

Signature · 1 line

1.7.8.110.g4cb5d

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

Signed-off-by Lans Zhang <[email protected]>

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

testcases/kernel/mem/cpuset/cpuset01.c +7 -4

index 1069335..99fbb9c 100644
@@ -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));
 	}

testcases/kernel/mem/include/mem.h +7 -0

index fdf558e..f54aa17 100644
@@ -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 */

testcases/kernel/mem/ksm/ksm02.c +3 -3

index 412b52e..6c96c74 100644
@@ -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");

testcases/kernel/mem/ksm/ksm04.c +3 -3

index e164b29..ed5e0b3 100644
@@ -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");

testcases/kernel/mem/lib/mem.c +5 -4

index c9525e5..69ea046 100644
@@ -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

Signature · 1 line

1.7.8.110.g4cb5d

------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service that delivers powerful full stack analytics. Optimize and monitor your browser, app, & servers with just a few lines of code. Try New Relic and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_apr

_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Jan Stancek

2013-04-17 07:37 UTC #1 lore raw

----- Original Message -----

Quoted 10 lines

From: "Lans Zhang" <[email protected]>
To: "Zhouping Liu" <[email protected]>, "Jan Stancek" <[email protected]>
Cc: "Wanlong Gao" <[email protected]>, [email protected]
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.

Quoted 7 lines

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 <[email protected]>

Hi,

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

testcases/kernel/mem/include/mem.h line 23

Quoted 6 lines

testcases/kernel/mem/include/mem.h +5 -0

+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

------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Lans Zhang

author

2013-04-17 08:31 UTC #2 lore raw

Quoted 28 lines · Jan Stancek

----- Original Message -----
Quoted 10 lines
From: "Lans Zhang"<[email protected]>
To: "Zhouping Liu"<[email protected]>, "Jan Stancek"<[email protected]>
Cc: "Wanlong Gao"<[email protected]>, [email protected]
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.
Quoted 7 lines
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<[email protected]>
Hi,
'v' should be probably dropped here, since it's not used and rest of patch uses 2 parameters anyway:

Quoted 8 lines

testcases/kernel/mem/include/mem.h line 23
Quoted 6 lines
testcases/kernel/mem/include/mem.h +5 -0
+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

Quoted 4 lines

Regards,
Jan

Zhouping Liu

2013-04-17 08:57 UTC #3 lore raw

----- Original Message -----

Quoted 45 lines

From: "Lans Zhang" <[email protected]>
To: "Jan Stancek" <[email protected]>
Cc: "Zhouping Liu" <[email protected]>, "Wanlong Gao" <[email protected]>, [email protected]
Sent: Wednesday, April 17, 2013 4:35:36 PM
Subject: Re: [LTP][PATCH V2] Fix short of nodemask array.
Quoted 29 lines · Jan Stancek
----- Original Message -----
Quoted 11 lines
From: "Lans Zhang"<[email protected]>
To: "Zhouping Liu"<[email protected]>, "Jan Stancek"<[email protected]>
Cc: "Wanlong Gao"<[email protected]>, [email protected]
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.
Quoted 7 lines
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<[email protected]>
Hi,
'v' should be probably dropped here, since it's not used and rest of patch uses 2 parameters anyway:
Quoted 8 lines
testcases/kernel/mem/include/mem.h line 23
Quoted 6 lines
testcases/kernel/mem/include/mem.h +5 -0
+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.

Quoted 2 lines

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.

Signature · 13 lines

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
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Lans Zhang

author

2013-04-17 09:13 UTC #4 lore raw

Quoted 48 lines · Zhouping Liu

----- Original Message -----
Quoted 45 lines
From: "Lans Zhang"<[email protected]>
To: "Jan Stancek"<[email protected]>
Cc: "Zhouping Liu"<[email protected]>, "Wanlong Gao"<[email protected]>, [email protected]
Sent: Wednesday, April 17, 2013 4:35:36 PM
Subject: Re: [LTP][PATCH V2] Fix short of nodemask array.
Quoted 37 lines · Jan Stancek
----- Original Message -----
Quoted 11 lines
From: "Lans Zhang"<[email protected]>
To: "Zhouping Liu"<[email protected]>, "Jan Stancek"<[email protected]>
Cc: "Wanlong Gao"<[email protected]>, [email protected]
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.
Quoted 7 lines
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<[email protected]>
Hi,
'v' should be probably dropped here, since it's not used and rest of patch uses 2 parameters anyway:
testcases/kernel/mem/include/mem.h line 23
Quoted 6 lines
testcases/kernel/mem/include/mem.h +5 -0
+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.

Quoted 7 lines

Quoted 2 lines
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

Quoted 1 line