Re: [Qemu-devel] [PATCH v3.1 00/31] NUMA series, and hostmem improvements

[Paolo Bonzini]

Il 09/05/2014 19:54, Eduardo Habkost ha scritto:
> On Fri, May 09, 2014 at 04:29:49PM +0800, Hu Tao wrote:
> > On Thu, May 08, 2014 at 04:51:56PM +0200, Paolo Bonzini wrote:
> > > Il 06/05/2014 11:27, Hu Tao ha scritto:
> > > > This series includes work on QOMifying the memory backends.
> > > > the idea is to delegate all properties of the memory backend to
> > > > a new QOM class hierarchy, in which the concrete classes
> > > > are hostmem-ram and hostmem-file.  The backend is passed to the
> > > > machine via "-numa node,memdev=foo" where "foo" is the id of the
> > > > backend object.
> > >
> > > Hello,
> > >
> > > I noticed now that if you have the host-nodes property set Linux
> > > requires you to set a policy other than "default" too.  If you don't,
> > > the mbind system call fails.
> > >
> > > What about squashing something like this?
> > >
> > > Paolo
> > >
> > > diff --git a/backends/hostmem.c b/backends/hostmem.c
> > > index d3f8476..a0a3111 100644
> > > --- a/backends/hostmem.c
> > > +++ b/backends/hostmem.c
> > > @@ -299,12 +299,23 @@ host_memory_backend_memory_init(UserCreatable *uc, Error 
> > > **errp)
> > >
> > >  #ifdef CONFIG_NUMA
> > >      unsigned long maxnode = find_last_bit(backend->host_nodes, MAX_NODES);
> > > +    unsigned policy = backend->policy;
> > > +
> > > +    /* Linux does not accept MPOL_DEFAULT with nonzero bitmap, but
> > > +     * "-object memory-ram,size=128M,hostnodes=0,policy=bind" is a
> > > +     * bit of a mouthful.  So if the host_nodes bitmap is nonzero,
> > > +     * pick the BIND policy.
>
> Are we sure MPOL_BIND is a better default than MPOL_INTERLEAVE or
> MPOL_PREFERRED?

Better than MPOL_PREFERRED, yes.  Better than MPOL_INTERLEAVE, I am not 
sure.

>  * If policy=default is set, it is always going to be MPOL_DEFAULT.
>  * If policy=bind is set, it is always going to be MPOL_BIND.
>  * if policy=preferred is set, it is always going to be MPOL_PREFERRED.
>  * If policy is omitted, it will be be MPOL_DEFAULT is host-nodes is
>    unset, and MPOL_BIND if host_nodes is set.

That's possible.  Or we can just detect the case in 
host_memory_backend_memory_init and provide a better error message than 
just "Invalid argument" (aka EINVAL).  We're always free to take an 
error situation and make it non-error in the future, but the reverse 
would be hard to do.

Paolo