Re: [Xen-devel] [PATCH v6 23/23] xl: vNUMA support

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Mar 02, 2015 at 03:59:19PM +0000, Ian Campbell wrote:
> On Thu, 2015-02-26 at 15:56 +0000, Wei Liu wrote:
> > This patch includes configuration options parser and documentation.
> > 
> > Please find the hunk to xl.cfg.pod.5 for more information.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > ---
> > Changes in v6:
> > 1. Disable NUMA auto-placement.
> > ---
> >  docs/man/xl.cfg.pod.5    |  54 ++++++++++++++++++
> >  tools/libxl/xl_cmdimpl.c | 140 
> > ++++++++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 193 insertions(+), 1 deletion(-)
> > 
> > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> > index 408653f..2a27b1c 100644
> > --- a/docs/man/xl.cfg.pod.5
> > +++ b/docs/man/xl.cfg.pod.5
> > @@ -266,6 +266,60 @@ it will crash.
> >  
> >  =back
> >  
> > +=head3 Guest Virtual NUMA Configuration
> > +
> > +=over 4
> > +
> > +=item B<vnuma=[ VNODE_SPEC, VNODE_SPEC, ... ]
> > +
> > +Specify virtual NUMA configuration with positional arguments. The
> > +nth B<VNODE_SPECE> in the list specifies the configuration of nth
> 
>                     ^stray E?
> 

Fixed.

> > +virtual node.
> > +
> > +Each B<VNODE_SPEC> is a list, which has a form of
> > +"[VNODE_CONFIG_OPTION,VNODE_CONFIG_OPTION, ... ]"  (without quotes).
> 
> BTW, near the top of this file is:
>         A list of C<VALUES> of the above types. Lists are homogeneous and are
>         not nested.
> 
> "are not nested" is no longer true.
> 

I will add a patch after my changes to libxlu, so that we don't mix that
up with vNUMA configuration options.

> > +
> > +For example vnuma = [ 
> > ["pnode=0","size=512","vcpus=0-4","vdistances=10,20"] ]
> > +means vnode 0 is mapped to pnode 0, has 512MB ram, has vcpus 0 to 4, the
> > +distance to itself is 10 and the distance to vnode 1 is 20.
> > +
> > +Each B<VNODE_CONFIG_OPTION> is a quoted string.
> 
> s/string/key=value pair/?

Fixed.

> 
> >  Supported
> > +B<VNODE_CONFIG_OPTION>s are:
> > +
> > +=over 4
> > +
> > +=item B<pnode=NUMBER>
> > +
> > +Specify which physical node this virtual node maps to.
> > +
> > +=item B<size=MBYTES>
> > +
> > +Specify the size of this virtual node. The sum of memory size of all
> > +vnodes must match B<maxmem=> (or B<memory=> if B<maxmem=> is not
> > +specified).
> 
> Didn't you suggest in a reply v4 that this need for the user to do
> tedious book keeping could go away after some other change: "This point
> becomes moot if we use other syntax to specify vcpu list."?
> 

That was for specifying vcpus I think.

> Should it be allowable to no specify maxmem and have xl do the sums over
> the vnodes  to come up with the correct value? (memory= could still be
> given in that case).
> 

I missed this point.

I think your suggestion is good. We can derive maxmem= from vnuma to
relieve user from tedious bookkeeping. If user has specified maxmem= and
vnuma we check in xl that if maxmem= equals to the sum of vnuma memory.

Wei.

> The code looks fine.
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel