Re: [PATCH v8 09/15] mm: Remove locking mf_mutex in is_raw_hwpoison_page_in_hugepage()

[email protected] Tue, 4 Aug 2026 13:32:47 -0700
Newsgroups gmane.linux.file-systems,gmane.linux.kernel.mm
Message-ID <[email protected]>

On 8/4/2026 9:58 AM, Matthew Wilcox wrote:
> On Tue, Aug 04, 2026 at 12:=E2=80=8A36:=E2=80=8A00PM -0400, Gregory Price=
 wrote: > On=20
> Fri, Jul 31, 2026 at 09:=E2=80=8A07:=E2=80=8A54PM +0100, Matthew Wilcox (=
Oracle) wrote:=20
>  > > Sleeping in this kind of predicate is unexpected. Add a new=20
> spinlock > > to protect
>=20
>=20
> On Tue, Aug 04, 2026 at 12:36:00PM -0400, Gregory Price wrote:
>> On Fri, Jul 31, 2026 at 09:07:54PM +0100, Matthew Wilcox (Oracle) wrote:
>> > Sleeping in this kind of predicate is unexpected.  Add a new spinlock
>> > to protect access to the list, and turn it into a normal singly linked
>> > list now that it doesn't need to be a lockless list.
>>=20
>> Out of curiosity, wouldn't sleeping in this context not just be
>> unexpected but theoretically cause issues? (mid-poison, sleep, scheduled
>> process generates more poison on the same page...) - at a minimum this
>> takes a thread out of the future potential poison-generating pool?
>=20
> A reasonable question.  We don't try to handle races terribly well --
> hwpoison can be generated asynchronously by patrol scrub, so it's always
> going to be possible for a thread to hit poison on something that the
> kernel didn't know about.
>=20
> What we're trying to do is avoid touching memory which we already knew
> to be poisoned, while also permitting touching memory which we haven't
> been told is poison.
>=20
> I see the locking in here as preventing list corruption and UAF rather
> than preventing races.  I'm also considering that we're now exposing this
> mutex fairly directly to userspace -- before it was only being touched
> through some fairly weird mechanisms like reading kcore.  With this
> patch series, every call to read() potentially touches this mutex.
> And that could hold off recording any hwpoison for some time, not to
> mention serialising all other calls to read() on a given folio.
>=20
> So we have all manner of protections that prevent us from taking this mut=
ex
> -- the folio must have a hwpoison page in it and must be hugetlb, but at
> the end of the day, one can still be preempted while holding a mutex and
> we never know when we might want to call this from a non-sleepable contex=
t.
>=20
> I'd be open to arguments that this should be an rwlock rather than
> a spinlock.  Or figure out a way to shard the lock per folio (lockdep
> etc make it very hard to embed a spinlock in struct folio).  I think
> it'd also be profitable to change how we record poison for hugetlb to
> not need locking or memory allocation.
>=20
> But this is all a very long way from where I want to be working -- cleani=
ng
> up the page fault path.  I think I've made a reasonable set of improvemen=
ts
> here, and somebody else can come along later to make the code even better.
>=20

We don't go down the path examining the raw_hwp_list unless the hugetlb=20
folio is poisoned which is a rare event, given that, I felt the lock is=20
fine.

thanks,
-jane