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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

> +        {
> +            vnode = XENMEMF_get_node(r->mem_flags);
> +
> +            if ( vnode < d->vnuma->nr_vnodes )
> +            {
> +                pnode = d->vnuma->vnode_to_pnode[vnode];
> +
> +                a->memflags &=
> +                    ~MEMF_node(XENMEMF_get_node(r->mem_flags));

I.e. this one needs to be pulled up, and probably include
MEMF_exact_node.

> @@ -747,6 +787,16 @@ long do_memory_op(unsigned long cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>              return start_extent;
>          args.domain = d;
>  
> +        args.memflags |= MEMF_node(XENMEMF_get_node(reservation.mem_flags));
> +        if ( reservation.mem_flags & XENMEMF_exact_node_request )
> +            args.memflags |= MEMF_exact_node;

Since you force MEMF_exact_node when XENMEMF_vnode (and the
necessary data is available), I also wonder whether the combination
of both flags in what the caller requests should be forbidden (to
potentially obtain some specific meaning in the future). Or
alternatively don't enforce the former?

Jan


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