----- Original Message -----
> From: "Zhouping Liu" <[email protected]>
> To: "gmail" <[email protected]>
> Cc: [email protected], "Wanlong Gao"
> <[email protected]>, "Jan Stancek" <[email protected]>
> Sent: Tuesday, 16 April, 2013 6:34:53 AM
> Subject: Re: [LTP] [PATCH] Fix short of nodemask array.
>
> 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.
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 <[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)))
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
> > [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
