Re: [XenPPC] [linux-ppc-2.6] [XEN][POWERPC] Use a bitmap to manage the foreign page area

[Jimi Xenidis <jimix@xxxxxxxxxxxxxx>]

Thanks for the watchful eye
On Nov 9, 2006, at 4:49 PM, Muli Ben-Yehuda wrote:

> On Thu, Nov 09, 2006 at 12:02:45AM +0000, Xen patchbot-linux- 
> ppc-2.6 wrote:
> > +struct page *alloc_foreign_page(void)
> > +{
> > +       int bit;
> > +       do {
> > +               bit = find_first_zero_bit(foreign_map_bitmap,
> > +                                         foreign_map_pgs);
> bit should be 'unsigned long'.
ACK

> > +               if (bit >= foreign_map_pgs)
> > +                       return NULL;
> I would print a message that the allocator has been exhausted here -
> it's a common source of bugs.
I'd prefer that in the caller report this, like any other allocator.

> Also, tiny optimization, but perhaps you
> next fit rather than first fit?
What is the optimization here? Some arches _do_ have different  
implementation for the 2, on PPC I would save a #define.
If you are suggesting I try to find the "next" zero bit if the first  
attempt does not work, I was thinking of that and IMHO the  
optimization is just not worth even the that trivial complication  
since we are only allocating on dev/mod init and freeing on dev/mod  
fini.
> > +       } while (test_and_set_bit(bit, foreign_map_bitmap) == 1);
> > +
> > +       return pfn_to_page(foreign_map_pfn + bit);
> > +}
> > +
> > +void free_foreign_page(struct page *page)
> > +{
> > +       int bit = page_to_pfn(page) - foreign_map_pfn;
> > +
> > +       BUG_ON(bit < 0);
> > +       BUG_ON(bit >= foreign_map_pgs);
> I would add BUG_ON(!test_bit(bit, foreign_map_bitmap)) here to catch
> another common source of bugs.
ACK

-JX


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