Subject: Re: [PATCH v10 02/37] mm: memory-failure: serialize TestSetPageHWPoison with zone->lock

From: "Michael S. Tsirkin" <[email-masked]>
Date: Mon, 8 Jun 2026 09:48:34 -0400

On Mon, Jun 08, 2026 at 10:43:21AM +0100, Lorenzo Stoakes wrote:
> On Mon, Jun 08, 2026 at 04:34:23AM -0400, Michael S. Tsirkin wrote:
> > TestSetPageHWPoison() is called without zone->lock, so its atomic
> > update to page->flags can race with non-atomic flag operations
> > that run under zone->lock in the buddy allocator.
> >
> > In particular, __free_pages_prepare() does:
> >
> >     page->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
> >
> > This non-atomic read-modify-write, while correctly excluding
> > __PG_HWPOISON from the mask, can still lose a concurrent
> > TestSetPageHWPoison if the read happens before the poison bit
> > is set and the write happens after.  Follow-up patches in this
> > series add similar non-atomic flag operations as well.
> >
> > Fix by acquiring zone->lock around TestSetPageHWPoison and
> > around ClearPageHWPoison in the retry path.  This
> > serializes with all buddy flag manipulation.  The cost is
> > negligible: one lock/unlock in an extremely rare path
> > (hardware memory errors).
> >
> > Note: SetPageHWPoison and TestClearPageHWPoison calls elsewhere
> > in this file operate on pages already removed from the buddy
> > allocator or on non-buddy pages (DAX, hugetlb), so they do not
> > need zone->lock protection.
> >
> > Acked-by: Miaohe Lin <[email-masked]>
> > Signed-off-by: Michael S. Tsirkin <[email-masked]>
> 
> Can we have Fixes: and Cc: stable and also send this separately please?
> 
> These patches seem like unrelated fixups that you've discovered along the way,
> and don't belong as part of the already rather large series, unless I'm missing
> something here.
> 
> Thanks, Lorenzo

I think you are mising that they are a dependency, not unrelated.
For example, this issue gets worse with the patchset as there are more
places that manipulate flags without atomics. No?

You are welcome to send this to stable, but I think stable rules
preclude theoretical bugfixes.

As for Fixes: the issue has been there for decades. I wouldn't know
what to attribute it for.

I guess I could send these separately, too, why not. Not sure
what this accomplishes, but hey.  But is that an ack? You want
this fix merged even before the feature?

> > Assisted-by: Claude:claude-opus-4-6
> > ---
> >  mm/memory-failure.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> > index ee42d4361309..3880486028a1 100644
> > --- a/mm/memory-failure.c
> > +++ b/mm/memory-failure.c
> > @@ -2348,6 +2348,8 @@ int memory_failure(unsigned long pfn, int flags)
> >  	unsigned long page_flags;
> >  	bool retry = true;
> >  	int hugetlb = 0;
> > +	struct zone *zone;
> > +	unsigned long mf_flags;
> >
> >  	if (!sysctl_memory_failure_recovery)
> >  		panic("Memory failure on page %lx", pfn);
> > @@ -2390,7 +2392,11 @@ int memory_failure(unsigned long pfn, int flags)
> >  	if (hugetlb)
> >  		goto unlock_mutex;
> >
> > +	/* Serialize with non-atomic buddy flag operations */
> > +	zone = page_zone(p);
> > +	spin_lock_irqsave(&zone->lock, mf_flags);
> >  	if (TestSetPageHWPoison(p)) {
> > +		spin_unlock_irqrestore(&zone->lock, mf_flags);
> >  		res = -EHWPOISON;
> >  		if (flags & MF_ACTION_REQUIRED)
> >  			res = kill_accessing_process(current, pfn, flags);
> > @@ -2399,6 +2405,7 @@ int memory_failure(unsigned long pfn, int flags)
> >  		action_result(pfn, MF_MSG_ALREADY_POISONED, MF_FAILED);
> >  		goto unlock_mutex;
> >  	}
> > +	spin_unlock_irqrestore(&zone->lock, mf_flags);
> >
> >  	/*
> >  	 * We need/can do nothing about count=0 pages.
> > @@ -2420,7 +2427,10 @@ int memory_failure(unsigned long pfn, int flags)
> >  			} else {
> >  				/* We lost the race, try again */
> >  				if (retry) {
> > +					/* Serialize with non-atomic buddy flag operations */
> > +					spin_lock_irqsave(&zone->lock, mf_flags);
> >  					ClearPageHWPoison(p);
> > +					spin_unlock_irqrestore(&zone->lock, mf_flags);
> >  					retry = false;
> >  					goto try_again;
> >  				}
> > --
> > MST
> >