Re: [RFC PATCH v2 4/7] mm: make alloc_contig_range work at pageblock granularity

2021-12-12 Thread Eric Ren

Hi,

On 2021/12/10 07:04, Zi Yan wrote:

From: Zi Yan 

alloc_contig_range() worked at MAX_ORDER-1 granularity to avoid merging
pageblocks with different migratetypes. It might unnecessarily convert
extra pageblocks at the beginning and at the end of the range. Change
alloc_contig_range() to work at pageblock granularity.

It is done by restoring pageblock types and split >pageblock_order free
pages after isolating at MAX_ORDER-1 granularity and migrating pages
away at pageblock granularity. The reason for this process is that
during isolation, some pages, either free or in-use, might have >pageblock
sizes and isolating part of them can cause free accounting issues.

Could you elaborate on how the acconting issue would happen in details?



Restoring the migratetypes of the pageblocks not in the interesting
range later is much easier.

Signed-off-by: Zi Yan 
---
  mm/page_alloc.c | 169 ++--
  1 file changed, 149 insertions(+), 20 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 107a5f186d3b..5ffbeb1b7512 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8981,8 +8981,8 @@ struct page *has_unmovable_pages(struct zone *zone, 
struct page *page,
  #ifdef CONFIG_CONTIG_ALLOC
  static unsigned long pfn_max_align_down(unsigned long pfn)
  {
-   return pfn & ~(max_t(unsigned long, MAX_ORDER_NR_PAGES,
-pageblock_nr_pages) - 1);
+   return ALIGN_DOWN(pfn, max_t(unsigned long, MAX_ORDER_NR_PAGES,
+pageblock_nr_pages));
  }
  
  static unsigned long pfn_max_align_up(unsigned long pfn)

@@ -9071,6 +9071,52 @@ static int __alloc_contig_migrate_range(struct 
compact_control *cc,
return 0;
  }
  
+static inline int save_migratetypes(unsigned char *migratetypes,

+   unsigned long start_pfn, unsigned long end_pfn)
+{
+   unsigned long pfn = start_pfn;
+   int num = 0;
+
+   while (pfn < end_pfn) {
+   migratetypes[num] = get_pageblock_migratetype(pfn_to_page(pfn));
+   num++;
+   pfn += pageblock_nr_pages;
+   }
+   return num;
+}
+
+static inline int restore_migratetypes(unsigned char *migratetypes,
+   unsigned long start_pfn, unsigned long end_pfn)
+{
+   unsigned long pfn = start_pfn;
+   int num = 0;
+
+   while (pfn < end_pfn) {
+   set_pageblock_migratetype(pfn_to_page(pfn), migratetypes[num]);
+   num++;
+   pfn += pageblock_nr_pages;
+   }
+   return num;
+}
+
+static inline void split_free_page_into_pageblocks(struct page *free_page,
+   int order, struct zone *zone)
+{
+   unsigned long pfn;
+
+   spin_lock(>lock);
+   del_page_from_free_list(free_page, zone, order);
+   for (pfn = page_to_pfn(free_page);
+pfn < page_to_pfn(free_page) + (1UL << order);
+pfn += pageblock_nr_pages) {
+   int mt = get_pfnblock_migratetype(pfn_to_page(pfn), pfn);
+
+   __free_one_page(pfn_to_page(pfn), pfn, zone, pageblock_order,
+   mt, FPI_NONE);
+   }
+   spin_unlock(>lock);
+}
+
  /**
   * alloc_contig_range() -- tries to allocate given range of pages
   * @start:start PFN to allocate
@@ -9096,8 +9142,15 @@ int alloc_contig_range(unsigned long start, unsigned 
long end,
   unsigned migratetype, gfp_t gfp_mask)
  {
unsigned long outer_start, outer_end;
+   unsigned long isolate_start = pfn_max_align_down(start);
+   unsigned long isolate_end = pfn_max_align_up(end);
+   unsigned long alloc_start = ALIGN_DOWN(start, pageblock_nr_pages);
+   unsigned long alloc_end = ALIGN(end, pageblock_nr_pages);

What is the differecence between isolate_* and alloc_*?

+   unsigned long num_pageblock_to_save;
unsigned int order;
int ret = 0;
+   unsigned char *saved_mt;
+   int num;
  
  	struct compact_control cc = {

.nr_migratepages = 0,
@@ -9111,11 +9164,30 @@ int alloc_contig_range(unsigned long start, unsigned 
long end,
};
INIT_LIST_HEAD();
  
+	/*

+* TODO: make MIGRATE_ISOLATE a standalone bit to avoid overwriting
+* the exiting migratetype. Then, we will not need the save and restore
+* process here.
+*/
+
+   /* Save the migratepages of the pageblocks before start and after end */
+   num_pageblock_to_save = (alloc_start - isolate_start) / 
pageblock_nr_pages
+   + (isolate_end - alloc_end) / 
pageblock_nr_pages;
+   saved_mt =
+   kmalloc_array(num_pageblock_to_save,
+ sizeof(unsigned char), GFP_KERNEL);
+   if (!saved_mt)
+   return -ENOMEM;
+
+   num = save_migratetypes(saved_mt, isolate_start, alloc_start);
+
+   num = save_migratetypes(_mt[num], 

[RFC PATCH v2 4/7] mm: make alloc_contig_range work at pageblock granularity

2021-12-09 Thread Zi Yan
From: Zi Yan 

alloc_contig_range() worked at MAX_ORDER-1 granularity to avoid merging
pageblocks with different migratetypes. It might unnecessarily convert
extra pageblocks at the beginning and at the end of the range. Change
alloc_contig_range() to work at pageblock granularity.

It is done by restoring pageblock types and split >pageblock_order free
pages after isolating at MAX_ORDER-1 granularity and migrating pages
away at pageblock granularity. The reason for this process is that
during isolation, some pages, either free or in-use, might have >pageblock
sizes and isolating part of them can cause free accounting issues.
Restoring the migratetypes of the pageblocks not in the interesting
range later is much easier.

Signed-off-by: Zi Yan 
---
 mm/page_alloc.c | 169 ++--
 1 file changed, 149 insertions(+), 20 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 107a5f186d3b..5ffbeb1b7512 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8981,8 +8981,8 @@ struct page *has_unmovable_pages(struct zone *zone, 
struct page *page,
 #ifdef CONFIG_CONTIG_ALLOC
 static unsigned long pfn_max_align_down(unsigned long pfn)
 {
-   return pfn & ~(max_t(unsigned long, MAX_ORDER_NR_PAGES,
-pageblock_nr_pages) - 1);
+   return ALIGN_DOWN(pfn, max_t(unsigned long, MAX_ORDER_NR_PAGES,
+pageblock_nr_pages));
 }
 
 static unsigned long pfn_max_align_up(unsigned long pfn)
@@ -9071,6 +9071,52 @@ static int __alloc_contig_migrate_range(struct 
compact_control *cc,
return 0;
 }
 
+static inline int save_migratetypes(unsigned char *migratetypes,
+   unsigned long start_pfn, unsigned long end_pfn)
+{
+   unsigned long pfn = start_pfn;
+   int num = 0;
+
+   while (pfn < end_pfn) {
+   migratetypes[num] = get_pageblock_migratetype(pfn_to_page(pfn));
+   num++;
+   pfn += pageblock_nr_pages;
+   }
+   return num;
+}
+
+static inline int restore_migratetypes(unsigned char *migratetypes,
+   unsigned long start_pfn, unsigned long end_pfn)
+{
+   unsigned long pfn = start_pfn;
+   int num = 0;
+
+   while (pfn < end_pfn) {
+   set_pageblock_migratetype(pfn_to_page(pfn), migratetypes[num]);
+   num++;
+   pfn += pageblock_nr_pages;
+   }
+   return num;
+}
+
+static inline void split_free_page_into_pageblocks(struct page *free_page,
+   int order, struct zone *zone)
+{
+   unsigned long pfn;
+
+   spin_lock(>lock);
+   del_page_from_free_list(free_page, zone, order);
+   for (pfn = page_to_pfn(free_page);
+pfn < page_to_pfn(free_page) + (1UL << order);
+pfn += pageblock_nr_pages) {
+   int mt = get_pfnblock_migratetype(pfn_to_page(pfn), pfn);
+
+   __free_one_page(pfn_to_page(pfn), pfn, zone, pageblock_order,
+   mt, FPI_NONE);
+   }
+   spin_unlock(>lock);
+}
+
 /**
  * alloc_contig_range() -- tries to allocate given range of pages
  * @start: start PFN to allocate
@@ -9096,8 +9142,15 @@ int alloc_contig_range(unsigned long start, unsigned 
long end,
   unsigned migratetype, gfp_t gfp_mask)
 {
unsigned long outer_start, outer_end;
+   unsigned long isolate_start = pfn_max_align_down(start);
+   unsigned long isolate_end = pfn_max_align_up(end);
+   unsigned long alloc_start = ALIGN_DOWN(start, pageblock_nr_pages);
+   unsigned long alloc_end = ALIGN(end, pageblock_nr_pages);
+   unsigned long num_pageblock_to_save;
unsigned int order;
int ret = 0;
+   unsigned char *saved_mt;
+   int num;
 
struct compact_control cc = {
.nr_migratepages = 0,
@@ -9111,11 +9164,30 @@ int alloc_contig_range(unsigned long start, unsigned 
long end,
};
INIT_LIST_HEAD();
 
+   /*
+* TODO: make MIGRATE_ISOLATE a standalone bit to avoid overwriting
+* the exiting migratetype. Then, we will not need the save and restore
+* process here.
+*/
+
+   /* Save the migratepages of the pageblocks before start and after end */
+   num_pageblock_to_save = (alloc_start - isolate_start) / 
pageblock_nr_pages
+   + (isolate_end - alloc_end) / 
pageblock_nr_pages;
+   saved_mt =
+   kmalloc_array(num_pageblock_to_save,
+ sizeof(unsigned char), GFP_KERNEL);
+   if (!saved_mt)
+   return -ENOMEM;
+
+   num = save_migratetypes(saved_mt, isolate_start, alloc_start);
+
+   num = save_migratetypes(_mt[num], alloc_end, isolate_end);
+
/*
 * What we do here is we mark all pageblocks in range as
 * MIGRATE_ISOLATE.  Because pageblock and max order pages may
 * have