Re: [PATCH v2] dax: enable dax fault handler to report VM_FAULT_HWPOISON

[Matthew Wilcox]

On Thu, Apr 27, 2023 at 06:35:57PM -0700, Dan Williams wrote:
> Jane Chu wrote:
> > Hi, Dan,
> > 
> > On 4/27/2023 2:36 PM, Dan Williams wrote:
> > > Jane Chu wrote:
> > >> When dax fault handler fails to provision the fault page due to
> > >> hwpoison, it returns VM_FAULT_SIGBUS which lead to a sigbus delivered
> > >> to userspace with .si_code BUS_ADRERR.  Channel dax backend driver's
> > >> detection on hwpoison to the filesystem to provide the precise reason
> > >> for the fault.
> > > 
> > > It's not yet clear to me by this description why this is an improvement
> > > or will not cause other confusion. In this case the reason for the
> > > SIGBUS is because the driver wants to prevent access to poison, not that
> > > the CPU consumed poison. Can you clarify what is lost by *not* making
> > > this change?
> > 
> > Elsewhere when hwpoison is detected by page fault handler and helpers as 
> > the direct cause to failure, VM_FAULT_HWPOISON or 
> > VM_FAULT_HWPOISON_LARGE is flagged to ensure accurate SIGBUS payload is 
> > produced, such as wp_page_copy() in COW case, do_swap_page() from 
> > handle_pte_fault(), hugetlb_fault() in hugetlb page fault case where the 
> > huge fault size would be indicated in the payload.
> > 
> > But dax fault has been an exception in that the SIGBUS payload does not 
> > indicate poison, nor fault size.  I don't see why it should be though,
> > recall an internal user expressing confusion regarding the different 
> > SIGBUS payloads.
> 
> ...but again this the typical behavior with block devices. If a block
> device has badblock that causes page cache page not to be populated
> that's a SIGBUS without hwpoison information. If the page cache is
> properly populated and then the CPU consumes poison that's a SIGBUS with
> the additional hwpoison information.

I'm not sure that's true when we mmap().  Yes, it's not consistent with
-EIO from read(), but we have additional information here, and it's worth
providing it.  You can think of it as *in this instance*, the error is
found "in the page cache", because that's effectively where the error
is from the point of view of the application?