[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BCE65484-36AD-4B08-B98F-7B4B8C50E5C2@nvidia.com>
Date: Fri, 30 May 2025 14:01:02 -0400
From: Zi Yan <ziy@...dia.com>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: David Hildenbrand <david@...hat.com>,
Johannes Weiner <hannes@...xchg.org>, linux-mm@...ck.org,
Andrew Morton <akpm@...ux-foundation.org>,
Oscar Salvador <osalvador@...e.de>,
Baolin Wang <baolin.wang@...ux.alibaba.com>,
"Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
Mel Gorman <mgorman@...hsingularity.net>,
Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
Brendan Jackman <jackmanb@...gle.com>, Richard Chang <richardycc@...gle.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter
from more functions.
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@...dia.com>
>
> Reviewed-by: Vlastimil Babka <vbabka@...e.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@...dia.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@...dia.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
Powered by blists - more mailing lists