Re: [PATCH v1] mm, pagemap: expose hwpoison entry

[Naoya Horiguchi <naoya.horiguchi@linux.dev>]

On Wed, Oct 13, 2021 at 10:49:04AM +0800, Peter Xu wrote:
> Hi, Naoya,
> 
> On Mon, Oct 04, 2021 at 08:50:01PM +0900, Naoya Horiguchi wrote:
> > +static inline struct page *hwpoison_entry_to_page(swp_entry_t entry)
> > +{
> > +	struct page *p = pfn_to_page(swp_offset(entry));
> > +
> > +	WARN_ON(!PageHWPoison(p));
> > +	return p;
> > +}
> 
> This is more a pure question..
> 
> I'm wondering whether that WARN_ON() could trigger.
> 
> IOW, what if we poison an anonymous page and then unpoison it?  

Thanks for the good question, this could trigger WARN for unpoisoned pages.
The impact is limited because the caller of unpoison should know that that
happens in testing workload, but maybe there's no good reason to prevent
from it. So I'll drop this WARN_ON().

> Will there be a
> hwpoison swap entry leftover in the ptes that it used to map?  

Yes it will, unpoison never affects exisiting hwpoison swap entries.

> Will it crash
> the program when the page is accessed?

Reading hwpoison_entry_to_page() via pagemap interface should not crash
because it just reads the page's metadata.
The process with the hwpoison swap entry still receives SIGBUS when doing
page fault (irrespective of doing unpoison or not) on the error address.

> 
> I had a feeling that when handling the page fault in do_swap_page before we
> SIGBUS the program, we should double-check the PageHWPoison on the pfn page,
> but I could be missing something..

The double-checking seems to allow processes to detect that the hwpoison page
is unpoisoned, some test programs could benefit from it. But maybe it could
be done independent of this patch.

Personally, I only use unpoison in cleanup phase of each test case,
and each test case newly starts test processes, so reusing error pages
with unpoison can be avoided.

Thanks,
Naoya Horiguchi