[PATCH v2 4/7] mm/compaction: simplify pfn iteration in isolate_freepages_range

Kemeng Shi posted 7 patches 2 years, 3 months ago
There is a newer version of this series
[PATCH v2 4/7] mm/compaction: simplify pfn iteration in isolate_freepages_range
Posted by Kemeng Shi 2 years, 3 months ago
We call isolate_freepages_block in strict mode, continuous pages in
pageblock will be isolated if isolate_freepages_block successed.
Then pfn + isolated will point to start of next pageblock to scan
no matter how many pageblocks are isolated in isolate_freepages_block.
Use pfn + isolated as start of next pageblock to scan to simplify the
iteration.

The pfn + isolated always points to start of next pageblock as:
In case isolated buddy page has order higher than pageblock:
1. page in buddy page is aligned with it's order
2. order of page is higher than pageblock order
Then page is aligned with pageblock order. So pfn of page and isolated
pages count are both aligned pageblock order. So pfn + isolated is
pageblock order aligned.

In case isolated buddy page has order lower than pageblock:
Buddy page with order N contains two order N - 1 pages as following:
|        order N        |
|order N - 1|order N - 1|
So buddy pages with order N - 1 will never cross boudary of order N.
Similar, buddy pages with order N - 2 will never cross boudary of order
N - 1 and so on. Then any pages with order less than pageblock order
will never crosa boudary of pageblock.

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

diff --git a/mm/compaction.c b/mm/compaction.c
index b4d03c9ffe7c..2937e754cfb7 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -739,21 +739,11 @@ isolate_freepages_range(struct compact_control *cc,
 	block_end_pfn = pageblock_end_pfn(pfn);
 
 	for (; pfn < end_pfn; pfn += isolated,
-				block_start_pfn = block_end_pfn,
-				block_end_pfn += pageblock_nr_pages) {
+				block_start_pfn = pfn,
+				block_end_pfn = pfn + pageblock_nr_pages) {
 		/* Protect pfn from changing by isolate_freepages_block */
 		unsigned long isolate_start_pfn = pfn;
 
-		/*
-		 * pfn could pass the block_end_pfn if isolated freepage
-		 * is more than pageblock order. In this case, we adjust
-		 * scanning range to right one.
-		 */
-		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);
 
 		if (!pageblock_pfn_to_page(block_start_pfn,
-- 
2.30.0
Re: [PATCH v2 4/7] mm/compaction: simplify pfn iteration in isolate_freepages_range
Posted by Mel Gorman 2 years, 3 months ago
On Sat, Aug 26, 2023 at 11:36:14PM +0800, Kemeng Shi wrote:
> We call isolate_freepages_block in strict mode, continuous pages in
> pageblock will be isolated if isolate_freepages_block successed.
> Then pfn + isolated will point to start of next pageblock to scan
> no matter how many pageblocks are isolated in isolate_freepages_block.
> Use pfn + isolated as start of next pageblock to scan to simplify the
> iteration.
> 
> The pfn + isolated always points to start of next pageblock as:
> In case isolated buddy page has order higher than pageblock:
> 1. page in buddy page is aligned with it's order
> 2. order of page is higher than pageblock order
> Then page is aligned with pageblock order. So pfn of page and isolated
> pages count are both aligned pageblock order. So pfn + isolated is
> pageblock order aligned.
> 
> In case isolated buddy page has order lower than pageblock:
> Buddy page with order N contains two order N - 1 pages as following:
> |        order N        |
> |order N - 1|order N - 1|
> So buddy pages with order N - 1 will never cross boudary of order N.
> Similar, buddy pages with order N - 2 will never cross boudary of order
> N - 1 and so on. Then any pages with order less than pageblock order
> will never crosa boudary of pageblock.
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

While I don't think the patch is wrong, I also don't think it
meaningfully simplifies the code or optimises enough to be justified.
Even though a branch is eliminated, the whole path is not cheap.

-- 
Mel Gorman
SUSE Labs
Re: [PATCH v2 4/7] mm/compaction: simplify pfn iteration in isolate_freepages_range
Posted by Kemeng Shi 2 years, 3 months ago

on 8/29/2023 11:01 PM, Mel Gorman wrote:
> On Sat, Aug 26, 2023 at 11:36:14PM +0800, Kemeng Shi wrote:
>> We call isolate_freepages_block in strict mode, continuous pages in
>> pageblock will be isolated if isolate_freepages_block successed.
>> Then pfn + isolated will point to start of next pageblock to scan
>> no matter how many pageblocks are isolated in isolate_freepages_block.
>> Use pfn + isolated as start of next pageblock to scan to simplify the
>> iteration.
>>
>> The pfn + isolated always points to start of next pageblock as:
>> In case isolated buddy page has order higher than pageblock:
>> 1. page in buddy page is aligned with it's order
>> 2. order of page is higher than pageblock order
>> Then page is aligned with pageblock order. So pfn of page and isolated
>> pages count are both aligned pageblock order. So pfn + isolated is
>> pageblock order aligned.
>>
>> In case isolated buddy page has order lower than pageblock:
>> Buddy page with order N contains two order N - 1 pages as following:
>> |        order N        |
>> |order N - 1|order N - 1|
>> So buddy pages with order N - 1 will never cross boudary of order N.
>> Similar, buddy pages with order N - 2 will never cross boudary of order
>> N - 1 and so on. Then any pages with order less than pageblock order
>> will never crosa boudary of pageblock.
>>
>> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
>> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> 
> While I don't think the patch is wrong, I also don't think it
> meaningfully simplifies the code or optimises enough to be justified.
> Even though a branch is eliminated, the whole path is not cheap.
> 
OK, I will drop this in next version if you insistant.