Mailing List Archive

On Mon, Aug 15, 2011 at 07:08:33PM +0100, David Vrabel wrote:
> # HG changeset patch
> # User David Vrabel <david.vrabel@citrix.com>
> # Date 1313404957 -3600
> # Node ID 1ec6c392d40f37032b0c845094c7ee6ba69740fd
> # Parent 8d6edc3d26d26931f3732a2008fb4818bc7bab2d
> x86: use 'dom0_mem' to limit the number of pages for dom0
>
> Use the 'dom0_mem' command line option to set the maximum number of
> pages for dom0. dom0 can use then use the XENMEM_maximum_reservation
> memory op to automatically find this limit and reduce the size of any
> page tables etc.

So .. I was actually curious about this -how did this use to work
with 2.6.18? In there, if you did 'dom0_mem=max:2GB' it would limit the
amount of memory - so how does it do that?

>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>
> 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);
> +

don't you need the amount of p2m as well? Or is that all limited
by d->max_pages?

> return nr_pages;
> }
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

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

On 16/08/11 01:31, Konrad Rzeszutek Wilk wrote:
> On Mon, Aug 15, 2011 at 07:08:33PM +0100, David Vrabel wrote:
>> x86: use 'dom0_mem' to limit the number of pages for dom0
>>
>> Use the 'dom0_mem' command line option to set the maximum number of
>> pages for dom0. dom0 can use then use the XENMEM_maximum_reservation
>> memory op to automatically find this limit and reduce the size of any
>> page tables etc.
>
> So .. I was actually curious about this -how did this use to work
> with 2.6.18? In there, if you did 'dom0_mem=max:2GB' it would limit the
> amount of memory - so how does it do that?

I don't know. I've not really looked at how the older kernels did it.

>> 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);
>> +
>
> don't you need the amount of p2m as well? Or is that all limited
> by d->max_pages?

Do you mean the initial p2m supplied by Xen? I think that just covers
the initial pages.

The p2m structure maintained by the kernel appears to be dynamically
allocated based on what mappings are required.

David

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

On Tue, 2011-08-16 at 13:29 +0100, David Vrabel wrote:
> On 16/08/11 01:31, Konrad Rzeszutek Wilk wrote:
> > On Mon, Aug 15, 2011 at 07:08:33PM +0100, David Vrabel wrote:
> >> x86: use 'dom0_mem' to limit the number of pages for dom0
> >>
> >> Use the 'dom0_mem' command line option to set the maximum number of
> >> pages for dom0. dom0 can use then use the XENMEM_maximum_reservation
> >> memory op to automatically find this limit and reduce the size of any
> >> page tables etc.
> >
> > So .. I was actually curious about this -how did this use to work
> > with 2.6.18? In there, if you did 'dom0_mem=max:2GB' it would limit the
> > amount of memory - so how does it do that?
>
> I don't know. I've not really looked at how the older kernels did it.

The classic-Xen kernels didn't try and munge the pseudo-physical (i.e.
RAM) address map with the machine (i.e. I/O) address map together in the
way pvops does, they just treated them as two orthogonal address spaces
(which has it's own issues, /proc/iomem is a complete fiction for
example). They don't really suffer from the same issues wrt e.g.
tracking where the I/O hole was before you clipped the initial amount of
RAM and all that.

The key difference is that classic-Xen kernels called both
XENMEM_memory_map and XENMEM_machine_memory_map and dealt with both,
whereas pvops (dom0) calls XENMEM_machine_memory_map and tries to figure
out how much of the E820_RAM therein it can actually use based on
nr_pages and such. You've used XENMEM_maximum_reservation in your
followup series which I think is a suitable surrogate for calling
XENMEM_memory_map (although I didn't check).

The linkage between mm setup and balloon driver setup in the classic
kernels was more explicit too IIRC.

> >> 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);
> >> +
> >
> > don't you need the amount of p2m as well? Or is that all limited
> > by d->max_pages?
>
> Do you mean the initial p2m supplied by Xen? I think that just covers
> the initial pages.

I think so too.

> The p2m structure maintained by the kernel appears to be dynamically
> allocated based on what mappings are required.

IIRC it is expanded around start of day to cover the start of day max
mappings etc (using BRK space) and further expanded later on
dynamically.

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



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

On 16/08/11 13:51, Ian Campbell wrote:
>
> The key difference is that classic-Xen kernels called both
> XENMEM_memory_map and XENMEM_machine_memory_map and dealt with both,
> whereas pvops (dom0) calls XENMEM_machine_memory_map and tries to figure
> out how much of the E820_RAM therein it can actually use based on
> nr_pages and such. You've used XENMEM_maximum_reservation in your
> followup series which I think is a suitable surrogate for calling
> XENMEM_memory_map (although I didn't check).

XENMEM_memory_map for dom0 doesn't work as nothing populates the map.

Classic-Xen kernels appear to invent a memory map based only on nr_pages
and it's non-obvious where it gets any other limit from (I didn't look
that hard though).

I did originally make XENMEM_memory_map work for dom0 but using
XENMEM_maximum_reservation looked more relevant.

David

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

Keir,

Since Konrad is considering submitting the Linux kernel patch needing
this Xen patch to the 3.0 stable tree, I think this should be a
candidate for Xen 4.1.2.

Thanks.

David

On 15/08/11 19:08, David Vrabel wrote:
> # HG changeset patch
> # User David Vrabel <david.vrabel@citrix.com>
> # Date 1313404957 -3600
> # Node ID 1ec6c392d40f37032b0c845094c7ee6ba69740fd
> # Parent 8d6edc3d26d26931f3732a2008fb4818bc7bab2d
> x86: use 'dom0_mem' to limit the number of pages for dom0
>
> Use the 'dom0_mem' command line option to set the maximum number of
> pages for dom0. dom0 can use then use the XENMEM_maximum_reservation
> memory op to automatically find this limit and reduce the size of any
> page tables etc.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>
> 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);
> +
> return nr_pages;
> }
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


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

>>> On 15.08.11 at 20:08, David Vrabel <david.vrabel@citrix.com> wrote:
> # HG changeset patch
> # User David Vrabel <david.vrabel@citrix.com>
> # Date 1313404957 -3600
> # Node ID 1ec6c392d40f37032b0c845094c7ee6ba69740fd
> # Parent 8d6edc3d26d26931f3732a2008fb4818bc7bab2d
> x86: use 'dom0_mem' to limit the number of pages for dom0
>
> Use the 'dom0_mem' command line option to set the maximum number of
> pages for dom0. dom0 can use then use the XENMEM_maximum_reservation
> memory op to automatically find this limit and reduce the size of any
> page tables etc.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>
> 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). If anything, this should be set to just max_pages, not
considering the amount of available memory at all.

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);

Jan

> +
> return nr_pages;
> }
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel




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

On 23/08/2011 10:01, "Jan Beulich" <JBeulich@novell.com> 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@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> On 23.08.11 at 11:26, Keir Fraser <keir.xen@gmail.com> wrote:
> On 23/08/2011 10:01, "Jan Beulich" <JBeulich@novell.com> 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@lists.xensource.com
http://lists.xensource.com/xen-devel

On 23/08/2011 10:41, "Jan Beulich" <JBeulich@novell.com> wrote:

>>> 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...

Ha, you'd be right! :-)

I'll leave it to David to champion his patch. I'll note I added it for 4.1.2
as well, if we decide there is a better way we will need to change it there
in advance of that release, and preferably -rc1 as well.

-- Keir



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

On 23/08/11 10:01, Jan Beulich wrote:
>>>> On 15.08.11 at 20:08, David Vrabel <david.vrabel@citrix.com> wrote:
>> x86: use 'dom0_mem' to limit the number of pages for dom0
>>
>> Use the 'dom0_mem' command line option to set the maximum number of
>> pages for dom0. dom0 can use then use the XENMEM_maximum_reservation
>> memory op to automatically find this limit and reduce the size of any
>> page tables etc.
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>>
>> 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). If anything, this should be set to just max_pages, not
> considering the amount of available memory at all.
>
> 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);

Yeah, this makes sense. I'll send another patch tomorrow.

David

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