Re: [PATCH v10 9/9] libxl: vnuma topology configuration parser and doc

[Wei Liu <wei.liu2@citrix.com>]

On Fri, Sep 12, 2014 at 11:02:30AM +0200, Dario Faggioli wrote:
> [Adding Wei]
> 
> On gio, 2014-09-11 at 22:04 -0400, Elena Ufimtseva wrote:
> > Hello Dario
> > 
> Hi,
> 
> > Thank you for such a thorough review.
> >
> Yeah, well, sorry it took a bit! :-/
> 

Sorry from me as well. I've been tracking this discussion for some time
but never weighted in because I needed to get my other 8 patches ready
for the freeze. No pressure. ;-)

> > Majority of comments make sense to me.
> >
> Cool. :-D
> 
> > Now I wanted to ask about the memory ranges :)
> > I am not sure at this point I should have them parsed in config file
> > (for the case where vNUMA node has multiple ranges) and as Ian has
> > mentioned I will be moving
> > these ranges away from libxl build info.
> > I guess for now It will be better to leave these ranges out of scope
> > of parsing code.
> > What do you think?
> > 
> If, as I'm quite sure, with 'memory ranges' you're now talking about the
> possibility of specifying more memory region for each node, yes, given
> that we're keeping that out of the libxl interface, I'd live them out of
> xl too.
> 

Agreed. We just need to make the syntax backward compatible.

> If, with 'memory ranges', you mean the possibility of specifying
> different memory size for each node, I think I'd keep this.
> 
> In fact, bbout memory, I'm fine with how you right now deal with
> vnuma_mem (which I suggested to rename to "vnuma_memory" or
> "vnuma_maxmem"), i.e.:
> 

I don't thin vnuma_maxmem reflects what this parameter means. The list
controls distribution of memory not the maximum amount. The sum of this
list should be equal to maxmem= in config file.

> vnuma_memory = [1024, 1024, 2048, 2048]
> 
> When, in future, we will introduce toolstack support for defining nodes
> with multiple memory regions, we can extend this syntax quite easily, to
> something like
> 
> vnuma_memory = ["128,896", 256,256,512"]
> 
> with the following meaning: we have two nodes; node 0 has two regions,
> one 128MB big, the other 896MB big. Node 1 has 3 regions, the first twos
> 256MB big, the third 512MB big.
> 
> Just to give an example.
> 
> If we want, that can be even more specific, i.e.:
> 
> vnuma_memory = ["128@0xffffaaa,896@0xccccdddd", ...]
> 
> meaning: node 0 has one region 128MB big, starting at address 0xffffaaa,
> and another 896MB big, starting at address 0xccccdddd (ISTR we agreed on
> something very similar to this for Arianna's iomem series).
> 

I think this syntax is good.

Wei.

> This is just an example, of course, the point being that the current
> interface is good for now, and can be easily extended, in a backward
> compatible way.
> 
> This is my take on this, hoping I understood your question. Ian's
> opinion is probably the one you want, though. :-)
> 
> Regards,
> Dario
> 
> -- 
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -----------------------------------------------------------------
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
>