Re: Suspicious error for CMA stress test

[Vlastimil Babka]

On 03/14/2016 07:49 AM, Joonsoo Kim wrote:
> On Fri, Mar 11, 2016 at 06:07:40PM +0100, Vlastimil Babka wrote:
> > On 03/11/2016 04:00 PM, Joonsoo Kim wrote:
> >
> > How about something like this? Just and idea, probably buggy (off-by-one etc.).
> > Should keep away cost from <pageblock_order iterations at the expense of the
> > relatively fewer >pageblock_order iterations.
>
> Hmm... I tested this and found that it's code size is a little bit
> larger than mine. I'm not sure why this happens exactly but I guess it would be
> related to compiler optimization. In this case, I'm in favor of my
> implementation because it looks like well abstraction. It adds one
> unlikely branch to the merge loop but compiler would optimize it to
> check it once.

I would be surprised if compiler optimized that to check it once, as 
order increases with each loop iteration. But maybe it's smart enough to 
do something like I did by hand? Guess I'll check the disassembly.

> Thanks.
>
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index ff1e3cbc8956..b8005a07b2a1 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -685,21 +685,13 @@ static inline void __free_one_page(struct page *page,
> >   	unsigned long combined_idx;
> >   	unsigned long uninitialized_var(buddy_idx);
> >   	struct page *buddy;
> > -	unsigned int max_order = MAX_ORDER;
> > +	unsigned int max_order = pageblock_order + 1;
> >
> >   	VM_BUG_ON(!zone_is_initialized(zone));
> >   	VM_BUG_ON_PAGE(page->flags & PAGE_FLAGS_CHECK_AT_PREP, page);
> >
> >   	VM_BUG_ON(migratetype == -1);
> > -	if (is_migrate_isolate(migratetype)) {
> > -		/*
> > -		 * We restrict max order of merging to prevent merge
> > -		 * between freepages on isolate pageblock and normal
> > -		 * pageblock. Without this, pageblock isolation
> > -		 * could cause incorrect freepage accounting.
> > -		 */
> > -		max_order = min_t(unsigned int, MAX_ORDER, pageblock_order + 1);
> > -	} else {
> > +	if (likely(!is_migrate_isolate(migratetype))) {
> >   		__mod_zone_freepage_state(zone, 1 << order, migratetype);
> >   	}
> >
> > @@ -708,11 +700,12 @@ static inline void __free_one_page(struct page *page,
> >   	VM_BUG_ON_PAGE(page_idx & ((1 << order) - 1), page);
> >   	VM_BUG_ON_PAGE(bad_range(zone, page), page);
> >
> > +continue_merging:
> >   	while (order < max_order - 1) {
> >   		buddy_idx = __find_buddy_index(page_idx, order);
> >   		buddy = page + (buddy_idx - page_idx);
> >   		if (!page_is_buddy(page, buddy, order))
> > -			break;
> > +			goto done_merging;
> >   		/*
> >   		 * Our buddy is free or it is CONFIG_DEBUG_PAGEALLOC guard page,
> >   		 * merge with it and move up one order.
> > @@ -729,6 +722,26 @@ static inline void __free_one_page(struct page *page,
> >   		page_idx = combined_idx;
> >   		order++;
> >   	}
> > +	if (max_order < MAX_ORDER) {
> > +		if (IS_ENABLED(CONFIG_CMA) &&
> > +				unlikely(has_isolate_pageblock(zone))) {
> > +
> > +			int buddy_mt;
> > +
> > +			buddy_idx = __find_buddy_index(page_idx, order);
> > +			buddy = page + (buddy_idx - page_idx);
> > +			buddy_mt = get_pageblock_migratetype(buddy);
> > +
> > +			if (migratetype != buddy_mt &&
> > +					(is_migrate_isolate(migratetype) ||
> > +					is_migrate_isolate(buddy_mt)))
> > +				goto done_merging;
> > +		}
> > +		max_order++;
> > +		goto continue_merging;
> > +	}
> > +
> > +done_merging:
> >   	set_page_order(page, order);
> >
> >   	/*
> >
> > --
> > To unsubscribe, send a message with 'unsubscribe linux-mm' in
> > the body to majordomo@xxxxxxxxxx  For more info on Linux MM,
> > see: http://www.linux-mm.org/ .
> > Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@xxxxxxxxxx  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>