Messages in this thread Patch in this message | | | Date | Wed, 13 Mar 2024 23:03:04 +0100 | Subject | Re: [PATCH] mm/memory: Fix missing pte marker for !page on pte zaps | From | David Hildenbrand <> |
| |
On 13.03.24 22:31, peterx@redhat.com wrote: > From: Peter Xu <peterx@redhat.com> > > Commit 0cf18e839f64 of large folio zap work broke uffd-wp. Now mm's uffd > unit test "wp-unpopulated" will trigger this WARN_ON_ONCE().
Good that I added the WARN_ON_ONCE() :)
> > The WARN_ON_ONCE() asserts that an VMA cannot be registered with > userfaultfd-wp if it contains a !normal page, but it's actually possible. > One example is an anonymous vma, register with uffd-wp, read anything will > install a zero page. Then when zap on it, this should trigger.
Are you sure? zap_install_uffd_wp_if_needed() contains right at the start:
/* Zap on anonymous always means dropping everything */ if (vma_is_anonymous(vma)) return;
So if that's the case the unit test triggers, I'm confused.
> > What's more, removing that WARN_ON_ONCE may not be enough either, because > we should also not rely on "whether it's a normal page" to decide whether > pte marker is needed. For example, one can register wr-protect over some > DAX regions to track writes when UFFD_FEATURE_WP_ASYNC enabled, in which > case it can have page==NULL for a devmap but we may want to keep the marker > around.
I thought uffd-wp was limited to specific backends only. But looks like that changed with UFFD_FEATURE_WP_ASYNC, I guess?
Change itself looks, good. Not sure about the anon_vma example above.
Thanks!
Acked-by: David Hildenbrand <david@redhat.com>
> > Cc: Muhammad Usama Anjum <usama.anjum@collabora.com> > Cc: David Hildenbrand <david@redhat.com> > Fixes: 0cf18e839f64 ("mm/memory: handle !page case in zap_present_pte() separately") > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > mm/memory.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/mm/memory.c b/mm/memory.c > index f2bc6dd15eb8..904f70b99498 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -1536,7 +1536,9 @@ static inline int zap_present_ptes(struct mmu_gather *tlb, > ptep_get_and_clear_full(mm, addr, pte, tlb->fullmm); > arch_check_zapped_pte(vma, ptent); > tlb_remove_tlb_entry(tlb, pte, addr); > - VM_WARN_ON_ONCE(userfaultfd_wp(vma)); > + if (userfaultfd_pte_wp(vma, ptent)) > + zap_install_uffd_wp_if_needed(vma, addr, pte, 1, > + details, ptent); > ksm_might_unmap_zero_page(mm, ptent); > return 1; > }
-- Cheers,
David / dhildenb
| |