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 X-Spam-Level: X-Spam-Status: No, score=-7.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E524BC49EA5 for ; Thu, 24 Jun 2021 14:19:11 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 8A172613B4 for ; Thu, 24 Jun 2021 14:19:11 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8A172613B4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 777726B0036; Thu, 24 Jun 2021 10:19:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 701036B005D; Thu, 24 Jun 2021 10:19:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 552CD6B006C; Thu, 24 Jun 2021 10:19:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0147.hostedemail.com [216.40.44.147]) by kanga.kvack.org (Postfix) with ESMTP id 1CFF56B0036 for ; Thu, 24 Jun 2021 10:19:10 -0400 (EDT) Received: from smtpin39.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 3771218031911 for ; Thu, 24 Jun 2021 14:19:10 +0000 (UTC) X-FDA: 78288824460.39.E3A5E90 Received: from mail-lj1-f169.google.com (mail-lj1-f169.google.com [209.85.208.169]) by imf30.hostedemail.com (Postfix) with ESMTP id A5785E00027E for ; Thu, 24 Jun 2021 14:19:09 +0000 (UTC) Received: by mail-lj1-f169.google.com with SMTP id c11so7955733ljd.6 for ; Thu, 24 Jun 2021 07:19:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=bfQbF4+5QrwD64lPgDMb7eHbcKxROnTe/uSIBqr6k6Y=; b=ctuzo1AyoljqVqRSaZ3ARYPrjykzji5HRjWWVnCCagrwPORxWLiIzyxn3MlLE+/dwT IUFc3NUuiukbRIzb1mIis21HHXbUHAfo//RRwMyiaMyK/PckKuQT8xcnK1iVccB+fTiz 3unxKuthiSKNqJj5WTUlFqdiFEiNey0+z69z5Lr0g2LBls4YShzmrowmVsidonEFVDF0 eV1uPEegLLpL+fcACEwXdMQ70+LsJCRLWkXGpTPmGBU/u71S2QHQkP3MX9lU5lMegQ5k dBlAtFNI3WMQJGXZh/z76hAC0a5TKENsOOwpGWXWR9SeEKuXXajW7JiQETP3HGs3usWs zWoQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=bfQbF4+5QrwD64lPgDMb7eHbcKxROnTe/uSIBqr6k6Y=; b=kNzJ92ERJKV0taILttKNyYQsVDcLIV8f0LOiRABV3a+Z3/evl8awPJGkVqGf15whJp /CBXDhqvgjFUop97XX6MzxZsV7PVeCHhPijOj6jxItgvBTlXfG8P5Tsr1OHmCTi33GPt LkItdRpuR+7bDfsU11ERscO32tu0l0PdfGwaw9UghaHLh9/XEePWSmEmoAy011X7kJsp O+Tqj42CscIwrvONJfZCCixK4dHn9JPSld9yCKxbVB8VfyHcbbFLgzKFnww0T+OYvs9/ 5TXCWOdkE3soV23HxaOgrcLmUlpAJL20ShDmHTde7DDE7tqGKJM4RJ9ay1qOPU/KI1E7 eVTg== X-Gm-Message-State: AOAM531bOvx0rHHgtrkzvqchJ7kSuf6YzYWDz1vq3MSa3WF4mz92+CbL heOwaTQ3pRdDcnbVoslf5YAjPnq/zRXdDxfbHSk= X-Google-Smtp-Source: ABdhPJwfuJMnFRIwmVlT32cOWirM21NjM+7tKY6OWUJlZNiNGl57wAZ76ekiqgvRAW+3RxYHqTdrV9X8/6WHe4aswgY= X-Received: by 2002:a2e:9cc3:: with SMTP id g3mr4110501ljj.366.1624544347987; Thu, 24 Jun 2021 07:19:07 -0700 (PDT) MIME-Version: 1.0 References: <20210620145742.54565-1-vvghjk1234@gmail.com> <20210623091509.GV30378@techsingularity.net> In-Reply-To: <20210623091509.GV30378@techsingularity.net> From: Wonhyuk Yang Date: Thu, 24 Jun 2021 23:18:57 +0900 Message-ID: Subject: Re: [PATCH] mm, compaction: fix 'limit' in fast_isolate_freepages To: Mel Gorman Cc: linux-mm@kvack.org, Andrew Morton , Vlastimil Babka Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: A5785E00027E Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=gmail.com header.s=20161025 header.b=ctuzo1Ay; spf=pass (imf30.hostedemail.com: domain of vvghjk1234@gmail.com designates 209.85.208.169 as permitted sender) smtp.mailfrom=vvghjk1234@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Stat-Signature: 6xug6wdydjfbaow9udjwbkmpyosp6gxc X-HE-Tag: 1624544349-342915 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 Wed, Jun 23, 2021 at 6:15 PM Mel Gorman wrote: > > On Sun, Jun 20, 2021 at 11:57:42PM +0900, Wonhyuk Yang wrote: > > Because of 'min(1, ...)', fast_isolate_freepages set 'limit' > > to 0 or 1. This takes away the opportunities of find candinate > > pages. Also, even if 'limit' reaches zero, it scan once. It is > > not consistent. So, modify the minimum value of 'limit' to 1. > > > > The changelog could do with a little polish. > > In addition, what were the effects of this and what load did you use to > evaluate it? While your patch is mostly correct, it has the potential > side-effect of increasing system CPU usage in some cases and I'm curious > to hear what you observed. Minimally it is worth noting in the changelog > that there is a risk of increasing system CPU usage but that there are > advantages too. Describe them in the changelog in case a regression > bisects to your patch. I tested it on the thpscale and the results are as follows. 5.12 5.12 vanilla patched Amean fault-both-1 598.15 ( 0.00%) 592.56 ( 0.93%) Amean fault-both-3 1494.47 ( 0.00%) 1514.35 ( -1.33%) Amean fault-both-5 2519.48 ( 0.00%) 2471.76 ( 1.89%) Amean fault-both-7 3173.85 ( 0.00%) 3079.19 ( 2.98%) Amean fault-both-12 8063.83 ( 0.00%) 7858.29 ( 2.55%) Amean fault-both-18 8781.20 ( 0.00%) 7827.70 * 10.86%* Amean fault-both-24 12576.44 ( 0.00%) 12250.20 ( 2.59%) Amean fault-both-30 18503.27 ( 0.00%) 17528.11 * 5.27%* Amean fault-both-32 16133.69 ( 0.00%) 13874.24 * 14.00%* 5.12 5.12 vanilla patched Ops Compaction migrate scanned 6547133.00 5963901.00 Ops Compaction free scanned 32452453.00 26609101.00 One thing to worry about is that the results are very different every time. Is there any precise way to measure this patch? > > @@ -1456,7 +1456,7 @@ fast_isolate_freepages(struct compact_control *cc) > > high_pfn = pfn; > > > > /* Shorten the scan if a candidate is found */ > > - limit >>= 1; > > + limit = max(1U, limit >> 1); > > } > > > > if (order_scanned >= limit) > > This hunk should be dropped. Once a candidate free page has been > identified, it's ok to decay the limit to 0. This hunk introduces a risk > of increasing system CPU usage unnecessarily. Yes, you are right. I'll take your opinion. > > @@ -1496,7 +1496,7 @@ fast_isolate_freepages(struct compact_control *cc) > > * to freelist_scan_limit. > > */ > > if (order_scanned >= limit) > > - limit = min(1U, limit >> 1); > > + limit = max(1U, limit >> 1); > > } > > The change is fine but I have a minor nitpick that you are free to > ignore. The comment above this block has a typo. > > s/scan ig related/scan is related/ > > Ordinarily patches to fix spelling are ignored but you are altering this > area anyway and it's helpful to see the full comment when reviewing this > patch. I think it would be harmless to fix the spelling in the context > of this patch. Okay, I'll fix this as well. Thank you for your review.