Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

mm/page_alloc: fix race condition between build_all_zonelists and page allocation

Patrick Daly reported the following problem;

NODE_DATA(nid)->node_zonelists[ZONELIST_FALLBACK] - before offline operation
[0] - ZONE_MOVABLE
[1] - ZONE_NORMAL
[2] - NULL

For a GFP_KERNEL allocation, alloc_pages_slowpath() will save the
offset of ZONE_NORMAL in ac->preferred_zoneref. If a concurrent
memory_offline operation removes the last page from ZONE_MOVABLE,
build_all_zonelists() & build_zonerefs_node() will update
node_zonelists as shown below. Only populated zones are added.

NODE_DATA(nid)->node_zonelists[ZONELIST_FALLBACK] - after offline operation
[0] - ZONE_NORMAL
[1] - NULL
[2] - NULL

The race is simple -- page allocation could be in progress when a memory
hot-remove operation triggers a zonelist rebuild that removes zones. The
allocation request will still have a valid ac->preferred_zoneref that is
now pointing to NULL and triggers an OOM kill.

This problem probably always existed but may be slightly easier to trigger
due to 6aa303defb74 ("mm, vmscan: only allocate and reclaim from zones
with pages managed by the buddy allocator") which distinguishes between
zones that are completely unpopulated versus zones that have valid pages
not managed by the buddy allocator (e.g. reserved, memblock, ballooning
etc). Memory hotplug had multiple stages with timing considerations
around managed/present page updates, the zonelist rebuild and the zone
span updates. As David Hildenbrand puts it

memory offlining adjusts managed+present pages of the zone
essentially in one go. If after the adjustments, the zone is no
longer populated (present==0), we rebuild the zone lists.

Once that's done, we try shrinking the zone (start+spanned
pages) -- which results in zone_start_pfn == 0 if there are no
more pages. That happens *after* rebuilding the zonelists via
remove_pfn_range_from_zone().

The only requirement to fix the race is that a page allocation request
identifies when a zonelist rebuild has happened since the allocation
request started and no page has yet been allocated. Use a seqlock_t to
track zonelist updates with a lockless read-side of the zonelist and
protecting the rebuild and update of the counter with a spinlock.

[akpm@linux-foundation.org: make zonelist_update_seq static]
Link: https://lkml.kernel.org/r/20220824110900.vh674ltxmzb3proq@techsingularity.net
Fixes: 6aa303defb74 ("mm, vmscan: only allocate and reclaim from zones with pages managed by the buddy allocator")
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Reported-by: Patrick Daly <quic_pdaly@quicinc.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Cc: <stable@vger.kernel.org> [4.9+]
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

authored by

Mel Gorman and committed by
Andrew Morton
3d36424b 1b513f61

+43 -10
+43 -10
mm/page_alloc.c
··· 4708 4708 EXPORT_SYMBOL_GPL(fs_reclaim_release); 4709 4709 #endif 4710 4710 4711 + /* 4712 + * Zonelists may change due to hotplug during allocation. Detect when zonelists 4713 + * have been rebuilt so allocation retries. Reader side does not lock and 4714 + * retries the allocation if zonelist changes. Writer side is protected by the 4715 + * embedded spin_lock. 4716 + */ 4717 + static DEFINE_SEQLOCK(zonelist_update_seq); 4718 + 4719 + static unsigned int zonelist_iter_begin(void) 4720 + { 4721 + if (IS_ENABLED(CONFIG_MEMORY_HOTREMOVE)) 4722 + return read_seqbegin(&zonelist_update_seq); 4723 + 4724 + return 0; 4725 + } 4726 + 4727 + static unsigned int check_retry_zonelist(unsigned int seq) 4728 + { 4729 + if (IS_ENABLED(CONFIG_MEMORY_HOTREMOVE)) 4730 + return read_seqretry(&zonelist_update_seq, seq); 4731 + 4732 + return seq; 4733 + } 4734 + 4711 4735 /* Perform direct synchronous page reclaim */ 4712 4736 static unsigned long 4713 4737 __perform_reclaim(gfp_t gfp_mask, unsigned int order, ··· 5025 5001 int compaction_retries; 5026 5002 int no_progress_loops; 5027 5003 unsigned int cpuset_mems_cookie; 5004 + unsigned int zonelist_iter_cookie; 5028 5005 int reserve_flags; 5029 5006 5030 5007 /* ··· 5036 5011 (__GFP_ATOMIC|__GFP_DIRECT_RECLAIM))) 5037 5012 gfp_mask &= ~__GFP_ATOMIC; 5038 5013 5039 - retry_cpuset: 5014 + restart: 5040 5015 compaction_retries = 0; 5041 5016 no_progress_loops = 0; 5042 5017 compact_priority = DEF_COMPACT_PRIORITY; 5043 5018 cpuset_mems_cookie = read_mems_allowed_begin(); 5019 + zonelist_iter_cookie = zonelist_iter_begin(); 5044 5020 5045 5021 /* 5046 5022 * The fast path uses conservative alloc_flags to succeed only until ··· 5213 5187 goto retry; 5214 5188 5215 5189 5216 - /* Deal with possible cpuset update races before we start OOM killing */ 5217 - if (check_retry_cpuset(cpuset_mems_cookie, ac)) 5218 - goto retry_cpuset; 5190 + /* 5191 + * Deal with possible cpuset update races or zonelist updates to avoid 5192 + * a unnecessary OOM kill. 5193 + */ 5194 + if (check_retry_cpuset(cpuset_mems_cookie, ac) || 5195 + check_retry_zonelist(zonelist_iter_cookie)) 5196 + goto restart; 5219 5197 5220 5198 /* Reclaim has failed us, start killing things */ 5221 5199 page = __alloc_pages_may_oom(gfp_mask, order, ac, &did_some_progress); ··· 5239 5209 } 5240 5210 5241 5211 nopage: 5242 - /* Deal with possible cpuset update races before we fail */ 5243 - if (check_retry_cpuset(cpuset_mems_cookie, ac)) 5244 - goto retry_cpuset; 5212 + /* 5213 + * Deal with possible cpuset update races or zonelist updates to avoid 5214 + * a unnecessary OOM kill. 5215 + */ 5216 + if (check_retry_cpuset(cpuset_mems_cookie, ac) || 5217 + check_retry_zonelist(zonelist_iter_cookie)) 5218 + goto restart; 5245 5219 5246 5220 /* 5247 5221 * Make sure that __GFP_NOFAIL request doesn't leak out and make sure ··· 6548 6514 int nid; 6549 6515 int __maybe_unused cpu; 6550 6516 pg_data_t *self = data; 6551 - static DEFINE_SPINLOCK(lock); 6552 6517 6553 - spin_lock(&lock); 6518 + write_seqlock(&zonelist_update_seq); 6554 6519 6555 6520 #ifdef CONFIG_NUMA 6556 6521 memset(node_load, 0, sizeof(node_load)); ··· 6586 6553 #endif 6587 6554 } 6588 6555 6589 - spin_unlock(&lock); 6556 + write_sequnlock(&zonelist_update_seq); 6590 6557 } 6591 6558 6592 6559 static noinline void __init