This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] Re: [PATCH] xen/balloon: Memory hotplug support for Xen ball

On Mon, Mar 28, 2011 at 08:55:27AM -0700, Dave Hansen wrote:
> On Mon, 2011-03-28 at 11:47 +0200, Daniel Kiper wrote:
> >
> > +static enum bp_state reserve_additional_memory(long credit)
> > +{
> > +       int nid, rc;
> > +       u64 start;
> > +       unsigned long balloon_hotplug = credit;
> > +
> > +       start = PFN_PHYS(SECTION_ALIGN_UP(max_pfn));
> > +       balloon_hotplug = (balloon_hotplug & PAGE_SECTION_MASK) + 
> > +       nid = memory_add_physaddr_to_nid(start);
> Is the 'balloon_hotplug' calculation correct?  I _think_ you're trying
> to round up to the SECTION_SIZE_PAGES.  But, if 'credit' was already
> section-aligned I think you'll unnecessarily round up to the next
> SECTION_SIZE_PAGES boundary.  Should it just be:
>       balloon_hotplug = ALIGN(balloon_hotplug, PAGES_PER_SECTION);

Yes, you are right. I am wrong. I will correct that. However, as I said
ealier I do not like ALIGN() in size context. For me ALIGN() is operation
on an address which aligns this address to specified boundary. That is
why I prefer use here open coded version (I agree that it is the same
to ALIGN()). I think that ROUND() macro would be better in size context.
However, I am not native english speaker and if I missed something correct
me, please.

> You might also want to consider some nicer units for those suckers.

What do you mind ??? I think that in that context PAGES_PER_SECTION
is quite good.

> 'start_paddr' is _much_ easier to grok than 'start', for instance.



Xen-devel mailing list