[PATCH v2 0/4] Fixes and cleanups to compaction

[PATCH v2 0/4] Fixes and cleanups to compaction

[Kemeng Shi]

Hi all, this series contains random fixes and cleanups to free page
isolation in compaction. This is based on another compact series[1].
More details can be found in respective patches. Thanks!

v1->v2:
-drop patch "mm/compaction: allow blockpfn outside of pageblock for high
order buddy page" no real issue will be fixed by this.
-Collect RVB from Baolin

Kemeng Shi (4):
  mm/compaction: set compact_cached_free_pfn correctly in 
    update_pageblock_skip
  mm/compaction: merge end_pfn boundary check in isolate_freepages_range
  mm/compaction: remove unnecessary cursor page in
    isolate_freepages_block
  mm/compaction: remove unnecessary "else continue" at end of loop in
    isolate_freepages_block

 mm/compaction.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

-- 
2.30.0

[PATCH v2 1/4] mm/compaction: set compact_cached_free_pfn correctly in update_pageblock_skip

[Kemeng Shi]

We will set skip to page block of block_start_pfn, it's more reasonable
to set compact_cached_free_pfn to page block before the block_start_pfn.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
 mm/compaction.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index fbf50a9e4d23..61f56cee3715 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1714,7 +1714,8 @@ static void isolate_freepages(struct compact_control *cc)
 
 		/* Update the skip hint if the full pageblock was scanned */
 		if (isolate_start_pfn == block_end_pfn)
-			update_pageblock_skip(cc, page, block_start_pfn);
+			update_pageblock_skip(cc, page, block_start_pfn -
+					      pageblock_nr_pages);
 
 		/* Are enough freepages isolated? */
 		if (cc->nr_freepages >= cc->nr_migratepages) {
-- 
2.30.0

[PATCH v2 2/4] mm/compaction: merge end_pfn boundary check in isolate_freepages_range

[Kemeng Shi]

From: Kemeng Shi <shikemeng@huawei.com>

Merge the end_pfn boundary check for single page block forward and multiple
page blocks forward to avoid do twice boundary check for multiple page
blocks forward.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
 mm/compaction.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index 61f56cee3715..b7a05b9843ed 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -735,8 +735,6 @@ isolate_freepages_range(struct compact_control *cc,
 		/* Protect pfn from changing by isolate_freepages_block */
 		unsigned long isolate_start_pfn = pfn;
 
-		block_end_pfn = min(block_end_pfn, end_pfn);
-
 		/*
 		 * pfn could pass the block_end_pfn if isolated freepage
 		 * is more than pageblock order. In this case, we adjust
@@ -745,9 +743,10 @@ isolate_freepages_range(struct compact_control *cc,
 		if (pfn >= block_end_pfn) {
 			block_start_pfn = pageblock_start_pfn(pfn);
 			block_end_pfn = pageblock_end_pfn(pfn);
-			block_end_pfn = min(block_end_pfn, end_pfn);
 		}
 
+		block_end_pfn = min(block_end_pfn, end_pfn);
+
 		if (!pageblock_pfn_to_page(block_start_pfn,
 					block_end_pfn, cc->zone))
 			break;
-- 
2.30.0

[PATCH v2 3/4] mm/compaction: remove unnecessary cursor page in isolate_freepages_block

[Kemeng Shi]

The cursor is only used for page forward currently. We can simply move page
forward directly to remove unnecessary cursor.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
 mm/compaction.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index b7a05b9843ed..cb1dd5c5bf3f 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -584,7 +584,7 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
 				bool strict)
 {
 	int nr_scanned = 0, total_isolated = 0;
-	struct page *cursor;
+	struct page *page;
 	unsigned long flags = 0;
 	bool locked = false;
 	unsigned long blockpfn = *start_pfn;
@@ -594,12 +594,11 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
 	if (strict)
 		stride = 1;
 
-	cursor = pfn_to_page(blockpfn);
+	page = pfn_to_page(blockpfn);
 
 	/* Isolate free pages. */
-	for (; blockpfn < end_pfn; blockpfn += stride, cursor += stride) {
+	for (; blockpfn < end_pfn; blockpfn += stride, page += stride) {
 		int isolated;
-		struct page *page = cursor;
 
 		/*
 		 * Periodically drop the lock (if held) regardless of its
@@ -624,7 +623,7 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
 
 			if (likely(order <= MAX_ORDER)) {
 				blockpfn += (1UL << order) - 1;
-				cursor += (1UL << order) - 1;
+				page += (1UL << order) - 1;
 				nr_scanned += (1UL << order) - 1;
 			}
 			goto isolate_fail;
@@ -661,7 +660,7 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
 		}
 		/* Advance to the end of split page */
 		blockpfn += isolated - 1;
-		cursor += isolated - 1;
+		page += isolated - 1;
 		continue;
 
 isolate_fail:
-- 
2.30.0

[PATCH v2 4/4] mm/compaction: remove unnecessary "else continue" at end of loop in isolate_freepages_block

[Kemeng Shi]

There is no behavior change to remove "else continue" code at end of scan loop.
Just remove it to make code cleaner.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
 mm/compaction.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index cb1dd5c5bf3f..006fc35bffa1 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -666,8 +666,6 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
 isolate_fail:
 		if (strict)
 			break;
-		else
-			continue;
 
 	}
 
-- 
2.30.0

Re: [PATCH v2 4/4] mm/compaction: remove unnecessary "else continue" at end of loop in isolate_freepages_block

[David Hildenbrand]

On 03.08.23 11:49, Kemeng Shi wrote:
> There is no behavior change to remove "else continue" code at end of scan loop.
> Just remove it to make code cleaner.
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> ---
>   mm/compaction.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/mm/compaction.c b/mm/compaction.c
> index cb1dd5c5bf3f..006fc35bffa1 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -666,8 +666,6 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
>   isolate_fail:
>   		if (strict)
>   			break;
> -		else
> -			continue;
>   
>   	}
>   

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb