[PATCH XEN-UNSTABLE PV_OPS] Balloon fix

[PATCH XEN-UNSTABLE PV_OPS] Balloon fix

[Konrad Rzeszutek Wilk]

The patch fixes booting a 64-bit dom0 on machine with 4GB. 

[PATCH] Include in balloon page tracking only PFNs which are marked as RAM in E820.

[Konrad Rzeszutek Wilk]

From: Konrad Rzeszutek Wilk <kliw@darnok.org>

Signed-off-by: Konrad Rzeszutek Wilk <kliw@darnok.org>
---
 drivers/xen/balloon.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 168e61f..ef2edd8 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -416,8 +416,12 @@ static int __init balloon_init(void)
 
 	/* Initialise the balloon with excess memory space. */
 	for (pfn = xen_start_info->nr_pages; pfn < max_pfn; pfn++) {
-		page = pfn_to_page(pfn);
-		balloon_append(page);
+   		int is_ram = page_is_ram(pfn);
+		if (is_ram) {
+			page = pfn_to_page(pfn);
+			if (PageReserved(page))
+				balloon_append(page);
+		}
 	}
 
 	target_watch.callback = watch_target;
-- 
1.6.2.5

Re: [PATCH] Include in balloon page tracking only PFNs which are marked as RAM in E820.

[Konrad Rzeszutek Wilk]

Gianluca,

I had not done any extensive testing. Would appreciate your feedback on this patch.

On Wed, Aug 19, 2009 at 03:12:29PM -0400, Konrad Rzeszutek Wilk wrote:
> From: Konrad Rzeszutek Wilk <kliw@darnok.org>
> 
> Signed-off-by: Konrad Rzeszutek Wilk <kliw@darnok.org>
> ---
>  drivers/xen/balloon.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 168e61f..ef2edd8 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -416,8 +416,12 @@ static int __init balloon_init(void)
>  
>  	/* Initialise the balloon with excess memory space. */
>  	for (pfn = xen_start_info->nr_pages; pfn < max_pfn; pfn++) {
> -		page = pfn_to_page(pfn);
> -		balloon_append(page);
> +   		int is_ram = page_is_ram(pfn);
> +		if (is_ram) {
> +			page = pfn_to_page(pfn);
> +			if (PageReserved(page))
> +				balloon_append(page);
> +		}
>  	}
>  
>  	target_watch.callback = watch_target;
> -- 
> 1.6.2.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] Include in balloon page tracking only PFNs which are marked as RAM in E820.

[Gianluca Guida]

Hi,

Ok, by looking at this patch it's clear that you are testing with a
kernel that tries to allow kernels to boot ballooned.

In this case the patch is correct. Since the patch is actually
reverted, I think we should wait to apply it until I succeed to
reproduce the bug that caused Jeremy to revert it.

Only one style comment:

Konrad Rzeszutek Wilk writes:
 > On Wed, Aug 19, 2009 at 03:12:29PM -0400, Konrad Rzeszutek Wilk wrote:
 > > From: Konrad Rzeszutek Wilk <kliw@darnok.org>
 > > 
 > > Signed-off-by: Konrad Rzeszutek Wilk <kliw@darnok.org>
 > > ---
 > >  drivers/xen/balloon.c |    8 ++++++--
 > >  1 files changed, 6 insertions(+), 2 deletions(-)
 > > 
 > > diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
 > > index 168e61f..ef2edd8 100644
 > > --- a/drivers/xen/balloon.c
 > > +++ b/drivers/xen/balloon.c
 > > @@ -416,8 +416,12 @@ static int __init balloon_init(void)
 > >  
 > >  	/* Initialise the balloon with excess memory space. */
 > >  	for (pfn = xen_start_info->nr_pages; pfn < max_pfn; pfn++) {
 > > -		page = pfn_to_page(pfn);
 > > -		balloon_append(page);
 > > +   		int is_ram = page_is_ram(pfn);

Do you really need to declare this as a variable? couldn't you put the
expression in the if statement?

 > > +		if (is_ram) {
 > > +			page = pfn_to_page(pfn);
 > > +			if (PageReserved(page))
 > > +				balloon_append(page);
 > > +		}
 > >  	}
 > >  
 > >  	target_watch.callback = watch_target;
 > > -- 
 > > 1.6.2.5


Thanks,
Gianluca

Re: [PATCH] Include in balloon page tracking only PFNs which are marked as RAM in E820.

[Konrad Rzeszutek Wilk]

On Thu, Aug 20, 2009 at 05:02:57PM +0100, Gianluca Guida wrote:
> Hi,
> 
> Ok, by looking at this patch it's clear that you are testing with a
> kernel that tries to allow kernels to boot ballooned.
> 
> In this case the patch is correct. Since the patch is actually
> reverted, I think we should wait to apply it until I succeed to
> reproduce the bug that caused Jeremy to revert it.

You are talking about the 7394b25905929a92e51771b832ee67b70ccebcf0 patch?
I will wait and if this patch is still needed I will send it out to LKML/xen-devel
with your style comment.


>  > > +   		int is_ram = page_is_ram(pfn);
> 
> Do you really need to declare this as a variable? couldn't you put the

Not at all. Right after I sent it out I realized it would have been looking
better without.

> expression in the if statement?