On Tue, May 21, 2024 at 10:25:43AM +0200, David Hildenbrand wrote: > On 17.05.24 17:07, Vincent Donnefort wrote: > > Hi David, > > > > [...] > > > > > -static int validate_page_before_insert(struct page *page) > > > +static bool vm_mixed_zeropage_allowed(struct vm_area_struct *vma) > > > +{ > > > + VM_WARN_ON_ONCE(vma->vm_flags & VM_PFNMAP); > > > + /* > > > + * Whoever wants to forbid the zeropage after some zeropages > > > + * might already have been mapped has to scan the page tables and > > > + * bail out on any zeropages. Zeropages in COW mappings can > > > + * be unshared using FAULT_FLAG_UNSHARE faults. > > > + */ > > > + if (mm_forbids_zeropage(vma->vm_mm)) > > > + return false; > > > + /* zeropages in COW mappings are common and unproblematic. */ > > > + if (is_cow_mapping(vma->vm_flags)) > > > + return true; > > > + /* Mappings that do not allow for writable PTEs are unproblematic. */ > > > + if (!(vma->vm_flags & (VM_WRITE | VM_MAYWRITE))) > > > + return false; > > > > Shouldn't we return true here? > > Indeed, thanks! I wish we would have user in the tree already that could > exercise that code path. I have a patch ready to use this path from the memory map tracing! I can either send it once this one is picked-up or you can add it to your series? > > [...] > > > > @@ -2043,7 +2085,7 @@ static int insert_page_in_batch_locked(struct vm_area_struct *vma, pte_t *pte, > > > if (!page_count(page)) > > > return -EINVAL; > > > > This test here prevents inserting the zero-page. > > You mean the existing page_count() check? or the (wrong) vma->vm_flags check > in vm_mixed_zeropage_allowed() ? I meant this page_count() here. As a quick test, I removed that check (also fixed the vm_flags above) and the zero-page was properly mapped! > > -- > Cheers, > > David / dhildenb >