>>> On 23.08.11 at 11:26, Keir Fraser <keir.xen@xxxxxxxxx> wrote:
> 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.
For a DomU it's natural that it may need adjustment, but for Dom0
it's not (at least not to me).
>> 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...
Why - there's still a big difference between using LONG_MAX
unconditionally and using whatever was specified via
"dom0_mem=max:".
>> 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.
I'd rather not, as for now there's no good reason for this to be a
64-bit variable (as we can't get even close to the 16Tb it would
take to overflow this). But then again I expect you to say that
the few extra REX prefixes don't matter all that much...
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home