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@lists.xensource.com
http://lists.xensource.com/xen-ppc-devel

Reply via email to