| Message ID | 20230729174354.2239980-2-shikemeng@huaweicloud.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fixes and cleanups to compaction | expand |
On Sun, 30 Jul 2023 01:43:50 +0800 Kemeng Shi <shikemeng@huaweicloud.com> wrote: > Commit 9fcd6d2e052ee ("mm, compaction: skip compound pages by order in > free scanner") skiped compound pages to save iterations and limit blockpfn > to reach outside of page block in case of bogus compound_order. While this > also limit pfnblock outside page block in case a buddy page with order > higher than page block is found. After this, isolate_freepages_range will > fail unexpectedly as it will fail to isolate the page block which was > isolated successfully by high order buddy page in previous page block > and abort the free page isolation. > > Fix this by allow blockpfn outside of pageblock in case of high order > buddy page. > > ... > > @@ -1418,7 +1420,7 @@ fast_isolate_around(struct compact_control *cc, unsigned long pfn) > isolate_freepages_block(cc, &start_pfn, end_pfn, &cc->freepages, 1, false); > > /* Skip this pageblock in the future as it's full or nearly full */ > - if (start_pfn == end_pfn && !cc->no_set_skip_hint) > + if (start_pfn >= end_pfn && !cc->no_set_skip_hint) > set_pageblock_skip(page); > } > This needed alteration for mm-unstable changes: @@ -1441,7 +1443,7 @@ fast_isolate_around(struct compact_contr isolate_freepages_block(cc, &start_pfn, end_pfn, &cc->freepages, 1, false); /* Skip this pageblock in the future as it's full or nearly full */ - if (start_pfn == end_pfn) + if (start_pfn >= end_pfn) set_pageblock_skip(page); return;
On 7/30/2023 1:43 AM, Kemeng Shi wrote: > Commit 9fcd6d2e052ee ("mm, compaction: skip compound pages by order in > free scanner") skiped compound pages to save iterations and limit blockpfn > to reach outside of page block in case of bogus compound_order. While this > also limit pfnblock outside page block in case a buddy page with order > higher than page block is found. After this, isolate_freepages_range will > fail unexpectedly as it will fail to isolate the page block which was > isolated successfully by high order buddy page in previous page block > and abort the free page isolation. Not sure I uderstand the problem, why the isolate_freepages_range() will fail? In isolate_freepages_range(), it did not use the 'blockpfn' cursor to try next candidate pfn, instead using the 'isolated' to calculate next cursor. Or I missed something else? > Fix this by allow blockpfn outside of pageblock in case of high order > buddy page. > > Fixes: 9fcd6d2e052ee ("mm, compaction: skip compound pages by order in free scanner") > Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com> > --- > mm/compaction.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/mm/compaction.c b/mm/compaction.c > index 6841c0496223..d1d28d57e5bd 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -681,8 +681,10 @@ static unsigned long isolate_freepages_block(struct compact_control *cc, > /* > * There is a tiny chance that we have read bogus compound_order(), > * so be careful to not go outside of the pageblock. > + * Allow blockpfn outside pageblock in normal case that we isolate > + * buddy page with order more than pageblock order. > */ > - if (unlikely(blockpfn > end_pfn)) > + if (unlikely(blockpfn > end_pfn) && total_isolated <= pageblock_nr_pages) > blockpfn = end_pfn; > > trace_mm_compaction_isolate_freepages(*start_pfn, blockpfn, > @@ -1418,7 +1420,7 @@ fast_isolate_around(struct compact_control *cc, unsigned long pfn) > isolate_freepages_block(cc, &start_pfn, end_pfn, &cc->freepages, 1, false); > > /* Skip this pageblock in the future as it's full or nearly full */ > - if (start_pfn == end_pfn && !cc->no_set_skip_hint) > + if (start_pfn >= end_pfn && !cc->no_set_skip_hint) > set_pageblock_skip(page); > } > > @@ -1687,7 +1689,7 @@ static void isolate_freepages(struct compact_control *cc) > block_end_pfn, freelist, stride, false); > > /* Update the skip hint if the full pageblock was scanned */ > - if (isolate_start_pfn == block_end_pfn) > + if (isolate_start_pfn >= block_end_pfn) > update_pageblock_skip(cc, page, block_start_pfn); > > /* Are enough freepages isolated? */
on 8/2/2023 9:57 AM, Baolin Wang wrote: > > > On 7/30/2023 1:43 AM, Kemeng Shi wrote: >> Commit 9fcd6d2e052ee ("mm, compaction: skip compound pages by order in >> free scanner") skiped compound pages to save iterations and limit blockpfn >> to reach outside of page block in case of bogus compound_order. While this >> also limit pfnblock outside page block in case a buddy page with order >> higher than page block is found. After this, isolate_freepages_range will >> fail unexpectedly as it will fail to isolate the page block which was >> isolated successfully by high order buddy page in previous page block >> and abort the free page isolation. > > Not sure I uderstand the problem, why the isolate_freepages_range() will fail? In isolate_freepages_range(), it did not use the 'blockpfn' cursor to try next candidate pfn, instead using the 'isolated' to calculate next cursor. Or I missed something else? > Ah, my bad. I will drop this in next version. >> Fix this by allow blockpfn outside of pageblock in case of high order >> buddy page. >> >> Fixes: 9fcd6d2e052ee ("mm, compaction: skip compound pages by order in free scanner") >> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com> >> --- >> mm/compaction.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/mm/compaction.c b/mm/compaction.c >> index 6841c0496223..d1d28d57e5bd 100644 >> --- a/mm/compaction.c >> +++ b/mm/compaction.c >> @@ -681,8 +681,10 @@ static unsigned long isolate_freepages_block(struct compact_control *cc, >> /* >> * There is a tiny chance that we have read bogus compound_order(), >> * so be careful to not go outside of the pageblock. >> + * Allow blockpfn outside pageblock in normal case that we isolate >> + * buddy page with order more than pageblock order. >> */ >> - if (unlikely(blockpfn > end_pfn)) >> + if (unlikely(blockpfn > end_pfn) && total_isolated <= pageblock_nr_pages) >> blockpfn = end_pfn; >> trace_mm_compaction_isolate_freepages(*start_pfn, blockpfn, >> @@ -1418,7 +1420,7 @@ fast_isolate_around(struct compact_control *cc, unsigned long pfn) >> isolate_freepages_block(cc, &start_pfn, end_pfn, &cc->freepages, 1, false); >> /* Skip this pageblock in the future as it's full or nearly full */ >> - if (start_pfn == end_pfn && !cc->no_set_skip_hint) >> + if (start_pfn >= end_pfn && !cc->no_set_skip_hint) >> set_pageblock_skip(page); >> } >> @@ -1687,7 +1689,7 @@ static void isolate_freepages(struct compact_control *cc) >> block_end_pfn, freelist, stride, false); >> /* Update the skip hint if the full pageblock was scanned */ >> - if (isolate_start_pfn == block_end_pfn) >> + if (isolate_start_pfn >= block_end_pfn) >> update_pageblock_skip(cc, page, block_start_pfn); >> /* Are enough freepages isolated? */ >
diff --git a/mm/compaction.c b/mm/compaction.c index 6841c0496223..d1d28d57e5bd 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -681,8 +681,10 @@ static unsigned long isolate_freepages_block(struct compact_control *cc, /* * There is a tiny chance that we have read bogus compound_order(), * so be careful to not go outside of the pageblock. + * Allow blockpfn outside pageblock in normal case that we isolate + * buddy page with order more than pageblock order. */ - if (unlikely(blockpfn > end_pfn)) + if (unlikely(blockpfn > end_pfn) && total_isolated <= pageblock_nr_pages) blockpfn = end_pfn; trace_mm_compaction_isolate_freepages(*start_pfn, blockpfn, @@ -1418,7 +1420,7 @@ fast_isolate_around(struct compact_control *cc, unsigned long pfn) isolate_freepages_block(cc, &start_pfn, end_pfn, &cc->freepages, 1, false); /* Skip this pageblock in the future as it's full or nearly full */ - if (start_pfn == end_pfn && !cc->no_set_skip_hint) + if (start_pfn >= end_pfn && !cc->no_set_skip_hint) set_pageblock_skip(page); } @@ -1687,7 +1689,7 @@ static void isolate_freepages(struct compact_control *cc) block_end_pfn, freelist, stride, false); /* Update the skip hint if the full pageblock was scanned */ - if (isolate_start_pfn == block_end_pfn) + if (isolate_start_pfn >= block_end_pfn) update_pageblock_skip(cc, page, block_start_pfn); /* Are enough freepages isolated? */
Commit 9fcd6d2e052ee ("mm, compaction: skip compound pages by order in free scanner") skiped compound pages to save iterations and limit blockpfn to reach outside of page block in case of bogus compound_order. While this also limit pfnblock outside page block in case a buddy page with order higher than page block is found. After this, isolate_freepages_range will fail unexpectedly as it will fail to isolate the page block which was isolated successfully by high order buddy page in previous page block and abort the free page isolation. Fix this by allow blockpfn outside of pageblock in case of high order buddy page. Fixes: 9fcd6d2e052ee ("mm, compaction: skip compound pages by order in free scanner") Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com> --- mm/compaction.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)