Re: Suspicious error for CMA stress test

From: Leizhen (ThunderTown)
Date: Mon Mar 07 2016 - 20:54:55 EST



On 2016/3/8 2:42, Laura Abbott wrote:
> On 03/07/2016 12:16 AM, Leizhen (ThunderTown) wrote:
>>
>>
>> On 2016/3/7 12:34, Joonsoo Kim wrote:
>>> On Fri, Mar 04, 2016 at 03:35:26PM +0800, Hanjun Guo wrote:
>>>> On 2016/3/4 14:38, Joonsoo Kim wrote:
>>>>> On Fri, Mar 04, 2016 at 02:05:09PM +0800, Hanjun Guo wrote:
>>>>>> On 2016/3/4 12:32, Joonsoo Kim wrote:
>>>>>>> On Fri, Mar 04, 2016 at 11:02:33AM +0900, Joonsoo Kim wrote:
>>>>>>>> On Thu, Mar 03, 2016 at 08:49:01PM +0800, Hanjun Guo wrote:
>>>>>>>>> On 2016/3/3 15:42, Joonsoo Kim wrote:
>>>>>>>>>> 2016-03-03 10:25 GMT+09:00 Laura Abbott <labbott@xxxxxxxxxx>:
>>>>>>>>>>> (cc -mm and Joonsoo Kim)
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 03/02/2016 05:52 AM, Hanjun Guo wrote:
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> I came across a suspicious error for CMA stress test:
>>>>>>>>>>>>
>>>>>>>>>>>> Before the test, I got:
>>>>>>>>>>>> -bash-4.3# cat /proc/meminfo | grep Cma
>>>>>>>>>>>> CmaTotal: 204800 kB
>>>>>>>>>>>> CmaFree: 195044 kB
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> After running the test:
>>>>>>>>>>>> -bash-4.3# cat /proc/meminfo | grep Cma
>>>>>>>>>>>> CmaTotal: 204800 kB
>>>>>>>>>>>> CmaFree: 6602584 kB
>>>>>>>>>>>>
>>>>>>>>>>>> So the freed CMA memory is more than total..
>>>>>>>>>>>>
>>>>>>>>>>>> Also the the MemFree is more than mem total:
>>>>>>>>>>>>
>>>>>>>>>>>> -bash-4.3# cat /proc/meminfo
>>>>>>>>>>>> MemTotal: 16342016 kB
>>>>>>>>>>>> MemFree: 22367268 kB
>>>>>>>>>>>> MemAvailable: 22370528 kB
>>>>>>>>> [...]
>>>>>>>>>>> I played with this a bit and can see the same problem. The sanity
>>>>>>>>>>> check of CmaFree < CmaTotal generally triggers in
>>>>>>>>>>> __move_zone_freepage_state in unset_migratetype_isolate.
>>>>>>>>>>> This also seems to be present as far back as v4.0 which was the
>>>>>>>>>>> first version to have the updated accounting from Joonsoo.
>>>>>>>>>>> Were there known limitations with the new freepage accounting,
>>>>>>>>>>> Joonsoo?
>>>>>>>>>> I don't know. I also played with this and looks like there is
>>>>>>>>>> accounting problem, however, for my case, number of free page is slightly less
>>>>>>>>>> than total. I will take a look.
>>>>>>>>>>
>>>>>>>>>> Hanjun, could you tell me your malloc_size? I tested with 1 and it doesn't
>>>>>>>>>> look like your case.
>>>>>>>>> I tested with malloc_size with 2M, and it grows much bigger than 1M, also I
>>>>>>>>> did some other test:
>>>>>>>> Thanks! Now, I can re-generate erronous situation you mentioned.
>>>>>>>>
>>>>>>>>> - run with single thread with 100000 times, everything is fine.
>>>>>>>>>
>>>>>>>>> - I hack the cam_alloc() and free as below [1] to see if it's lock issue, with
>>>>>>>>> the same test with 100 multi-thread, then I got:
>>>>>>>> [1] would not be sufficient to close this race.
>>>>>>>>
>>>>>>>> Try following things [A]. And, for more accurate test, I changed code a bit more
>>>>>>>> to prevent kernel page allocation from cma area [B]. This will prevent kernel
>>>>>>>> page allocation from cma area completely so we can focus cma_alloc/release race.
>>>>>>>>
>>>>>>>> Although, this is not correct fix, it could help that we can guess
>>>>>>>> where the problem is.
>>>>>>> More correct fix is something like below.
>>>>>>> Please test it.
>>>>>> Hmm, this is not working:
>>>>> Sad to hear that.
>>>>>
>>>>> Could you tell me your system's MAX_ORDER and pageblock_order?
>>>>>
>>>>
>>>> MAX_ORDER is 11, pageblock_order is 9, thanks for your help!
>>>
>>> Hmm... that's same with me.
>>>
>>> Below is similar fix that prevents buddy merging when one of buddy's
>>> migrate type, but, not both, is MIGRATE_ISOLATE. In fact, I have
>>> no idea why previous fix (more correct fix) doesn't work for you.
>>> (It works for me.) But, maybe there is a bug on the fix
>>> so I make new one which is more general form. Please test it.
>>
>> Hi,
>> Hanjun Guo has gone to Tailand on business, so I help him to run this patch. The result
>> shows that the count of "CmaFree:" is OK now. But sometimes printed some information as below:
>>
>> alloc_contig_range: [28500, 28600) PFNs busy
>> alloc_contig_range: [28300, 28380) PFNs busy
>>
>
> Those messages aren't necessarily a problem. Those messages indicate that
OK.

> those pages weren't able to be isolated. Given the test here is a
> concurrency test, I suspect some concurrent allocation or free prevented
> isolation which is to be expected some times. I'd only be concerned if
> seeing those messages cause allocation failure or some other notable impact.
I chose memory block size: 512K, 1M, 2M ran serveral times, there was no memory allocation failure.

>
> Thanks,
> Laura
>
>>>
>>> Thanks.
>>>
>>> ---------->8-------------
>>> >From dd41e348572948d70b935fc24f82c096ff0fb417 Mon Sep 17 00:00:00 2001
>>> From: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
>>> Date: Fri, 4 Mar 2016 13:28:17 +0900
>>> Subject: [PATCH] mm/cma: fix race
>>>
>>> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
>>> ---
>>> mm/page_alloc.c | 33 +++++++++++++++++++--------------
>>> 1 file changed, 19 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>> index c6c38ed..d80d071 100644
>>> --- a/mm/page_alloc.c
>>> +++ b/mm/page_alloc.c
>>> @@ -620,8 +620,8 @@ static inline void rmv_page_order(struct page *page)
>>> *
>>> * For recording page's order, we use page_private(page).
>>> */
>>> -static inline int page_is_buddy(struct page *page, struct page *buddy,
>>> - unsigned int order)
>>> +static inline int page_is_buddy(struct zone *zone, struct page *page,
>>> + struct page *buddy, unsigned int order)
>>> {
>>> if (!pfn_valid_within(page_to_pfn(buddy)))
>>> return 0;
>>> @@ -644,6 +644,20 @@ static inline int page_is_buddy(struct page *page, struct page *buddy,
>>> if (page_zone_id(page) != page_zone_id(buddy))
>>> return 0;
>>>
>>> + if (IS_ENABLED(CONFIG_CMA) &&
>>> + unlikely(has_isolate_pageblock(zone)) &&
>>> + unlikely(order >= pageblock_order)) {
>>> + int page_mt, buddy_mt;
>>> +
>>> + page_mt = get_pageblock_migratetype(page);
>>> + buddy_mt = get_pageblock_migratetype(buddy);
>>> +
>>> + if (page_mt != buddy_mt &&
>>> + (is_migrate_isolate(page_mt) ||
>>> + is_migrate_isolate(buddy_mt)))
>>> + return 0;
>>> + }
>>> +
>>> VM_BUG_ON_PAGE(page_count(buddy) != 0, buddy);
>>>
>>> return 1;
>>> @@ -691,17 +705,8 @@ static inline void __free_one_page(struct page *page,
>>> 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 (!is_migrate_isolate(migratetype))
>>> __mod_zone_freepage_state(zone, 1 << order, migratetype);
>>> - }
>>>
>>> page_idx = pfn & ((1 << max_order) - 1);
>>>
>>> @@ -711,7 +716,7 @@ static inline void __free_one_page(struct page *page,
>>> 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))
>>> + if (!page_is_buddy(zone, page, buddy, order))
>>> break;
>>> /*
>>> * Our buddy is free or it is CONFIG_DEBUG_PAGEALLOC guard page,
>>> @@ -745,7 +750,7 @@ static inline void __free_one_page(struct page *page,
>>> higher_page = page + (combined_idx - page_idx);
>>> buddy_idx = __find_buddy_index(combined_idx, order + 1);
>>> higher_buddy = higher_page + (buddy_idx - combined_idx);
>>> - if (page_is_buddy(higher_page, higher_buddy, order + 1)) {
>>> + if (page_is_buddy(zone, higher_page, higher_buddy, order + 1)) {
>>> list_add_tail(&page->lru,
>>> &zone->free_area[order].free_list[migratetype]);
>>> goto out;
>>>
>>
>
>
> .
>