[RFC PATCH 02/26] mm: compaction: avoid GFP_NOFS deadlocks

Johannes Weiner
2023-04-18T19:14:25Z

During stress testing, two deadlock scenarios were observed:

1. One GFP_NOFS allocation was sleeping on too_many_isolated(), and
all CPUs were busy with compactors that appeared to be spinning on
buffer locks.

Give GFP_NOFS compactors additional isolation headroom, the same
way we do during reclaim, to eliminate this deadlock scenario.

2. In a more pernicious scenario, the GFP_NOFS allocation was
busy-spinning in compaction, but seemingly never making
progress. Upon closer inspection, memory was dominated by file
pages, which the fs compactor isn't allowed to touch. The remaining
anon pages didn't have the contiguity to satisfy the request.

Allow GFP_NOFS allocations to bypass watermarks when compaction
failed at the highest priority.

While these deadlocks were encountered only in tests with the
subsequent patches (which put a lot more demand on compaction), in
theory these problems already exist in the code today. Fix them now.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---
mm/compaction.c | 15 +++++++++++++--
mm/page_alloc.c | 10 +++++++++-
2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index 8238e83385a7..84db84e8fd3a 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -745,8 +745,9 @@ isolate_freepages_range(struct compact_control *cc,
}

/* Similar to reclaim, but different enough that they don't share logic */
-static bool too_many_isolated(pg_data_t *pgdat)
+static bool too_many_isolated(struct compact_control *cc)
{
+ pg_data_t *pgdat = cc->zone->zone_pgdat;
bool too_many;

unsigned long active, inactive, isolated;
@@ -758,6 +759,16 @@ static bool too_many_isolated(pg_data_t *pgdat)
isolated = node_page_state(pgdat, NR_ISOLATED_FILE) +
node_page_state(pgdat, NR_ISOLATED_ANON);

+ /*
+ * GFP_NOFS callers are allowed to isolate more pages, so they
+ * won't get blocked by normal direct-reclaimers, forming a
+ * circular deadlock. GFP_NOIO won't get here.
+ */
+ if (cc->gfp_mask & __GFP_FS) {
+ inactive >>= 3;
+ active >>= 3;
+ }
+
too_many = isolated > (inactive + active) / 2;
if (!too_many)
wake_throttle_isolated(pgdat);
@@ -806,7 +817,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
* list by either parallel reclaimers or compaction. If there are,
* delay for some time until fewer pages are isolated
*/
- while (unlikely(too_many_isolated(pgdat))) {
+ while (unlikely(too_many_isolated(cc))) {
/* stop isolation if there are still pages not migrated */
if (cc->nr_migratepages)
return -EAGAIN;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 3bb3484563ed..ac03571e0532 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4508,8 +4508,16 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
prep_new_page(page, order, gfp_mask, alloc_flags);

/* Try get a page from the freelist if available */
- if (!page)
+ if (!page) {
+ /*
+ * It's possible that the only migration sources are
+ * file pages, and the GFP_NOFS stack is holding up
+ * other compactors. Use reserves to avoid deadlock.
+ */
+ if (prio == MIN_COMPACT_PRIORITY && !(gfp_mask & __GFP_FS))
+ alloc_flags |= ALLOC_NO_WATERMARKS;
page = get_page_from_freelist(gfp_mask, order, alloc_flags, ac);
+ }

if (page) {
struct zone *zone = page_zone(page);
--
2.39.2