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

[David Hildenbrand <david@redhat.com>]

On 02.06.25 18:59, Zi Yan wrote:
> On 2 Jun 2025, at 11:18, 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 PB_ISOLATE_MODE_CMA_ALLOC 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
>> PB_ISOLATE_MODE_MEM_OFFLINE, since only PB_ISOLATE_MODE_MEM_OFFLINE
>> reports isolation failures.
>>
>> alloc_contig_range() no longer needs migratetype. Replace it with
>> PB_ISOLATE_MODE_CMA_ALLOC to tell if an allocation is for CMA. So does
>> __alloc_contig_migrate_range().
> 
> This paragraph should be changed to:
> 
> alloc_contig_range() no longer needs migratetype. Replace it with
> a newly defined acr_flags_t to tell if an allocation is for CMA. So does
> __alloc_contig_migrate_range(). Add ACR_OTHER (set to 0) to indicate
> other cases.
> 
>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
>> ---
>>   drivers/virtio/virtio_mem.c    |  4 +-
>>   include/linux/gfp.h            | 18 ++++++++-
>>   include/linux/page-isolation.h |  7 +---
>>   include/trace/events/kmem.h    | 14 ++++---
>>   mm/cma.c                       |  3 +-
>>   mm/memory_hotplug.c            |  6 +--
>>   mm/page_alloc.c                | 27 ++++++-------
>>   mm/page_isolation.c            | 70 +++++++++++++++-------------------
>>   8 files changed, 79 insertions(+), 70 deletions(-)
>>
> 
> 
> The fixup to restore acr_flags_t:
> 
> 
>  From d0205580ab70aaf93f3f7c04b53dc595ee387bac Mon Sep 17 00:00:00 2001
> From: Zi Yan <ziy@nvidia.com>
> Date: Mon, 2 Jun 2025 12:53:58 -0400
> Subject: [PATCH] restore acr_flags_t.
> 
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
>   drivers/virtio/virtio_mem.c    |  4 ++--
>   include/linux/gfp.h            | 21 +++++----------------
>   include/linux/page-isolation.h | 15 +++++++++++++++
>   include/trace/events/kmem.h    | 12 ++++++------
>   mm/cma.c                       |  3 +--
>   mm/page_alloc.c                | 24 ++++++++++++------------
>   6 files changed, 41 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> index 535680a54ff5..6bce70b139b2 100644
> --- a/drivers/virtio/virtio_mem.c
> +++ b/drivers/virtio/virtio_mem.c
> @@ -1243,8 +1243,8 @@ 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,
> -					PB_ISOLATE_MODE_OTHER, GFP_KERNEL);
> +		rc = alloc_contig_range(pfn, pfn + nr_pages, ACR_OTHER,
> +					GFP_KERNEL);
>   		if (rc == -ENOMEM)
>   			/* whoops, out of memory */
>   			return rc;
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 17b92888d6de..95065cec85e5 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -423,25 +423,14 @@ 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
> -/*
> - * Pageblock isolation modes:
> - * PB_ISOLATE_MODE_MEM_OFFLINE - isolate to offline (!allocate) memory
> - *				 e.g., skip over PageHWPoison() pages and
> - *				 PageOffline() pages. Unmovable pages will be
> - *				 reported in this mode.
> - * PB_ISOLATE_MODE_CMA_ALLOC   - isolate for CMA allocations
> - * PB_ISOLATE_MODE_OTHER       - isolate for other purposes
> - */
> -enum pb_isolate_mode {
> -	PB_ISOLATE_MODE_MEM_OFFLINE,
> -	PB_ISOLATE_MODE_CMA_ALLOC,
> -	PB_ISOLATE_MODE_OTHER,
> -};
> +
> +typedef unsigned int __bitwise acr_flags_t;
> +#define ACR_OTHER	((__force acr_flags_t)0)	// other allocations
> +#define ACR_CMA		((__force acr_flags_t)BIT(0))	// allocate for CMA

For FPI I called that "FPI_NONE" -- no special request -- and similarly 
for RMAP "RMAP_NONE".

So if ACR_CMA is set, it's a CMA allocation, otherwise just an ordinary one.


Apart from that LGTM.

-- 
Cheers,

David / dhildenb