linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Khalid Aziz <khalid.aziz@oracle.com>
To: Charan Teja Reddy <charante@codeaurora.org>,
	akpm@linux-foundation.org, vbabka@suse.cz, mhocko@suse.com,
	ngupta@nitingupta.dev, vinmenon@codeaurora.org
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH V3] mm/compaction: correct deferral logic for proactive compaction
Date: Tue, 19 Jan 2021 08:42:55 -0700	[thread overview]
Message-ID: <5343d1b9-a9b1-9967-1f88-5d37ec93274c@oracle.com> (raw)
In-Reply-To: <1610989938-31374-1-git-send-email-charante@codeaurora.org>

On 1/18/21 10:12 AM, Charan Teja Reddy wrote:
> should_proactive_compact_node() returns true when sum of the
> weighted 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 its weighted
> fragmentation score is greater than wmark_low(=wmark_high - 10).
> 
> This means that the sum of the weighted fragmentation scores of all the
> zones can exceed the wmark_high but individual weighted fragmentation
> zone scores can still be less than wmark_low which makes the unnecessary
> trigger of the proactive compaction only to return doing nothing.
> 
> 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 weighted 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 zones weighted fragmentation scores are below wmark_low, it
> returns without trying the proactive compaction. As a result the
> weighted 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.
> 
> Fix this by checking just zone fragmentation score, not the weighted, in
> __compact_finished() and use the zones weighted fragmentation score in
> fragmentation_score_node(). In the test case above, If the weighted
> average of is above wmark_high, then individual score (not adjusted) of
> atleast one zone has to be above wmark_high. Thus it avoids the
> unnecessary trigger and deferrals of the proactive compaction.
> 
> Fix-suggested-by: Vlastimil Babka <vbabka@suse.cz>
> Signed-off-by: Charan Teja Reddy <charante@codeaurora.org>
> ---
> 
> Changes in V3: Addressed suggestions from Vlastimil
> 
> Changes in V2: https://lore.kernel.org/patchwork/patch/1366862/
> 
> Changes in V1: https://lore.kernel.org/patchwork/patch/1364646/
> 
>   mm/compaction.c | 20 ++++++++++++++------
>   1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/compaction.c b/mm/compaction.c
> index e5acb97..ccddb3a 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -1925,20 +1925,28 @@ static bool kswapd_is_running(pg_data_t *pgdat)
>   
>   /*
>    * A zone's fragmentation score is the external fragmentation wrt to the
> - * COMPACTION_HPAGE_ORDER scaled by the zone's size. It returns a value
> - * in the range [0, 100].
> + * COMPACTION_HPAGE_ORDER. It returns a value in the range [0, 100].
> + */
> +static unsigned int fragmentation_score_zone(struct zone *zone)
> +{
> +	return extfrag_for_order(zone, COMPACTION_HPAGE_ORDER);
> +}
> +
> +/*
> + * A weighted zone's fragmentation score is the external fragmentation
> + * wrt to the COMPACTION_HPAGE_ORDER scaled by the zone's size. It
> + * returns a value in the range [0, 100].
>    *
>    * The scaling factor ensures that proactive compaction focuses on larger
>    * zones like ZONE_NORMAL, rather than smaller, specialized zones like
>    * ZONE_DMA32. For smaller zones, the score value remains close to zero,
>    * and thus never exceeds the high threshold for proactive compaction.
>    */
> -static unsigned int fragmentation_score_zone(struct zone *zone)
> +static unsigned int fragmentation_score_zone_weighted(struct zone *zone)
>   {
>   	unsigned long score;
>   
> -	score = zone->present_pages *
> -			extfrag_for_order(zone, COMPACTION_HPAGE_ORDER);
> +	score = zone->present_pages * fragmentation_score_zone(zone);
>   	return div64_ul(score, zone->zone_pgdat->node_present_pages + 1);
>   }
>   
> @@ -1958,7 +1966,7 @@ static unsigned int fragmentation_score_node(pg_data_t *pgdat)
>   		struct zone *zone;
>   
>   		zone = &pgdat->node_zones[zoneid];
> -		score += fragmentation_score_zone(zone);
> +		score += fragmentation_score_zone_weighted(zone);
>   	}
>   
>   	return score;
> 

Looks good.

Reviewed-by: Khalid Aziz <khalid.aziz@oracle.com>



  parent reply	other threads:[~2021-01-19 15:43 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-18 17:12 [PATCH V3] mm/compaction: correct deferral logic for proactive compaction Charan Teja Reddy
2021-01-18 17:41 ` Vlastimil Babka
2021-01-19 15:42 ` Khalid Aziz [this message]
2021-01-19 19:26 ` David Rientjes
2021-01-20 11:04   ` Vlastimil Babka
2021-01-24 22:54     ` David Rientjes
2021-01-27 15:47       ` Charan Teja Kalla
2021-01-25 15:56 ` Vlastimil Babka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5343d1b9-a9b1-9967-1f88-5d37ec93274c@oracle.com \
    --to=khalid.aziz@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=charante@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=ngupta@nitingupta.dev \
    --cc=vbabka@suse.cz \
    --cc=vinmenon@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).