* [LTP] [PATCH v2] mem/oom: fixed a cpuset error
@ 2013-04-27 4:23 Zhouping Liu
2013-04-27 6:04 ` Mike Frysinger
0 siblings, 1 reply; 5+ messages in thread
From: Zhouping Liu @ 2013-04-27 4:23 UTC (permalink / raw)
To: LTP List
For the below special NUMA system, oom0[4|5] failed:
# numactl -H
available: 2 nodes (0-1)
node 0 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27
node 0 size: 0 MB
node 0 free: 0 MB
node 1 cpus:
node 1 size: 16384 MB
node 1 free: 14173 MB
node distances:
node 0 1
0: 10 40
1: 40 10
failed log:
oom04 1 TBROK : write /dev/cpuset/1/cpuset.mems: errno=EINVAL(22): Invalid argument
The reason is that node0 only contains all CPUs, no any memory,
and node1 contains all memory, but no CPUs, in the previous codes,
we only do cpuset testing on a independent node, which caused the
sub-cpuset cgroup only contains CPUs or memory in one node, but
that's not permitted in the special machine. The patch fixed it.
Signed-off-by: Zhouping Liu <zliu@redhat.com>
Reviewed-by: Caspar Zhang <caspar@casparzhang.com>
---
Change log:
v1 - v2:
fixed some grammar errors, and updated some comments.
testcases/kernel/mem/lib/mem.c | 14 +++++++++++++-
testcases/kernel/mem/oom/oom04.c | 19 +++++++++++++------
testcases/kernel/mem/oom/oom05.c | 25 +++++++++++++++----------
3 files changed, 41 insertions(+), 17 deletions(-)
diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
index 5397177..62bbc95 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -792,7 +792,19 @@ void write_cpusets(long nd)
write_cpuset_files(CPATH_NEW, "mems", buf);
gather_node_cpus(cpus, nd);
- write_cpuset_files(CPATH_NEW, "cpus", cpus);
+ /*
+ * If the 'nd' node doesn't contain any CPUs,
+ * the first ID of CPU '0' will be used as
+ * the value of cpuset.cpus.
+ */
+ if (strlen(cpus) != 0) {
+ write_cpuset_files(CPATH_NEW, "cpus", cpus);
+ } else {
+ tst_resm(TINFO, "None CPUs in the node%ld", nd);
+ tst_resm(TINFO, "Only use CPU0 in the cpuset cgroup "
+ "for the special scenario");
+ write_cpuset_files(CPATH_NEW, "cpus", "0");
+ }
SAFE_FILE_PRINTF(NULL, CPATH_NEW "/tasks", "%d", getpid());
}
diff --git a/testcases/kernel/mem/oom/oom04.c b/testcases/kernel/mem/oom/oom04.c
index 4d3f2f4..dd9acd1 100644
--- a/testcases/kernel/mem/oom/oom04.c
+++ b/testcases/kernel/mem/oom/oom04.c
@@ -85,6 +85,8 @@ int main(int argc, char *argv[])
void setup(void)
{
+ int memnode, ret;
+
tst_require_root(NULL);
tst_sig(FORK, DEF_HANDLER, cleanup);
TEST_PAUSE;
@@ -93,12 +95,17 @@ void setup(void)
set_sys_tune("overcommit_memory", 1, 1);
mount_mem("cpuset", "cpuset", NULL, CPATH, CPATH_NEW);
- if (is_numa(cleanup) > 0)
- /* For NUMA system, using the first node for cpuset.mems */
- write_cpusets(get_a_numa_node(cleanup));
- else
- /* For nonNUMA system, using node0 for cpuset.mems */
- write_cpusets(0);
+
+ /*
+ * Not any nodes contain memory, so using get_allowed_nodes(NH_MEMS)
+ * to get a memory node, the operation also applies to Non-NUMA
+ * system.
+ */
+ ret = get_allowed_nodes(NH_MEMS, 1, &memnode);
+ if (ret < 0)
+ tst_brkm(TBROK, NULL, "Failed to get a memory node "
+ "using get_allowed_nodes()");
+ write_cpusets(memnode);
}
void cleanup(void)
diff --git a/testcases/kernel/mem/oom/oom05.c b/testcases/kernel/mem/oom/oom05.c
index 15feba5..fd1b671 100644
--- a/testcases/kernel/mem/oom/oom05.c
+++ b/testcases/kernel/mem/oom/oom05.c
@@ -108,24 +108,29 @@ int main(int argc, char *argv[])
void setup(void)
{
+ int ret, memnode;
+
tst_require_root(NULL);
tst_sig(FORK, DEF_HANDLER, cleanup);
TEST_PAUSE;
+ overcommit = get_sys_tune("overcommit_memory");
+ set_sys_tune("overcommit_memory", 1, 1);
+
mount_mem("memcg", "cgroup", "memory", MEMCG_PATH, MEMCG_PATH_NEW);
mount_mem("cpuset", "cpuset", NULL, CPATH, CPATH_NEW);
write_memcg();
- set_sys_tune("overcommit_memory", 1, 1);
-
- if (is_numa(cleanup))
- /* For NUMA system, using the first node for cpuset.mems */
- write_cpusets(get_a_numa_node(cleanup));
- else
- /* For nonNUMA system, using node0 for cpuset.mems */
- write_cpusets(0);
-
- overcommit = get_sys_tune("overcommit_memory");
+ /*
+ * Not any nodes contain memory, so using get_allowed_nodes(NH_MEMS)
+ * to get a memory node, the operation also applies to Non-NUMA
+ * system.
+ */
+ ret = get_allowed_nodes(NH_MEMS, 1, &memnode);
+ if (ret < 0)
+ tst_brkm(TBROK, NULL, "Failed to get a memory node "
+ "using get_allowed_nodes()");
+ write_cpusets(memnode);
}
void cleanup(void)
--
1.7.11.7
------------------------------------------------------------------------------
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 related [flat|nested] 5+ messages in thread
* Re: [LTP] [PATCH v2] mem/oom: fixed a cpuset error
2013-04-27 4:23 [LTP] [PATCH v2] mem/oom: fixed a cpuset error Zhouping Liu
@ 2013-04-27 6:04 ` Mike Frysinger
2013-04-27 6:34 ` Zhouping Liu
0 siblings, 1 reply; 5+ messages in thread
From: Mike Frysinger @ 2013-04-27 6:04 UTC (permalink / raw)
To: ltp-list
[-- Attachment #1.1: Type: Text/Plain, Size: 1791 bytes --]
On Saturday 27 April 2013 00:23:14 Zhouping Liu wrote:
> The reason is that node0 only contains all CPUs, no any memory,
no any memory -> no memory
> and node1 contains all memory, but no CPUs, in the previous codes,
change "CPUs, in" to "CPUs. In"
> we only do cpuset testing on a independent node, which caused the
a -> an
> sub-cpuset cgroup only contains CPUs or memory in one node, but
only contains -> to only contain
> that's not permitted in the special machine. The patch fixed it.
what is a "special machine" ?
also, generally there's no need to say "The patch fixed it". the fact that
you've described a bug and have a patch implies that this fixes things.
> + tst_resm(TINFO, "None CPUs in the node%ld", nd);
i think you mean "no" instead of "None"
> + tst_resm(TINFO, "Only use CPU0 in the cpuset cgroup "
> + "for the special scenario");
what is "the special scenario" ?
> mount_mem("cpuset", "cpuset", NULL, CPATH, CPATH_NEW);
> - if (is_numa(cleanup) > 0)
> - /* For NUMA system, using the first node for cpuset.mems */
> - write_cpusets(get_a_numa_node(cleanup));
> - else
> - /* For nonNUMA system, using node0 for cpuset.mems */
> - write_cpusets(0);
> +
> + /*
> + * Not any nodes contain memory, so using get_allowed_nodes(NH_MEMS)
"Not any" -> "No"
"using" -> "use"
> + * to get a memory node, the operation also applies to Non-NUMA
change the ", the" to ". This".
> + * system.
systems.
> + /*
looks like you mixed spaces & tabs. only use tabs.
> + * Not any nodes contain memory, so using get_allowed_nodes(NH_MEMS)
> + * to get a memory node, the operation also applies to Non-NUMA
> + * system.
same comments about the grammar here as above
-mike
[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 421 bytes --]
------------------------------------------------------------------------------
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
[-- Attachment #3: Type: text/plain, Size: 155 bytes --]
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [LTP] [PATCH v2] mem/oom: fixed a cpuset error
2013-04-27 6:04 ` Mike Frysinger
@ 2013-04-27 6:34 ` Zhouping Liu
2013-04-29 0:08 ` Mike Frysinger
0 siblings, 1 reply; 5+ messages in thread
From: Zhouping Liu @ 2013-04-27 6:34 UTC (permalink / raw)
To: Mike Frysinger; +Cc: ltp-list
----- Original Message -----
> From: "Mike Frysinger" <vapier@gentoo.org>
> To: ltp-list@lists.sourceforge.net
> Cc: "Zhouping Liu" <zliu@redhat.com>
> Sent: Saturday, April 27, 2013 2:04:32 PM
> Subject: Re: [LTP] [PATCH v2] mem/oom: fixed a cpuset error
>
> On Saturday 27 April 2013 00:23:14 Zhouping Liu wrote:
> > The reason is that node0 only contains all CPUs, no any memory,
>
> no any memory -> no memory
OK.
>
> > and node1 contains all memory, but no CPUs, in the previous codes,
>
> change "CPUs, in" to "CPUs. In"
OK.
>
> > we only do cpuset testing on a independent node, which caused the
>
> a -> an
OK.
>
> > sub-cpuset cgroup only contains CPUs or memory in one node, but
>
> only contains -> to only contain
why 'contain', I think it should be 'contains', isn't it?
>
> > that's not permitted in the special machine. The patch fixed it.
>
> what is a "special machine" ?
the special machine is that it has such nodes(describe above),
in which there's only CPUs or memory.
how about this:
"that's not permitted in the such above special machine."
>
> also, generally there's no need to say "The patch fixed it". the fact that
> you've described a bug and have a patch implies that this fixes things.
>
> > + tst_resm(TINFO, "None CPUs in the node%ld", nd);
>
> i think you mean "no" instead of "None"
>
> > + tst_resm(TINFO, "Only use CPU0 in the cpuset cgroup "
> > + "for the special scenario");
>
> what is "the special scenario" ?
the special scenario is "no CPUs in the node%ld", I think it's clear in log message.
>
> > mount_mem("cpuset", "cpuset", NULL, CPATH, CPATH_NEW);
> > - if (is_numa(cleanup) > 0)
> > - /* For NUMA system, using the first node for cpuset.mems */
> > - write_cpusets(get_a_numa_node(cleanup));
> > - else
> > - /* For nonNUMA system, using node0 for cpuset.mems */
> > - write_cpusets(0);
> > +
> > + /*
> > + * Not any nodes contain memory, so using get_allowed_nodes(NH_MEMS)
>
> "Not any" -> "No"
it's not that meaning, what I meant here is that there's not any nodes contain
memory in a NUMA system, Some nodes contain memory, but some nodes don't.
>
> "using" -> "use"
OK.
>
> > + * to get a memory node, the operation also applies to Non-NUMA
>
> change the ", the" to ". This".
OK
>
> > + * system.
>
> systems.
>
> > + /*
>
> looks like you mixed spaces & tabs. only use tabs.
OK.
>
> > + * Not any nodes contain memory, so using get_allowed_nodes(NH_MEMS)
> > + * to get a memory node, the operation also applies to Non-NUMA
> > + * system.
>
> same comments about the grammar here as above
> -mike
>
--
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] 5+ messages in thread
* Re: [LTP] [PATCH v2] mem/oom: fixed a cpuset error
2013-04-27 6:34 ` Zhouping Liu
@ 2013-04-29 0:08 ` Mike Frysinger
2013-04-29 6:12 ` Zhouping Liu
0 siblings, 1 reply; 5+ messages in thread
From: Mike Frysinger @ 2013-04-29 0:08 UTC (permalink / raw)
To: Zhouping Liu; +Cc: ltp-list
[-- Attachment #1.1: Type: Text/Plain, Size: 2081 bytes --]
On Saturday 27 April 2013 02:34:49 Zhouping Liu wrote:
> From: "Mike Frysinger" <vapier@gentoo.org>
> > On Saturday 27 April 2013 00:23:14 Zhouping Liu wrote:
> > > sub-cpuset cgroup only contains CPUs or memory in one node, but
> >
> > only contains -> to only contain
>
> why 'contain', I think it should be 'contains', isn't it?
when you add the "to", it changes to "contain"
> > > that's not permitted in the special machine. The patch fixed it.
> >
> > what is a "special machine" ?
>
> the special machine is that it has such nodes(describe above),
> in which there's only CPUs or memory.
>
> how about this:
>
> "that's not permitted in the such above special machine."
i would use:
that's not permitted in the scenario described above.
> > > + tst_resm(TINFO, "None CPUs in the node%ld", nd);
> >
> > i think you mean "no" instead of "None"
> >
> > > + tst_resm(TINFO, "Only use CPU0 in the cpuset cgroup "
> > > + "for the special scenario");
> >
> > what is "the special scenario" ?
>
> the special scenario is "no CPUs in the node%ld", I think it's clear in log
> message.
when you say "Only use", that's a command to the user. i think you meant to
say "Only using".
i think the two messages can be combined into one then:
tst_resm(TINFO, "No CPUs in node%ld; using only CPU0", nd);
> > > mount_mem("cpuset", "cpuset", NULL, CPATH, CPATH_NEW);
> > >
> > > - if (is_numa(cleanup) > 0)
> > > - /* For NUMA system, using the first node for cpuset.mems */
> > > - write_cpusets(get_a_numa_node(cleanup));
> > > - else
> > > - /* For nonNUMA system, using node0 for cpuset.mems */
> > > - write_cpusets(0);
> > > +
> > > + /*
> > > + * Not any nodes contain memory, so using get_allowed_nodes(NH_MEMS)
> >
> > "Not any" -> "No"
>
> it's not that meaning, what I meant here is that there's not any nodes
> contain memory in a NUMA system, Some nodes contain memory, but some nodes
> don't.
ok, so i think you want to say instead:
Some nodes do not contain memory, ....
-mike
[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 421 bytes --]
------------------------------------------------------------------------------
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
[-- Attachment #3: Type: text/plain, Size: 155 bytes --]
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [LTP] [PATCH v2] mem/oom: fixed a cpuset error
2013-04-29 0:08 ` Mike Frysinger
@ 2013-04-29 6:12 ` Zhouping Liu
0 siblings, 0 replies; 5+ messages in thread
From: Zhouping Liu @ 2013-04-29 6:12 UTC (permalink / raw)
To: Mike Frysinger; +Cc: ltp-list
----- Original Message -----
> From: "Mike Frysinger" <vapier@gentoo.org>
> To: "Zhouping Liu" <zliu@redhat.com>
> Cc: ltp-list@lists.sourceforge.net
> Sent: Monday, April 29, 2013 8:08:03 AM
> Subject: Re: [LTP] [PATCH v2] mem/oom: fixed a cpuset error
>
> On Saturday 27 April 2013 02:34:49 Zhouping Liu wrote:
> > From: "Mike Frysinger" <vapier@gentoo.org>
> > > On Saturday 27 April 2013 00:23:14 Zhouping Liu wrote:
> > > > sub-cpuset cgroup only contains CPUs or memory in one node, but
> > >
> > > only contains -> to only contain
> >
> > why 'contain', I think it should be 'contains', isn't it?
>
> when you add the "to", it changes to "contain"
yes, I didn't catch the "to" :(
>
> > > > that's not permitted in the special machine. The patch fixed it.
> > >
> > > what is a "special machine" ?
> >
> > the special machine is that it has such nodes(describe above),
> > in which there's only CPUs or memory.
> >
> > how about this:
> >
> > "that's not permitted in the such above special machine."
>
> i would use:
> that's not permitted in the scenario described above.
Agreed.
>
> > > > + tst_resm(TINFO, "None CPUs in the node%ld", nd);
> > >
> > > i think you mean "no" instead of "None"
> > >
> > > > + tst_resm(TINFO, "Only use CPU0 in the cpuset cgroup "
> > > > + "for the special scenario");
> > >
> > > what is "the special scenario" ?
> >
> > the special scenario is "no CPUs in the node%ld", I think it's clear in log
> > message.
>
> when you say "Only use", that's a command to the user. i think you meant to
> say "Only using".
>
> i think the two messages can be combined into one then:
> tst_resm(TINFO, "No CPUs in node%ld; using only CPU0", nd);
It sounds better, agreed.
>
> > > > mount_mem("cpuset", "cpuset", NULL, CPATH, CPATH_NEW);
> > > >
> > > > - if (is_numa(cleanup) > 0)
> > > > - /* For NUMA system, using the first node for cpuset.mems */
> > > > - write_cpusets(get_a_numa_node(cleanup));
> > > > - else
> > > > - /* For nonNUMA system, using node0 for cpuset.mems */
> > > > - write_cpusets(0);
> > > > +
> > > > + /*
> > > > + * Not any nodes contain memory, so using get_allowed_nodes(NH_MEMS)
> > >
> > > "Not any" -> "No"
> >
> > it's not that meaning, what I meant here is that there's not any nodes
> > contain memory in a NUMA system, Some nodes contain memory, but some nodes
> > don't.
>
> ok, so i think you want to say instead:
> Some nodes do not contain memory, ....
Yes, I have updated it, please review v3.
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] 5+ messages in thread
end of thread, other threads:[~2013-04-29 6:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-27 4:23 [LTP] [PATCH v2] mem/oom: fixed a cpuset error Zhouping Liu
2013-04-27 6:04 ` Mike Frysinger
2013-04-27 6:34 ` Zhouping Liu
2013-04-29 0:08 ` Mike Frysinger
2013-04-29 6:12 ` Zhouping Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox