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 31CEBC0015E for ; Tue, 15 Aug 2023 09:22:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 716088E000A; Tue, 15 Aug 2023 05:22:16 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 69FD18D0001; Tue, 15 Aug 2023 05:22:16 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 53F658E000A; Tue, 15 Aug 2023 05:22:16 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 3D23D8D0001 for ; Tue, 15 Aug 2023 05:22:16 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 0E882A0856 for ; Tue, 15 Aug 2023 09:22:16 +0000 (UTC) X-FDA: 81125797872.07.228E986 Received: from dggsgout11.his.huawei.com (unknown [45.249.212.51]) by imf29.hostedemail.com (Postfix) with ESMTP id 9961F120017 for ; Tue, 15 Aug 2023 09:22:11 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=none; dmarc=none; spf=none (imf29.hostedemail.com: domain of shikemeng@huaweicloud.com has no SPF policy when checking 45.249.212.51) smtp.mailfrom=shikemeng@huaweicloud.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1692091333; 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=wcrDmTslqbBxCWAdA8VjKteYRY47qgbFgQvIjoj4czk=; b=xPy+p25a7dJ1eXUx7d2PUbjde5ZzWWvg07KYKOwWs1HCEPYNTw+0FfzO954K+b0AQB0c60 epSxHq+wTltyANcRyXfJ+oxHZ7DlVlF7xGvMtBHSmA1/JZHULJKjoDtjnyzTzYmPuZluRC qJo6370+rst29m4LawV63heEipqvOPw= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=none; dmarc=none; spf=none (imf29.hostedemail.com: domain of shikemeng@huaweicloud.com has no SPF policy when checking 45.249.212.51) smtp.mailfrom=shikemeng@huaweicloud.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1692091333; a=rsa-sha256; cv=none; b=CFHwAcp4+4bS3nv7E9OZVUl1Qo2Xr4IlvWxI8/k9LAQ/VlhF0lOkoFXTLMXj1ZDXBq/wEy Uozj1+07ItmecnxfIKHgLbgttKb1RnrUGCAsHGgqU5o3kAnqQVPjHC6w0xb095sHdexPHa djeOlyefLx4UE1Im5FLaCrCs51rQnSk= Received: from mail02.huawei.com (unknown [172.30.67.169]) by dggsgout11.his.huawei.com (SkyGuard) with ESMTP id 4RQ5Pj6vVGz4f3jqy for ; Tue, 15 Aug 2023 17:22:01 +0800 (CST) Received: from [10.174.178.129] (unknown [10.174.178.129]) by APP3 (Coremail) with SMTP id _Ch0CgA3DcW5Q9tkeCneAg--.10131S2; Tue, 15 Aug 2023 17:22:02 +0800 (CST) Subject: Re: [PATCH 3/9] mm/compaction: correctly return failure with bogus compound_order in strict mode To: Baolin Wang , 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: Kemeng Shi Message-ID: Date: Tue, 15 Aug 2023 17:22:00 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-CM-TRANSID:_Ch0CgA3DcW5Q9tkeCneAg--.10131S2 X-Coremail-Antispam: 1UD129KBjvJXoWxCF4xXrW8ArWkGr1fWw13twb_yoWrtF15pF yktFy7JryUCw1FqF17Jr1DAryUCw48t3WUJr48XFyUArn8Jr12qr1jvr1j9ryUXr4xAr1j qr4UtF9rZF17Xa7anT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDU0xBIdaVrnRJUUUyEb4IE77IF4wAFF20E14v26r1j6r4UM7CY07I20VC2zVCF04k2 6cxKx2IYs7xG6rWj6s0DM7CIcVAFz4kK6r1j6r18M28lY4IEw2IIxxk0rwA2F7IY1VAKz4 vEj48ve4kI8wA2z4x0Y4vE2Ix0cI8IcVAFwI0_tr0E3s1l84ACjcxK6xIIjxv20xvEc7Cj xVAFwI0_Gr1j6F4UJwA2z4x0Y4vEx4A2jsIE14v26rxl6s0DM28EF7xvwVC2z280aVCY1x 0267AKxVW0oVCq3wAS0I0E0xvYzxvE52x082IY62kv0487Mc02F40EFcxC0VAKzVAqx4xG 6I80ewAv7VC0I7IYx2IY67AKxVWUJVWUGwAv7VC2z280aVAFwI0_Jr0_Gr1lOx8S6xCaFV Cjc4AY6r1j6r4UM4x0Y48IcVAKI48JMxk0xIA0c2IEe2xFo4CEbIxvr21l42xK82IYc2Ij 64vIr41l4I8I3I0E4IkC6x0Yz7v_Jr0_Gr1lx2IqxVAqx4xG67AKxVWUJVWUGwC20s026x 8GjcxK67AKxVWUGVWUWwC2zVAF1VAY17CE14v26r126r1DMIIYrxkI7VAKI48JMIIF0xvE 2Ix0cI8IcVAFwI0_Jr0_JF4lIxAIcVC0I7IYx2IY6xkF7I0E14v26r1j6r4UMIIF0xvE42 xK8VAvwI8IcIk0rVWrZr1j6s0DMIIF0xvEx4A2jsIE14v26r1j6r4UMIIF0xvEx4A2jsIE c7CjxVAFwI0_Jr0_GrUvcSsGvfC2KfnxnUUI43ZEXa7IU1CPfJUUUUU== X-CM-SenderInfo: 5vklyvpphqwq5kxd4v5lfo033gof0z/ X-CFilter-Loop: Reflected X-Rspamd-Queue-Id: 9961F120017 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: me56gq8x19p448nyy5imhznicebsnbqi X-HE-Tag: 1692091331-595210 X-HE-Meta: U2FsdGVkX1+Di378JQ5a7cGzCUI5PLbKS59rT9SIUs/ZpQyEY05/66vG4garqKee5ZbETqJYeDky4u6rH102OvaAF6M0drH0HCufTSCc+YM5mp35PFSAZ49stkXb4XGTic5STArDAqIMYY9bMgsMqLN/ou6TGeLUfK8jjKpALJSJdA8zXPbKV9GsHy+/1ezn7VsNtpp8emvGo7t9fWIYoaDLHPnA5OxGzL52RVSZzULCugPGIb83/anR4UXhiT5RQBzcTNz4e+xzz9RWyV0/E/nmxNkXcz7I2pw8ONKKINy40NsQTkwGAqCCydqVGj5s7itbN0sCm/v+fvMled8i16HInzbPeLFu5qlYyVlgSaqGntvJY4WvibFfsr8S5MYBZzx1cHgxPiLI4CWLZfVDi5TY40Lx6a11vOAWEHy1pYasApglX8fSV7iDWbtwZ4+Xm7NbDj4n8thDliEUOk/dQBFcmlCeXDKoZzqQ9/Z8GiVKs55BogO2aKtq4poYIDkWPdkkrUvvg/7jhdK8PA3mZj9TbBeHXtc1XrhGjqZfywFAJLxZIxWF28Tl5sFi1FD0TdtwoF5bjPlifiR+xq5Ji650WS7UNufwRpFY9w3K0ST/KsumBRjD1StGFbVfUNP1W5WotaBxgBD2yJJ7R8acoZ2X4rKFiJafyqREqTHe90qVmS8TdHn265y/QrssVUQrqZoag0L/juhDt2kGaszgbGPN3/T1Eiu1bsV56kQrNhdi3Gt8XHTQZp4ZGpEidOBilDggqMut7ROrDFuPKQolxSE4w/0kId8A0/qWJCZTzXtA81Y5WThf91a9DVzvBBEOP1os0RbW0t5R2lA47QYoZV/KngaAHu6qYqVlQ/MOXvjS3ZC0yDLCWuSXWuWPB5qExP2ZvDf1c+BxmHSGvq4N85H6FXIT9xv6SCyxrDt4JJu0bXo+pZTNqF+TEnV5fPq/IhX9yIgmuP4Q1vEcoia ExSdcWhl g3mSnkn5cKcDvmvKq1BhAyioSsqAm2oInSeieXfmraUuxxJ/ScBNF1p/D32mg5iVOBEyr+kjdTjeDiOAGr9hni7FeZ6nvW3obR+JphPmn/PuHBCzZhbnyFzWnxpGkqQyEi/opovZ319pp1ENaa5NBbnyyACrY0M0KFeHR0TygOGGwpF8utTSn1k9UFFO9ViK468gO4XI5KJk9qwekaHhrHuwDc/uC+7BiO8eouQY//7766pLKoB0WNedFvou/49ifGGot0nEZN4TWVmF7PiSK+NInMNwSyoC62ckK 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/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 >> --- >>   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); >>   >