Hi Lans,

----- Original Message -----
> From: "gmail" <[email protected]>
> To: [email protected]
> 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 <[email protected]>
> ---
>   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
> [email protected]
> 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
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Reply via email to