From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH v4 02/21] xen: make two memory hypercalls vNUMA-aware Date: Fri, 23 Jan 2015 15:37:51 +0000 Message-ID: <54C278DF0200007800058F0F@mail.emea.novell.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> <20150123144627.GC1801@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20150123144627.GC1801@zion.uk.xensource.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Liu Cc: 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 23.01.15 at 15:46, wrote: > 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. 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