Suspicious error for CMA stress test

[vbabka@suse.cz (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@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 at kvack.org.  For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a href=mailto:"dont@kvack.org"> email at kvack.org </a>
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo at kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email at kvack.org </a>
>