diff mbox series

[RFC,2/4] mm/page_isolation: remove migratetype from move_freepages_block_isolate()

Message ID 20240828202240.2809740-3-ziy@nvidia.com (mailing list archive)
State New
Headers show
Series Make MIGRATE_ISOLATE a standalone bit | expand

Commit Message

Zi Yan Aug. 28, 2024, 8:22 p.m. UTC 
Since migratetype is no longer overwritten during pageblock isolation,
moving pageblocks to and from MIGRATE_ISOLATE do not need migratetype.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 include/linux/page-isolation.h |  3 +--
 mm/page_alloc.c                | 27 +++++++++++++++++++++------
 mm/page_isolation.c            | 19 +++++++++----------
 3 files changed, 31 insertions(+), 18 deletions(-)

Comments

David Hildenbrand Sept. 2, 2024, 2:42 p.m. UTC | #1 
On 28.08.24 22:22, Zi Yan wrote:
> Since migratetype is no longer overwritten during pageblock isolation,
> moving pageblocks to and from MIGRATE_ISOLATE do not need migratetype.
> 
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
>   include/linux/page-isolation.h |  3 +--
>   mm/page_alloc.c                | 27 +++++++++++++++++++++------
>   mm/page_isolation.c            | 19 +++++++++----------
>   3 files changed, 31 insertions(+), 18 deletions(-)
> 
> diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
> index 11b8695115ea..6a62401410c3 100644
> --- a/include/linux/page-isolation.h
> +++ b/include/linux/page-isolation.h
> @@ -35,8 +35,7 @@ static inline bool is_migrate_isolate(int migratetype)
>   
>   void set_pageblock_migratetype(struct page *page, int migratetype);
>   
> -bool move_freepages_block_isolate(struct zone *zone, struct page *page,
> -				  int migratetype);
> +bool move_freepages_block_isolate(struct zone *zone, struct page *page);
>   
>   int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
>   			     int migratetype, int flags, gfp_t gfp_flags);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 4ea5cd1a07e2..dc7c36461953 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1764,10 +1764,12 @@ static unsigned long find_large_buddy(unsigned long start_pfn)
>    *
>    * Returns %true if pages could be moved, %false otherwise.
>    */
> -bool move_freepages_block_isolate(struct zone *zone, struct page *page,
> -				  int migratetype)
> +bool move_freepages_block_isolate(struct zone *zone, struct page *page)
>   {
>   	unsigned long start_pfn, pfn;
> +	bool is_block_isolated = get_pageblock_isolate(page);
> +	int from_mt;

I think we should have two functions, one that isolates, another one 
that un-isolates. Or at least make the semantics not depend on the 
current state of the pageblock.

bool pageblock_set_isolated_and_move_free_pages(struct zone *zone, 
struct page *page, bool isolated);

vs.

pageblock_isolate_and_move_free_pages()
pageblock_unisolate_and_move_free_pages()
Zi Yan Sept. 2, 2024, 3:30 p.m. UTC | #2 
On 2 Sep 2024, at 10:42, David Hildenbrand wrote:

> On 28.08.24 22:22, Zi Yan wrote:
>> Since migratetype is no longer overwritten during pageblock isolation,
>> moving pageblocks to and from MIGRATE_ISOLATE do not need migratetype.
>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> ---
>>   include/linux/page-isolation.h |  3 +--
>>   mm/page_alloc.c                | 27 +++++++++++++++++++++------
>>   mm/page_isolation.c            | 19 +++++++++----------
>>   3 files changed, 31 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
>> index 11b8695115ea..6a62401410c3 100644
>> --- a/include/linux/page-isolation.h
>> +++ b/include/linux/page-isolation.h
>> @@ -35,8 +35,7 @@ static inline bool is_migrate_isolate(int migratetype)
>>    void set_pageblock_migratetype(struct page *page, int migratetype);
>>  -bool move_freepages_block_isolate(struct zone *zone, struct page *page,
>> -				  int migratetype);
>> +bool move_freepages_block_isolate(struct zone *zone, struct page *page);
>>    int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
>>   			     int migratetype, int flags, gfp_t gfp_flags);
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 4ea5cd1a07e2..dc7c36461953 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -1764,10 +1764,12 @@ static unsigned long find_large_buddy(unsigned long start_pfn)
>>    *
>>    * Returns %true if pages could be moved, %false otherwise.
>>    */
>> -bool move_freepages_block_isolate(struct zone *zone, struct page *page,
>> -				  int migratetype)
>> +bool move_freepages_block_isolate(struct zone *zone, struct page *page)
>>   {
>>   	unsigned long start_pfn, pfn;
>> +	bool is_block_isolated = get_pageblock_isolate(page);
>> +	int from_mt;
>
> I think we should have two functions, one that isolates, another one that un-isolates. Or at least make the semantics not depend on the current state of the pageblock.
>
> bool pageblock_set_isolated_and_move_free_pages(struct zone *zone, struct page *page, bool isolated);
>
> vs.
>
> pageblock_isolate_and_move_free_pages()
> pageblock_unisolate_and_move_free_pages()

Sure. Will do.

--
Best Regards,
Yan, Zi
diff mbox series

Patch

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 11b8695115ea..6a62401410c3 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -35,8 +35,7 @@  static inline bool is_migrate_isolate(int migratetype)
 
 void set_pageblock_migratetype(struct page *page, int migratetype);
 
-bool move_freepages_block_isolate(struct zone *zone, struct page *page,
-				  int migratetype);
+bool move_freepages_block_isolate(struct zone *zone, struct page *page);
 
 int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 			     int migratetype, int flags, gfp_t gfp_flags);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4ea5cd1a07e2..dc7c36461953 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1764,10 +1764,12 @@  static unsigned long find_large_buddy(unsigned long start_pfn)
  *
  * Returns %true if pages could be moved, %false otherwise.
  */
-bool move_freepages_block_isolate(struct zone *zone, struct page *page,
-				  int migratetype)
+bool move_freepages_block_isolate(struct zone *zone, struct page *page)
 {
 	unsigned long start_pfn, pfn;
+	bool is_block_isolated = get_pageblock_isolate(page);
+	int from_mt;
+	int to_mt;
 
 	if (!prep_move_freepages_block(zone, page, &start_pfn, NULL, NULL))
 		return false;
@@ -1784,7 +1786,10 @@  bool move_freepages_block_isolate(struct zone *zone, struct page *page,
 
 		del_page_from_free_list(buddy, zone, order,
 					get_pfnblock_migratetype(buddy, pfn));
-		set_pageblock_migratetype(page, migratetype);
+		if (is_block_isolated)
+			clear_pageblock_isolate(page);
+		else
+			set_pageblock_isolate(page);
 		split_large_buddy(zone, buddy, pfn, order, FPI_NONE);
 		return true;
 	}
@@ -1795,14 +1800,24 @@  bool move_freepages_block_isolate(struct zone *zone, struct page *page,
 
 		del_page_from_free_list(page, zone, order,
 					get_pfnblock_migratetype(page, pfn));
-		set_pageblock_migratetype(page, migratetype);
+		if (is_block_isolated)
+			clear_pageblock_isolate(page);
+		else
+			set_pageblock_isolate(page);
 		split_large_buddy(zone, page, pfn, order, FPI_NONE);
 		return true;
 	}
 move:
+	if (is_block_isolated)
+		clear_pageblock_isolate(page);
+
+	from_mt = is_block_isolated ? MIGRATE_ISOLATE : get_pageblock_migratetype(page);
+	to_mt = is_block_isolated ? get_pageblock_migratetype(page) : MIGRATE_ISOLATE;
+
+	if (!is_block_isolated)
+		set_pageblock_isolate(page);
 	__move_freepages_block(zone, start_pfn,
-			       get_pfnblock_migratetype(page, start_pfn),
-			       migratetype);
+			       from_mt, to_mt);
 	return true;
 }
 #endif /* CONFIG_MEMORY_ISOLATION */
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 7e04047977cf..3ffdfddbdd50 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -181,7 +181,7 @@  static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
 	unmovable = has_unmovable_pages(check_unmovable_start, check_unmovable_end,
 			migratetype, isol_flags);
 	if (!unmovable) {
-		if (!move_freepages_block_isolate(zone, page, MIGRATE_ISOLATE)) {
+		if (!move_freepages_block_isolate(zone, page)) {
 			spin_unlock_irqrestore(&zone->lock, flags);
 			return -EBUSY;
 		}
@@ -202,7 +202,7 @@  static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
 	return -EBUSY;
 }
 
-static void unset_migratetype_isolate(struct page *page, int migratetype)
+static void unset_migratetype_isolate(struct page *page)
 {
 	struct zone *zone;
 	unsigned long flags;
@@ -255,10 +255,10 @@  static void unset_migratetype_isolate(struct page *page, int migratetype)
 		 * Isolating this block already succeeded, so this
 		 * should not fail on zone boundaries.
 		 */
-		WARN_ON_ONCE(!move_freepages_block_isolate(zone, page, migratetype));
+		WARN_ON_ONCE(!move_freepages_block_isolate(zone, page));
 	} else {
-		set_pageblock_migratetype(page, migratetype);
-		__putback_isolated_page(page, order, migratetype);
+		clear_pageblock_isolate(page);
+		__putback_isolated_page(page, order, get_pageblock_migratetype(page));
 	}
 	zone->nr_isolate_pageblock--;
 out:
@@ -428,7 +428,7 @@  static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
 failed:
 	/* restore the original migratetype */
 	if (!skip_isolation)
-		unset_migratetype_isolate(pfn_to_page(isolate_pageblock), migratetype);
+		unset_migratetype_isolate(pfn_to_page(isolate_pageblock));
 	return -EBUSY;
 }
 
@@ -501,7 +501,7 @@  int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 	ret = isolate_single_pageblock(isolate_end, flags, gfp_flags, true,
 			skip_isolation, migratetype);
 	if (ret) {
-		unset_migratetype_isolate(pfn_to_page(isolate_start), migratetype);
+		unset_migratetype_isolate(pfn_to_page(isolate_start));
 		return ret;
 	}
 
@@ -514,8 +514,7 @@  int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 					start_pfn, end_pfn)) {
 			undo_isolate_page_range(isolate_start, pfn, migratetype);
 			unset_migratetype_isolate(
-				pfn_to_page(isolate_end - pageblock_nr_pages),
-				migratetype);
+				pfn_to_page(isolate_end - pageblock_nr_pages));
 			return -EBUSY;
 		}
 	}
@@ -545,7 +544,7 @@  void undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 		page = __first_valid_page(pfn, pageblock_nr_pages);
 		if (!page || !is_migrate_isolate_page(page))
 			continue;
-		unset_migratetype_isolate(page, migratetype);
+		unset_migratetype_isolate(page);
 	}
 }
 /*