From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: Re: [PATCH v4 02/21] xen: make two memory hypercalls vNUMA-aware Date: Fri, 23 Jan 2015 14:46:27 +0000 Message-ID: <20150123144627.GC1801@zion.uk.xensource.com> References: <1422011632-22018-1-git-send-email-wei.liu2@citrix.com> <1422011632-22018-3-git-send-email-wei.liu2@citrix.com> <54C257B30200007800058CF7@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <54C257B30200007800058CF7@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Wei Liu , ian.campbell@citrix.com, andrew.cooper3@citrix.com, dario.faggioli@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, ufimtseva@gmail.com List-Id: xen-devel@lists.xenproject.org On Fri, Jan 23, 2015 at 01:16:19PM +0000, Jan Beulich wrote: > >>> On 23.01.15 at 12:13, 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 > > Acked-by: Jan Beulich > > 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. > > + { > > + 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? > We can't forbid guests from setting both flags, the reason is the same as above. So don't enforce the MEMF_exact_node seems when XENMEMF_vnode seems the way to go. Wei. > Jan