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