mm: compaction: avoid migrating non-cma pages to a cma area
Subsystems (3)
RFC PATCH 1/1 mm: compaction: avoid migrating non-cma pages to a cma area
Compaction does treat cma pageblocks on pair with any movable pageblocks. It means it can easily move non-cma pages into a cma zone.
It can create problems for the cma allocator.
The particular problem I'm looking at is related to btrfs metadata pages, which are allocated without __GFP_MOVABLE, but beside that are generic pagecache pages. In fact, they are sometimes movable and sometimes not, depending on whether they are dirty and also on the extent buffer reference counter.
Compaction moves them to the hugetlb_cma area, and then sometimes the cma allocator fails to move them back from the cma area. It results in failures of gigantic hugepages allocations.
Also in general cma areas are reserved close to the end of a zone, and it's where compaction tries to migrate pages. It means compaction will aggressively fill cma areas, which makes not much sense.
So to avoid it, let's preserve non-cma pages from being moved into a cma area. Because cma areas are usually quite large and the number of areas is small, it should not significantly affect the memory fragmentation.
--- mm/compaction.c | 6 ++++++ mm/internal.h | 1 + 2 files changed, 7 insertions(+)
mm/compaction.c +6 -0
@@ -1159,6 +1159,10 @@ static bool suitable_migration_target(struct compact_control *cc,return false; } +/* Do not bring pages non-cma pages into a cma area */ +if (is_migrate_cma (get_pageblock_migratetype (page)) && !cc->cma) +return false; +if (cc->ignore_block_suitable)return true;
@@ -1832,6 +1836,8 @@ static isolate_migrate_t isolate_migratepages(struct compact_control *cc)if (!low_pfn)return ISOLATE_ABORT ; + cc->cma =is_migrate_cma (get_pageblock_migratetype (page)); +/* * Either we isolated something and proceed with migration. Or * we failed and compact_zone should decide if we should
mm/internal.h +1 -0
@@ -232,6 +232,7 @@ struct compact_control {bool contended;/* Signal lock or sched contention */ bool rescan;/* Rescanning the same pageblock */ bool alloc_contig;/* alloc_contig_range allocation */ +bool cma;/* migratepages contains cma pages */ };/*
Signature · 1 line
2.25.1
Quoted 20 lines · Roman Gushchin
Compaction does treat cma pageblocks on pair with any movable pageblocks. It means it can easily move non-cma pages into a cma zone.
It can create problems for the cma allocator.
The particular problem I'm looking at is related to btrfs metadata pages, which are allocated without
__GFP_MOVABLE, but beside that are generic pagecache pages. In fact, they are sometimes movable and sometimes not, depending on whether they are dirty and also on the extent buffer reference counter.Compaction moves them to the
hugetlb_cmaarea, and then sometimes the cma allocator fails to move them back from the cma area. It results in failures of gigantic hugepages allocations.Also in general cma areas are reserved close to the end of a zone, and it's where compaction tries to migrate pages. It means compaction will aggressively fill cma areas, which makes not much sense.
Quoted 6 lines
So to avoid it, let's preserve non-cma pages from being moved into a cma area. Because cma areas are usually quite large and the number of areas is small, it should not significantly affect the memory fragmentation.
Signed-off-by Roman Gushchin <[email protected]>
Friendly ping... Any thoughts, comments, ideas?
Thanks!
Roman
Quoted 10 lines · Roman Gushchin
Compaction does treat cma pageblocks on pair with any movable pageblocks. It means it can easily move non-cma pages into a cma zone.
It can create problems for the cma allocator.
The particular problem I'm looking at is related to btrfs metadata pages, which are allocated without
__GFP_MOVABLE, but beside that are generic pagecache pages. In fact, they are sometimes movable and sometimes not, depending on whether they are dirty and also on the extent buffer reference counter.
Hm I think I'd rather make such pages really unmovable (by a pin?) than deny the whole CMA area to compaction. Would it be feasible?
Quoted 8 lines
Compaction moves them to the
hugetlb_cmaarea, and then sometimes the cma allocator fails to move them back from the cma area. It results in failures of gigantic hugepages allocations.Also in general cma areas are reserved close to the end of a zone, and it's where compaction tries to migrate pages. It means compaction will aggressively fill cma areas, which makes not much sense.
So now the free page scanner will have to skip those areas, which is not much effective. But I suspect a worse problem in __compaction_suitable() which will now falsely report that there are enough free pages, so compaction will start but fail to do anytning. Minimally the __zone_watermark_ok() check there would have to lose ALLOC_CMA, but there might be other similar checks that would need adjusting.
And long-term what happens if the "CMA using ZONE_MOVABLE" approach is merged and there are not more CMA migratetypes to test? Might this change actually also avoid your issue, as said pages without __GFP_MOVABLE won't end up in a ZONE_MOVABLE?
Quoted 48 lines
So to avoid it, let's preserve non-cma pages from being moved into a cma area. Because cma areas are usually quite large and the number of areas is small, it should not significantly affect the memory fragmentation.
Signed-off-by Roman Gushchin <[email protected]>--- mm/compaction.c | 6 ++++++ mm/internal.h | 1 + 2 files changed, 7 insertions(+)mm/compaction.c +6
-0@@ -1159,6 +1159,10 @@ static bool suitable_migration_target(struct compact_control *cc,return false; } +/* Do not bring pages non-cma pages into a cma area */ +if (is_migrate_cma (get_pageblock_migratetype (page)) && !cc->cma) +return false; +if (cc->ignore_block_suitable)return true;@@ -1832,6 +1836,8 @@ static isolate_migrate_t isolate_migratepages(struct compact_control *cc)if (!low_pfn)return ISOLATE_ABORT ; + cc->cma =is_migrate_cma (get_pageblock_migratetype (page)); +/* * Either we isolated something and proceed with migration. Or * we failed and compact_zone should decide if we should mm/internal.h +1
-0@@ -232,6 +232,7 @@ struct compact_control {bool contended;/* Signal lock or sched contention */ bool rescan;/* Rescanning the same pageblock */ bool alloc_contig;/* alloc_contig_range allocation */ +bool cma;/* migratepages contains cma pages */ };/*
Quoted 4 lines · Vlastimil Babka
Quoted 3 lines · Roman Gushchin
Compaction does treat cma pageblocks on pair with any movable pageblocks. It means it can easily move non-cma pages into a cma zone.
Quoted 8 lines
Quoted 7 lines
It can create problems for the cma allocator.
The particular problem I'm looking at is related to btrfs metadata pages, which are allocated without
__GFP_MOVABLE, but beside that are generic pagecache pages. In fact, they are sometimes movable and sometimes not, depending on whether they are dirty and also on the extent buffer reference counter.
Hello, Vlastimil!
Thank you for looking into it.
Quoted 2 lines
Hm I think I'd rather make such pages really unmovable (by a pin?) than deny the whole CMA area to compaction. Would it be feasible?
Well, it's an option too, however I'm not sure it's the best one. First, because these pages can be moved quite often, making them completely unmovable will make the compaction less efficient. Second, because it's not only about these pages, but in general about migrating pages into a cma area without a clear need.
As I wrote in the commit log, if a cma area is located close to end of a node (which seems to be default at least on x86 without a movable zone), compaction can fill it quite aggressively. And it might bring some hot pages (e.g. executable pagecache pages), which will cause cma allocation failures. I've seen something like this in our prod.
Quoted 10 lines
Quoted 9 lines
Compaction moves them to the
hugetlb_cmaarea, and then sometimes the cma allocator fails to move them back from the cma area. It results in failures of gigantic hugepages allocations.Also in general cma areas are reserved close to the end of a zone, and it's where compaction tries to migrate pages. It means compaction will aggressively fill cma areas, which makes not much sense.
Quoted 6 lines
So now the free page scanner will have to skip those areas, which is not much effective. But I suspect a worse problem in
__compaction_suitable()which will now falsely report that there are enough free pages, so compaction will start but fail to do anytning. Minimally the__zone_watermark_ok()check there would have to loseALLOC_CMA, but there might be other similar checks that would need adjusting.
A really good point! I've looked around for any other checks, but haven't found anything. Please, find an updated version of the patch below.
Quoted 5 lines
And long-term what happens if the "CMA using
ZONE_MOVABLE" approach is merged and there are not more CMA migratetypes to test? Might this change actually also avoid your issue, as said pages without__GFP_MOVABLEwon't end up in aZONE_MOVABLE?
Yeah, this is what I was thinking about. Basically I want to mimic this behavior right now. Once this approach will be implemented and merged, it will happen
automatically: obviously, compaction won't move pages between different zones.
Thank you!
--
From f4a0cfff41c7acab78116478e8e69ae8773b405c Mon Sep 17 00:00:00 2001
From: Roman Gushchin <[email protected]>
Date: Wed, 8 Apr 2020 11:16:38 -0700
Subject: [PATCH v2] mm: compaction: avoid migrating non-cma pages to a cma
area
Compaction does treat cma pageblocks on pair with any movable pageblocks. It means it can easily move non-cma pages into a cma zone.
It can create problems for the cma allocator.
The particular problem I'm looking at is related to btrfs metadata pages, which are allocated without __GFP_MOVABLE, but beside that are generic pagecache pages. In fact, they are sometimes movable and sometimes not, depending on whether they are dirty and also on the extent buffer reference counter.
Compaction moves them to the hugetlb_cma area, and then sometimes the cma allocator fails to move them back from the cma area. It results in failures of gigantic hugepages allocations.
Also in general cma areas are reserved close to the end of a zone, and it's where compaction tries to migrate pages. It means compaction will aggressively fill cma areas, which makes not much sense.
So to avoid it, let's preserve non-cma pages from being moved into a cma area. Because cma areas are usually quite large and the number of areas is small, it should not significantly affect the memory fragmentation.
v2:
1) adjusted the __zone_watermark_ok() check, suggested by Vlastimil Babka
2) fixed a typo in a comment
--- mm/compaction.c | 10 ++++++++-- mm/internal.h | 1 + 2 files changed, 9 insertions(+), 2 deletions(-)
mm/compaction.c +8 -2
@@ -1159,6 +1159,10 @@ static bool suitable_migration_target(struct compact_control *cc,return false; } +/* Do not bring non-cma pages into a cma area */ +if (is_migrate_cma (get_pageblock_migratetype (page)) && !cc->cma) +return false; +if (cc->ignore_block_suitable)return true;
@@ -1832,6 +1836,8 @@ static isolate_migrate_t isolate_migratepages(struct compact_control *cc)if (!low_pfn)return ISOLATE_ABORT ; + cc->cma =is_migrate_cma (get_pageblock_migratetype (page)); +/* * Either we isolated something and proceed with migration. Or * we failed and compact_zone should decide if we should
@@ -2000,8 +2006,8 @@ static enum compact_result __compaction_suitable(struct zone *zone, int order, watermark = (order >PAGE_ALLOC_COSTLY_ORDER ) ?low_wmark_pages (zone) :min_wmark_pages (zone); watermark +=compact_gap (order); -if (!__zone_watermark_ok (zone,0 , watermark, classzone_idx, -ALLOC_CMA , wmark_target)) +if (!__zone_watermark_ok (zone,0 , watermark, classzone_idx,0 , + wmark_target))return COMPACT_SKIPPED ;return COMPACT_CONTINUE ;
mm/internal.h +1 -0
@@ -232,6 +232,7 @@ struct compact_control {bool contended;/* Signal lock or sched contention */ bool rescan;/* Rescanning the same pageblock */ bool alloc_contig;/* alloc_contig_range allocation */ +bool cma;/* migratepages contains cma pages */ };/*
Signature · 1 line
2.25.2
Quoted 15 lines · Roman Gushchin
On Tue, Apr 14, 2020 at 01:49:45PM +0200, Vlastimil Babka wrote:
Hello, Vlastimil!
Thank you for looking into it.
Quoted 2 lines
Hm I think I'd rather make such pages really unmovable (by a pin?) than deny the whole CMA area to compaction. Would it be feasible?
Well, it's an option too, however I'm not sure it's the best one. First, because these pages can be moved quite often, making them completely unmovable will make the compaction less efficient. Second, because it's not only about these pages, but in general about migrating pages into a cma area without a clear need.
Quoted 5 lines
As I wrote in the commit log, if a cma area is located close to end of a node (which seems to be default at least on x86 without a movable zone), compaction can fill it quite aggressively. And it might bring some hot pages (e.g. executable pagecache pages), which will cause cma allocation failures. I've seen something like this in our prod.
Hmm, I see.
Quoted 17 lines
Quoted 10 lines
Quoted 9 lines
Compaction moves them to the
hugetlb_cmaarea, and then sometimes the cma allocator fails to move them back from the cma area. It results in failures of gigantic hugepages allocations.Also in general cma areas are reserved close to the end of a zone, and it's where compaction tries to migrate pages. It means compaction will aggressively fill cma areas, which makes not much sense.
Quoted 6 lines
So now the free page scanner will have to skip those areas, which is not much effective. But I suspect a worse problem in
__compaction_suitable()which will now falsely report that there are enough free pages, so compaction will start but fail to do anytning. Minimally the__zone_watermark_ok()check there would have to loseALLOC_CMA, but there might be other similar checks that would need adjusting.
Quoted 2 lines
A really good point! I've looked around for any other checks, but haven't found anything. Please, find an updated version of the patch below.
Technically there's also __isolate_free_page() using ALLOC_CMA for watermark check, but it's shared by compaction and alloc_contig_range(), so we can't just remove ALLOC_CMA from there. It's probably not worth to complicate it though. If we pass compaction_suitable() without ALLOC_CMA and then reach __isolate_free_page() and meanwhile watermarks changed so we wouldn't pass without ALLOC_CMA anymore, it should be rare hopefully and not cause us deplete non-CMA free pages too badly.
But I've only now also realized how dynamic setting cc->cma is. So in case a zone consists mostly of CMA blocks, removing ALLOC_CMA in __compaction_suitable() would be actually wrong and prevent compaction from doing any work? Sigh. Any idea about that?
Quoted 9 lines
Quoted 5 lines
And long-term what happens if the "CMA using
ZONE_MOVABLE" approach is merged and there are not more CMA migratetypes to test? Might this change actually also avoid your issue, as said pages without__GFP_MOVABLEwon't end up in aZONE_MOVABLE?Yeah, this is what I was thinking about. Basically I want to mimic this behavior right now. Once this approach will be implemented and merged, it will happen
automatically: obviously, compaction won't move pages between different zones.
That will be much better. Can't wait, then :)
Quoted 51 lines
Thank you!
--
From
f4a0cfff41c7acab78116478e8e69ae8773b405cMon Sep 17 00:00:00 2001
From: Roman Gushchin <[email protected]>
Date: Wed, 8 Apr 2020 11:16:38 -0700
Subject: [PATCH v2] mm: compaction: avoid migrating non-cma pages to a cma
areaCompaction does treat cma pageblocks on pair with any movable pageblocks. It means it can easily move non-cma pages into a cma zone.
It can create problems for the cma allocator.
The particular problem I'm looking at is related to btrfs metadata pages, which are allocated without
__GFP_MOVABLE, but beside that are generic pagecache pages. In fact, they are sometimes movable and sometimes not, depending on whether they are dirty and also on the extent buffer reference counter.Compaction moves them to the
hugetlb_cmaarea, and then sometimes the cma allocator fails to move them back from the cma area. It results in failures of gigantic hugepages allocations.Also in general cma areas are reserved close to the end of a zone, and it's where compaction tries to migrate pages. It means compaction will aggressively fill cma areas, which makes not much sense.
So to avoid it, let's preserve non-cma pages from being moved into a cma area. Because cma areas are usually quite large and the number of areas is small, it should not significantly affect the memory fragmentation.
v2:
1) adjusted the__zone_watermark_ok()check, suggested by Vlastimil Babka
2) fixed a typo in a commentSigned-off-by Roman Gushchin <[email protected]>>--- mm/compaction.c | 10 ++++++++-- mm/internal.h | 1 + 2 files changed, 9 insertions(+), 2 deletions(-)mm/compaction.c +0
-0
mm/compaction.c static bool suitable_migration_target(struct compact_control *cc, · line 1163
Quoted 5 lines
mm/compaction.c +2
-0@@ -1159,6 +1159,10 @@ static bool suitable_migration_target(struct compact_control *cc,return false; } +/* Do not bring non-cma pages into a cma area */ +if (is_migrate_cma (get_pageblock_migratetype (page)) && !cc->cma)
Nit: probably reverse this as the second test is cheaper?
Quoted 5 lines
mm/compaction.c +2
-0+return false; +if (cc->ignore_block_suitable)return true;
Quoted 52 lines · Vlastimil Babka
Quoted 15 lines · Roman Gushchin
On Tue, Apr 14, 2020 at 01:49:45PM +0200, Vlastimil Babka wrote:
Hello, Vlastimil!
Thank you for looking into it.
Quoted 2 lines
Hm I think I'd rather make such pages really unmovable (by a pin?) than deny the whole CMA area to compaction. Would it be feasible?
Well, it's an option too, however I'm not sure it's the best one. First, because these pages can be moved quite often, making them completely unmovable will make the compaction less efficient. Second, because it's not only about these pages, but in general about migrating pages into a cma area without a clear need.
Quoted 5 lines
As I wrote in the commit log, if a cma area is located close to end of a node (which seems to be default at least on x86 without a movable zone), compaction can fill it quite aggressively. And it might bring some hot pages (e.g. executable pagecache pages), which will cause cma allocation failures. I've seen something like this in our prod.
Hmm, I see.
Quoted 17 lines
Quoted 10 lines
Quoted 9 lines
Compaction moves them to the
hugetlb_cmaarea, and then sometimes the cma allocator fails to move them back from the cma area. It results in failures of gigantic hugepages allocations.Also in general cma areas are reserved close to the end of a zone, and it's where compaction tries to migrate pages. It means compaction will aggressively fill cma areas, which makes not much sense.
Quoted 6 lines
So now the free page scanner will have to skip those areas, which is not much effective. But I suspect a worse problem in
__compaction_suitable()which will now falsely report that there are enough free pages, so compaction will start but fail to do anytning. Minimally the__zone_watermark_ok()check there would have to loseALLOC_CMA, but there might be other similar checks that would need adjusting.Quoted 2 lines
A really good point! I've looked around for any other checks, but haven't found anything. Please, find an updated version of the patch below.
Technically there's also
__isolate_free_page()usingALLOC_CMAfor watermark check, but it's shared by compaction andalloc_contig_range(), so we can't just removeALLOC_CMAfrom there. It's probably not worth to complicate it though. If we passcompaction_suitable()withoutALLOC_CMAand then reach__isolate_free_page()and meanwhile watermarks changed so we wouldn't pass withoutALLOC_CMAanymore, it should be rare hopefully and not cause us deplete non-CMA free pages too badly.
Quoted 4 lines
But I've only now also realized how dynamic setting
cc->cmais. So in case a zone consists mostly of CMA blocks, removingALLOC_CMAin__compaction_suitable()would be actually wrong and prevent compaction from doing any work? Sigh. Any idea about that?
Hm, idk, is it a realistic setup? Looks like it depends significantly on the exact usecase.
Another option is to move the cma area closer to the beginning of a zone.
Quoted 10 lines
Quoted 10 lines
Quoted 6 lines
And long-term what happens if the "CMA using
ZONE_MOVABLE" approach is merged and there are not more CMA migratetypes to test? Might this change actually also avoid your issue, as said pages without__GFP_MOVABLEwon't end up in aZONE_MOVABLE?Yeah, this is what I was thinking about. Basically I want to mimic this behavior right now. Once this approach will be implemented and merged, it will happen
automatically: obviously, compaction won't move pages between different zones.
After the second thought it's not so obvious: CMA would need to migrate pages (data) between zones, right? It might bring some other complications.
Quoted 2 lines
That will be much better. Can't wait, then :)
Yeah, if it will happen soon-ish, we can just wait. I just don't know how hard it is and how many edge cases are there to be figured out first.
Do you think that it's better to wait and do not merge this patch upstream?
Thanks!
Quoted 8 lines · Roman Gushchin
Quoted 4 lines · Vlastimil Babka
But I've only now also realized how dynamic setting
cc->cmais. So in case a zone consists mostly of CMA blocks, removingALLOC_CMAin__compaction_suitable()would be actually wrong and prevent compaction from doing any work? Sigh. Any idea about that?Hm, idk, is it a realistic setup? Looks like it depends significantly on the exact usecase.
Yes :/ depends e.g. on how many hugepages are reserved, right?
Quoted 1 line
Another option is to move the cma area closer to the beginning of a zone.
That wouldn't hurt I guess.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
And long-term what happens if the "CMA using
ZONE_MOVABLE" approach is merged and there are not more CMA migratetypes to test? Might this change actually also avoid your issue, as said pages without__GFP_MOVABLEwon't end up in aZONE_MOVABLE?
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines
Yeah, this is what I was thinking about. Basically I want to mimic this behavior right now. Once this approach will be implemented and merged, it will happen
automatically: obviously, compaction won't move pages between different zones.After the second thought it's not so obvious: CMA would need to migrate pages (data) between zones, right? It might bring some other complications.
I don't recall how it was implemented, but I assume yeah. There shouldn't be complications I think, migrating out of CMA area should cause no issue for CMA, and such migrations between zones or even nodes already happen.
Quoted 7 lines
Quoted 2 lines
That will be much better. Can't wait, then :)
Yeah, if it will happen soon-ish, we can just wait. I just don't know how hard it is and how many edge cases are there to be figured out first.
Do you think that it's better to wait and do not merge this patch upstream?
We could probably merge it and watch for issues, but I'd like to have also Mel and Joonsoo comment on it :)
Thanks,
Vlastimil
Quoted 2 lines
Thanks!