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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Jan 23, 2015 at 03:37:51PM +0000, Jan Beulich wrote:
> >>> On 23.01.15 at 15:46, <wei.liu2@xxxxxxxxxx> wrote:
> > On Fri, Jan 23, 2015 at 01:16:19PM +0000, Jan Beulich wrote:
> >> >>> On 23.01.15 at 12:13, <wei.liu2@xxxxxxxxxx> 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@xxxxxxxxxx>
> >> > Acked-by: Jan Beulich <JBeulich@xxxxxxxx>
> >> 
> >> 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.
> 

OK, thanks for clarification.

Wei.

> Jan

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