From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752066AbcF2KFR (ORCPT ); Wed, 29 Jun 2016 06:05:17 -0400 Received: from mx2.suse.de ([195.135.220.15]:39331 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751168AbcF2KFP (ORCPT ); Wed, 29 Jun 2016 06:05:15 -0400 Subject: Re: [merged] mm-compaction-abort-free-scanner-if-split-fails.patch removed from -mm tree To: Joonsoo Kim , linux-kernel@vger.kernel.org, akpm@linux-foundation.org References: <57732c58.xVgPTyhpf0KHQcpw%akpm@linux-foundation.org> <20160629081204.GC25102@js1304-P5Q-DELUXE> Cc: rientjes@google.com, hughd@google.com, mgorman@techsingularity.net, minchan@kernel.org, stable@vger.kernel.org, mm-commits@vger.kernel.org From: Vlastimil Babka Message-ID: Date: Wed, 29 Jun 2016 12:05:09 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <20160629081204.GC25102@js1304-P5Q-DELUXE> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/29/2016 10:12 AM, Joonsoo Kim wrote: >> @@ -1035,8 +1034,12 @@ static void isolate_freepages(struct com >> continue; >> >> /* Found a block suitable for isolating free pages from. */ >> - isolate_freepages_block(cc, &isolate_start_pfn, >> - block_end_pfn, freelist, false); >> + isolated = isolate_freepages_block(cc, &isolate_start_pfn, >> + block_end_pfn, freelist, false); >> + /* If isolation failed early, do not continue needlessly */ >> + if (!isolated && isolate_start_pfn < block_end_pfn && >> + cc->nr_migratepages > cc->nr_freepages) >> + break; > > Hello, David. > > Minchan found the bug on this patch. Was it reported in some mail to linux-mm before the patch was sent to Linus, or just now? > isolate_freepages_block() could return positive number if it is > stopped due to memory shortage. In this case, above branch would not > catch this situation due to positive 'isolated' and go through the Uh, right. It could have isolated something successfully and only then fail to isolate some more, so isolated is non-zero. I've missed this corner case in review :( > following code. > > if ((cc->nr_freepages >= cc->nr_migratepages) || XXX) > else > VM_BUG_ON(isolate_start_pfn < block_end_pfn); > > In this case, cc->nr_freepages could be lower than cc->nr_migratepages > and isolate_start_pfn < block_end_pfn so it triggers VM_BUG_ON(). > > If my analysis is correct, please fix it. David? > Thanks. >