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=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 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 EA6B6C433DB for ; Fri, 15 Jan 2021 12:45:18 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3B3FC2184D for ; Fri, 15 Jan 2021 12:45:17 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3B3FC2184D Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 16D6D8D015E; Fri, 15 Jan 2021 07:45:17 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 11F4B8D015B; Fri, 15 Jan 2021 07:45:17 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F28EC8D015E; Fri, 15 Jan 2021 07:45:16 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0107.hostedemail.com [216.40.44.107]) by kanga.kvack.org (Postfix) with ESMTP id DC8918D015B for ; Fri, 15 Jan 2021 07:45:16 -0500 (EST) Received: from smtpin27.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 9BAB8181AC9BF for ; Fri, 15 Jan 2021 12:45:16 +0000 (UTC) X-FDA: 77707979832.27.army76_3211b7e2752f Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin27.hostedemail.com (Postfix) with ESMTP id 72FBC3D668 for ; Fri, 15 Jan 2021 12:45:16 +0000 (UTC) X-HE-Tag: army76_3211b7e2752f X-Filterd-Recvd-Size: 6239 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf18.hostedemail.com (Postfix) with ESMTP for ; Fri, 15 Jan 2021 12:45:15 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 86926AC63; Fri, 15 Jan 2021 12:45:14 +0000 (UTC) To: Charan Teja Reddy , akpm@linux-foundation.org, mhocko@suse.com, khalid.aziz@oracle.com, ngupta@nitingupta.dev, vinmenon@codeaurora.org Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <1610546586-18998-1-git-send-email-charante@codeaurora.org> From: Vlastimil Babka Subject: Re: [PATCH] mm/compaction: return proper state in should_proactive_compact_node Message-ID: Date: Fri, 15 Jan 2021 13:45:14 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: <1610546586-18998-1-git-send-email-charante@codeaurora.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable 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 1/13/21 3:03 PM, 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(=3Dwmark_high - 10). >=20 > This means that the sum of the fragmentation scores of all the zones ca= n > 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. >=20 > 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 i= s > 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. >=20 > Test scenario is that: compaction_proactiveness=3D50 thus the wmark_low= =3D > 50 and wmark_high =3D 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. >=20 > So, run the proactive compaction on the node zones only when atleast on= e > of the zones fragmentation score is greater than wmark_low. This avoids > the unnecessary deferral and retries of the compaction. >=20 > Signed-off-by: Charan Teja Reddy Good catch about the problem, but I wonder if the solution could be bette= r. fragmentation_score_node() is a weighted average of scores of all zones, = that's why fragmentation_score_zone() adjusts the score by zone_present/node_pre= sent. But when considering an individual zone in __compact_finished(), we shoul= dn't be using fragmentation_score_zone() with the adjustment. We are not calculat= ing the weighted average for the whole node there, so it doesn't make sense to do= the adjustment by size. So if it simply took extfrag_for_order(...) as the sc= ore, it should work as expected. In your example above, the score of each zone wo= uld be above 60. If the weighted average is above wmark_high, then individual sc= ore (not adjusted) of at least one zone has to be above wmark_high, and the e= xtra check using max() is not necessary. So I would split fragmentation_score_zone() to e.g. fragmentation_score_z= one() and fragmentation_score_zone_weighted() and call the latter only from fragmentation_score_node(), and not from __compact_finished(). Vlastimil > --- > mm/compaction.c | 27 +++++++++++++++++++++++++-- > 1 file changed, 25 insertions(+), 2 deletions(-) >=20 > diff --git a/mm/compaction.c b/mm/compaction.c > index e5acb97..f7a772a 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -1964,6 +1964,26 @@ static unsigned int fragmentation_score_node(pg_= data_t *pgdat) > return score; > } > =20 > +/* > + * Returns the maximum of fragmentation scores of zones in a node. Thi= s is > + * used in taking the decission of whether to trigger the proactive co= mpaction > + * on the zones of this node. > + */ > +static unsigned int fragmentation_score_node_zones_max(pg_data_t *pgda= t) > +{ > + int zoneid; > + unsigned int max =3D 0; > + > + for (zoneid =3D 0; zoneid < MAX_NR_ZONES; zoneid++) { > + struct zone *zone; > + > + zone =3D &pgdat->node_zones[zoneid]; > + max =3D max_t(unsigned int, fragmentation_score_zone(zone), max); > + } > + > + return max; > +} > + > static unsigned int fragmentation_score_wmark(pg_data_t *pgdat, bool l= ow) > { > unsigned int wmark_low; > @@ -1979,13 +1999,16 @@ static unsigned int fragmentation_score_wmark(p= g_data_t *pgdat, bool low) > =20 > static bool should_proactive_compact_node(pg_data_t *pgdat) > { > - int wmark_high; > + int wmark_low, wmark_high; > =20 > if (!sysctl_compaction_proactiveness || kswapd_is_running(pgdat)) > return false; > =20 > wmark_high =3D fragmentation_score_wmark(pgdat, false); > - return fragmentation_score_node(pgdat) > wmark_high; > + wmark_low =3D fragmentation_score_wmark(pgdat, true); > + > + return fragmentation_score_node(pgdat) > wmark_high && > + fragmentation_score_node_zones_max(pgdat) > wmark_low; > } > =20 > static enum compact_result __compact_finished(struct compact_control *= cc) >=20