Re: [PATCH v4 02/21] xen: make two memory hypercalls vNUMA-aware

["Jan Beulich" <JBeulich@suse.com>]

>>> On 23.01.15 at 15:46, <wei.liu2@citrix.com> wrote:
> On Fri, Jan 23, 2015 at 01:16:19PM +0000, Jan Beulich wrote:
>> >>> On 23.01.15 at 12:13, <wei.liu2@citrix.com> wrote:
>> > Make XENMEM_increase_reservation and XENMEM_populate_physmap
>> > vNUMA-aware.
>> > 
>> > That is, if guest requests Xen to allocate memory for specific vnode,
>> > Xen can translate vnode to pnode using vNUMA information of that guest.
>> > 
>> > XENMEMF_vnode is introduced for the guest to mark the node number is in
>> > fact virtual node number and should be translated by Xen.
>> > 
>> > XENFEAT_memory_op_vnode_supported is introduced to indicate that Xen is
>> > able to translate virtual node to physical node.
>> > 
>> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> > Acked-by: Jan Beulich <JBeulich@suse.com>
>> 
>> I'm afraid there's another change needed for this to hold:
>> 
>> > --- a/xen/common/memory.c
>> > +++ b/xen/common/memory.c
>> > @@ -692,6 +692,50 @@ out:
>> >      return rc;
>> >  }
>> >  
>> > +static int translate_vnode_to_pnode(struct domain *d,
>> > +                                    struct xen_memory_reservation *r,
>> > +                                    struct memop_args *a)
>> > +{
>> > +    int rc = 0;
>> > +    unsigned int vnode, pnode;
>> > +
>> > +    /*
>> > +     * Note: we don't strictly require non-supported bits set to zero,
>> > +     * so we may have exact_vnode bit set for old guests that don't
>> > +     * support vNUMA.
>> > +     *
>> > +     * To distinguish spurious vnode request v.s. real one, check if
>> > +     * d->vnuma exists.
>> > +     */
>> > +    if ( r->mem_flags & XENMEMF_vnode )
>> > +    {
>> > +        read_lock(&d->vnuma_rwlock);
>> > +        if ( d->vnuma )
>> 
>> if r->mem_flags has XENMEMF_vnode set but d->vnuma is NULL,
>> you need to clear the node from the flags.
>> 
> 
> As said in the comment, we don't seem to enforce non-supported bits set
> to zero (IIRC you told me that). So an old guest that sets XENMEMF_vnode
> by accident will get its other flags cleared if I follow your suggestion.

Which is an acceptable thing to do I think - they called for
undefined behavior, and they now get unexpected behavior.
Mistaking the virtual node specified for a physical one is certainly
less desirable.

Jan