Re: [Xen-devel] [PATCH v6 02/23] xen: move NUMA_NO_NODE to public memory.h as XEN_NUMA_NO_NODE

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Mar 02, 2015 at 04:50:48PM +0000, Jan Beulich wrote:
> >>> On 02.03.15 at 17:39, <wei.liu2@xxxxxxxxxx> wrote:
> > On Mon, Mar 02, 2015 at 04:27:25PM +0000, Jan Beulich wrote:
> >> >>> On 02.03.15 at 17:08, <wei.liu2@xxxxxxxxxx> wrote:
> >> > On Mon, Mar 02, 2015 at 03:51:37PM +0000, Jan Beulich wrote:
> >> >> >>> On 02.03.15 at 16:38, <wei.liu2@xxxxxxxxxx> wrote:
> >> >> > On Mon, Mar 02, 2015 at 03:30:21PM +0000, Ian Campbell wrote:
> >> >> >> On Mon, 2015-03-02 at 07:04 +0000, Jan Beulich wrote:
> >> >> >> > >>> Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 02/27/15 5:58 PM >>>
> >> >> >> > >On 27/02/15 16:51, Wei Liu wrote:
> >> >> >> > >> During last round review, Andrew wanted me to move this to Xen 
> >> >> >> > >> public
> >> >> >> > >> header to avoid reinventing it in libxc. Now this value is used 
> >> >> >> > >> in libxc
> >> >> >> > >> patch.
> >> >> >> > >>
> >> >> >> > >> But I don't particularly mind whether we move it or not, it's 
> >> >> >> > >> up to you
> >> >> >> > >> maintainers to decide.
> >> >> >> > >
> >> >> >> > >It is a sentinel value used in the public ABI.  It should 
> >> >> >> > >therefore
> >> >> >> > >appear in the public API.
> >> >> >> > 
> >> >> >> > Which it already does, as XENMEMF_get_node(0). I don't think it 
> >> >> >> > needs
> >> >> >> > particular naming as a new constant, even more that it isn't 
> >> >> >> > intended to
> >> >> >> > be used explicitly in any of the memops.
> >> >> >> 
> >> >> >> IMHO the named constant does seem to make the tools code at least 
> >> >> >> more
> >> >> >> readable, but without Wei having said where this is to be used I'm 
> >> >> >> not
> >> >> >> sure where it should live. In particular I'm unsure if/how/where this
> >> >> >> value gets passed to a hypercall, as opposed to perhaps being used 
> >> >> >> as a
> >> >> > 
> >> >> > This is used to fill in vnode_to_pnode array. That array get
> >> >> > subsequently passed down to hypervisor.
> >> >> 
> >> >> Do we really accept NUMA_NO_NODE to be passed that way?
> >> >> 
> >> > 
> >> > public/domctl.h:struct xen_domctl_vnuma has vnode_to_pnode array.
> >> 
> >> That wasn't my concern - I was rather wondering why we would
> >> accept any of this array's fields to be set to "no node".
> >> 
> > 
> > If you want to have numa topology exposed to guest but doesn't care
> > about underly memory affinity?
> 
> Is this useful for anything in reality? I.e. why would you want to
> tell the guest it's NUMA when it really isn't? The only case I could
> see is testing vNUMA code changes without having a NUMA box
> around, but that hardly counts as a real use.
> 

Dario has the idea that vNUMA is not only about performance. AIUI he
thinks it's perfectly OK to have that kind of use case.

> Furthermore iirc that array is an array of uint32_t, and the
> sentinel (if any) there ought to be 0xffffffff irrespective of what
> we use internally in the hypervisor.
> 

There are two issues here irrespective of this series:

1. should we expose that sentinel in ABI?
2. if so, what should the sentinel be?

I think Andrew and you disagree on the first one. We can work out the
answer to the second question later.

Currently my code doesn't really pass that down to hypervisor (that is,
requires a valid pnode to map a vnode to), so I'm fine with either
exposing or not exposing.

Wei.

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

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