From mboxrd@z Thu Jan 1 00:00:00 1970 From: Elena Ufimtseva Subject: Re: [PATCH v10 5/9] libxl: vnuma types declararion Date: Mon, 8 Sep 2014 15:47:31 -0400 Message-ID: References: <1409718258-3276-1-git-send-email-ufimtseva@gmail.com> <1409718258-3276-3-git-send-email-ufimtseva@gmail.com> <1409760274.15505.9.camel@kazak.uk.xensource.com> <1409826927.15057.22.camel@kazak.uk.xensource.com> <1410192781.4005.112.camel@Solace.lan> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1410192781.4005.112.camel@Solace.lan> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Dario Faggioli Cc: Keir Fraser , Ian Campbell , Stefano Stabellini , George Dunlap , Matt Wilson , Li Yechen , Ian Jackson , "xen-devel@lists.xen.org" , Jan Beulich List-Id: xen-devel@lists.xenproject.org On Mon, Sep 8, 2014 at 12:13 PM, Dario Faggioli wrote: > On gio, 2014-09-04 at 11:35 +0100, Ian Campbell wrote: >> On Wed, 2014-09-03 at 23:48 -0400, Elena Ufimtseva wrote: >> > >> > Ok, now going back to these types. >> > >> > ("vnodes", uint32), >> > ("vmemranges", uint32), >> > ("vnuma_mem", Array(uint64, "num_vnuma_mem")), >> > ("vnuma_vcpumap", Array(uint32, "num_vnuma_vcpumap")), >> > ("vdistance", Array(uint32, "num_vdistance")), >> > ("vnuma_vnodemap", Array(uint32, "num_vnuma_vnondemap")), >> > >> > >> > vnodes - number of vnuma nodes; >> > vmemranges - number of vmemranges (not used, but added support in >> > Xen); Not sure how to deal with this. Should I just remove it as its >> > not used by tool stack yet? >> >> I think that would be best, otherwise we risk tying ourselves to an API >> which might not actually work when we come to try and use it. >> > I agree. FTR, this is mostly mine and other h/v folks noticing that it > would have been nice to have multirange support very late. Elena adapted > the hypervisor patches quite quickly, so, thanks Elena! :-) Thanks Dario! > > For the tools part, I also think it'd be best to keep this out for now. > >> > vnuma_mem - vnode memory sizes (conflicts now with vmemranges); >> > vnuma_vcpumap - map of vnuma nodes and vcpus (length is number of >> > vcpus, each element is vnuma node number); >> > vdistance - distances, num_vdistance = vnodes * vnodes; >> > vnuma_vnodemap - mapping of vnuma nodes to physical NUMA nodes; >> > >> > And vnuma_autoplacement probably not needed here at all. >> > > I *REALLY* don't think this is necessary. Yes, I already removed that. > >> Then I would take the rest and wrap them in a libxl_vnode_info struct: >> >> libxl_vnode_info = Struct("libxl_vnode_info", [ >> ("mem", MemKB), # Size of this node's memory >> ("distances", Array(...)), # Distances from this node to the others (single dimensional array) >> ("pnode", TYPE), # which pnode this vnode is associated with >> ]) >> >> Then in the (b|c)_config >> ("vnuma_nodes", Array(libxl_vnode_info...)) >> >> This avoids the issue of having to have various num_* which are expected >> to always be identical. Except for vnuma_nodes[N].num_distances, but I >> think that is unavoidable. >> > My +1 to this way of arranging the interface. > >> The only thing I wasn't sure of was how to fit the vcpumap into this. >> AIUI this is a map from vcpu number to vnode number (i.e. for each vcpu >> it tells you which vnuma node it is part of). >> > Correct. > >> I presume it isn't >> possible/desirable to have a vcpu be part of multiple vnuma nodes. >> >> One possibility would be to have a libxl_bitmap/cpumap as part of the >> libxl_vnode_info (containing the set of vcpus which are part of this >> node) but that would be prone to allowing a vcpu to be in multiple >> nodes. >> > Not ideal, but not too bad from my point of view. > >> Another possibility would be an array in (b|c)_config as you have now, >> which would be annoying because it's num_foo really ought to be the >> ->vcpus count and the IDL doesn't really allow that. >> >> I'm not sure what is the lesser of the two evils here. >> > Sure, both are not ideal. I think I like the former, the one using > libxl_cpumap-s, better. It looks more consistent to me, and it fit > nicely with the array of struct way of restructuring the API Ian > described above. > > It allows the user to ask for a vcpu to be part of more than one node, > yes, but identifying and at least warning about this, or even sanity > checking things, should not be impossible (a bit expensive, perhaps, but > probably not to much for a toolstack level check). Thank you Dario. I will work on this and on re-arranging libxl vnuma structure. > > Regards, > Dario > > -- > <> (Raistlin Majere) > ----------------------------------------------------------------- > Dario Faggioli, Ph.D, http://about.me/dario.faggioli > Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) > -- Elena