Re: [PATCH v4 2/4] mm,compaction: Let isolate_migratepages_{range,block} return error codes
On 3/10/21 4:08 PM, Oscar Salvador wrote:
> Currently, isolate_migratepages_{range,block} and their callers use
> a pfn == 0 vs pfn != 0 scheme to let the caller know whether there was
> any error during isolation.
> This does not work as soon as we need to start reporting different error
> codes and make sure we pass them down the chain, so they are properly
> interpreted by functions like e.g: alloc_contig_range.
>
> Let us rework isolate_migratepages_{range,block} so we can report error
> codes.
> Since isolate_migratepages_block will stop returning the next pfn to be
> scanned, we reuse the cc->migrate_pfn field to keep track of that.
>
> Signed-off-by: Oscar Salvador
Acked-by: Vlastimil Babka
> ---
> mm/compaction.c | 48
> mm/internal.h | 2 +-
> mm/page_alloc.c | 7 +++
> 3 files changed, 28 insertions(+), 29 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index e04f4476e68e..5769753a8f60 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -787,15 +787,16 @@ static bool too_many_isolated(pg_data_t *pgdat)
> *
> * Isolate all pages that can be migrated from the range specified by
> * [low_pfn, end_pfn). The range is expected to be within same pageblock.
> - * Returns zero if there is a fatal signal pending, otherwise PFN of the
> - * first page that was not scanned (which may be both less, equal to or more
> - * than end_pfn).
> + * Returns -EINTR in case we need to abort when we have too many isolated
> pages
> + * due to e.g: signal pending, async mode or having still pages to migrate,
> or 0.
> + * cc->migrate_pfn will contain the next pfn to scan (which may be both less,
> + * equal to or more that end_pfn).
> *
> * The pages are isolated on cc->migratepages list (not required to be
> empty),
> * and cc->nr_migratepages is updated accordingly. The cc->migrate_pfn field
> * is neither read nor updated.
> */
> -static unsigned long
> +static int
> isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
> unsigned long end_pfn, isolate_mode_t isolate_mode)
> {
> @@ -810,6 +811,8 @@ isolate_migratepages_block(struct compact_control *cc,
> unsigned long low_pfn,
> unsigned long next_skip_pfn = 0;
> bool skip_updated = false;
>
> + cc->migrate_pfn = low_pfn;
> +
> /*
>* Ensure that there are not too many pages isolated from the LRU
>* list by either parallel reclaimers or compaction. If there are,
> @@ -818,16 +821,16 @@ isolate_migratepages_block(struct compact_control *cc,
> unsigned long low_pfn,
> while (unlikely(too_many_isolated(pgdat))) {
> /* stop isolation if there are still pages not migrated */
> if (cc->nr_migratepages)
> - return 0;
> + return -EINTR;
>
> /* async migration should just abort */
> if (cc->mode == MIGRATE_ASYNC)
> - return 0;
> + return -EINTR;
>
> congestion_wait(BLK_RW_ASYNC, HZ/10);
>
> if (fatal_signal_pending(current))
> - return 0;
> + return -EINTR;
> }
>
> cond_resched();
> @@ -1130,7 +1133,9 @@ isolate_migratepages_block(struct compact_control *cc,
> unsigned long low_pfn,
> if (nr_isolated)
> count_compact_events(COMPACTISOLATED, nr_isolated);
>
> - return low_pfn;
> + cc->migrate_pfn = low_pfn;
> +
> + return 0;
> }
>
> /**
> @@ -1139,15 +1144,15 @@ isolate_migratepages_block(struct compact_control
> *cc, unsigned long low_pfn,
> * @start_pfn: The first PFN to start isolating.
> * @end_pfn: The one-past-last PFN.
> *
> - * Returns zero if isolation fails fatally due to e.g. pending signal.
> - * Otherwise, function returns one-past-the-last PFN of isolated page
> - * (which may be greater than end_pfn if end fell in a middle of a THP page).
> + * Returns -EINTR in case isolation fails fatally due to e.g. pending signal,
> + * or 0.
> */
> -unsigned long
> +int
> isolate_migratepages_range(struct compact_control *cc, unsigned long
> start_pfn,
> unsigned long end_pfn)
> {
> unsigned long pfn, block_start_pfn, block_end_pfn;
> + int ret = 0;
>
> /* Scan block by block. First and last block may be incomplete */
> pfn = start_pfn;
> @@ -1166,17 +1171,17 @@ isolate_migratepages_range(struct compact_control
> *cc, unsigned long start_pfn,
> block_end_pfn, cc->zone))
> continue;
>
> - pfn = isolate_migratepages_block(cc, pfn, block_end_pfn,
> - ISOLATE_UNEVICTABLE);
> + ret = isolate_migratepages_block(cc, pfn, block_end_pfn,
> +
[PATCH v4 2/4] mm,compaction: Let isolate_migratepages_{range,block} return error codes
Currently, isolate_migratepages_{range,block} and their callers use
a pfn == 0 vs pfn != 0 scheme to let the caller know whether there was
any error during isolation.
This does not work as soon as we need to start reporting different error
codes and make sure we pass them down the chain, so they are properly
interpreted by functions like e.g: alloc_contig_range.
Let us rework isolate_migratepages_{range,block} so we can report error
codes.
Since isolate_migratepages_block will stop returning the next pfn to be
scanned, we reuse the cc->migrate_pfn field to keep track of that.
Signed-off-by: Oscar Salvador
---
mm/compaction.c | 48
mm/internal.h | 2 +-
mm/page_alloc.c | 7 +++
3 files changed, 28 insertions(+), 29 deletions(-)
diff --git a/mm/compaction.c b/mm/compaction.c
index e04f4476e68e..5769753a8f60 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -787,15 +787,16 @@ static bool too_many_isolated(pg_data_t *pgdat)
*
* Isolate all pages that can be migrated from the range specified by
* [low_pfn, end_pfn). The range is expected to be within same pageblock.
- * Returns zero if there is a fatal signal pending, otherwise PFN of the
- * first page that was not scanned (which may be both less, equal to or more
- * than end_pfn).
+ * Returns -EINTR in case we need to abort when we have too many isolated pages
+ * due to e.g: signal pending, async mode or having still pages to migrate, or
0.
+ * cc->migrate_pfn will contain the next pfn to scan (which may be both less,
+ * equal to or more that end_pfn).
*
* The pages are isolated on cc->migratepages list (not required to be empty),
* and cc->nr_migratepages is updated accordingly. The cc->migrate_pfn field
* is neither read nor updated.
*/
-static unsigned long
+static int
isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
unsigned long end_pfn, isolate_mode_t isolate_mode)
{
@@ -810,6 +811,8 @@ isolate_migratepages_block(struct compact_control *cc,
unsigned long low_pfn,
unsigned long next_skip_pfn = 0;
bool skip_updated = false;
+ cc->migrate_pfn = low_pfn;
+
/*
* Ensure that there are not too many pages isolated from the LRU
* list by either parallel reclaimers or compaction. If there are,
@@ -818,16 +821,16 @@ isolate_migratepages_block(struct compact_control *cc,
unsigned long low_pfn,
while (unlikely(too_many_isolated(pgdat))) {
/* stop isolation if there are still pages not migrated */
if (cc->nr_migratepages)
- return 0;
+ return -EINTR;
/* async migration should just abort */
if (cc->mode == MIGRATE_ASYNC)
- return 0;
+ return -EINTR;
congestion_wait(BLK_RW_ASYNC, HZ/10);
if (fatal_signal_pending(current))
- return 0;
+ return -EINTR;
}
cond_resched();
@@ -1130,7 +1133,9 @@ isolate_migratepages_block(struct compact_control *cc,
unsigned long low_pfn,
if (nr_isolated)
count_compact_events(COMPACTISOLATED, nr_isolated);
- return low_pfn;
+ cc->migrate_pfn = low_pfn;
+
+ return 0;
}
/**
@@ -1139,15 +1144,15 @@ isolate_migratepages_block(struct compact_control *cc,
unsigned long low_pfn,
* @start_pfn: The first PFN to start isolating.
* @end_pfn: The one-past-last PFN.
*
- * Returns zero if isolation fails fatally due to e.g. pending signal.
- * Otherwise, function returns one-past-the-last PFN of isolated page
- * (which may be greater than end_pfn if end fell in a middle of a THP page).
+ * Returns -EINTR in case isolation fails fatally due to e.g. pending signal,
+ * or 0.
*/
-unsigned long
+int
isolate_migratepages_range(struct compact_control *cc, unsigned long start_pfn,
unsigned long end_pfn)
{
unsigned long pfn, block_start_pfn, block_end_pfn;
+ int ret = 0;
/* Scan block by block. First and last block may be incomplete */
pfn = start_pfn;
@@ -1166,17 +1171,17 @@ isolate_migratepages_range(struct compact_control *cc,
unsigned long start_pfn,
block_end_pfn, cc->zone))
continue;
- pfn = isolate_migratepages_block(cc, pfn, block_end_pfn,
- ISOLATE_UNEVICTABLE);
+ ret = isolate_migratepages_block(cc, pfn, block_end_pfn,
+ISOLATE_UNEVICTABLE);
- if (!pfn)
+ if (ret)
break;
if (cc->nr_migratepages >= COMPACT_CLUSTER_MAX)
break;
}
- return pfn;
+ return
