Commit 4fbbb3fd authored by Johannes Weiner's avatar Johannes Weiner Committed by Andrew Morton

mm: compaction: avoid GFP_NOFS ABBA deadlock

During stress testing with higher-order allocations, a deadlock scenario
was observed in compaction: One GFP_NOFS allocation was sleeping on
mm/compaction.c::too_many_isolated(), while all CPUs in the system were
busy with compactors spinning on buffer locks held by the sleeping
GFP_NOFS allocation.

Reclaim is susceptible to this same deadlock; we fixed it by granting
GFP_NOFS allocations additional LRU isolation headroom, to ensure it makes
forward progress while holding fs locks that other reclaimers might
acquire.  Do the same here.

This code has been like this since compaction was initially merged, and I
only managed to trigger this with out-of-tree patches that dramatically
increase the contexts that do GFP_NOFS compaction.  While the issue is
real, it seems theoretical in nature given existing allocation sites. 
Worth fixing now, but no Fixes tag or stable CC.

Link: https://lkml.kernel.org/r/20230519111359.40475-1-hannes@cmpxchg.orgSigned-off-by: default avatarJohannes Weiner <hannes@cmpxchg.org>
Acked-by: default avatarMel Gorman <mgorman@techsingularity.net>
Acked-by: default avatarVlastimil Babka <vbabka@suse.cz>
Cc: Michal Hocko <mhocko@suse.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
parent 3cf04937
...@@ -740,8 +740,9 @@ isolate_freepages_range(struct compact_control *cc, ...@@ -740,8 +740,9 @@ isolate_freepages_range(struct compact_control *cc,
} }
/* Similar to reclaim, but different enough that they don't share logic */ /* 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; bool too_many;
unsigned long active, inactive, isolated; unsigned long active, inactive, isolated;
...@@ -753,6 +754,17 @@ static bool too_many_isolated(pg_data_t *pgdat) ...@@ -753,6 +754,17 @@ static bool too_many_isolated(pg_data_t *pgdat)
isolated = node_page_state(pgdat, NR_ISOLATED_FILE) + isolated = node_page_state(pgdat, NR_ISOLATED_FILE) +
node_page_state(pgdat, NR_ISOLATED_ANON); node_page_state(pgdat, NR_ISOLATED_ANON);
/*
* Allow GFP_NOFS to isolate past the limit set for regular
* compaction runs. This prevents an ABBA deadlock when other
* compactors have already isolated to the limit, but are
* blocked on filesystem locks held by the GFP_NOFS thread.
*/
if (cc->gfp_mask & __GFP_FS) {
inactive >>= 3;
active >>= 3;
}
too_many = isolated > (inactive + active) / 2; too_many = isolated > (inactive + active) / 2;
if (!too_many) if (!too_many)
wake_throttle_isolated(pgdat); wake_throttle_isolated(pgdat);
...@@ -801,7 +813,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, ...@@ -801,7 +813,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
* list by either parallel reclaimers or compaction. If there are, * list by either parallel reclaimers or compaction. If there are,
* delay for some time until fewer pages are isolated * 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 */ /* stop isolation if there are still pages not migrated */
if (cc->nr_migratepages) if (cc->nr_migratepages)
return -EAGAIN; return -EAGAIN;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment