Re: Suspicious error for CMA stress test

Vlastimil Babka
2016-03-14T07:06:25Z

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@kvack.org. For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: email@kvack.org
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: email@kvack.org
>