should_reclaim_retry() hands the reclaim_retry_zone tracepoint the
zoneref it is walking, and the tracepoint reads the slot again for the
zone's node and index. Offlining a zone's last block rebuilds the
zonelists in place, and the shrunk list's NULL terminator can land on
that slot in between:
CPU 0 CPU 1
should_reclaim_retry()
zone = zonelist_zone(slot 1) /* NORMAL1 */
zone_reclaimable_pages(zone)
offline_pages()
build_all_zonelists()
[MOVABLE1, NORMAL1, NULL]
-> [NORMAL1, NULL]
trace_reclaim_retry_zone(slot 1)
zonelist_node_idx(slot 1)
zone_to_nid(NULL)
BUG: kernel NULL pointer dereference, address: 0000000000000060
RIP: 0010:trace_event_raw_event_reclaim_retry_zone+0x57/0xd0
Call Trace:
__alloc_pages_slowpath.constprop.0+0xa80/0x16d0
__alloc_frozen_pages_noprof+0x2d5/0x360
alloc_pages_node_noprof+0x53/0xe0
The second read follows zone_reclaimable_pages() and a per-CPU vmstat
snapshot, so unlike the iterator's double read no compiler merges it.
Pass the zone the loop already loaded. The event's fields are unchanged.
With the tracepoint enabled and kprobes holding should_reclaim_retry()
across the offline, the base kernel oopsed in 3 of 3 runs and this
change in none.
Fixes: 11cd8638c37f ("mm, page_alloc: remove stop_machine from
build_all_zonelists")
Cc: [email protected]
Signed-off-by: Gregory Price (Meta) <[email protected]>
---
include/trace/events/oom.h | 8 ++++----
mm/page_alloc.c | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/trace/events/oom.h b/include/trace/events/oom.h
index 9f0a5d1482c44..cbcf6fe4cc615 100644
--- a/include/trace/events/oom.h
+++ b/include/trace/events/oom.h
@@ -33,7 +33,7 @@ TRACE_EVENT(oom_score_adj_update,
TRACE_EVENT(reclaim_retry_zone,
- TP_PROTO(struct zoneref *zoneref,
+ TP_PROTO(struct zone *zone,
int order,
unsigned long reclaimable,
unsigned long available,
@@ -41,7 +41,7 @@ TRACE_EVENT(reclaim_retry_zone,
int no_progress_loops,
bool wmark_check),
- TP_ARGS(zoneref, order, reclaimable, available, min_wmark,
no_progress_loops, wmark_check),
+ TP_ARGS(zone, order, reclaimable, available, min_wmark,
no_progress_loops, wmark_check),
TP_STRUCT__entry(
__field( int, node)
@@ -55,8 +55,8 @@ TRACE_EVENT(reclaim_retry_zone,
),
TP_fast_assign(
- __entry->node = zonelist_node_idx(zoneref);
- __entry->zone_idx = zonelist_zone_idx(zoneref);
+ __entry->node = zone_to_nid(zone);
+ __entry->zone_idx = zone_idx(zone);
__entry->order = order;
__entry->reclaimable = reclaimable;
__entry->available = available;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4cf2982c5b107..1bb308296fcdc 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4679,7 +4679,7 @@ should_reclaim_retry(gfp_t gfp_mask, unsigned order,
*/
wmark = __zone_watermark_ok(zone, order, min_wmark,
ac->highest_zoneidx, alloc_flags, available);
- trace_reclaim_retry_zone(z, order, reclaimable,
+ trace_reclaim_retry_zone(zone, order, reclaimable,
available, min_wmark, *no_progress_loops,
wmark);
if (wmark) {
ret = true;
--
2.53.0-Meta
