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 Tue, Mar 03, 2015 at 07:44:25AM +0000, Jan Beulich wrote:
> >>> On 02.03.15 at 18:00, <wei.liu2@xxxxxxxxxx> wrote:
> > 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.
> 
> I very much think that if we want to allow a "no node" specification
> via the domctl, then this should be part of the ABI. But that value
> and its (implicit) equivalent used for memops don't need to be the
> same, and it looks like they can't. And looking at this I think the
> code we have right now needs fixing: The internal vnode_to_pnode
> array should become nodeid_t[], and input from the domctl should
> be validated to either be a valid pnode or the to be defined sentinel
> (which then, due to being stored as a more narrow type, needs
> translation to NUMA_NO_NODE).
> 
> If we don't want to allow "no node", then input should be validated
> to present valid pnodes.
> 

This is currently the case, I don't allow "no node". Not because the
toolstack is designed / expected to work, but because there are many
implications that I am not quite sure of (and this thread sorta confirms
that everybody has his / her idea of how this should work). Libxl always
requires the pnode to be a valid one. A sentinel is used between libxc
and libxl to mark if the node specified is a valid pnode.  Other than
that, that sentinel is not used, not passed down to hypervisor.

I think Andrew and Dario came to the idea of needing to have a unified
sentinel from different angels. Andrew happened to see me using (~0UL)
as a sentinel then wanted me to propagate the hypervisor sentinel. Dario
thought more about other use cases that might need provisioning.

In any case, whether exposing this sentinel or not has no immediate user
visible effect (it's only needed between libxc and libxl at the moment),
and we don't have any burden maintaining a different sentinel in libxc
because libxc is not defined as stable interface.

Due to the controversy of this patch, I'm fine with dropping this patch
and move forward (now 20+ patches are blocked by this one). If we
can't come to a resolution within this week, I will just drop this patch
and resend the others next week.

Wei.

> Jan

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