Re: [PATCH v2 1/6] mm: userfaultfd: add new UFFDIO_POISON ioctl

[Peter Xu <peterx@redhat.com>]

On Wed, Jul 05, 2023 at 09:27:15AM -0700, James Houghton wrote:
> On Wed, Jul 5, 2023 at 9:15 AM Peter Xu <peterx@redhat.com> wrote:
> >
> > On Wed, Jul 05, 2023 at 09:09:19AM -0700, James Houghton wrote:
> > > > > diff --git a/include/linux/swapops.h b/include/linux/swapops.h
> > > > > index 4c932cb45e0b..8259fee32421 100644
> > > > > --- a/include/linux/swapops.h
> > > > > +++ b/include/linux/swapops.h
> > > > > @@ -394,7 +394,8 @@ typedef unsigned long pte_marker;
> > > > >
> > > > >  #define  PTE_MARKER_UFFD_WP                  BIT(0)
> > > > >  #define  PTE_MARKER_SWAPIN_ERROR             BIT(1)
> > > > > -#define  PTE_MARKER_MASK                     (BIT(2) - 1)
> > > > > +#define  PTE_MARKER_UFFD_POISON                      BIT(2)
> > > >
> > > > One more tab.
> > > >
> > > > Though I remembered the last time we discussed IIRC we plan to rename
> > > > SWAPIN_ERROR and reuse it, could you explain why a new bit is still needed?
> > > >
> > > > I think I commented this but I'll do it again: IIUC any existing host
> > > > swapin errors for guest pages should be reported as MCE too, afaict,
> > > > happened in kvm context.
> > >
> > > I think swapin errors are treated differently than poison. Swapin
> > > errors get VM_FAULT_SIGBUS, and poison gets VM_FAULT_HWPOISON, so
> > > UFFDIO_POISON should also get VM_FAULT_HWPOISON (so that's what Axel
> > > has implemented). And I think that needs a separate PTE marker.
> >
> > My question was, should we also make SWAPIN_ERROR return VM_FAULT_HWPOISON
> > always?
> >
> > Just to recap from what I already commented above - if a guest page got
> > error in swapin due to block sector failures, it should be treated as
> > VM_FAULT_HWPOISON too, IMHO.  IOW, I think current SWAPIN_ERROR is wrong
> > when in kvm context and we should fix it first.
> 
> Oh! Yes, I agree, though I'm not familiar enough with the users of
> SWAPIN_ERROR to know if we can actually make this change.

Miaohe initially proposed this swapin error facility, let's see whether he
can comment; he's already in the cc list.

AFAICT that's the right thing to do, and it shouldn't affect any existing
user of swapin error if there is.

Or say, VM_FAULT_HWPOISON should be the same as VM_FAULT_SIGBUS when not in
kvm context, so shouldn't change a thing in !kvm, while changing that
should fix kvm from crashing a guest where we shouldn't need to.

-- 
Peter Xu