On Thu, 10 May 2007 17:42:54 +0100 (IST)
Mel Gorman <[EMAIL PROTECTED]> wrote:

> > +   if (!pfn_valid(pfn))
> > +           return -EINVAL;
> 
> This may lead to boundary cases where pages cannot be captured at the 
> start and end of non-aligned zones due to memory holes.
> 
Hm, ok. maybe we can remove this.

> > +   zone = info->zone;
> > +   if ((zone != page_zone(pfn_to_page(pfn))) ||
> > +       (zone != page_zone(pfn_to_page(last_pfn))))
> > +           return -EINVAL;
> 
> Is this check really necessary? Surely a caller to 
> capture_isolate_freed_pages() will have already made all the necessary 
> checks when adding the struct insolation_info ?
> 
just because isolation_info is treated per zone.
Maybe MIGRATE_ISOLATING can allow us more flexible approach.


> > +   drain_all_pages();
> > +   spin_lock(&zone->lock);
> > +   while (pfn < info->end_pfn) {
> > +           if (!pfn_valid(pfn)) {
> > +                   pfn++;
> > +                   continue;
> > +           }
> > +           page = pfn_to_page(pfn);
> > +           /* See page_is_buddy()  */
> > +           if (page_count(page) == 0 && PageBuddy(page)) {
> 
> If PageBuddy is set it's free, you shouldn't have to check the page_count.
> 
ok.

> > +                   order = page_order(page);
> > +                   order_size = 1 << order;
> > +                   zone->free_area[order].nr_free--;
> > +                   __mod_zone_page_state(zone, NR_FREE_PAGES, -order_size);
> > +                   list_del(&page->lru);
> > +                   rmv_page_order(page);
> > +                   isolate_page_nolock(info, page, order);
> > +                   nr_pages += order_size;
> > +                   pfn += order_size;
> > +           } else {
> > +                   pfn++;
> > +           }
> > +   }
> > +   spin_unlock(&zone->lock);
> > +   return nr_pages;
> > +}
> > #endif /* CONFIG_PAGE_ISOLATION */
> >
> 
> This is all similar to move_freepages() other than the locking part. It 
> would be worth checking if there is code that could be shared or at least 
> have similar styles.

Thank you, I'll look into move_freepages().

-Kame


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to