From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id A0EB2C001B0 for ; Tue, 15 Aug 2023 08:29:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 147B090000B; Tue, 15 Aug 2023 04:29:01 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0F8208D0001; Tue, 15 Aug 2023 04:29:01 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F28F790000B; Tue, 15 Aug 2023 04:29:00 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id E47908D0001 for ; Tue, 15 Aug 2023 04:29:00 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id B11E6160CBB for ; Tue, 15 Aug 2023 08:29:00 +0000 (UTC) X-FDA: 81125663640.06.59CB8A6 Received: from out30-119.freemail.mail.aliyun.com (out30-119.freemail.mail.aliyun.com [115.124.30.119]) by imf04.hostedemail.com (Postfix) with ESMTP id CF44040015 for ; Tue, 15 Aug 2023 08:28:57 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=none; spf=pass (imf04.hostedemail.com: domain of baolin.wang@linux.alibaba.com designates 115.124.30.119 as permitted sender) smtp.mailfrom=baolin.wang@linux.alibaba.com; dmarc=pass (policy=none) header.from=alibaba.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1692088139; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=54dXLPHRDs3JZcsD614W2psJ+lbhPTGrBi+tOpFsR1c=; b=ECSeCZEmHPGvtrIaPNsSFeOIPyb54zUt2Lm6aUyeNN8hhjGz9tdYaBueoSrogvgohitWz3 aqlbKqbuhxUzkBpVivVZrcGSSd4fsQG29SSAnuyGMTaGDSc8IC8Ereje8Z5yu+DL3XStkO kHlPR403mF+Pl7Va1aCfrMBD9W0qI1E= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1692088139; a=rsa-sha256; cv=none; b=UW3F3aSp57elLR0qwYXMpQUf9AVVT0E8hEVdoYLnEdasPuPpLAYl6+SmhV0WzoBHSG0G59 mZJXC0Mx7t4YY8ZmCqfDAiZvrQqglJIwjs97pKE7rjhuHi9oDXlWkjQm5iQMNOIXXeX6pv Y1BRLPK+cwAqV80FUV+OkejyxEnJZYs= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=none; spf=pass (imf04.hostedemail.com: domain of baolin.wang@linux.alibaba.com designates 115.124.30.119 as permitted sender) smtp.mailfrom=baolin.wang@linux.alibaba.com; dmarc=pass (policy=none) header.from=alibaba.com X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R161e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018046059;MF=baolin.wang@linux.alibaba.com;NM=1;PH=DS;RN=6;SR=0;TI=SMTPD_---0VprFL1k_1692088132; Received: from 30.97.48.59(mailfrom:baolin.wang@linux.alibaba.com fp:SMTPD_---0VprFL1k_1692088132) by smtp.aliyun-inc.com; Tue, 15 Aug 2023 16:28:53 +0800 Message-ID: Date: Tue, 15 Aug 2023 16:28:54 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.14.0 Subject: Re: [PATCH 3/9] mm/compaction: correctly return failure with bogus compound_order in strict mode To: Kemeng Shi , linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, mgorman@techsingularity.net, david@redhat.com References: <20230805110711.2975149-1-shikemeng@huaweicloud.com> <20230805110711.2975149-4-shikemeng@huaweicloud.com> From: Baolin Wang In-Reply-To: <20230805110711.2975149-4-shikemeng@huaweicloud.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: CF44040015 X-Rspam-User: X-Stat-Signature: mquwc3g4ggp4rd6wnca1pmat5pekusyf X-Rspamd-Server: rspam03 X-HE-Tag: 1692088137-323413 X-HE-Meta: U2FsdGVkX1+xI44grYhGRnzHjb44p4LN7nGMQ2WJ55xrqxLwwX4vMuUn2+kSHSGnTFpJ4tDBAmFdtLNH0l2ZiIvmJKovE5jZ6faH/CtNkp0YOnghKXnV/xBCQB3fHfpUe5tCbZJXdC43oe8p1eYMSyIYJX2XUA5IiCPKTsn5adUQy/psi6d8O+reycKXGBhDKgl0KOUObettxjtYKpXMV2vaql0RuCu+KVStz4XWnDGVwHdHKD5X2Of1/WSzvGxybs/Sgl/f+Kknk7LBZNsDmX8A1UKUdj9AkdEzKfT9A5Z+kX2UtYiiPxXp3WWq73HLOA32IsCwCMa9s4rNsIA5eoN9Nh9cN64EE6YtMUrRcRe/vE8cJYMxPPzMp0SMH1X9Pv7F7XeQQGP36j7uPaE1wW1wtoCS+OEue9+ks56jjqwSx8I/aCUn5Zws/e4FNKATSNO8wMVgKQCuJ3gV81J/39GT+uXhF9GK9ba0/nQhvz832LCN1AdapuKFSoDB/los+L6g9pnbdLe9HYvHu7QmAD7Lez+odYjZy2tvAaObe3MJkdJNLQLx2v2kTfOnLHJNUZT51j/2VSGLrW9ZrJvPoGCv+zx1dYNPXlGV2e247o/I6jKzTRO40jKwoVEC269rpl6IhKZGcI5FK2oTDu+CvJ6OCupBJjglHzLUDeWmyjqPfLsFR8kVWG5yKYF2greE9yrPaLjFuSUIeMNvGM8TNPEBVOeS9a+YwN3tsrB46aAkRPTSRe5FkseIqF4gC0t77XNPCrl34ra3JbDRkEVfEAA0xFWgRQns38DsM2OTs97tRoFpYKVSCxNseRFYyNAz+65p7RD77Duk38gz8Ic8VAP36bo8409xtfb/qOKVqgsEwHX6AsbwpumJbUBfGdvoRfdzWaXXQymEvuj/QwtRHZ62aZOpYOKy2OHQr5hEl7O2V1ZOvAwnxpixbCOJy/ErKTU/8n8M49Jwuj9bhl+ N5dpF7Gz 5cbiizTSQM4jt1mUkwiwb/D0SmALQIYalNdfBWELds8i/pgoubc1+aYeonKNVGIO8CjZ7R7qldonxouVlkD4p/9evtgcjGnT36p6nlJGQ9aFWr3Y06o2gmcbuaotc8GyS+e33ED39IeHYtBX0wsaywb0Vat0Aw1T17Eqt2txFSk7TC+s4q1TVRTHQJYlT3ULdCyV2 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 8/5/2023 7:07 PM, Kemeng Shi wrote: > In strict mode, we should return 0 if there is any hole in pageblock. If > we successfully isolated pages at beginning at pageblock and then have a > bogus compound_order outside pageblock in next page. We will abort search > loop with blockpfn > end_pfn. Although we will limit blockpfn to end_pfn, > we will treat it as a successful isolation in strict mode as blockpfn is > not < end_pfn and return partial isolated pages. Then > isolate_freepages_range may success unexpectly with hole in isolated > range. Yes, that can be happened. > This patch also removes unnecessary limit for blockpfn to go outside > by buddy page introduced in fixed commit or by stride introduced after > fixed commit. Caller could use returned blockpfn to check if full > pageblock is scanned by test if blockpfn >= end and to get next pfn to > scan inside isolate_freepages_block on demand. IMO, I don't think removing the pageblock restriction is worth it, since it did not fix anything and will make people more confused, at least to me. That is to say, it will be surprised that the blockpfn can go outside of the pageblock after calling isolate_freepages_block() to just scan only one pageblock, and I did not see in detail if this can cause other potential problems. > Fixes: 9fcd6d2e052ee ("mm, compaction: skip compound pages by order in free scanner") > Signed-off-by: Kemeng Shi > --- > mm/compaction.c | 21 +++++++++++---------- > 1 file changed, 11 insertions(+), 10 deletions(-) > > diff --git a/mm/compaction.c b/mm/compaction.c > index fa1b100b0d10..684f6e6cd8bc 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -631,6 +631,14 @@ static unsigned long isolate_freepages_block(struct compact_control *cc, > page += (1UL << order) - 1; > nr_scanned += (1UL << order) - 1; > } > + /* > + * There is a tiny chance that we have read bogus > + * compound_order(), so be careful to not go outside > + * of the pageblock. > + */ > + if (unlikely(blockpfn >= end_pfn)) > + blockpfn = end_pfn - 1; So we can just add this validation to ensure that the isolate_freepages_block() can return 0 if failure is happened, which can fix your problem. > + > goto isolate_fail; > } > > @@ -677,17 +685,10 @@ static unsigned long isolate_freepages_block(struct compact_control *cc, > if (locked) > spin_unlock_irqrestore(&cc->zone->lock, flags); > > - /* > - * There is a tiny chance that we have read bogus compound_order(), > - * so be careful to not go outside of the pageblock. > - */ > - if (unlikely(blockpfn > end_pfn)) > - blockpfn = end_pfn; > - > trace_mm_compaction_isolate_freepages(*start_pfn, blockpfn, > nr_scanned, total_isolated); > > - /* Record how far we have got within the block */ > + /* Record how far we have got */ > *start_pfn = blockpfn; > > /* > @@ -1443,7 +1444,7 @@ fast_isolate_around(struct compact_control *cc, unsigned long pfn) > isolate_freepages_block(cc, &start_pfn, end_pfn, &cc->freepages, 1, false); > > /* Skip this pageblock in the future as it's full or nearly full */ > - if (start_pfn == end_pfn && !cc->no_set_skip_hint) > + if (start_pfn >= end_pfn && !cc->no_set_skip_hint) > set_pageblock_skip(page); > } > > @@ -1712,7 +1713,7 @@ static void isolate_freepages(struct compact_control *cc) > block_end_pfn, freelist, stride, false); > > /* Update the skip hint if the full pageblock was scanned */ > - if (isolate_start_pfn == block_end_pfn) > + if (isolate_start_pfn >= block_end_pfn) > update_pageblock_skip(cc, page, block_start_pfn - > pageblock_nr_pages); >