Re: [PATCH 0/6] mm: fix pfn walker races with memory offline

[Gregory Price (Meta)]

On 2026-10-27 13:51 +0100, David Hildenbrand (Arm) wrote:
> On 10/27/26 12:31, Gregory Price (Meta) wrote:
> > On 2026-10-27 09:57 +0100, David Hildenbrand (Arm) wrote:
> > 
> > I actually have a reason for these fixes other than "bot said bad".
> 
> So you found them on production kernels?
> 
> We did not have a single real report of that for probably a decade or
> even longer in production.
> 
> Recently, we had the single first report about the compaction case from a
> stress test (no production):
> 
> https://lore.kernel.org/all/20260903095543.3752338-1-yuan1.liu@xxxxxxxxx/
> 
> "We are not sure whether this race is reachable under realistic
> workloads or only under this synthetic stress test."
> 
> Where I motivated that we need something clean.
> 

tl;dr:

while these are ai-generated, I'm not posting untested, unreviewed
schlopp for the sake of getting patches upstream.

I actually care about this issue.

---

Some of these don't show up as crashes but instead as hung offline
attempts (GFP_KERNEL landing on movable).  That shows up as our
hypervisor just failing to overcommit / guests hanging.

The compaction/alloc_contig bug is one I've seen in production.

I've seen a handful of NULL derefs in production i suspect are the
zonelist derefs, but it's hard to say (they're rare enough that we
lose the dumps before someone has a thought to pull them).

I expect this will scale up as we increase the number of VMs and
the amount of hotplug we're doing.

For clarity, we use ballooning + hotplug as a combo:
 - ballooning to do cooperative overcommit
 - hotplug to defend host stability when the overcommit ratio is extreme
   (basically deflate_on_oom because a host stability issue)

We're pretty aggressive about ballooning guests to squeeze them and
dropping the balloon to hot-unplug that memory when a guest is cold.


skippable side note on virtio-balloon/mem:

I have some improvements to virtio-balloon and virtio-mem that allow for
handoff from balloon directly to mem for hot-unplug without having to
drop the balloon, but I haven't posted those (and we don't run those in
production, so this is not the nexus of crashes).

Similarly I have another extension that lets the host set a "NO_ALLOC"
bit on the zone so a balloon-only system can provide the host a way to
defend against mass deflate_on_oom events (we prefer guests to OOM and
have the host to kill them due to PSI, rather than threaten host
stability). Also not in production and not posted, but there have been
a lot of experiments on overcommit here.

> The thing is that the race between offlining and actual memory removal
> is extremely long. Poisoning is just a mechanism to find these theoretical
> issues that could cause issues faster.
> 
> And with all the other things going on, this has lower priority, and I
> wanted to get back to the AI patches I have once the other thing is sorted.
> 

Sure, I realize I'm pushing this out during the merge window, but I
felt it reasonable to get some thoughts on whether these fixes are
worth considering for the window.

If the answer is no just put em on the back burner. We can carry
fixes as-needed.

> > 
> > I have a reproducer for every single one of these (and the other
> > series) - but a handful are unreachable due to a 2-statement read
> > being emitted as a single read by the compiler (which can be undone
> > by a targeted compiler flag). Those were included for completeness.
> 
> 
> In general, I think we want something that will protect all
> pfn_to_online_page() + zone->contig thingies, not just selected ones.
> We discussed in the past using some sort of rcu for that.
>
> get_online_mems is usually the wrong approach.
> 

Agreed, although the PFN walks seem genuinely harder to wrangle into an
good rcu shape.

get_online_mems seems like the backportable solution, but i can see not
wanting to have to go clean it back up later.

> protecting individual pfn_to_online_page is just whack a mole. Likely we want a
> way to check whether pfn_to_online_page() is called from the wrong context, and
> maybe we need a variant that can be called from "wrong context" for a good reason.
> 

Agreed.  Although I fear the structural fix for this will have uglier
performance issue than we think.  Then again, maybe it doesn't matter
(if you're doing a PFN walk, then you're likely already doing something
stupidly expensive).

Where i'm largely concerned is the page allocator and the
compaction/contig code not taking a regression.

For zonelists i've ended up with something like this:

@@ -1461,7 +1466,7 @@ typedef struct pglist_data {
         * Generally the first zones will be references to this node's
         * node_zones.
         */
-       struct zonelist node_zonelists[MAX_ZONELISTS];
+       struct zonelist __rcu *node_zonelists[MAX_ZONELISTS];

...

static inline struct srcu_ctr __percpu *zonelist_read_lock(void)
{
       return srcu_read_lock_fast(&zonelist_srcu);
}

static inline void zonelist_read_unlock(struct srcu_ctr __percpu *scp)
{
       srcu_read_unlock_fast(&zonelist_srcu, scp);
}

DEFINE_LOCK_GUARD_0(zonelist_read, _T->scp = zonelist_read_lock(),
                   zonelist_read_unlock(_T->scp),
                   struct srcu_ctr __percpu *scp)

static inline struct zonelist *pgdat_zonelist(pg_data_t *pgdat, int type)
{
       return zonelist_dereference(pgdat->node_zonelists[type]);
}

With this the zonelist cookie checks evaporate, but you still end up
with guard() at every walk location - not great, but better i suppose.

I was hoping this could be sunken into the zonelist walk macros, but
I haven't seen that pan out.  I've been spending a lot of time auditing
that code for private nodes as well.


On performance:

I've been hammering this with will-it-scale and a bunch of workload
tests on a variety of hosts and have not seen any obvious regressions
outside of powerpc (which i can't test directly, but SRCU-fast requires
atomics for powerpc from what i can tell - so it will eat overhead
without a fallback... if there is actually any hotplug ppc hardware out
there to care about?)

-- 
~Gregory