Re: [PATCH v11 4/6] mm: page_isolation: enable arbitrary range page isolation.

From: Zi Yan
Date: Tue May 24 2022 - 15:06:48 EST

Next message: William McVicker: "Bug when mapping designware PCIe MSI msg"
Previous message: Yosry Ahmed: "Re: [PATCH] memcg: provide reclaim stats via 'memory.reclaim'"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 25 Apr 2022, at 10:31, Zi Yan wrote:

> From: Zi Yan <ziy@xxxxxxxxxx>
>
> Now start_isolate_page_range() is ready to handle arbitrary range
> isolation, so move the alignment check/adjustment into the function body.
> Do the same for its counterpart undo_isolate_page_range().
> alloc_contig_range(), its caller, can pass an arbitrary range instead of
> a MAX_ORDER_NR_PAGES aligned one.
>
> Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
> ---
> mm/page_alloc.c | 16 ++--------------
> mm/page_isolation.c | 33 ++++++++++++++++-----------------
> 2 files changed, 18 insertions(+), 31 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 70ddd9a0bcf3..a002cf12eb6c 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -8924,16 +8924,6 @@ void *__init alloc_large_system_hash(const char *tablename,
> }
>
> #ifdef CONFIG_CONTIG_ALLOC
> -static unsigned long pfn_max_align_down(unsigned long pfn)
> -{
> - return ALIGN_DOWN(pfn, MAX_ORDER_NR_PAGES);
> -}
> -
> -static unsigned long pfn_max_align_up(unsigned long pfn)
> -{
> - return ALIGN(pfn, MAX_ORDER_NR_PAGES);
> -}
> -
> #if defined(CONFIG_DYNAMIC_DEBUG) || \
> (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE))
> /* Usage: See admin-guide/dynamic-debug-howto.rst */
> @@ -9075,8 +9065,7 @@ int alloc_contig_range(unsigned long start, unsigned long end,
> * put back to page allocator so that buddy can use them.
> */
>
> - ret = start_isolate_page_range(pfn_max_align_down(start),
> - pfn_max_align_up(end), migratetype, 0, gfp_mask);
> + ret = start_isolate_page_range(start, end, migratetype, 0, gfp_mask);
> if (ret)
> goto done;
>
> @@ -9157,8 +9146,7 @@ int alloc_contig_range(unsigned long start, unsigned long end,
> free_contig_range(end, outer_end - end);
>
> done:
> - undo_isolate_page_range(pfn_max_align_down(start),
> - pfn_max_align_up(end), migratetype);
> + undo_isolate_page_range(start, end, migratetype);
> return ret;
> }
> EXPORT_SYMBOL(alloc_contig_range);
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index 94b3467e5ba2..75e454f5cf45 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -435,7 +435,6 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags,
> * be MIGRATE_ISOLATE.
> * @start_pfn: The lower PFN of the range to be isolated.
> * @end_pfn: The upper PFN of the range to be isolated.
> - * start_pfn/end_pfn must be aligned to pageblock_order.
> * @migratetype: Migrate type to set in error recovery.
> * @flags: The following flags are allowed (they can be combined in
> * a bit mask)
> @@ -482,33 +481,33 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
> {
> unsigned long pfn;
> struct page *page;
> + /* isolation is done at page block granularity */
> + unsigned long isolate_start = ALIGN_DOWN(start_pfn, pageblock_nr_pages);
> + unsigned long isolate_end = ALIGN(end_pfn, pageblock_nr_pages);
> int ret;
>
> - BUG_ON(!IS_ALIGNED(start_pfn, pageblock_nr_pages));
> - BUG_ON(!IS_ALIGNED(end_pfn, pageblock_nr_pages));
> -
> - /* isolate [start_pfn, start_pfn + pageblock_nr_pages) pageblock */
> - ret = isolate_single_pageblock(start_pfn, gfp_flags, false);
> + /* isolate [isolate_start, isolate_start + pageblock_nr_pages) pageblock */
> + ret = isolate_single_pageblock(isolate_start, gfp_flags, false);
> if (ret)
> return ret;
>
> - /* isolate [end_pfn - pageblock_nr_pages, end_pfn) pageblock */
> - ret = isolate_single_pageblock(end_pfn, gfp_flags, true);
> + /* isolate [isolate_end - pageblock_nr_pages, isolate_end) pageblock */
> + ret = isolate_single_pageblock(isolate_end, gfp_flags, true);
> if (ret) {
> - unset_migratetype_isolate(pfn_to_page(start_pfn), migratetype);
> + unset_migratetype_isolate(pfn_to_page(isolate_start), migratetype);
> return ret;
> }
>
> /* skip isolated pageblocks at the beginning and end */
> - for (pfn = start_pfn + pageblock_nr_pages;
> - pfn < end_pfn - pageblock_nr_pages;
> + for (pfn = isolate_start + pageblock_nr_pages;
> + pfn < isolate_end - pageblock_nr_pages;
> pfn += pageblock_nr_pages) {
> page = __first_valid_page(pfn, pageblock_nr_pages);
> if (page && set_migratetype_isolate(page, migratetype, flags,
> start_pfn, end_pfn)) {
> - undo_isolate_page_range(start_pfn, pfn, migratetype);
> + undo_isolate_page_range(isolate_start, pfn, migratetype);
> unset_migratetype_isolate(
> - pfn_to_page(end_pfn - pageblock_nr_pages),
> + pfn_to_page(isolate_end - pageblock_nr_pages),
> migratetype);
> return -EBUSY;
> }
> @@ -524,12 +523,12 @@ void undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
> {
> unsigned long pfn;
> struct page *page;
> + unsigned long isolate_start = ALIGN_DOWN(start_pfn, pageblock_nr_pages);
> + unsigned long isolate_end = ALIGN(end_pfn, pageblock_nr_pages);
>
> - BUG_ON(!IS_ALIGNED(start_pfn, pageblock_nr_pages));
> - BUG_ON(!IS_ALIGNED(end_pfn, pageblock_nr_pages));
>
> - for (pfn = start_pfn;
> - pfn < end_pfn;
> + for (pfn = isolate_start;
> + pfn < isolate_end;
> pfn += pageblock_nr_pages) {
> page = __first_valid_page(pfn, pageblock_nr_pages);
> if (!page || !is_migrate_isolate_page(page))
> --
> 2.35.1

The fixup patch below should be applied to address the infinite loop issue reported by Qian Cai:

diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 46cbc4621d84..b70a03d9c52b 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -520,12 +520,12 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
int ret;

/* isolate [isolate_start, isolate_start + pageblock_nr_pages) pageblock */
- ret = isolate_single_pageblock(isolate_start, gfp_flags, false);
+ ret = isolate_single_pageblock(isolate_start, flags, gfp_flags, false);
if (ret)
return ret;

/* isolate [isolate_end - pageblock_nr_pages, isolate_end) pageblock */
- ret = isolate_single_pageblock(isolate_end, gfp_flags, true);
+ ret = isolate_single_pageblock(isolate_end, flags, gfp_flags, true);
if (ret) {
unset_migratetype_isolate(pfn_to_page(isolate_start), migratetype);
return ret;

The complete commit with the fixup patch applied is:

From 211ef82d35d3a0cf108846a440145688f7cfa21f Mon Sep 17 00:00:00 2001
From: Zi Yan <ziy@xxxxxxxxxx>
Date: Thu, 12 May 2022 20:22:58 -0700
Subject: [PATCH] mm: page_isolation: enable arbitrary range page
isolation.

Now start_isolate_page_range() is ready to handle arbitrary range
isolation, so move the alignment check/adjustment into the function body.
Do the same for its counterpart undo_isolate_page_range().
alloc_contig_range(), its caller, can pass an arbitrary range instead of a
MAX_ORDER_NR_PAGES aligned one.

Link: https://lkml.kernel.org/r/20220425143118.2850746-5-zi.yan@xxxxxxxx
Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
Cc: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
Cc: David Hildenbrand <david@xxxxxxxxxx>
Cc: Eric Ren <renzhengeek@xxxxxxxxx>
Cc: kernel test robot <lkp@xxxxxxxxx>
Cc: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx>
Cc: Mike Rapoport <rppt@xxxxxxxxxxxxx>
Cc: Minchan Kim <minchan@xxxxxxxxxx>
Cc: Oscar Salvador <osalvador@xxxxxxx>
Cc: Vlastimil Babka <vbabka@xxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
mm/page_alloc.c | 16 ++--------------
mm/page_isolation.c | 33 ++++++++++++++++-----------------
2 files changed, 18 insertions(+), 31 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 76551933bb1d..9a21ea9af35c 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8959,16 +8959,6 @@ void *__init alloc_large_system_hash(const char *tablename,
}

#ifdef CONFIG_CONTIG_ALLOC
-static unsigned long pfn_max_align_down(unsigned long pfn)
-{
- return ALIGN_DOWN(pfn, MAX_ORDER_NR_PAGES);
-}
-
-static unsigned long pfn_max_align_up(unsigned long pfn)
-{
- return ALIGN(pfn, MAX_ORDER_NR_PAGES);
-}
-
#if defined(CONFIG_DYNAMIC_DEBUG) || \
(defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE))
/* Usage: See admin-guide/dynamic-debug-howto.rst */
@@ -9110,8 +9100,7 @@ int alloc_contig_range(unsigned long start, unsigned long end,
* put back to page allocator so that buddy can use them.
*/

- ret = start_isolate_page_range(pfn_max_align_down(start),
- pfn_max_align_up(end), migratetype, 0, gfp_mask);
+ ret = start_isolate_page_range(start, end, migratetype, 0, gfp_mask);
if (ret)
goto done;

@@ -9192,8 +9181,7 @@ int alloc_contig_range(unsigned long start, unsigned long end,
free_contig_range(end, outer_end - end);

done:
- undo_isolate_page_range(pfn_max_align_down(start),
- pfn_max_align_up(end), migratetype);
+ undo_isolate_page_range(start, end, migratetype);
return ret;
}
EXPORT_SYMBOL(alloc_contig_range);
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 6b47acaf51f3..706915c9a380 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -472,7 +472,6 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
* be MIGRATE_ISOLATE.
* @start_pfn: The lower PFN of the range to be isolated.
* @end_pfn: The upper PFN of the range to be isolated.
- * start_pfn/end_pfn must be aligned to pageblock_order.
* @migratetype: Migrate type to set in error recovery.
* @flags: The following flags are allowed (they can be combined in
* a bit mask)
@@ -519,33 +518,33 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
{
unsigned long pfn;
struct page *page;
+ /* isolation is done at page block granularity */
+ unsigned long isolate_start = ALIGN_DOWN(start_pfn, pageblock_nr_pages);
+ unsigned long isolate_end = ALIGN(end_pfn, pageblock_nr_pages);
int ret;

- BUG_ON(!IS_ALIGNED(start_pfn, pageblock_nr_pages));
- BUG_ON(!IS_ALIGNED(end_pfn, pageblock_nr_pages));
-
- /* isolate [start_pfn, start_pfn + pageblock_nr_pages) pageblock */
- ret = isolate_single_pageblock(start_pfn, flags, gfp_flags, false);
+ /* isolate [isolate_start, isolate_start + pageblock_nr_pages) pageblock */
+ ret = isolate_single_pageblock(isolate_start, flags, gfp_flags, false);
if (ret)
return ret;

- /* isolate [end_pfn - pageblock_nr_pages, end_pfn) pageblock */
- ret = isolate_single_pageblock(end_pfn, flags, gfp_flags, true);
+ /* isolate [isolate_end - pageblock_nr_pages, isolate_end) pageblock */
+ ret = isolate_single_pageblock(isolate_end, flags, gfp_flags, true);
if (ret) {
- unset_migratetype_isolate(pfn_to_page(start_pfn), migratetype);
+ unset_migratetype_isolate(pfn_to_page(isolate_start), migratetype);
return ret;
}

/* skip isolated pageblocks at the beginning and end */
- for (pfn = start_pfn + pageblock_nr_pages;
- pfn < end_pfn - pageblock_nr_pages;
+ for (pfn = isolate_start + pageblock_nr_pages;
+ pfn < isolate_end - pageblock_nr_pages;
pfn += pageblock_nr_pages) {
page = __first_valid_page(pfn, pageblock_nr_pages);
if (page && set_migratetype_isolate(page, migratetype, flags,
start_pfn, end_pfn)) {
- undo_isolate_page_range(start_pfn, pfn, migratetype);
+ undo_isolate_page_range(isolate_start, pfn, migratetype);
unset_migratetype_isolate(
- pfn_to_page(end_pfn - pageblock_nr_pages),
+ pfn_to_page(isolate_end - pageblock_nr_pages),
migratetype);
return -EBUSY;
}
@@ -561,12 +560,12 @@ void undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
{
unsigned long pfn;
struct page *page;
+ unsigned long isolate_start = ALIGN_DOWN(start_pfn, pageblock_nr_pages);
+ unsigned long isolate_end = ALIGN(end_pfn, pageblock_nr_pages);

- BUG_ON(!IS_ALIGNED(start_pfn, pageblock_nr_pages));
- BUG_ON(!IS_ALIGNED(end_pfn, pageblock_nr_pages));

- for (pfn = start_pfn;
- pfn < end_pfn;
+ for (pfn = isolate_start;
+ pfn < isolate_end;
pfn += pageblock_nr_pages) {
page = __first_valid_page(pfn, pageblock_nr_pages);
if (!page || !is_migrate_isolate_page(page))
--
2.35.1

--
Best Regards,
Yan, Zi

Attachment: signature.asc
Description: OpenPGP digital signature

Next message: William McVicker: "Bug when mapping designware PCIe MSI msg"
Previous message: Yosry Ahmed: "Re: [PATCH] memcg: provide reclaim stats via 'memory.reclaim'"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]