[3/9] mm/compaction: correctly return failure with bogus compound_order in strict mode
Message ID | 20230805110711.2975149-4-shikemeng@huaweicloud.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c44e:0:b0:3f2:4152:657d with SMTP id w14csp241224vqr; Fri, 4 Aug 2023 20:22:28 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEAKfTYpt1USHiiJcO372Vm52L5IOD2anQsLnFhwyP1Gof66Hz1/fHXWR6s2FzBiTjZ9wFw X-Received: by 2002:a05:6358:89d:b0:135:615a:309d with SMTP id m29-20020a056358089d00b00135615a309dmr3852365rwj.7.1691205747747; Fri, 04 Aug 2023 20:22:27 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691205747; cv=none; d=google.com; s=arc-20160816; b=p2AHyCY2pm4hwnWMtqgimB3kMrBTy+DWix12JsRIVo5zT0BbOyoIgxonbwiCVcEFMw mPK60TcVof91jtL04+bpj8sriLMTAQieyJEkv3dNBhOKKtn/NM6mAr5LYq4t/ZgvnoFU 0U557AO4STzsSXe4+KvdHi/gIJEtzkFsfSp7PAzSXo1LPuTQGXBCz/lNSqiEtgiLExTy 1oabFYv0jOlglEgvnApQiIT+53nGWdzSDKDAYuQJuGv+TTIlQcvI+Z4RpcZ5UJ0TkY8j WXbvFXzEA5Omvr9lxH87w2pKhUNvlCj+QfJM6e8cifuctog1tY4CmOWOQ5gto7P4/PW+ IsBA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=RVjH75A/UOEb4U4eXTxJQcqx3CjPe408RWD1yLELWBY=; fh=zpCCF/J8Z0sUUNZfCNdnaRvm94yhXPVxl9PzV22VU6k=; b=ZxmE3vpYTp5aMeLcDHY2ooQjygEQHF930eZSm6tN3wAg9F+4QhkadEpPUFQgaZjweP YI1hRVCSUfJEiVsw8QFWOKgLc6z0+YS4TNCLnPYrx6SDbnXssrwrofVwRbg/Am7lvAq6 c/fsBDoANCLKEe8FERgnqPV5tvLmDbAxAz0Be8gq65OZ8/LSBT2JjzqEEXEMIU8ZPvfK Jgw7GbmrE4MJszsOpF15aQ3/YhoarYHdJdiHpBotd/aRX582+1wjm85abLiVWFyhIx8U 2dUgqmmS7kH+bqHD7JaVFMHPH+sjm2fBzJcrhY36yF8HT3NGTLSbn3x+/GPq/v/XUoTw SWrw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id li14-20020a170903294e00b001bb94eaa311si2480800plb.567.2023.08.04.20.22.14; Fri, 04 Aug 2023 20:22:27 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229668AbjHEDHO (ORCPT <rfc822;liqunnana@gmail.com> + 99 others); Fri, 4 Aug 2023 23:07:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35302 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229534AbjHEDHH (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 4 Aug 2023 23:07:07 -0400 Received: from dggsgout12.his.huawei.com (unknown [45.249.212.56]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 73DAC4EE4 for <linux-kernel@vger.kernel.org>; Fri, 4 Aug 2023 20:07:02 -0700 (PDT) Received: from mail02.huawei.com (unknown [172.30.67.143]) by dggsgout12.his.huawei.com (SkyGuard) with ESMTP id 4RHnYX4D99z4f3lKg for <linux-kernel@vger.kernel.org>; Sat, 5 Aug 2023 11:06:56 +0800 (CST) Received: from huaweicloud.com (unknown [10.175.124.27]) by APP4 (Coremail) with SMTP id gCh0CgAHvrHQvM1k6A5ePg--.23962S5; Sat, 05 Aug 2023 11:06:59 +0800 (CST) From: Kemeng Shi <shikemeng@huaweicloud.com> To: linux-mm@kvack.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, baolin.wang@linux.alibaba.com, mgorman@techsingularity.net, david@redhat.com Cc: shikemeng@huaweicloud.com Subject: [PATCH 3/9] mm/compaction: correctly return failure with bogus compound_order in strict mode Date: Sat, 5 Aug 2023 19:07:05 +0800 Message-Id: <20230805110711.2975149-4-shikemeng@huaweicloud.com> X-Mailer: git-send-email 2.30.0 In-Reply-To: <20230805110711.2975149-1-shikemeng@huaweicloud.com> References: <20230805110711.2975149-1-shikemeng@huaweicloud.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-CM-TRANSID: gCh0CgAHvrHQvM1k6A5ePg--.23962S5 X-Coremail-Antispam: 1UD129KBjvJXoWxWF18CFWfXr13ArWrXF47Arb_yoW5Ar4DpF 9rK3WxAF1Dua4YqF9rXr4kZ3W29ws5Ka17J3y3KF18ZF9Yya4xG3sFyr1jvFy0qr9xArZ0 vr4DtFWxC3WDXa7anT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDU0xBIdaVrnRJUUUBE14x267AKxVWrJVCq3wAFc2x0x2IEx4CE42xK8VAvwI8IcIk0 rVWrJVCq3wAFIxvE14AKwVWUJVWUGwA2jI8I6cxK62vIxIIY0VWUZVW8XwA2048vs2IY02 0E87I2jVAFwI0_JrWl82xGYIkIc2x26xkF7I0E14v26ryj6s0DM28lY4IEw2IIxxk0rwA2 F7IY1VAKz4vEj48ve4kI8wA2z4x0Y4vE2Ix0cI8IcVAFwI0_tr0E3s1l84ACjcxK6xIIjx v20xvEc7CjxVAFwI0_Gr1j6F4UJwA2z4x0Y4vEx4A2jsIE14v26rxl6s0DM28EF7xvwVC2 z280aVCY1x0267AKxVW0oVCq3wAS0I0E0xvYzxvE52x082IY62kv0487Mc02F40EFcxC0V AKzVAqx4xG6I80ewAv7VC0I7IYx2IY67AKxVWUJVWUGwAv7VC2z280aVAFwI0_Jr0_Gr1l Ox8S6xCaFVCjc4AY6r1j6r4UM4x0Y48IcxkI7VAKI48JM4x0x7Aq67IIx4CEVc8vx2IErc IFxwCF04k20xvY0x0EwIxGrwCFx2IqxVCFs4IE7xkEbVWUJVW8JwC20s026c02F40E14v2 6r1j6r18MI8I3I0E7480Y4vE14v26r106r1rMI8E67AF67kF1VAFwI0_JF0_Jw1lIxkGc2 Ij64vIr41lIxAIcVC0I7IYx2IY67AKxVWUJVWUCwCI42IY6xIIjxv20xvEc7CjxVAFwI0_ Gr0_Cr1lIxAIcVCF04k26cxKx2IYs7xG6r1j6r1xMIIF0xvEx4A2jsIE14v26r1j6r4UMI IF0xvEx4A2jsIEc7CjxVAFwI0_Gr0_Gr1UYxBIdaVFxhVjvjDU0xZFpf9x0pRPEf5UUUUU = X-CM-SenderInfo: 5vklyvpphqwq5kxd4v5lfo033gof0z/ X-CFilter-Loop: Reflected X-Spam-Status: No, score=0.1 required=5.0 tests=BAYES_00,DATE_IN_FUTURE_06_12, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773357757849916824 X-GMAIL-MSGID: 1773357757849916824 |
Series |
Fixes and cleanups to compaction
|
|
Commit Message
Kemeng Shi
Aug. 5, 2023, 11:07 a.m. UTC
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.
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.
Fixes: 9fcd6d2e052ee ("mm, compaction: skip compound pages by order in free scanner")
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
mm/compaction.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
Comments
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 <shikemeng@huaweicloud.com> > --- > 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); >
on 8/15/2023 4:28 PM, Baolin Wang wrote: > > > 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 <shikemeng@huaweicloud.com> >> --- >> 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. > Thanks for feedback! Sure, I will do this in next version. >> + >> 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); >> >
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; + 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);