On Wed, Nov 02, 2022 at 07:21:19PM +0000, Matthew Wilcox wrote: > On Wed, Nov 02, 2022 at 03:02:35PM -0400, Peter Xu wrote: > > Does the patch attached look reasonable to you? > > Mmm, no. If the page is in the swap cache, this will be "true". It will not happen in practise, right? I mean, shmem_get_folio() should have done the swap-in, and we should have the page lock held at the meantime. For anon, mcopy_atomic_pte() is the only user and it's passing in a newly allocated page here. > > > diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c > > index 3d0fef3980b3..650ab6cfd5f4 100644 > > --- a/mm/userfaultfd.c > > +++ b/mm/userfaultfd.c > > @@ -64,7 +64,7 @@ int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd, > > pte_t _dst_pte, *dst_pte; > > bool writable = dst_vma->vm_flags & VM_WRITE; > > bool vm_shared = dst_vma->vm_flags & VM_SHARED; > > - bool page_in_cache = page->mapping; > > + bool page_in_cache = page_mapping(page); > > We could do: > > struct page *head = compound_head(page); > bool page_in_cache = head->mapping && !PageMappingFlags(head); Sounds good to me, but it just gets a bit complicated. If page_mapping() doesn't sound good, how about we just pass that over from callers? We only have three, so quite doable too. -- Peter Xu
