On 21.05.24 11:06, Vincent Donnefort wrote:
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?
Whatever works for you! To debug, it would be good if you could send me
the patch and simple instructions on how to test it (do we have a
selftest as well?).
[...]
@@ -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!
That's weird and might indicate another issue.
The refcount of the shared zeropage should be initialized to 1, just
like for any other reserved pages
(mm/mm_init.c:__init_single_page()->init_page_count())
Hm ...
--
Cheers,
David / dhildenb
