Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932982AbcCQJZs (ORCPT ); Thu, 17 Mar 2016 05:25:48 -0400 Received: from szxga02-in.huawei.com ([119.145.14.65]:51309 "EHLO szxga02-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932329AbcCQJZp (ORCPT ); Thu, 17 Mar 2016 05:25:45 -0400 Subject: Re: Suspicious error for CMA stress test To: Joonsoo Kim References: <56DD38E7.3050107@huawei.com> <56DDCB86.4030709@redhat.com> <56DE30CB.7020207@huawei.com> <56DF7B28.9060108@huawei.com> <56E2FB5C.1040602@suse.cz> <20160314064925.GA27587@js1304-P5Q-DELUXE> <56E662E8.700@suse.cz> <20160314071803.GA28094@js1304-P5Q-DELUXE> <56E92AFC.9050208@huawei.com> <20160317065426.GA10315@js1304-P5Q-DELUXE> CC: Vlastimil Babka , "Leizhen (ThunderTown)" , Laura Abbott , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , Andrew Morton , Sasha Levin , Laura Abbott , qiuxishi , Catalin Marinas , Will Deacon , Arnd Bergmann , dingtinahong , , "linux-mm@kvack.org" From: Hanjun Guo Message-ID: <56EA77BC.2090702@huawei.com> Date: Thu, 17 Mar 2016 17:24:12 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20160317065426.GA10315@js1304-P5Q-DELUXE> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.17.188] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020205.56EA77C8.01BC,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-06-18 04:22:30, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: a52e2d9e2f22d387a60808804e4e1c14 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3204 Lines: 74 On 2016/3/17 14:54, Joonsoo Kim wrote: > On Wed, Mar 16, 2016 at 05:44:28PM +0800, Hanjun Guo wrote: >> On 2016/3/14 15:18, Joonsoo Kim wrote: >>> On Mon, Mar 14, 2016 at 08:06:16AM +0100, Vlastimil Babka wrote: >>>> 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 >>>>> 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. >>> Okay. I used following slightly optimized version and I need to >>> add 'max_order = min_t(unsigned int, MAX_ORDER, pageblock_order + 1)' >>> to yours. Please consider it, too. >> Hmm, this one is not work, I still can see the bug is there after applying >> this patch, did I miss something? > I may find that there is a bug which was introduced by me some time > ago. Could you test following change in __free_one_page() on top of > Vlastimil's patch? > > -page_idx = pfn & ((1 << max_order) - 1); > +page_idx = pfn & ((1 << MAX_ORDER) - 1); I tested Vlastimil's patch + your change with stress for more than half hour, the bug I reported is gone :) I have some questions, Joonsoo, you provided a patch as following: diff --git a/mm/cma.c b/mm/cma.c index 3a7a67b..952a8a3 100644 --- a/mm/cma.c +++ b/mm/cma.c @@ -448,7 +448,10 @@ bool cma_release(struct cma *cma, const struct page *pages, unsigned int count) VM_BUG_ON(pfn + count > cma->base_pfn + cma->count); + mutex_lock(&cma_mutex); free_contig_range(pfn, count); + mutex_unlock(&cma_mutex); + cma_clear_bitmap(cma, pfn, count); trace_cma_release(pfn, pages, count); diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 7f32950..68ed5ae 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1559,7 +1559,8 @@ void free_hot_cold_page(struct page *page, bool cold) * excessively into the page allocator */ if (migratetype >= MIGRATE_PCPTYPES) { - if (unlikely(is_migrate_isolate(migratetype))) { + if (is_migrate_cma(migratetype) || + unlikely(is_migrate_isolate(migratetype))) { free_one_page(zone, page, pfn, 0, migratetype); goto out; } This patch also works to fix the bug, why not just use this one? is there any side effects for this patch? maybe there is performance issue as the mutex lock is used, any other issues? Thanks Hanjun