On Fri, Nov 11, 2011 at 02:29:53PM +0000, Mel Gorman wrote:
> > if (PageBuddy(buddy) && page_order(buddy) == order) {
> > VM_BUG_ON(page_count(buddy) != 0);
> > return 1;
> > @@ -518,9 +562,15 @@ static inline void __free_one_page(struct page *page,
> > break;
> >
> > /* Our buddy is free, merge with it and move up one order. */
> > - list_del(&buddy->lru);
> > - zone->free_area[order].nr_free--;
> > - rmv_page_order(buddy);
> > + if (page_is_corrupt_dbg(buddy)) {
> > + clear_page_corrupt_dbg(buddy);
> > + set_page_private(page, 0);
> > + __mod_zone_page_state(zone, NR_FREE_PAGES, 1 << order);
>
> Why are the buddies not merged?
I believe they are merged, but I'll double check.
> > static inline void expand(struct zone *zone, struct page *page,
> > - int low, int high, struct free_area *area,
> > + unsigned int low, unsigned int high, struct free_area *area,
> > int migratetype)
> > {
> > unsigned long size = 1 << high;
> > @@ -746,9 +796,16 @@ static inline void expand(struct zone *zone, struct page *page,
> > high--;
> > size >>= 1;
> > VM_BUG_ON(bad_range(zone, &page[size]));
> > - list_add(&page[size].lru, &area->free_list[migratetype]);
> > - area->nr_free++;
> > - set_page_order(&page[size], high);
> > + if (high < corrupt_dbg()) {
> > + INIT_LIST_HEAD(&page[size].lru);
> > + set_page_corrupt_dbg(&page[size]);
> > + set_page_private(&page[size], high);
> > + __mod_zone_page_state(zone, NR_FREE_PAGES, -(1 << high));
> > + } else {
>
> Because high is a signed integer, I don't think this would necessarily
> optimised away at compile time when DEBUG_PAGEALLOC is not set adding a
> new branch to a heavily executed fast path.
>
> For the fast paths, you should not add new branches if you can. Move the
> debugging code to inline functions that only exist when DEBUG_PAGEALLOC
> is set so there is no additional overhead in the !CONFIG_DEBUG_PAGEALLOC
> case.
I changed "high" type from int to unsigned int in the patch, and checked
that this branch is removed by compiler in !CONFIG_DEBUG_PAGEALLOC case.
But perhaps having this inside preprocessor checks is cleaner, so I'll do
that.
Thanks for the comments, I'll rework and repost.
Stanislaw
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>