[PATCH] mm/compaction: Mark concurrent compact_considered accesses

[PATCH] mm/compaction: Mark concurrent compact_considered accesses

[Zhu Jun]

Direct compaction and kcompactd update compact_considered without a
common lock. KCSAN reports the plain increment racing with a reset.
This counter is an approximate backoff heuristic and already permits
concurrent increments to be lost.

Use a local snapshot for the saturating increment and mark all shared
loads and stores, including the tracepoint snapshot. Preserve the
existing saturation limit and deferral decisions.

Fixes: 4f92e2586b43 ("mm: compaction: defer compaction using an exponential backoff when compaction fails")
Signed-off-by: Zhu Jun <zhujun2@cmss.chinamobile.com>
---
 include/trace/events/compaction.h |  2 +-
 mm/compaction.c                   | 15 ++++++++-------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/trace/events/compaction.h b/include/trace/events/compaction.h
index d05759d18538..42303a4835f5 100644
--- a/include/trace/events/compaction.h
+++ b/include/trace/events/compaction.h
@@ -255,7 +255,7 @@ DECLARE_EVENT_CLASS(mm_compaction_defer_template,
 		__entry->nid = zone_to_nid(zone);
 		__entry->idx = zone_idx(zone);
 		__entry->order = order;
-		__entry->considered = zone->compact_considered;
+		__entry->considered = READ_ONCE(zone->compact_considered);
 		__entry->defer_shift = zone->compact_defer_shift;
 		__entry->order_failed = zone->compact_order_failed;
 	),
diff --git a/mm/compaction.c b/mm/compaction.c
index f1b2060eb201..04b93d3545af 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -126,7 +126,7 @@ static unsigned long release_free_list(struct list_head *freepages)
  */
 static void defer_compaction(struct zone *zone, int order)
 {
-	zone->compact_considered = 0;
+	WRITE_ONCE(zone->compact_considered, 0);
 	zone->compact_defer_shift++;
 
 	if (order < zone->compact_order_failed)
@@ -142,15 +142,16 @@ static void defer_compaction(struct zone *zone, int order)
 static bool compaction_deferred(struct zone *zone, int order)
 {
 	unsigned long defer_limit = 1UL << zone->compact_defer_shift;
+	unsigned long considered;
 
 	if (order < zone->compact_order_failed)
 		return false;
 
-	/* Avoid possible overflow */
-	if (++zone->compact_considered >= defer_limit) {
-		zone->compact_considered = defer_limit;
+	/* This is a heuristic; concurrent updates may be lost. */
+	considered = READ_ONCE(zone->compact_considered) + 1;
+	WRITE_ONCE(zone->compact_considered, min(considered, defer_limit));
+	if (considered >= defer_limit)
 		return false;
-	}
 
 	trace_mm_compaction_deferred(zone, order);
 
@@ -166,7 +167,7 @@ void compaction_defer_reset(struct zone *zone, int order,
 		bool alloc_success)
 {
 	if (alloc_success) {
-		zone->compact_considered = 0;
+		WRITE_ONCE(zone->compact_considered, 0);
 		zone->compact_defer_shift = 0;
 	}
 	if (order >= zone->compact_order_failed)
@@ -182,7 +183,7 @@ static bool compaction_restarting(struct zone *zone, int order)
 		return false;
 
 	return zone->compact_defer_shift == COMPACT_MAX_DEFER_SHIFT &&
-		zone->compact_considered >= 1UL << zone->compact_defer_shift;
+		READ_ONCE(zone->compact_considered) >= 1UL << zone->compact_defer_shift;
 }
 
 /* Returns true if the pageblock should be scanned for pages to isolate. */
-- 
2.51.0

Re: [PATCH] mm/compaction: Mark concurrent compact_considered accesses

[Vlastimil Babka (SUSE)]

On 11/11/26 09:42, Zhu Jun wrote:
> Direct compaction and kcompactd update compact_considered without a
> common lock. KCSAN reports the plain increment racing with a reset.
> This counter is an approximate backoff heuristic and already permits
> concurrent increments to be lost.
> 
> Use a local snapshot for the saturating increment and mark all shared
> loads and stores, including the tracepoint snapshot. Preserve the
> existing saturation limit and deferral decisions.
> 
> Fixes: 4f92e2586b43 ("mm: compaction: defer compaction using an exponential backoff when compaction fails")
> Signed-off-by: Zhu Jun <zhujun2@cmss.chinamobile.com>

Shouldn't KCSAN be suppressed by data_race() and not READ_ONCE/WRITE_ONCE?

> ---
>  include/trace/events/compaction.h |  2 +-
>  mm/compaction.c                   | 15 ++++++++-------
>  2 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/include/trace/events/compaction.h b/include/trace/events/compaction.h
> index d05759d18538..42303a4835f5 100644
> --- a/include/trace/events/compaction.h
> +++ b/include/trace/events/compaction.h
> @@ -255,7 +255,7 @@ DECLARE_EVENT_CLASS(mm_compaction_defer_template,
>  		__entry->nid = zone_to_nid(zone);
>  		__entry->idx = zone_idx(zone);
>  		__entry->order = order;
> -		__entry->considered = zone->compact_considered;
> +		__entry->considered = READ_ONCE(zone->compact_considered);
>  		__entry->defer_shift = zone->compact_defer_shift;
>  		__entry->order_failed = zone->compact_order_failed;
>  	),
> diff --git a/mm/compaction.c b/mm/compaction.c
> index f1b2060eb201..04b93d3545af 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -126,7 +126,7 @@ static unsigned long release_free_list(struct list_head *freepages)
>   */
>  static void defer_compaction(struct zone *zone, int order)
>  {
> -	zone->compact_considered = 0;
> +	WRITE_ONCE(zone->compact_considered, 0);
>  	zone->compact_defer_shift++;
>  
>  	if (order < zone->compact_order_failed)
> @@ -142,15 +142,16 @@ static void defer_compaction(struct zone *zone, int order)
>  static bool compaction_deferred(struct zone *zone, int order)
>  {
>  	unsigned long defer_limit = 1UL << zone->compact_defer_shift;
> +	unsigned long considered;
>  
>  	if (order < zone->compact_order_failed)
>  		return false;
>  
> -	/* Avoid possible overflow */
> -	if (++zone->compact_considered >= defer_limit) {
> -		zone->compact_considered = defer_limit;
> +	/* This is a heuristic; concurrent updates may be lost. */
> +	considered = READ_ONCE(zone->compact_considered) + 1;
> +	WRITE_ONCE(zone->compact_considered, min(considered, defer_limit));
> +	if (considered >= defer_limit)
>  		return false;
> -	}
>  
>  	trace_mm_compaction_deferred(zone, order);
>  
> @@ -166,7 +167,7 @@ void compaction_defer_reset(struct zone *zone, int order,
>  		bool alloc_success)
>  {
>  	if (alloc_success) {
> -		zone->compact_considered = 0;
> +		WRITE_ONCE(zone->compact_considered, 0);
>  		zone->compact_defer_shift = 0;
>  	}
>  	if (order >= zone->compact_order_failed)
> @@ -182,7 +183,7 @@ static bool compaction_restarting(struct zone *zone, int order)
>  		return false;
>  
>  	return zone->compact_defer_shift == COMPACT_MAX_DEFER_SHIFT &&
> -		zone->compact_considered >= 1UL << zone->compact_defer_shift;
> +		READ_ONCE(zone->compact_considered) >= 1UL << zone->compact_defer_shift;
>  }
>  
>  /* Returns true if the pageblock should be scanned for pages to isolate. */