Re: [Xen-devel] [RFC/PATCH v2] XENMEM_claim_pages (subop of existing) hypercall

[Dan Magenheimer <dan.magenheimer@xxxxxxxxxx>]

> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Thursday, November 15, 2012 3:25 AM
> To: Dan Magenheimer
> Cc: xen-devel@xxxxxxxxxxxxx; KonradWilk; Zhigang Wang; Keir Fraser; TimDeegan
> Subject: Re: [RFC/PATCH v2] XENMEM_claim_pages (subop of existing) hypercall

Hi Jan --

Thanks for the quick review!

Mostly fixed for v3 but a couple things:
 
> > +     * Claimed memory is considered unavailable unless the request
> > +     * is made by a domain with sufficient unclaimed pages.
> > +     */
> > +    if ( (total_unclaimed_pages + request >
> > +           total_avail_pages + tmem_freeable_pages()) &&
> > +          (d == NULL || d->unclaimed_pages < request) )
> > +        goto not_found;
> 
> The treatment of d being NULL certainly needs further thought:
> Is it really better to fulfill the claim and fail some (perhaps
> important) _xmalloc()?

Ideally, allocation in the presence of existing claims should
behave as if the claiming domains had actually already allocated
the unclaimed-amount-of-memory.  So I'd argue that enforcing
the claim should be sacrosanct here.
 
> Also, I'm missing a mechanism by which the tools could find out
> how much unclaimed memory is available, in order to determine
> (if in use) how much memory needs to be ballooned out of Dom0.

OK.  I'm not certain if this will be useful on a per-domain
basis as well but, for completeness, I will also add
unclaimed_pages into xc_dominfo etc (which causes a bump
in XEN_DOMCTL_INTERFACE_VERSION).

> Similarly, but perhaps of lower priority, there is no integration
> with the low-mem handling.

I'd also consider this lower priority as Olaf and Andre
have argued that the claim mechanism is not needed for
sharing/paging so the two mechanisms may not
be used together, at least for the foreseeable future.
So I plan to skip this, unless you change your mind and
consider it a showstopper for acceptance.

> Finally, there still are a number of formatting issues.

Hmmm... I found one I think.  Is there an equivalent to
checkpatch for hypervisor code?  If you see any formatting
issues in v3, please call them out explicitly as I am
sincerely trying to avoid them.

Thanks,
Dan

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