linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3] mm/compaction: correct deferral logic for proactive compaction
@ 2021-01-18 17:12 Charan Teja Reddy
  2021-01-18 17:41 ` Vlastimil Babka
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Charan Teja Reddy @ 2021-01-18 17:12 UTC (permalink / raw)
  To: akpm, vbabka, mhocko, khalid.aziz, ngupta, vinmenon
  Cc: linux-mm, linux-kernel, Charan Teja Reddy

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;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH V3] mm/compaction: correct deferral logic for proactive compaction
  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
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Vlastimil Babka @ 2021-01-18 17:41 UTC (permalink / raw)
  To: Charan Teja Reddy, akpm, mhocko, khalid.aziz, ngupta, vinmenon
  Cc: linux-mm, linux-kernel

On 1/18/21 6:12 PM, 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>

Acked-by: Vlastimil Babka <vbabka@suse.cz>
Thanks!



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3] mm/compaction: correct deferral logic for proactive compaction
  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
  2021-01-19 19:26 ` David Rientjes
  2021-01-25 15:56 ` Vlastimil Babka
  3 siblings, 0 replies; 8+ messages in thread
From: Khalid Aziz @ 2021-01-19 15:42 UTC (permalink / raw)
  To: Charan Teja Reddy, akpm, vbabka, mhocko, ngupta, vinmenon
  Cc: linux-mm, linux-kernel

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>



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3] mm/compaction: correct deferral logic for proactive compaction
  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
@ 2021-01-19 19:26 ` David Rientjes
  2021-01-20 11:04   ` Vlastimil Babka
  2021-01-25 15:56 ` Vlastimil Babka
  3 siblings, 1 reply; 8+ messages in thread
From: David Rientjes @ 2021-01-19 19:26 UTC (permalink / raw)
  To: Charan Teja Reddy
  Cc: akpm, vbabka, mhocko, khalid.aziz, ngupta, vinmenon, linux-mm,
	linux-kernel

On Mon, 18 Jan 2021, 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.

Isn't this an issue in deferred compaction as well?  It seems like 
deferred compaction should check that work was actually performed before 
deferring subsequent calls to compaction.

In other words, I don't believe deferred compaction is intended to avoid 
checks to determine if compaction is worth it; it should only defer 
*additional* work that was not productive.

Thoughts?

> 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>

Suggested-by

> Signed-off-by: Charan Teja Reddy <charante@codeaurora.org>

Acked-by: David Rientjes <rientjes@google.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3] mm/compaction: correct deferral logic for proactive compaction
  2021-01-19 19:26 ` David Rientjes
@ 2021-01-20 11:04   ` Vlastimil Babka
  2021-01-24 22:54     ` David Rientjes
  0 siblings, 1 reply; 8+ messages in thread
From: Vlastimil Babka @ 2021-01-20 11:04 UTC (permalink / raw)
  To: David Rientjes, Charan Teja Reddy
  Cc: akpm, mhocko, khalid.aziz, ngupta, vinmenon, linux-mm, linux-kernel

On 1/19/21 8:26 PM, David Rientjes wrote:
> On Mon, 18 Jan 2021, 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.
> 
> Isn't this an issue in deferred compaction as well?  It seems like 
> deferred compaction should check that work was actually performed before 
> deferring subsequent calls to compaction.

Direct compaction does, proactive not.

> In other words, I don't believe deferred compaction is intended to avoid 
> checks to determine if compaction is worth it; it should only defer 
> *additional* work that was not productive.

Yeah, that should be more optimal.

> Thoughts?
> 



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3] mm/compaction: correct deferral logic for proactive compaction
  2021-01-20 11:04   ` Vlastimil Babka
@ 2021-01-24 22:54     ` David Rientjes
  2021-01-27 15:47       ` Charan Teja Kalla
  0 siblings, 1 reply; 8+ messages in thread
From: David Rientjes @ 2021-01-24 22:54 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Charan Teja Reddy, akpm, mhocko, khalid.aziz, ngupta, vinmenon,
	linux-mm, linux-kernel

On Wed, 20 Jan 2021, Vlastimil Babka wrote:

> On 1/19/21 8:26 PM, David Rientjes wrote:
> > On Mon, 18 Jan 2021, 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.
> > 
> > Isn't this an issue in deferred compaction as well?  It seems like 
> > deferred compaction should check that work was actually performed before 
> > deferring subsequent calls to compaction.
> 
> Direct compaction does, proactive not.
> 
> > In other words, I don't believe deferred compaction is intended to avoid 
> > checks to determine if compaction is worth it; it should only defer 
> > *additional* work that was not productive.
> 
> Yeah, that should be more optimal.
> 

Charan, is this something you'd like to follow up on, or should I take a 
look instead?

Thanks!


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3] mm/compaction: correct deferral logic for proactive compaction
  2021-01-18 17:12 [PATCH V3] mm/compaction: correct deferral logic for proactive compaction Charan Teja Reddy
                   ` (2 preceding siblings ...)
  2021-01-19 19:26 ` David Rientjes
@ 2021-01-25 15:56 ` Vlastimil Babka
  3 siblings, 0 replies; 8+ messages in thread
From: Vlastimil Babka @ 2021-01-25 15:56 UTC (permalink / raw)
  To: Charan Teja Reddy, akpm, mhocko, khalid.aziz, ngupta, vinmenon
  Cc: linux-mm, linux-kernel

On 1/18/21 6:12 PM, 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/

Andrew, I've noticed that v1 is still in mmotm [1] together with v3, which
doesn't make sense together, please drop [1]. Thanks

[1]
https://www.ozlabs.org/~akpm/mmots/broken-out/mm-compaction-return-proper-state-in-should_proactive_compact_node.patch

>  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;
> 



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V3] mm/compaction: correct deferral logic for proactive compaction
  2021-01-24 22:54     ` David Rientjes
@ 2021-01-27 15:47       ` Charan Teja Kalla
  0 siblings, 0 replies; 8+ messages in thread
From: Charan Teja Kalla @ 2021-01-27 15:47 UTC (permalink / raw)
  To: David Rientjes, Vlastimil Babka
  Cc: akpm, mhocko, khalid.aziz, ngupta, vinmenon, linux-mm, linux-kernel



On 1/25/2021 4:24 AM, David Rientjes wrote:
> On Wed, 20 Jan 2021, Vlastimil Babka wrote:
> 
>> On 1/19/21 8:26 PM, David Rientjes wrote:
>>> On Mon, 18 Jan 2021, 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.
>>>
>>> Isn't this an issue in deferred compaction as well?  It seems like 
>>> deferred compaction should check that work was actually performed before 
>>> deferring subsequent calls to compaction.
>>
>> Direct compaction does, proactive not.
>>
>>> In other words, I don't believe deferred compaction is intended to avoid 
>>> checks to determine if compaction is worth it; it should only defer 
>>> *additional* work that was not productive.
>>
>> Yeah, that should be more optimal.
>>
> 
> Charan, is this something you'd like to follow up on, or should I take a 
> look instead?
> 

Sure David. Happy to follow up on this. Thanks!

> Thanks!
> 

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, a Linux Foundation Collaborative Project


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-01-27 15:48 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

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).