Re: [Xen-devel] [PATCH] x86: use 'dom0_mem' to limit the number of pages for dom0

[Keir Fraser <keir.xen@xxxxxxxxx>]

On 23/08/2011 10:01, "Jan Beulich" <JBeulich@xxxxxxxxxx> wrote:

>> diff -r 8d6edc3d26d2 -r 1ec6c392d40f xen/arch/x86/domain_build.c
>> --- a/xen/arch/x86/domain_build.c Sat Aug 13 10:14:58 2011 +0100
>> +++ b/xen/arch/x86/domain_build.c Mon Aug 15 11:42:37 2011 +0100
>> @@ -254,6 +254,8 @@ static unsigned long __init compute_dom0
>>      }
>>  #endif
>>  
>> +    d->max_pages = min(max_pages, avail);
> 
> Only having noticed this hypervisor side change in the patch set as it
> now got applied to -unstable already, I have to object to this: It is
> not memory hotplug compatible (i.e. Dom0, at least with a non-pvops
> kernel, could have got ballooned up past the original limit once memory
> got added to the system, whereas now you limit it to what was there
> at boot time).

That's true for any domain isn't it? Yet we've got by so far. The max-mem
can be adjusted after a domain is created, so it's not incompatible.

> If anything, this should be set to just max_pages, not
> considering the amount of available memory at all.

I suspect that nullifies the aim of the patch...

> Besides that the assignment needs to be saturating, as struct domain's
> max_pages member is only "unsigned int", whereas the local variables
> are all "unsigned long". So in the end this should really be an equivalent
> of
> 
>     d->max_pages = min(max_pages, UINT_MAX);

We should make d->max_pages a ulong or u64.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel