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 5B0F7C433EF for ; Fri, 15 Jul 2022 15:26:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B8CDD9401FD; Fri, 15 Jul 2022 11:26:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B3EA09401FB; Fri, 15 Jul 2022 11:26:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A03F69401FD; Fri, 15 Jul 2022 11:26:10 -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 8EE439401FB for ; Fri, 15 Jul 2022 11:26:10 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 6258B358FD for ; Fri, 15 Jul 2022 15:26:10 +0000 (UTC) X-FDA: 79689710100.29.2D75323 Received: from mail-pj1-f47.google.com (mail-pj1-f47.google.com [209.85.216.47]) by imf24.hostedemail.com (Postfix) with ESMTP id 079BD1800AB for ; Fri, 15 Jul 2022 15:26:07 +0000 (UTC) Received: by mail-pj1-f47.google.com with SMTP id b8so5916682pjo.5 for ; Fri, 15 Jul 2022 08:26:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20210112.gappssmtp.com; s=20210112; h=message-id:date:mime-version:user-agent:from:subject:to:cc :references:in-reply-to:content-transfer-encoding; bh=Cly3uiirEJxaVmNgo6rPXHXvSk+U8xZrBUijQJe8Q7c=; b=H3jnQZy2DpPkrVg5O/BIQ7sk6CwdYoq+ksn5eEXZzZfZfgYWsjfcX7pLqYKoz9vtmN Owtj4Q/FO2h/4X96VPPpmaJSe6tp8ShRJr+bzgpStAbhBOYdLrRPBbvnR5DG820ENVLT 3U/fE2csDgc7zxJhFLcj2F3XVediPB5b9t/LKrIkEGU5drZApCCZ+5X2St+vP6AHAJN+ Jiz9cjFv4o2xte90/MsH+Oqzsa44Ig9jHFDQ/VaQxwt8MDzpYhbhe7DoMmOoODk+ER+z T0GZVHXu9s/W47TOtMl5Q6d/MDwcB8brJzdOldq02MSQO7wh4PBFrZUJbwDLSSXAcKYV Mo9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:from :subject:to:cc:references:in-reply-to:content-transfer-encoding; bh=Cly3uiirEJxaVmNgo6rPXHXvSk+U8xZrBUijQJe8Q7c=; b=G9OyQX3062jc/MN+IPjKbVd1NZRvtlwOl5OODgy5XRujkNqGGEwl/Z2OyjBA+0TI9o u7acOkPHJGID9s62dajn6IgW2jtQaLJ3vD6Z3hpnxRYWwkXbJQCIz2SoIOjrd22zCGwp MqmyXMfBbBaWk0GYnUNCvCRoXoovSx5oQ0/W0L2+p9y3+lcRDtSBWacz6ZfX+CHFGZh0 vfMvCXO5gTYjXtLoMV4awJjpnDDf3/EaUDJZgweGveOnmidaAk0ONaBNWgvPW4fG9NFc 3PTo343NV8e4JhtUZCAiHHnrs7qmVandi/JQXDl/dAQsYfNyGnPujJVN3U2tpF1HpoCi 6agQ== X-Gm-Message-State: AJIora9fRnuv2tk6JrHWhFUqXrC3x/BH2y1wAhUlkFHtFSeEwCGG72Ny mAe98z9PGyiln4uYgy+0eIdZmg== X-Google-Smtp-Source: AGRyM1vrPBVFwycOacJJ7UPinf22aXK8PkguBDyl2+9q8k68zY8QhRdIEQuytmJU/vrvqO7ckHjE/Q== X-Received: by 2002:a17:902:744c:b0:16b:dacb:5724 with SMTP id e12-20020a170902744c00b0016bdacb5724mr14079637plt.57.1657898766639; Fri, 15 Jul 2022 08:26:06 -0700 (PDT) Received: from [10.254.50.189] ([139.177.225.234]) by smtp.gmail.com with ESMTPSA id j6-20020a634a46000000b0040d22243295sm3338758pgl.79.2022.07.15.08.26.04 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 15 Jul 2022 08:26:06 -0700 (PDT) Message-ID: <3529e159-8c22-5d67-5e5d-c912784df9a0@bytedance.com> Date: Fri, 15 Jul 2022 23:26:01 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 From: Chuyi Zhou Subject: Re: [PATCH v3] mm/compaction: fix set skip in fast_find_migrateblock To: Mel Gorman , Andrew Morton Cc: linux-mm@kvack.org References: <20220713062009.597255-1-zhouchuyi@bytedance.com> <20220713082814.bed234e00d7f5ceb3858352a@linux-foundation.org> <20220714115020.GA3563@techsingularity.net> In-Reply-To: <20220714115020.GA3563@techsingularity.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=bytedance-com.20210112.gappssmtp.com header.s=20210112 header.b=H3jnQZy2; spf=pass (imf24.hostedemail.com: domain of zhouchuyi@bytedance.com designates 209.85.216.47 as permitted sender) smtp.mailfrom=zhouchuyi@bytedance.com; dmarc=pass (policy=none) header.from=bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1657898770; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Cly3uiirEJxaVmNgo6rPXHXvSk+U8xZrBUijQJe8Q7c=; b=zaE268y+yV3AsBTdMDkzGH991WuKjJrJV0Ada+LH6bPeu0y+XyvBF6Imr8rgImmnsMor1T Q/+BwgE8F+fdQXlqVE3YrzFk5UuuMY6EhMk3tiqiJte7+fJOFaD0D8aqoOBM7/LuSBbEEo s/eayeTiL9pzu/ZALjPVYIZAjFBejv4= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1657898770; a=rsa-sha256; cv=none; b=l5t/Z9dki0YQaE+2d+FGczhoACoMj+G4yLpABUYxCHBrgjgKL4gj57y9zoP9j/M6tazX/Q rTfZQU8xqQVtUEokBMdT2O9Sj19ILYcBFbCePsdm1OVlMyBv80xtzAoIVYtjBffM2b4KA6 2rZGup7bF+35tr2w27nAvyokNctwa3U= X-Stat-Signature: 5f7rx934nz1mihbiofqm5k1h74oxyo7x X-Rspamd-Queue-Id: 079BD1800AB X-Rspamd-Server: rspam07 Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=bytedance-com.20210112.gappssmtp.com header.s=20210112 header.b=H3jnQZy2; spf=pass (imf24.hostedemail.com: domain of zhouchuyi@bytedance.com designates 209.85.216.47 as permitted sender) smtp.mailfrom=zhouchuyi@bytedance.com; dmarc=pass (policy=none) header.from=bytedance.com X-Rspam-User: X-HE-Tag: 1657898767-223429 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: 在 2022/7/14 下午7:50, Mel Gorman 写道: > On Wed, Jul 13, 2022 at 08:28:14AM -0700, Andrew Morton wrote: >> (cc Mel) >> >> On Wed, 13 Jul 2022 14:20:09 +0800 Chuyi Zhou wrote: >> >>> From: zhouchuyi >>> >>> When we successfully find a pageblock in fast_find_migrateblock(), >>> the block will be set skip-flag through set_pageblock_skip(). However, >>> when entering isolate_migratepages_block(), the whole pageblock will >>> be skipped due to the branch >>> 'if (!valid_page && IS_ALIGNED(low_pfn, pageblock_nr_pages))'. >>> Eventually we will goto isolate_abort and isolate nothing. That cause >>> fast_find_migrateblock useless. >>> > > It's not very clear *why* this failed from the changelog because > superficially !valid_page will be true for the first pageblock and there > is a reasonable expectation it will be aligned. Is the following accurate > based on your analysis? > > However, when entering isolate_migratepages_block(), the first > pageblock will be skipped in the branch 'if (!valid_page && > IS_ALIGNED(low_pfn, pageblock_nr_pages))' as isolation_suitable > returns true due to the skip bit set by fast_find_migrateblock(). > > If so, please update the changelog as a reviewer handling backports may > wonder what exactly is wrong with that branch. > Hi Mel, thanks for your review. If fast scanning failed, the return block may not be aligned, because we get pfn from *free_pfn*. When fast-find success, the return value *pfn* is get from pageblock_start_pfn, and it will be passed to isolate_migratepages_block as low_pfn. I think normally the value get from pageblock_start_pfn should be aligned with pageblock_nr_pages. I have used printk test it. Maby I miss something important? pfn = pageblock_start_pfn(free_pfn); ... found_block = true; set_pageblock_skip(freepage); break; > Second, what guarantees a block returned by fast_find that is not > aligned gets marked skipped after it is scanned? The set_pageblock_skip > is only called when there is a valid page and it may not be set if > !IS_ALIGNED(low_pfn). Is something like this untested hunk also necessary? > You are right, we do need some machenism to ensure mark skipped after scanned a !IS_ALIGNED block from fast_find. However, I think the following code may not working. Because *skip_updated* has been reset: if (!skip_updated) { skip_updated = true; if (test_and_set_skip(cc, page, low_pfn)) goto isolate_abort; } Why not ignore skip_updated after scanned a block, just like this: diff --git a/mm/compaction.c b/mm/compaction.c index 962d05d1e187..1c388c45f127 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -1180,7 +1180,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, * rescanned twice in a row. */ if (low_pfn == end_pfn && (!nr_isolated || cc->rescan)) { - if (valid_page && !skip_updated) + if (valid_page) set_pageblock_skip(valid_page); update_cached_migrate(cc, low_pfn); } > diff --git a/mm/compaction.c b/mm/compaction.c > index 1f89b969c12b..112346b2f716 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -888,8 +888,9 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, > * COMPACT_CLUSTER_MAX at a time so the second call must > * not falsely conclude that the block should be skipped. > */ > - if (!valid_page && IS_ALIGNED(low_pfn, pageblock_nr_pages)) { > - if (!isolation_suitable(cc, page)) { > + if (!valid_page) { > + if (IS_ALIGNED(low_pfn, pageblock_nr_pages) && > + !isolation_suitable(cc, page)) { > low_pfn = end_pfn; > page = NULL; > goto isolate_abort; > >>> In this Patch, when we find a suitable pageblock in fast_find_ >>> migrateblock, we do noting but let isolate_migratepages_block >>> to set skip flag to the pageblock after scan it. Normally, >>> we would isolate some pages from the fast-find block. >>> >>> I use mmtest/thpscale-madvhugepage test it. Here is the result: >>> baseline patch >>> Amean fault-both-1 1331.66 ( 0.00%) 1261.04 * 5.30%* >>> Amean fault-both-3 1383.95 ( 0.00%) 1191.69 * 13.89%* >>> Amean fault-both-5 1568.13 ( 0.00%) 1445.20 * 7.84%* >>> Amean fault-both-7 1819.62 ( 0.00%) 1555.13 * 14.54%* >>> Amean fault-both-12 1106.96 ( 0.00%) 1149.43 * -3.84%* >>> Amean fault-both-18 2196.93 ( 0.00%) 1875.77 * 14.62%* >>> Amean fault-both-24 2642.69 ( 0.00%) 2671.21 * -1.08%* >>> Amean fault-both-30 2901.89 ( 0.00%) 2857.32 * 1.54%* >>> Amean fault-both-32 3747.00 ( 0.00%) 3479.23 * 7.15%* >>> >>> Fixes: 70b44595eafe9 ("mm, compaction: use free lists to quickly locate a migration source") >>> >>> Signed-off-by: zhouchuyi > > No need for a newline between Fixes and Signed-off-by. The Signed-off-by > should have your full name, not a username. >