Message ID | 20231206110054.61617-1-v-songbaohua@oppo.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp4026751vqy; Wed, 6 Dec 2023 03:04:00 -0800 (PST) X-Google-Smtp-Source: AGHT+IE1NzKpdJm3izn/RGsF0wCoS6fsJsVXyyvPFjA2rJd76xwvJctJykeUarNrjZXF2edirhq6 X-Received: by 2002:a05:6a00:1817:b0:6bd:b7c5:f776 with SMTP id y23-20020a056a00181700b006bdb7c5f776mr493389pfa.8.1701860640691; Wed, 06 Dec 2023 03:04:00 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701860640; cv=none; d=google.com; s=arc-20160816; b=ZBTCMgK6LhU08S/Gk/tQ920hb0mCc2sxzTDx3aRcspLG0rYh1IoWj3nzguF7w1WZII SXuwJkHCQ54pGYlo8TgDzTF3VeBkoBkB0XKMQniOG5Gh5BbsTwYweA5DuAKywh8WzBBi mF+M+Q73nPg9nLwggBq0X2D2cU3OV3TUKN3f5r1jm+nGTjmu1EK6HGrO39Xpjwe/sQtU 6LByHQHI5uDNJg+9TQ+QvTrB0dOBBuoO1LDYTLqoSzlHEcX9ErRIPnQFmo5vtoB3D38g mYh5DHzG6Y9YCZgov0FFd9nN4Fzlg62ReCV/vLzJF9tR4NvHsXSkmDgcL8xaItCCM2CU 9axg== 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 :message-id:date:subject:cc:to:from:dkim-signature; bh=1tmT8glKoA1mZRisM7CPsvj8rp8Fj5zx5XkNMnqVUSc=; fh=9t1x+0O0bYjWEWTkheyob2MwebMmr8ZYCcmWw2h6yXQ=; b=a6i/+hQM8sTkJ6LVQaoYmzntXRCyoUqg4RrDOxjIJ9041GVW35a8F/ustYm0dnthJB lToccRUFe3Sy74uGk600WtP7j3W3xA/zYbVJ8eERCEColKqaXL/sWShun6TLvl3jtm8R hKyMi29aOfDBAfzvleyJmIfCkhYR1VfQskKlh5wi6bSzqk+Uw3VMEaJYhpX6q1GkPebA 6VzahRZHW1caghi3YZMg4k9A6tWB/hziYfBHUmiYTrpORKhapdxV/u4mPLPrIeovrHJk h/qk4/AriTVJJbAg0Ida9sdTIQugrAwrAxzULbUK8kimvBU0iAP9rp39vxX0SHBZ+xzb zAew== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=Ej79UEC5; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from morse.vger.email (morse.vger.email. [2620:137:e000::3:1]) by mx.google.com with ESMTPS id dr2-20020a056a004a8200b006cbef05beddsi11053081pfb.11.2023.12.06.03.03.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Dec 2023 03:04:00 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=Ej79UEC5; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by morse.vger.email (Postfix) with ESMTP id AF1D980A9E0D; Wed, 6 Dec 2023 03:03:47 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at morse.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1378115AbjLFLD2 (ORCPT <rfc822;pusanteemu@gmail.com> + 99 others); Wed, 6 Dec 2023 06:03:28 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49910 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1378022AbjLFLDK (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 6 Dec 2023 06:03:10 -0500 Received: from mail-pj1-x1035.google.com (mail-pj1-x1035.google.com [IPv6:2607:f8b0:4864:20::1035]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 61073384E for <linux-kernel@vger.kernel.org>; Wed, 6 Dec 2023 03:01:49 -0800 (PST) Received: by mail-pj1-x1035.google.com with SMTP id 98e67ed59e1d1-285d1101868so4448031a91.0 for <linux-kernel@vger.kernel.org>; Wed, 06 Dec 2023 03:01:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701860474; x=1702465274; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=1tmT8glKoA1mZRisM7CPsvj8rp8Fj5zx5XkNMnqVUSc=; b=Ej79UEC5r6V/O8zOKvyMXFR8manJkyM6bHwRTOfUfpCbk6LmIBIGlajYvLtmAVqDk+ fbjIDtxty1TGPktNTVaf3vD50a8a/BKJq9UgxiNukwLjhSK3hL+N/7WKHu2/5NuFt9p2 4JFXSq5qFlomRSogXlYxhomJW08+U6WYioF2f9Ca61plVgPTTBkDG9/VggNARyz6I2Ku Ye0y5tUk7ea3mkglMH6xwtU9pncP/+Ke8XwVDpVfQ0WJNS9ujocuTaC/NSLKR+pWI5rd YbEiZ6gnBOSibtqbtuT8i6YJbCCmRBhuhBeq6l6J/qOYVCEKJbKt8DMqfzYlc4/mMKkX hCZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701860474; x=1702465274; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=1tmT8glKoA1mZRisM7CPsvj8rp8Fj5zx5XkNMnqVUSc=; b=aCyQ5fgYTibtrHHegEPoF6ZFIIZ+2b8i1Ma1JDvuuE0yZuAx8IfPmmKQM7b0ThYpq2 MJ0g5Nn2rwMv56iPbh6ugdZJyjKcmWUGebdvZaod2xmPWyLB3icLx0DIlcuZuY2P1ktb hgRJvunrosOJpvqI2W4czn/b0zlPQEzAGCbtkm5OOPC1PjlJu7nzyP6yBfZ4DcASkI7j ysfdKAohYgFjziY9Fes9XPh1QVyqiM0G56xLyNPog8V3zocBiNaBxdTXolnjFsKlPYws P50KcpTQGvdBHD/qGN1V+iX48AdsdKhHx2Y9PR82BvapI1LKUzWpDehq67ZjzonnOLzj OfiA== X-Gm-Message-State: AOJu0YxMVsQD1U+4S2HnWf4HD/thSj04PS17DAXSP2pj4VY5/9Bclzaw fS58Lw9zm9XT9N5zd1Ea+/Y= X-Received: by 2002:a17:90a:e7c2:b0:286:e2bb:26a4 with SMTP id kb2-20020a17090ae7c200b00286e2bb26a4mr426280pjb.40.1701860474132; Wed, 06 Dec 2023 03:01:14 -0800 (PST) Received: from barry-desktop.hub ([2407:7000:8942:5500:c02a:270e:39c4:ae78]) by smtp.gmail.com with ESMTPSA id r17-20020a17090a941100b00263f41a655esm10890158pjo.43.2023.12.06.03.01.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 06 Dec 2023 03:01:13 -0800 (PST) From: Barry Song <21cnbao@gmail.com> X-Google-Original-From: Barry Song <v-songbaohua@oppo.com> To: akpm@linux-foundation.org, baolin.wang@linux.alibaba.com, linux-mm@kvack.org Cc: david@redhat.com, hannes@cmpxchg.org, huzhanyuan@oppo.com, linux-kernel@vger.kernel.org, mgorman@techsingularity.net, shikemeng@huaweicloud.com, v-songbaohua@oppo.com, willy@infradead.org Subject: [PATCH] mm: compaction: avoid fast_isolate_freepages blindly choose improper pageblock Date: Thu, 7 Dec 2023 00:00:54 +1300 Message-Id: <20231206110054.61617-1-v-songbaohua@oppo.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-0.6 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Wed, 06 Dec 2023 03:03:47 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783894952369681351 X-GMAIL-MSGID: 1784530223218560509 |
Series |
mm: compaction: avoid fast_isolate_freepages blindly choose improper pageblock
|
|
Commit Message
Barry Song
Dec. 6, 2023, 11 a.m. UTC
Testing shows fast_isolate_freepages can blindly choose an unsuitable
pageblock from time to time particularly while the min mark is used
from XXX path:
if (!page) {
cc->fast_search_fail++;
if (scan_start) {
/*
* Use the highest PFN found above min. If one was
* not found, be pessimistic for direct compaction
* and use the min mark.
*/
if (highest >= min_pfn) {
page = pfn_to_page(highest);
cc->free_pfn = highest;
} else {
if (cc->direct_compaction && pfn_valid(min_pfn)) { /* XXX */
page = pageblock_pfn_to_page(min_pfn,
min(pageblock_end_pfn(min_pfn),
zone_end_pfn(cc->zone)),
cc->zone);
cc->free_pfn = min_pfn;
}
}
}
}
The reason is that no code is doing any check on the min_pfn
min_pfn = pageblock_start_pfn(cc->free_pfn - (distance >> 1));
In contrast, slow path of isolate_freepages() is always skipping unsuitable
pageblocks in a decent way.
This issue doesn't happen quite often. When running 25 machines with 16GiB
memory for one night, most of them can hit this unexpected code path.
However the frequency isn't like many times per second. It might be one
time in a couple of hours. Thus, it is very hard to measure the visible
performance impact in my machines though the affection of choosing the
unsuitable migration_target should be negative in theory.
I feel it's still worth fixing this to at least make the code theoretically
self-explanatory as it is quite odd an unsuitable migration_target can be
still migration_target.
Reported-by: Zhanyuan Hu <huzhanyuan@oppo.com>
Signed-off-by: Barry Song <v-songbaohua@oppo.com>
---
v1:
move the fix to the specific min_pfn path with respect to Baolin's comment
rfc:
https://lore.kernel.org/linux-mm/20231129104530.63787-1-v-songbaohua@oppo.com/#t
mm/compaction.c | 3 +++
1 file changed, 3 insertions(+)
Comments
On 12/6/2023 7:00 PM, Barry Song wrote: > Testing shows fast_isolate_freepages can blindly choose an unsuitable > pageblock from time to time particularly while the min mark is used > from XXX path: > if (!page) { > cc->fast_search_fail++; > if (scan_start) { > /* > * Use the highest PFN found above min. If one was > * not found, be pessimistic for direct compaction > * and use the min mark. > */ > if (highest >= min_pfn) { > page = pfn_to_page(highest); > cc->free_pfn = highest; > } else { > if (cc->direct_compaction && pfn_valid(min_pfn)) { /* XXX */ > page = pageblock_pfn_to_page(min_pfn, > min(pageblock_end_pfn(min_pfn), > zone_end_pfn(cc->zone)), > cc->zone); > cc->free_pfn = min_pfn; > } > } > } > } > > The reason is that no code is doing any check on the min_pfn > min_pfn = pageblock_start_pfn(cc->free_pfn - (distance >> 1)); > > In contrast, slow path of isolate_freepages() is always skipping unsuitable > pageblocks in a decent way. > > This issue doesn't happen quite often. When running 25 machines with 16GiB > memory for one night, most of them can hit this unexpected code path. > However the frequency isn't like many times per second. It might be one > time in a couple of hours. Thus, it is very hard to measure the visible > performance impact in my machines though the affection of choosing the > unsuitable migration_target should be negative in theory. > > I feel it's still worth fixing this to at least make the code theoretically > self-explanatory as it is quite odd an unsuitable migration_target can be > still migration_target. > > Reported-by: Zhanyuan Hu <huzhanyuan@oppo.com> > Signed-off-by: Barry Song <v-songbaohua@oppo.com> LGTM. Thanks. Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com> > --- > v1: > move the fix to the specific min_pfn path with respect to Baolin's comment > rfc: > https://lore.kernel.org/linux-mm/20231129104530.63787-1-v-songbaohua@oppo.com/#t > > mm/compaction.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mm/compaction.c b/mm/compaction.c > index 01ba298739dd..de15a2ef0af5 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -1611,6 +1611,9 @@ static void fast_isolate_freepages(struct compact_control *cc) > min(pageblock_end_pfn(min_pfn), > zone_end_pfn(cc->zone)), > cc->zone); > + if (page && !suitable_migration_target(cc, page)) > + page = NULL; > + > cc->free_pfn = min_pfn; > } > }
On Thu, Dec 07, 2023 at 12:00:54AM +1300, Barry Song wrote: > Testing shows fast_isolate_freepages can blindly choose an unsuitable > pageblock from time to time particularly while the min mark is used > from XXX path: > if (!page) { > cc->fast_search_fail++; > if (scan_start) { > /* > * Use the highest PFN found above min. If one was > * not found, be pessimistic for direct compaction > * and use the min mark. > */ > if (highest >= min_pfn) { > page = pfn_to_page(highest); > cc->free_pfn = highest; > } else { > if (cc->direct_compaction && pfn_valid(min_pfn)) { /* XXX */ > page = pageblock_pfn_to_page(min_pfn, > min(pageblock_end_pfn(min_pfn), > zone_end_pfn(cc->zone)), > cc->zone); > cc->free_pfn = min_pfn; > } > } > } > } > > The reason is that no code is doing any check on the min_pfn > min_pfn = pageblock_start_pfn(cc->free_pfn - (distance >> 1)); > > In contrast, slow path of isolate_freepages() is always skipping unsuitable > pageblocks in a decent way. > > This issue doesn't happen quite often. When running 25 machines with 16GiB > memory for one night, most of them can hit this unexpected code path. > However the frequency isn't like many times per second. It might be one > time in a couple of hours. Thus, it is very hard to measure the visible > performance impact in my machines though the affection of choosing the > unsuitable migration_target should be negative in theory. > > I feel it's still worth fixing this to at least make the code theoretically > self-explanatory as it is quite odd an unsuitable migration_target can be > still migration_target. > > Reported-by: Zhanyuan Hu <huzhanyuan@oppo.com> > Signed-off-by: Barry Song <v-songbaohua@oppo.com> Acked-by: Mel Gorman <mgorman@techsingularity.net>
On Wed, Dec 13, 2023 at 9:02 PM Mel Gorman <mgorman@techsingularity.net> wrote: > > On Thu, Dec 07, 2023 at 12:00:54AM +1300, Barry Song wrote: > > Testing shows fast_isolate_freepages can blindly choose an unsuitable > > pageblock from time to time particularly while the min mark is used > > from XXX path: > > if (!page) { > > cc->fast_search_fail++; > > if (scan_start) { > > /* > > * Use the highest PFN found above min. If one was > > * not found, be pessimistic for direct compaction > > * and use the min mark. > > */ > > if (highest >= min_pfn) { > > page = pfn_to_page(highest); > > cc->free_pfn = highest; > > } else { > > if (cc->direct_compaction && pfn_valid(min_pfn)) { /* XXX */ > > page = pageblock_pfn_to_page(min_pfn, > > min(pageblock_end_pfn(min_pfn), > > zone_end_pfn(cc->zone)), > > cc->zone); > > cc->free_pfn = min_pfn; > > } > > } > > } > > } > > > > The reason is that no code is doing any check on the min_pfn > > min_pfn = pageblock_start_pfn(cc->free_pfn - (distance >> 1)); > > > > In contrast, slow path of isolate_freepages() is always skipping unsuitable > > pageblocks in a decent way. > > > > This issue doesn't happen quite often. When running 25 machines with 16GiB > > memory for one night, most of them can hit this unexpected code path. > > However the frequency isn't like many times per second. It might be one > > time in a couple of hours. Thus, it is very hard to measure the visible > > performance impact in my machines though the affection of choosing the > > unsuitable migration_target should be negative in theory. > > > > I feel it's still worth fixing this to at least make the code theoretically > > self-explanatory as it is quite odd an unsuitable migration_target can be > > still migration_target. > > > > Reported-by: Zhanyuan Hu <huzhanyuan@oppo.com> > > Signed-off-by: Barry Song <v-songbaohua@oppo.com> > > Acked-by: Mel Gorman <mgorman@techsingularity.net> Hi Mel, Thanks! Hi Andrew, Given this patch has been in mm-stable, https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/commit/?h=mm-stable&id=d19b1a1797 does it still have a chance to collect Mel's tag? > > -- > Mel Gorman > SUSE Labs Thanks Barry
On Thu, 14 Dec 2023 09:34:35 +0800 Barry Song <21cnbao@gmail.com> wrote: > > > Reported-by: Zhanyuan Hu <huzhanyuan@oppo.com> > > > Signed-off-by: Barry Song <v-songbaohua@oppo.com> > > > > Acked-by: Mel Gorman <mgorman@techsingularity.net> > > Hi Mel, > Thanks! > > Hi Andrew, > Given this patch has been in mm-stable, > https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/commit/?h=mm-stable&id=d19b1a1797 > does it still have a chance to collect Mel's tag? That would require a rebuild of mm-stable, which is also a rebase. I don't think I've had to do that before - I'll occasionally rebase, but just to drop things - not a full rebuild from the quilt patches. If I end up having to do that then I'll somehow try to remember to make this change.
diff --git a/mm/compaction.c b/mm/compaction.c index 01ba298739dd..de15a2ef0af5 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -1611,6 +1611,9 @@ static void fast_isolate_freepages(struct compact_control *cc) min(pageblock_end_pfn(min_pfn), zone_end_pfn(cc->zone)), cc->zone); + if (page && !suitable_migration_target(cc, page)) + page = NULL; + cc->free_pfn = min_pfn; } }