Re: [Xen-devel] [PATCH 1/6] mmu: Introduce XENMEM_claim_pages (subop of memory ops).

[Tim Deegan <tim@xxxxxxx>]

At 10:36 -0500 on 06 Mar (1362566187), Konrad Rzeszutek Wilk wrote:
> On Wed, Mar 06, 2013 at 09:07:46AM +0000, Tim Deegan wrote:
> > 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)

No, I meant to get rid of the XENMEMF_claim_* flags altogether (so all
claims are 'tmem' claims) and, at a suitable level in the toolstack, do
something like this:

  LOCK
  free := [XEN_SYSCTL_physinfo]->free
  claimed := [XENMEM_get_outstanding_claims]
  IF need_free_memory AND free - claimed < needed THEN
    DESPAIR (claim would need tmem freeable pages)
  ELSE
    IF [XENMEM_claim](needed) THEN
      REJOICE
    ELSE
      DESPAIR (could not claim enough memory)
    ENDIF
  ENDIF
  UNLOCK

(using whatever wrappers around the hypercalls are appropriate)

If tmem consumes free memory after the test but before the claim, that's
OK: it's equivalent to using a XENMEMF_claim_normal claim and
tmem allocating the same memory immediately _after_ the claim.

The lock is only there to stop two parallel instances claiming the same
free memory.  Or you could invert it, for the optimistic approach:

  IF NOT [XENMEM_claim](needed) THEN
    DESPAIR (could not claim enough memory)
  ENDIF
  free := [XEN_SYSCTL_physinfo]->free
  claimed := [XENMEM_get_outstanding_claims]
  IF need_free_memory AND free < claimed THEN
    [XENMEM_claim](0)
    DESPAIR (claim would need tmem freeable pages)
  ELSE

Not _quite_ the same semantics you had before, but good enough, no?
Especially in the presence of the widely advertised high-speed
allocations.

> 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:
> 
>       xc_hvm_build and xc_dom_mem_init
> 

AIUI, xl already has a lock around all of domain creation, so that's not
making anything worse.  But if libxc exposes the claim operations to
libxl, then libxl could do the locking just around the claim.

In any case, with my grumpy kernel hacker's hat on, odd architecture in
the tools is no excuse for bizarre hypercall semantics. :)  

Cheers,

Tim.

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