Re: [Qemu-devel] [PATCH v4 1/1] numa: Print warning if no node is assigned to a CPU

[Eduardo Habkost]

On Wed, Mar 04, 2015 at 02:50:59PM +0100, Michael S. Tsirkin wrote:
> On Wed, Mar 04, 2015 at 10:35:51AM -0300, Eduardo Habkost wrote:
> > On Wed, Mar 04, 2015 at 01:30:35PM +0100, Michael S. Tsirkin wrote:
> > > On Tue, Feb 24, 2015 at 03:57:23PM -0300, Eduardo Habkost wrote:
> > > > Instead of silently assigning CPU to node 0 when it is omitted from the
> > > > command-line, check if all CPUs up to max_cpus are present in the NUMA
> > > > configuration.
> > > 
> > > Could you please describe the problematic configuration
> > > in more detail in commit log?
> > > Also, how does this interact with cpu hotplug?
> > 
> > I will try to reword the message to make it clearer. Suggestions are
> > welcome.
> 
> 
> Just include the command line that reproduces the problem,
> and describe the guest behaviour that results.

There's no guest behavior to describe, except that it will see the CPU assigned
to node 0 (as already described above).

I submitted a new version without a command-line example, already. I assumed it
would be obvious to anybody who used the -numa command-line argument before,
but here it is:

Correct command-line, no warning:

  $ qemu-system-x86_64 -smp 2,maxcpus=4
  $ qemu-system-x86_64 -smp 2,maxcpus=4 -numa node,cpus=0-3

Incomplete command-line, with warnings:

  $ qemu-system-x86_64 -smp 2,maxcpus=4 -numa node,cpus=0
  qemu-system-x86_64: warning: CPU(s) not present in any NUMA nodes: 1 2 3
  qemu-system-x86_64: warning: All CPU(s) up to maxcpus should be described in 
NUMA config

  $ qemu-system-x86_64 -smp 2,maxcpus=4 -numa node,cpus=0-2
  qemu-system-x86_64: warning: CPU(s) not present in any NUMA nodes: 3
  qemu-system-x86_64: warning: All CPU(s) up to maxcpus should be described in 
NUMA config


> 
> > Basically, the problem is:
> > 
> > * Currently all possible CPUs (including hotplug ones) need to be
> >   present in the SRAT when QEMU is started[1]. QEMU already does that.
> > * When a CPU is ommitted from the command-line, QEMU is silently
> >   assigning it to node 0. This is not a very useful default and
> >   just confuse users.
> 
> It's reasonable if user doesn't want to play with numa, isn't it?

This is validation of the configuration when NUMA nodes are configured.
If the user doesn't want to play with numa, there won't be any -numa
arguments in the command-line.

> 
> > 
> > > > 
> > > > I am making this a warning and not a fatal error, to allow management
> > > > software to be updated if necessary.
> > > >
> > > > Signed-off-by: Eduardo Habkost <address@hidden>
> > > > ---
> > > > v1 -> v2: (no changes)
> > > > 
> > > > v2 -> v3:
> > > >  * Use enumerate_cpus() and error_report() for error message
> > > >  * Simplify logic using bitmap_full()
> > > > 
> > > > v3 -> v4:
> > > >  * Clarify error message, mention that all CPUs up to
> > > >    maxcpus need to be described in NUMA config
> > > > ---
> > > >  numa.c | 10 ++++++++++
> > > >  1 file changed, 10 insertions(+)
> > > > 
> > > > diff --git a/numa.c b/numa.c
> > > > index 9a3fc15..d8021b9 100644
> > > > --- a/numa.c
> > > > +++ b/numa.c
> > > > @@ -201,6 +201,16 @@ static void validate_numa_cpus(void)
> > > >          bitmap_or(seen_cpus, seen_cpus,
> > > >                    numa_info[i].node_cpu, MAX_CPUMASK_BITS);
> > > >      }
> > > > +
> > > > +    if (!bitmap_full(seen_cpus, max_cpus)) {
> > > > +        char *msg;
> > > > +        bitmap_complement(seen_cpus, seen_cpus, max_cpus);
> > > > +        msg = enumerate_cpus(seen_cpus, max_cpus);
> > > > +        error_report("warning: CPU(s) not present in any NUMA nodes: 
> > > > %s", msg);
> > > > +        error_report("warning: All CPU(s) up to maxcpus should be 
> > > > described "
> > > > +                     "in NUMA config");
> > > 
> > > What happens with e.g. windows guests when this warning is emitted?
> > > do they blue-screen?
> > 
> > As described on the commit log above, the CPU is silently assigned to
> > node 0 when it is omitted from the command-line. In other words, this
> > won't make guests confused, but can make users confused.
> > 
> > All possible VCPUs (including the hotplug ones, i.e. up to max_cpus) are
> > expected to be present in the SRAT, and they are already present. The
> > only problem is that instead of complaining about missing CPUs, QEMU is
> > assigning node 0 to them.
> > 
> > -- 
> > Eduardo

-- 
Eduardo