Re: [LTP] [PATCH v2] mem/oom: fixed a cpuset error

Mike Frysinger
2013-04-29T00:08:22Z

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