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=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 CA28BC433E0 for ; Wed, 13 Jan 2021 23:42:53 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 5E9C62339F for ; Wed, 13 Jan 2021 23:42:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5E9C62339F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linux-foundation.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id A14EC8D00A1; Wed, 13 Jan 2021 18:42:52 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9C4008D008E; Wed, 13 Jan 2021 18:42:52 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8DA638D00A1; Wed, 13 Jan 2021 18:42:52 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0030.hostedemail.com [216.40.44.30]) by kanga.kvack.org (Postfix) with ESMTP id 78A728D008E for ; Wed, 13 Jan 2021 18:42:52 -0500 (EST) Received: from smtpin14.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 421728245571 for ; Wed, 13 Jan 2021 23:42:52 +0000 (UTC) X-FDA: 77702379384.14.game51_0d0cf0e27522 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin14.hostedemail.com (Postfix) with ESMTP id 243721822989D for ; Wed, 13 Jan 2021 23:42:52 +0000 (UTC) X-HE-Tag: game51_0d0cf0e27522 X-Filterd-Recvd-Size: 4806 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by imf47.hostedemail.com (Postfix) with ESMTP for ; Wed, 13 Jan 2021 23:42:51 +0000 (UTC) Received: by mail.kernel.org (Postfix) with ESMTPSA id 67A2B22248; Wed, 13 Jan 2021 23:42:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1610581370; bh=ertrlZE0rwiNSTRbz6JlyUmrWKabmyACTp9lthgNjBo=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=riRbTyls2G1jpPe9KSJLt4CMRENZtXdUGbOsfSHcRN9pOksh/W8eajnRztz7kvNg2 C0IAt/wIFbQs/5iqnTYpPCsP6X8zyAxjeVsjsXbUYntOUgBsBWZoJhvNehnxGMnXDK 9/2H6Oy/1LNWTnOGQ+j+CVsgC5EuOcf9bLIfNRjI= Date: Wed, 13 Jan 2021 15:42:49 -0800 From: Andrew Morton To: Charan Teja Reddy Cc: mhocko@suse.com, vbabka@suse.cz, khalid.aziz@oracle.com, ngupta@nitingupta.dev, vinmenon@codeaurora.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm/compaction: return proper state in should_proactive_compact_node Message-Id: <20210113154249.51d3410f3c658d6bedd2bff4@linux-foundation.org> In-Reply-To: <1610546586-18998-1-git-send-email-charante@codeaurora.org> References: <1610546586-18998-1-git-send-email-charante@codeaurora.org> X-Mailer: Sylpheed 3.5.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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, 13 Jan 2021 19:33:06 +0530 Charan Teja Reddy wrote: > should_proactive_compact_node() returns true when sum of the > fragmentation score of all the zones in the node is greater than the > wmark_high of compaction which then triggers the proactive compaction > that operates on the individual zones of the node. But proactive > compaction runs on the zone only when the fragmentation score of the > zone is greater than wmark_low(=wmark_high - 10). > > This means that the sum of the fragmentation scores of all the zones can > exceed the wmark_high but individual zone scores can still be less than > the wmark_low which makes the unnecessary trigger of the proactive > compaction only to return doing nothing. > > Another issue with the return of proactive compaction with out even > trying is its deferral. It is simply deferred for 1 << > COMPACT_MAX_DEFER_SHIFT if the scores across the proactive compaction is > same, thinking that compaction didn't make any progress but in reality > it didn't even try. With the delay between successive retries for > proactive compaction is 500msec, it can result into the deferral for > ~30sec with out even trying the proactive compaction. > > Test scenario is that: compaction_proactiveness=50 thus the wmark_low = > 50 and wmark_high = 60. System have 2 zones(Normal and Movable) with > sizes 5GB and 6GB respectively. After opening some apps on the android, > the fragmentation scores of these zones are 47 and 49 respectively. > Since the sum of these fragmentation scores are above the wmark_high > which triggers the proactive compaction and there since the individual > zone scores are below wmark_low, it returns without trying the > compaction. As a result the fragmentation scores of the zones are still > 47 and 49 which makes the existing logic to defer the compaction > thinking that noprogress is made across the compaction. > > So, run the proactive compaction on the node zones only when atleast one > of the zones fragmentation score is greater than wmark_low. This avoids > the unnecessary deferral and retries of the compaction. > > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -1964,6 +1964,26 @@ static unsigned int fragmentation_score_node(pg_data_t *pgdat) > return score; > } > > +/* > + * Returns the maximum of fragmentation scores of zones in a node. This is > + * used in taking the decission of whether to trigger the proactive compaction > + * on the zones of this node. > + */ > +static unsigned int fragmentation_score_node_zones_max(pg_data_t *pgdat) > +{ > + int zoneid; > + unsigned int max = 0; > + > + for (zoneid = 0; zoneid < MAX_NR_ZONES; zoneid++) { > + struct zone *zone; > + > + zone = &pgdat->node_zones[zoneid]; > + max = max_t(unsigned int, fragmentation_score_zone(zone), max); Both args are unsigned int, so I think the max_t is unnecessary? --- a/mm/compaction.c~mm-compaction-return-proper-state-in-should_proactive_compact_node-fix +++ a/mm/compaction.c @@ -1975,7 +1975,7 @@ static unsigned int fragmentation_score_ struct zone *zone; zone = &pgdat->node_zones[zoneid]; - max = max_t(unsigned int, fragmentation_score_zone(zone), max); + max = max(fragmentation_score_zone(zone), max); } return max; _