Re: [PATCH v9 08/15] hugetlb: Use the has_hwpoisoned flag

[Matthew Wilcox <willy@infradead.org>]

On Fri, Sep 18, 2026 at 03:44:12PM +0200, David Hildenbrand (Arm) wrote:
> I'm a terrible person and it took me way too long to get back to this :(
> 
> > 
> > You make it sound so easy ;-)
> 
> Heh, as long as you hold a folio reference it really is :)
> 
> > 
> > If we don't hold a reference on the folio, then the folio (whether it's
> > hugetlb or not) can be split.  And then folio_test_hwpoison() can hit
> > the assertion that it's now a tail page.
> 
> Right. I am/was missing the connection to "generic_file_read_iter() in hugetlbfs".

We might be able to split this into two series at this point.  Is that
worth me spending time on doing?  Originally I was going to use
is_page_hwpoison() in filemap, then it became obvious that this was
inefficient when we had a reference to the folio, and so we ended up with
is_ref_page_hwpoison().

> > Before this patch series, we don't always hold the hugetlb lock when
> > splitting a hugetlb folio that contains hwpoison.  And we don't want
> > to have to grab the hugetlb lock if we can avoid it -- unprivileged
> > userspace can hammer these paths hard, and I don't want to see this
> > lock be contended.
> 
> Right. But doesn't at least the pagecache always hold a folio reference when
> testing for hwpoison?
> 
> I mean, there is other code that might need care (PFN walkers like memory
> offlining), but I was surprised to see pagecache code require a rework of
> lockless hwpoison checking.
> 
> I'm sure I am missing something.

It's just how everything evolved.

> > Oh, I'm not happy about it.  But we need an atomic way to determine if
> > a page belongs to a hugetlb folio with hwpoison detected.  Short of a
> > complete rearchitecture of how we handle hwpoison, this is the best I've
> > come up with.
> 
> (did I express how much I hate the hwpoison infrastructure and how it's racy
> left and right? :) )

Yes, and you'll have the chance to do it again on Wednesday ;-)

> >> (what on earth is "huge_poison" is this supposed to be "hugetlb_poison" ? Why
> >> "poison" and not "hwpoison"? Really odd)
> > 
> > We're inconsistent in our naming on both of these things.  It doesn't
> > help that somebody decided to reuse the term "poison" to mean
> > "uninitialised struct page".
> 
> Right, but let's be consistent with hugetlb and with hwpoison. ;)

I'll take another look ...

> >> I'd expect that we actually get rid of folio_test_hwpoison entirely and
> >> exclusively work on per-page state and has_hwpoison. But IIUC, now it's some
> >> mixture of folio checks, page checks, folio_has, mixed with some hugetlb oddity.
> > 
> > If we could get rid of HVO, we could do it entirely on struct page.
> 
> That's what I hope we will achieve at some point.

I'm not sure that's a realistic hope in the next few years.  Even if
we get struct page down to 8 bytes, that's 2MB of memory per 1GiB allocation
(assuming 4KiB pages).  Always a tempting target for someone looking for
memory savings.

I would prefer an out-of-line tree that doesn't rely on a bit in struct
page.  Maybe a bit in struct folio wuld be fine (which tells you whether
it's OK to skip the tree search).

> > Or, as above, entirely rearchitecture hwpoison handling to not be based
> > around pages or folios any more.
> > 
> >> I am not quite clear whether the change you propose here is actually required
> >> for the remainder of this series?
> >>
> >> 	[PATCH v9 00/15] Use generic_file_read_iter() in hugetlbfs
> >>
> >> IOW, do we really need all this hugetlb hwpoison handling just to accomplish
> >> that, or could some of that (bigger hwpoison rework) be done separately?
> > 
> > Blame Sashiko!  I'm going to ignore it in future, but it's really good
> > at nerd-sniping "hey all of this is already broken and you could fix it
> > as part of this series".
> 
> Right, and this is only the tip of the iceberg, because the entire hwpoison
> infrastructure is a complete hacked-on racy piece of ... engineering excellence.
> 
> To summarize my question: is it possible to separate for this series the
> pagecache part (Use generic_file_read_iter() in hugetlbfs) from all the hugetlb
> hwpoison rework?
> 
> OTOH, if it's really not avoidable, please let me know.

I think Sashiko will whine and complain incessantly if we drop the
race elimination patches off the front.  I can try it, but is it worth
doing?  Eliminating the races seems like good engineering anyway.

> > try_to_unmap_one() is already 200 lines and contains some tricky code flow.
> > I'm trying to at least make the problem not-worse.  I think we should
> > pull out even more code into helpers, but not in this patch series.
> 
> Note that this code now was partly reworked such that this helper will soon no
> longer be required.

Rebasing it on current Linus simplified this a lot.