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

[Gianluca Guida <gianluca.guida@xxxxxxxxxx>]

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@xxxxxxxxxx>
 > > 
 > > Signed-off-by: Konrad Rzeszutek Wilk <kliw@xxxxxxxxxx>
 > > ---
 > >  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

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