lkml.org 
[lkml]   [2020]   [Dec]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] mm: Don't fault around userfaultfd-registered regions on reads
    On Thu, Dec 03, 2020 at 02:44:14PM -0500, Andrea Arcangeli wrote:
    > On Thu, Dec 03, 2020 at 01:02:34PM -0500, Peter Xu wrote:
    > > On Wed, Dec 02, 2020 at 09:36:45PM -0800, Hugh Dickins wrote:
    > > > On Wed, 2 Dec 2020, Peter Xu wrote:
    > > > > On Wed, Dec 02, 2020 at 02:37:33PM -0800, Hugh Dickins wrote:
    > > > > > On Tue, 1 Dec 2020, Andrea Arcangeli wrote:
    > > > > > >
    > > > > > > Any suggestions on how to have the per-vaddr per-mm _PAGE_UFFD_WP bit
    > > > > > > survive the pte invalidates in a way that remains associated to a
    > > > > > > certain vaddr in a single mm (so it can shoot itself in the foot if it
    > > > > > > wants, but it can't interfere with all other mm sharing the shmem
    > > > > > > file) would be welcome...
    > > > > >
    > > > > > I think it has to be a new variety of swap-like non_swap_entry() pte,
    > > > > > see include/linux/swapops.h. Anything else would be more troublesome.
    >
    > Agreed. Solving it by changing the unmapping of the ptes is also some
    > trouble but less troublesome than adding new bitmaps to the vma to
    > handle in vma_merge/vma_split.
    >
    > > > But those ptes will be pte_present(), so you must provide a pfn,
    > >
    > > Could I ask why?
    >
    > _PAGE_PROTNONE exists only for one reason, so pte_present returns true
    > and the page is as good as mapped, the pfn is real and mapped,
    > everything is up and running fine except _PAGE_PRESENT is not set in
    > the pte. _PAGE_PROTNONE doesn't unmap the pte, it only triggers faults
    > on a mapped pte.
    >
    > When we set _PAGE_PROTNONE and clear _PAGE_PRESENT atomically, nothing
    > changes at all for all pte_present and all other VM common code,
    > except now you get page faults.
    >
    > So numa hinting faults use that and it's a perfect fit for that,
    > because you just want to change nothing, but still be notified on
    > access.
    >
    > Here instead you need to really unmap the page and lose any pfn or
    > page reference and everything along with it, just one bit need to
    > survive the unmap: the _PAGE_UFFD_WP bit.
    >
    > I tend to agree this needs to work more similarly to the migration
    > entry like Hugh suggested or an entirely new mechanism to keep "vma
    > specific state" alive, for filebacked mappings that get zapped but
    > that have still a vma intact.

    Thanks Andrea & Hugh. I think I get the point now.

    I guess missed the fact that !pte_present() means it's a swap entry by
    definition... So my previous thoughts on using either _PAGE_PROTNONE or
    _PAGE_SPECIAL could be a bit silly because if without pte_present() they'll
    just be misunderstood as swp entries, then all the _PAGE_* bits won't make any
    sense any more, since they could mean some strange (type, offset) tuple of swp
    entries.

    So yes, I think we need a swp entry, with some special format.

    >
    > The vma removal in munmap() syscall, is then the only point where the
    > pte is only allowed to be cleared for good and only then the pte can
    > be freed.
    >
    > Not even MADV_DONTNEED should be allowed to zero out the pte, it can
    > drop everything but that single bit.

    Right. Ideally I wouldn't expect any sane userspace that uses uffd-wp with
    shmem to MADV_DONTNEED a page right under wr-protection, because it really
    shouldn't do that... However from the semantics of MADV_DONTNEED, indeed it
    should be the same case as the shmem fallocate code where there's a pre-unmap,
    so we should be prepared for that too.

    >
    > > Meanwhile, this reminded me another option - besides _PAGE_PROTNONE, can we use
    > > _PAGE_SPECIAL? That sounds better at least from its naming that it tells it's
    > > a special page already. We can also leverage existing pte_special() checks here
    > > and there, then mimic what we do with pte_devmap(), maybe?
    >
    > That's also for mapped pages VM_PFNMAP or similar I think.
    >
    > By memory the !pte_present case for filebacked vmas only exists as
    > migration entry so I think we'll just add a branch to that case so
    > that it can disambiguate the migration entry from the _PAGE_UFFDP_WP
    > bit.
    >
    > So we can reserve one bit in the migration entry that is always
    > enforced zero when it is a migration entry.
    >
    > When that bit is set on a non-present page in a filebacked vma, it
    > will mean _UFFD_PAGE_WP is set for that vaddr in that mm.

    I'm just afraid there's no space left for a migration entry, because migration
    entries fills in the pfn information into swp offset field rather than a real
    offset (please refer to make_migration_entry())? I assume PFN can use any bit.
    Or did I miss anything?

    I went back to see the original proposal from Hugh:

    IIUC you only need a single value, no need to carve out another whole
    swp_type: could probably be swp_offset 0 of any swp_type other than 0.

    Hugh/Andrea, sorry if this is a stupid swap question: could you help explain
    why swp_offset=0 won't be used by any swap device? I believe it's correct,
    it's just that I failed to figure out the reason myself. :(

    Besides above, to reuse the _PAGE_SWP_UFFD_WP (as it's already there), maybe we
    can also define the special swp entry as:

    swp_entry(0, _PAGE_SWP_UFFD_WP)

    Then as long as we check this against the vma so that we know it's not
    anonymous memory, then it's the special pte. Combined as:

    vma_is_anonymous(vma)==false && swp_entry(0, _PAGE_SWP_UFFD_WP);

    >
    > Then we need to pass a parameter in all pte zapping operations to tell
    > if the unmap event is happening because the vma has been truncated, or
    > if it's happening for any other reason.
    >
    > If it's happening for any other reasons (page truncate, MADV_DONTNEED,
    > memory pressure to swap/writepage) we just convert any present pte
    > with _UFFD_PAGE_WP set, to the non-migration entry non-present pte
    > with the reserved migration entry bit set.
    >
    > If the present pte has no _UFFD_PAGE_WP then it'll be zapped as usual
    > regardless of VM_UFFD_WP in the vma or not.
    >
    > If the pte zapping is instead invoked because of a vma truncation, and
    > it means it's the last unmap operation on that vaddr, the pte zap
    > logic will be told to ignore the _UFFD_PAGE_WP in any present pte so
    > the ptes will be zeroed out and later freed as needed.

    Agreed.

    Thanks,

    --
    Peter Xu

    \
     
     \ /
      Last update: 2020-12-04 03:33    [W:3.690 / U:0.092 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site