Re: [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions.

[Zi Yan <ziy@nvidia.com>]

On 30 May 2025, at 13:15, Vlastimil Babka wrote:

> On 5/30/25 18:22, Zi Yan wrote:
>> migratetype is no longer overwritten during pageblock isolation,
>> start_isolate_page_range(), has_unmovable_pages(), and
>> set_migratetype_isolate() no longer need which migratetype to restore
>> during isolation failure.
>>
>> For has_unmoable_pages(), it needs to know if the isolation is for CMA
>> allocation, so adding CMA_ALLOCATION to provide the information. At the
>> same time change isolation flags to enum pb_isolate_mode
>> (PB_ISOLATE_MODE_MEM_OFFLINE, PB_ISOLATE_MODE_CMA_ALLOC,
>> PB_ISOLATE_MODE_OTHER). Remove REPORT_FAILURE and check
>> MEMORY_OFFLINE instead, since only PB_ISOLATE_MODE_MEM_OFFLINE reports
>> isolation failures.
>>
>> alloc_contig_range() no longer needs migratetype. Replace it with
>> enum acr_flags_t to tell if an allocation is for CMA. So does
>> __alloc_contig_migrate_range().
>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
>
>> ---
>>  drivers/virtio/virtio_mem.c    |  2 +-
>>  include/linux/gfp.h            |  9 ++++-
>>  include/linux/page-isolation.h | 20 ++++++++--
>>  include/trace/events/kmem.h    | 14 ++++---
>>  mm/cma.c                       |  2 +-
>>  mm/memory_hotplug.c            |  6 +--
>>  mm/page_alloc.c                | 27 ++++++-------
>>  mm/page_isolation.c            | 70 +++++++++++++++-------------------
>>  8 files changed, 82 insertions(+), 68 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>> index 56d0dbe62163..6bce70b139b2 100644
>> --- a/drivers/virtio/virtio_mem.c
>> +++ b/drivers/virtio/virtio_mem.c
>> @@ -1243,7 +1243,7 @@ static int virtio_mem_fake_offline(struct virtio_mem *vm, unsigned long pfn,
>>  		if (atomic_read(&vm->config_changed))
>>  			return -EAGAIN;
>>
>> -		rc = alloc_contig_range(pfn, pfn + nr_pages, MIGRATE_MOVABLE,
>> +		rc = alloc_contig_range(pfn, pfn + nr_pages, ACR_OTHER,
>>  					GFP_KERNEL);
>>  		if (rc == -ENOMEM)
>>  			/* whoops, out of memory */
>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>> index be160e8d8bcb..51990d571e3e 100644
>> --- a/include/linux/gfp.h
>> +++ b/include/linux/gfp.h
>> @@ -423,9 +423,16 @@ static inline bool gfp_compaction_allowed(gfp_t gfp_mask)
>>  extern gfp_t vma_thp_gfp_mask(struct vm_area_struct *vma);
>>
>>  #ifdef CONFIG_CONTIG_ALLOC
>> +
>> +enum acr_flags_t {
>> +	ACR_CMA,	// CMA allocation
>> +	ACR_OTHER,	// other allocation
>> +};
>> +
>>  /* The below functions must be run on a range from a single zone. */
>>  extern int alloc_contig_range_noprof(unsigned long start, unsigned long end,
>> -			      unsigned migratetype, gfp_t gfp_mask);
>> +				     enum acr_flags_t alloc_flags,
>> +				     gfp_t gfp_mask);
>
> Nit: when used this way I think it's also rather a "mode" than "flags". It's
> probably sufficient though, I don't expect we'll be adding more and want to
> combine them in a flags way. So it's just about the enum's name.

This is the fixup (variable alloc_flags is renamed to alloc_mode too):

From 9674b741eb93eb74de52fb28c644f523387d1e9f Mon Sep 17 00:00:00 2001
From: Zi Yan <ziy@nvidia.com>
Date: Fri, 30 May 2025 13:58:11 -0400
Subject: [PATCH] rename acr_flags_t to acr_mode.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 include/linux/gfp.h         |  4 ++--
 include/trace/events/kmem.h | 12 ++++++------
 mm/page_alloc.c             | 14 +++++++-------
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 51990d571e3e..363771903be3 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -424,14 +424,14 @@ extern gfp_t vma_thp_gfp_mask(struct vm_area_struct *vma);

 #ifdef CONFIG_CONTIG_ALLOC

-enum acr_flags_t {
+enum acr_mode {
 	ACR_CMA,	// CMA allocation
 	ACR_OTHER,	// other allocation
 };

 /* The below functions must be run on a range from a single zone. */
 extern int alloc_contig_range_noprof(unsigned long start, unsigned long end,
-				     enum acr_flags_t alloc_flags,
+				     enum acr_mode alloc_mode,
 				     gfp_t gfp_mask);
 #define alloc_contig_range(...)			alloc_hooks(alloc_contig_range_noprof(__VA_ARGS__))

diff --git a/include/trace/events/kmem.h b/include/trace/events/kmem.h
index 7c4e2e703a23..4ac8fbff224c 100644
--- a/include/trace/events/kmem.h
+++ b/include/trace/events/kmem.h
@@ -312,9 +312,9 @@ TRACE_EVENT(mm_alloc_contig_migrate_range_info,
 		 unsigned long nr_migrated,
 		 unsigned long nr_reclaimed,
 		 unsigned long nr_mapped,
-		 enum acr_flags_t alloc_flags),
+		 enum acr_mode alloc_mode),

-	TP_ARGS(start, end, nr_migrated, nr_reclaimed, nr_mapped, alloc_flags),
+	TP_ARGS(start, end, nr_migrated, nr_reclaimed, nr_mapped, alloc_mode),

 	TP_STRUCT__entry(
 		__field(unsigned long, start)
@@ -322,7 +322,7 @@ TRACE_EVENT(mm_alloc_contig_migrate_range_info,
 		__field(unsigned long, nr_migrated)
 		__field(unsigned long, nr_reclaimed)
 		__field(unsigned long, nr_mapped)
-		__field(enum acr_flags_t, alloc_flags)
+		__field(enum acr_mode, alloc_mode)
 	),

 	TP_fast_assign(
@@ -331,13 +331,13 @@ TRACE_EVENT(mm_alloc_contig_migrate_range_info,
 		__entry->nr_migrated = nr_migrated;
 		__entry->nr_reclaimed = nr_reclaimed;
 		__entry->nr_mapped = nr_mapped;
-		__entry->alloc_flags = alloc_flags;
+		__entry->alloc_mode = alloc_mode;
 	),

-	TP_printk("start=0x%lx end=0x%lx alloc_flags=%d nr_migrated=%lu nr_reclaimed=%lu nr_mapped=%lu",
+	TP_printk("start=0x%lx end=0x%lx alloc_mode=%d nr_migrated=%lu nr_reclaimed=%lu nr_mapped=%lu",
 		  __entry->start,
 		  __entry->end,
-		  __entry->alloc_flags,
+		  __entry->alloc_mode,
 		  __entry->nr_migrated,
 		  __entry->nr_reclaimed,
 		  __entry->nr_mapped)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index dd761f5e6310..dc418510aba2 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6696,12 +6696,12 @@ static void alloc_contig_dump_pages(struct list_head *page_list)

 /*
  * [start, end) must belong to a single zone.
- * @alloc_flags: using acr_flags_t to filter the type of migration in
+ * @alloc_mode: using acr_mode to filter the type of migration in
  *		trace_mm_alloc_contig_migrate_range_info.
  */
 static int __alloc_contig_migrate_range(struct compact_control *cc,
 					unsigned long start, unsigned long end,
-					enum acr_flags_t alloc_flags)
+					enum acr_mode alloc_mode)
 {
 	/* This function is based on compact_zone() from compaction.c. */
 	unsigned int nr_reclaimed;
@@ -6773,7 +6773,7 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
 		putback_movable_pages(&cc->migratepages);
 	}

-	trace_mm_alloc_contig_migrate_range_info(start, end, alloc_flags,
+	trace_mm_alloc_contig_migrate_range_info(start, end, alloc_mode,
 						 total_migrated,
 						 total_reclaimed,
 						 total_mapped);
@@ -6844,7 +6844,7 @@ static int __alloc_contig_verify_gfp_mask(gfp_t gfp_mask, gfp_t *gfp_cc_mask)
  * alloc_contig_range() -- tries to allocate given range of pages
  * @start:	start PFN to allocate
  * @end:	one-past-the-last PFN to allocate
- * @alloc_flags:	allocation information
+ * @alloc_mode:	allocation information
  * @gfp_mask:	GFP mask. Node/zone/placement hints are ignored; only some
  *		action and reclaim modifiers are supported. Reclaim modifiers
  *		control allocation behavior during compaction/migration/reclaim.
@@ -6861,7 +6861,7 @@ static int __alloc_contig_verify_gfp_mask(gfp_t gfp_mask, gfp_t *gfp_cc_mask)
  * need to be freed with free_contig_range().
  */
 int alloc_contig_range_noprof(unsigned long start, unsigned long end,
-			      enum acr_flags_t alloc_flags, gfp_t gfp_mask)
+			      enum acr_mode alloc_mode, gfp_t gfp_mask)
 {
 	unsigned long outer_start, outer_end;
 	int ret = 0;
@@ -6876,7 +6876,7 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
 		.alloc_contig = true,
 	};
 	INIT_LIST_HEAD(&cc.migratepages);
-	enum pb_isolate_mode mode = (alloc_flags == ACR_CMA) ?
+	enum pb_isolate_mode mode = (alloc_mode == ACR_CMA) ?
 					    PB_ISOLATE_MODE_CMA_ALLOC :
 					    PB_ISOLATE_MODE_OTHER;

@@ -6921,7 +6921,7 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
 	 * allocated.  So, if we fall through be sure to clear ret so that
 	 * -EBUSY is not accidentally used or returned to caller.
 	 */
-	ret = __alloc_contig_migrate_range(&cc, start, end, alloc_flags);
+	ret = __alloc_contig_migrate_range(&cc, start, end, alloc_mode);
 	if (ret && ret != -EBUSY)
 		goto done;

-- 
2.47.2



Best Regards,
Yan, Zi