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 A1BADC4167B for ; Wed, 6 Dec 2023 10:18:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3C72C6B0087; Wed, 6 Dec 2023 05:18:48 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 377286B0088; Wed, 6 Dec 2023 05:18:48 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 23FAB6B0089; Wed, 6 Dec 2023 05:18:48 -0500 (EST) 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 0F3B26B0087 for ; Wed, 6 Dec 2023 05:18:48 -0500 (EST) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id C8BA3A0169 for ; Wed, 6 Dec 2023 10:18:47 +0000 (UTC) X-FDA: 81535994694.27.97F31D1 Received: from mail-vs1-f50.google.com (mail-vs1-f50.google.com [209.85.217.50]) by imf17.hostedemail.com (Postfix) with ESMTP id B876240014 for ; Wed, 6 Dec 2023 10:18:45 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=YQmJhwly; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf17.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.217.50 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1701857925; a=rsa-sha256; cv=none; b=Th1+BubdbwaQ9/8vepbWUEMFb5dkfsJ0psbCnAKBTNc3fs40T+Z+K2AjtJkPaROTxCi/3t 0WY00E58T/wmw9J71bFU6QItGwqxaqtD4KLK227qy8ZrOqm4nmeb1ySSIyYj/EI6Hao3qd 3HxygfXdg4W0EHCv+76HZenqL7ZT0yw= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=YQmJhwly; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf17.hostedemail.com: domain of 21cnbao@gmail.com designates 209.85.217.50 as permitted sender) smtp.mailfrom=21cnbao@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1701857925; 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=PB5sZ5MSZmZsnlatsjzNG0ijjFc2lOy26kZJCTP898Q=; b=lUmvLZvJ+9AxXClLfS9CpB+Iraso3p0giDtMlHGAHzruvzLjrj5kx95NvHTIA7I3N+nnFe khpUwYZGu8WBuX5+G8sZZVcS0fuKYL9ozuxFGibbRm1rX9UTxZ9XYE/j73hvzUgB112qQW olAxkfOSKxt7ycYV1riT/oVLagfR61A= Received: by mail-vs1-f50.google.com with SMTP id ada2fe7eead31-464889ff296so678360137.1 for ; Wed, 06 Dec 2023 02:18:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701857925; x=1702462725; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=PB5sZ5MSZmZsnlatsjzNG0ijjFc2lOy26kZJCTP898Q=; b=YQmJhwlyp5XLM16jIlZI/ZrR3/FfHp2D4pxD9ecsFKF7ovChoHliwBlqIh+VNWc2dS f7CyMVd4OLUIC0NY3htF6ZvAblGSgnCCQmSKZrmqKppY+R5Fm0azQBCaDd5LyIUsAEs0 JzDvlXQRY/WDufyHzY4N7f5pWlBaE1xpiZLMbJKON5PdtEMagp1to4rcYrKHSuBZPBdP 53AgWfdh4lABmqLBvTHn8QoO3gUy/EEi+3XQNV+LZMeyYqCwuLu6b8Wwmb3jN7duOi3Q qevlcCDJ3CLHBEMm9h8ZWNDz/UwXqN3n+y2RSC1aUijBZJQLJteJTDCvLZb7d1qA3Oa8 KMEA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701857925; x=1702462725; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=PB5sZ5MSZmZsnlatsjzNG0ijjFc2lOy26kZJCTP898Q=; b=e+uGDk931ln/YTcwf5UGsYF9HfBMfyzQrPS0AO5pWgcYzpCvl/2908PFBvpkIVoLj4 VgQdP+4dsTa7HI2pfgvAGSQsG9Znh8vwifOJWWCQ8yjY/CLN8kLhJOOjO2XLN6Rhe4mX 2RioL7SPM8leopBd4mXlQ/NcQngCB6aW8UPW5T0cjc1nUFTHXeyKiBBoP/+5t5naeN6m /I0SRh9FRzcO/iHvKq/YpPCEBYF/jXYpkmZWFC0IFMjFPtVD29R94e5EHmUF/EZrC0PW hvdkIUkkVjWvKNHwrD47i7puD7H3LFDSl/voC0lsHjamZtKpGWU6BdFk0G3fKJJK2LSz AbOA== X-Gm-Message-State: AOJu0YzFXoiIfpOyWDbTfOe01FFcC0kfzTHn9HtfO3wukTGP7ehOsyTN ZB3YFom7K8qzjhlRevTMmvT2MfwVw8GG2AduoII= X-Google-Smtp-Source: AGHT+IFh+MMuiWe4LiMzv/KJTqc5ICpDaA5Y7AOzgAcrrvz+FZPl+kGnCbxOkUR4ACPrCPbILgIpjx0Mmm/ihmaWg+I= X-Received: by 2002:a05:6102:7b8:b0:464:95d9:8d39 with SMTP id x24-20020a05610207b800b0046495d98d39mr170221vsg.27.1701857924667; Wed, 06 Dec 2023 02:18:44 -0800 (PST) MIME-Version: 1.0 References: <20231129104530.63787-1-v-songbaohua@oppo.com> <079610c2-04ed-4495-8eb7-518b04f911f7@linux.alibaba.com> In-Reply-To: <079610c2-04ed-4495-8eb7-518b04f911f7@linux.alibaba.com> From: Barry Song <21cnbao@gmail.com> Date: Wed, 6 Dec 2023 23:18:32 +1300 Message-ID: Subject: Re: [RFC PATCH] mm: compaction: avoid fast_isolate_freepages blindly choose improper pageblock To: Baolin Wang Cc: akpm@linux-foundation.org, linux-mm@kvack.org, david@redhat.com, shikemeng@huaweicloud.com, willy@infradead.org, mgorman@techsingularity.net, hannes@cmpxchg.org, linux-kernel@vger.kernel.org, Barry Song , Zhanyuan Hu Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: B876240014 X-Stat-Signature: c46xugt3fyqzdog79qsjuhdaonkm1bbu X-HE-Tag: 1701857925-862059 X-HE-Meta: U2FsdGVkX19lRl8GrYucWaqZ22d6yoxRzJgRrAHwObhnWX0lU+1TsBY4+apHWxr5jePObQarXf+7B7mMb4hN8L2PGkheSnwHpfAch4nZ/aLaG81IOIp33itWQVeGn8sSDcw9SqNlaN5qxUzkJCsrT1gkHska97lhGpHvW9dEhPu3gcmqNbMetCyGhQ5YO3NQ8WdhhKtAleCX+Y9FCGONCxwDFhZ9vACCle607A4/1cmRewxF3WjsWvVybtQiAte/MTPq8LiZUTE32YJwWflBk8l6Q5i5iMGd6dXffnBLaok9CBo1vGYzk9sbAk3WStJf080fyoXgF7wCw6EBPC0ovcItdK4qmOxiv3GDn/ilpZp4hFqGuvKz+koI0HrglKg+Shgc+g5pItKoL0N7e1rIVg9S0oWaBzsv+HLxbDVtcXSM6j4IPt9h3GBUprQkh9ZLQAJJbV+OnpAOMm7epT67rRPym6tVCJqiRajkv6Q+cEoedw0iaymlVtMWbfGpIlcDoNRE21lhXnZ2mNkxxVJKyP0tzceqKXDpAaxwzKTvIzerD3WK9oxIOAbhgeCaQubD1iPoeJMRchuKG7gnu3u9XIaFL1H7XVOXA46JD/vLVt3KLxyNqe4LA2k05xzLJfnqhuJUWjdBBp7ZbG6DgvbKjalMb4IgUoiFyjy6Y4QcdZVII02Y5YVh013RDKMoK8ILbAcblZgN4YpKTGIHSOD6i9lWnsm44gLnui54ANyLn6ds3DXQRzp42uu4szYUehKDdXUQfdzqprBcmXgVPgccmg3ywKOxPmcEZ/cQVR+cNnmYMRyN2tdkcfV0jjMKbKOpxtgTcGdBotAqU3y/EVyENMUgVOJKD9UG+apYuO65J8uEKu3GndKCErjt7AoOSLJ3S4qxQjT6Mbn40KCNK5t764OQ4AJLx/MDwiCLfVKKqCR3BaP1vLF6nMPV6dVy/z5Jhcz02V4B8LNK6j238PQ v7rsqpAt YhMhPCRdmR/nDl6L3Uq16lWo/WeopF/QzCmwXEktcvzdA3ullm+xMIPbfVxRKf5hp3kWfwlS550kOTuuJelrXqdzYmXb+4yCUvgUvic9n1YfF93uOycKCzdwZE3wv0lkTaelQsHS5JjIE2ABuXID/dvxiGhZ8HKlAXw0KHqmhjiyZaZxf9IeFhLaiQkh0OwaAT/OM/zZiq6ngPOpZHWrQmx2ZUnDOA9DqzHMGmBfxLyNQhuh1HFHl5UxIAuBJLUi2ttkjEMEzfeTp4nyHRvPetNqaYjTnhQjGOUsqyGVAUF4qKD53/GfwrJThYNM0zmbbAHSt41e3ZkjPp0UgMkSKqlRj0jf4jqNJbAwfwDkPlOcLfgMjjzL6YZhiUs85KBoC227vQGK8ZttmEkiVUfVtQKcv9DNIZe637KYLi9WVOMGDClEk3hEAI9VbhZPHjXBbQAZouA81phUXVPikdcsdkHvD+Dn0aTDCNftO 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: List-Subscribe: List-Unsubscribe: On Wed, Dec 6, 2023 at 10:54=E2=80=AFPM Baolin Wang wrote: > > > > On 11/29/2023 6:45 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 >=3D min_pfn) { > > page =3D pfn_to_page(highest); > > cc->free_pfn =3D highest; > > } else { > > if (cc->direct_compaction && pfn_valid(min_pf= n)) { /* XXX */ > > page =3D pageblock_pfn_to_page(min_pf= n, > > min(pageblock_end_pfn(min_pfn= ), > > zone_end_pfn(cc->zone)), > > cc->zone); > > cc->free_pfn =3D min_pfn; > > } > > } > > } > > } > > Yes, the min_pfn can be an unsuitable migration target. But I think we > can just add the suitable_migration_target() validation into 'min_pfn' > case? Since other cases must be suitable target which found from > MIGRATE_MOVABLE free list. Something like below: > > diff --git a/mm/compaction.c b/mm/compaction.c > index 01ba298739dd..4e8eb4571909 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -1611,6 +1611,8 @@ static void fast_isolate_freepages(struct > compact_control *cc) > > min(pageblock_end_pfn(min_pfn), > > zone_end_pfn(cc->zone)), > cc->zone); > + if > (!suitable_migration_target(cc, page)) > + page =3D NULL; > cc->free_pfn =3D min_pfn; > } > } > yes. this makes more senses. > By the way, I wonder if this patch can improve the efficiency of > compaction in your test case? This happens not quite often. when running 25 machines for one night, most of them can hit this unexpected code path. but the frequency isn't many times in one second. it might be one time in a couple of hours. so it is very difficult to measure the visible performance impact in my machines though the affection of choosing the unsuitable migration_target should be negative. I feel like it's worth fixing this to at least make the code theoretically self-explanatory? as it is quite odd unsuitable_migration_target can be still migration_target? > > > In contrast, slow path is skipping unsuitable pageblocks in a decent wa= y. > > > > I don't know if it is an intended design or just an oversight. But > > it seems more sensible to skip unsuitable pageblock. > > > > Reported-by: Zhanyuan Hu > > Signed-off-by: Barry Song > > --- > > mm/compaction.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/mm/compaction.c b/mm/compaction.c > > index 01ba298739dd..98c485a25614 100644 > > --- a/mm/compaction.c > > +++ b/mm/compaction.c > > @@ -1625,6 +1625,12 @@ static void fast_isolate_freepages(struct compac= t_control *cc) > > cc->total_free_scanned +=3D nr_scanned; > > if (!page) > > return; > > + /* > > + * Otherwise, we can blindly choose an improper pageblock especia= lly > > + * while using the min mark > > + */ > > + if (!suitable_migration_target(cc, page)) > > + return; > > > > low_pfn =3D page_to_pfn(page); > > fast_isolate_around(cc, low_pfn); Thanks Barry