* [LTP] [PATCH] Fix short of nodemask array.
@ 2013-03-04 4:49 gmail
2013-04-16 2:03 ` Wanlong Gao
2013-04-16 4:34 ` Zhouping Liu
0 siblings, 2 replies; 6+ messages in thread
From: gmail @ 2013-03-04 4:49 UTC (permalink / raw)
To: ltp-list
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@gmail.com>
---
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
------------------------------------------------------------------------------
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] Fix short of nodemask array.
2013-03-04 4:49 [LTP] [PATCH] Fix short of nodemask array gmail
@ 2013-04-16 2:03 ` Wanlong Gao
2013-04-16 4:34 ` Zhouping Liu
1 sibling, 0 replies; 6+ messages in thread
From: Wanlong Gao @ 2013-04-16 2:03 UTC (permalink / raw)
To: Zhouping Liu, Jan Stancek; +Cc: ltp-list, gmail
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@gmail.com>
> ---
> 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
>
------------------------------------------------------------------------------
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] Fix short of nodemask array.
2013-03-04 4:49 [LTP] [PATCH] Fix short of nodemask array gmail
2013-04-16 2:03 ` Wanlong Gao
@ 2013-04-16 4:34 ` Zhouping Liu
2013-04-16 6:58 ` Jan Stancek
[not found] ` <516E15C3.5080507@gmail.com>
1 sibling, 2 replies; 6+ messages in thread
From: Zhouping Liu @ 2013-04-16 4:34 UTC (permalink / raw)
To: gmail; +Cc: ltp-list
Hi Lans,
----- Original Message -----
> From: "gmail" <lans.zhang2008@gmail.com>
> 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.
> 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@gmail.com>
> ---
> 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@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] Fix short of nodemask array.
2013-04-16 4:34 ` Zhouping Liu
@ 2013-04-16 6:58 ` Jan Stancek
2013-04-17 3:13 ` Lans Zhang
[not found] ` <516E15C3.5080507@gmail.com>
1 sibling, 1 reply; 6+ messages in thread
From: Jan Stancek @ 2013-04-16 6:58 UTC (permalink / raw)
To: Zhouping Liu; +Cc: ltp-list, gmail
----- Original Message -----
> From: "Zhouping Liu" <zliu@redhat.com>
> To: "gmail" <lans.zhang2008@gmail.com>
> Cc: ltp-list@lists.sourceforge.net, "Wanlong Gao" <gaowanlong@cn.fujitsu.com>, "Jan Stancek" <jstancek@redhat.com>
> 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@gmail.com>
> > 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 <lans.zhang2008@gmail.com>
> > ---
> > 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] Fix short of nodemask array.
2013-04-16 6:58 ` Jan Stancek
@ 2013-04-17 3:13 ` Lans Zhang
0 siblings, 0 replies; 6+ messages in thread
From: Lans Zhang @ 2013-04-17 3:13 UTC (permalink / raw)
To: Jan Stancek; +Cc: ltp-list
On 04/16/2013 02:58 PM, Jan Stancek wrote:
>
>
>
>
> ----- Original Message -----
>> From: "Zhouping Liu"<zliu@redhat.com>
>> To: "gmail"<lans.zhang2008@gmail.com>
>> Cc: ltp-list@lists.sourceforge.net, "Wanlong Gao"<gaowanlong@cn.fujitsu.com>, "Jan Stancek"<jstancek@redhat.com>
>> 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@gmail.com>
>>> 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.
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@gmail.com>
>>> ---
>>> 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@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/ltp-list
>>>
>>
>> --
>> Thanks,
>> Zhouping
>>
>
------------------------------------------------------------------------------
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH] Fix short of nodemask array.
[not found] ` <516E15C3.5080507@gmail.com>
@ 2013-04-17 3:22 ` Wanlong Gao
0 siblings, 0 replies; 6+ messages in thread
From: Wanlong Gao @ 2013-04-17 3:22 UTC (permalink / raw)
To: Lans Zhang; +Cc: LTP List
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@gmail.com>
>>> 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.
>
> 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@gmail.com>
>>> ---
>>> 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@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/ltp-list
>>>
>>
>
>
------------------------------------------------------------------------------
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-17 3:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-04 4:49 [LTP] [PATCH] Fix short of nodemask array gmail
2013-04-16 2:03 ` Wanlong Gao
2013-04-16 4:34 ` Zhouping Liu
2013-04-16 6:58 ` Jan Stancek
2013-04-17 3:13 ` Lans Zhang
[not found] ` <516E15C3.5080507@gmail.com>
2013-04-17 3:22 ` Wanlong Gao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox