From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH 1/6] mmu: Introduce XENMEM_claim_pages (subop of memory ops). Date: Thu, 7 Mar 2013 09:59:45 -0500 Message-ID: <20130307145945.GB15130@phenom.dumpdata.com> References: <1362419234-29446-1-git-send-email-konrad.wilk@oracle.com> <1362419234-29446-2-git-send-email-konrad.wilk@oracle.com> <20130305120156.GA1139@ocelot.phlegethon.org> <20130305214320.GB8235@phenom.dumpdata.com> <20130306090746.GA15637@ocelot.phlegethon.org> <20130306153627.GA12500@phenom.dumpdata.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20130306153627.GA12500@phenom.dumpdata.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: Tim Deegan Cc: dan.magenheimer@oracle.com, xen-devel@lists.xensource.com, keir@xen.org, keir.xen@gmail.com List-Id: xen-devel@lists.xenproject.org > > > The big thing is *might*. I put this in the code path to explain better: > > > > > > /* note; The usage of tmem claim means that allocation from a guest *might* > > > + * have to come from freeable memory. Using free memory is always better, if > > > + * it is available, than using freeable memory. This flag is for the use > > > + * case where the toolstack cannot be constantly aware of the exact current > > > + * value of free and/or freeable on each machine and is multi-machine > > > + * capable. It can try/fail a "normal" claim on all machines first then, > > > + * and if the normal claim on all machines fail, then "fallback" to a > > > + * tmem-flag type claim. > > > > Oh I see. That's pretty strange semantics for a 'claim', though. > > Wouldn't it make sense for the toolstack just to query free and claimed > > memory on the first pass and fail if there's not enough space? > > So do something like this: > > if ( dom->claim_enabled ) { > unsigned long outstanding = xc_domain_get_outstanding_pages(dom->xch); > xc_physinfo_t xcphysinfo = { 0 }; > int flag = XENMEMF_claim_normal; > > rc = xc_physinfo(dom->xch, &xcphysinfo); > > if (xcphysinfo.total_pages + outstanding > dom->total_pages) > flag = XENMEMF_claim_tmem; > > rc = xc_domain_claim_pages(dom->xch, dom->guest_domid, dom->total_pages, > flag); > } > > (Ignorning the checks for 'rc' and bailing out as neccessary) > > > The race between that query and the claim call (i.e. enough > > actually-free space at query time but only freeable memory at claim > > time) isn't materially different from the claim succeeding and then tmem > > consuming the memory. And a race between multiple claims can be > > trivially avoided with a mutex in toolstack. > > The location where the claim call is in the libxc toolstack (it seemed > the most appropiate as it deals with the populate calls). There is no locking > semantics at all in that library - that is something the other libraries - > such as libxl, have. The libxl has the lock, but it would be a coarse lock > as it would be around: ..bla bla.. The other thought is just to drop the XENMEMF_claim_normal flag and just have XENMEMF_claim_tmem flag and use that by default. I think that will simplify this a lot more. Let me prep a patch and have it ready by tomorrow.