Messages in this thread | | | Subject | Re: [PATCH v1 2/5] mm/madvise: introduce MADV_POPULATE_(READ|WRITE) to prefault/prealloc memory | From | David Hildenbrand <> | Date | Tue, 30 Mar 2021 18:31:41 +0200 |
| |
On 30.03.21 18:30, David Hildenbrand wrote: > On 30.03.21 18:21, Jann Horn wrote: >> On Tue, Mar 30, 2021 at 5:01 PM David Hildenbrand <david@redhat.com> wrote: >>>>> +long faultin_vma_page_range(struct vm_area_struct *vma, unsigned long start, >>>>> + unsigned long end, bool write, int *locked) >>>>> +{ >>>>> + struct mm_struct *mm = vma->vm_mm; >>>>> + unsigned long nr_pages = (end - start) / PAGE_SIZE; >>>>> + int gup_flags; >>>>> + >>>>> + VM_BUG_ON(!PAGE_ALIGNED(start)); >>>>> + VM_BUG_ON(!PAGE_ALIGNED(end)); >>>>> + VM_BUG_ON_VMA(start < vma->vm_start, vma); >>>>> + VM_BUG_ON_VMA(end > vma->vm_end, vma); >>>>> + mmap_assert_locked(mm); >>>>> + >>>>> + /* >>>>> + * FOLL_HWPOISON: Return -EHWPOISON instead of -EFAULT when we hit >>>>> + * a poisoned page. >>>>> + * FOLL_POPULATE: Always populate memory with VM_LOCKONFAULT. >>>>> + * !FOLL_FORCE: Require proper access permissions. >>>>> + */ >>>>> + gup_flags = FOLL_TOUCH | FOLL_POPULATE | FOLL_MLOCK | FOLL_HWPOISON; >>>>> + if (write) >>>>> + gup_flags |= FOLL_WRITE; >>>>> + >>>>> + /* >>>>> + * See check_vma_flags(): Will return -EFAULT on incompatible mappings >>>>> + * or with insufficient permissions. >>>>> + */ >>>>> + return __get_user_pages(mm, start, nr_pages, gup_flags, >>>>> + NULL, NULL, locked); >>>> >>>> You mentioned in the commit message that you don't want to actually >>>> dirty all the file pages and force writeback; but doesn't >>>> POPULATE_WRITE still do exactly that? In follow_page_pte(), if >>>> FOLL_TOUCH and FOLL_WRITE are set, we mark the page as dirty: >>> >>> Well, I mention that POPULATE_READ explicitly doesn't do that. I >>> primarily set it because populate_vma_page_range() also sets it. >>> >>> Is it safe to *not* set it? IOW, fault something writable into a page >>> table (where the CPU could dirty it without additional page faults) >>> without marking it accessed? For me, this made logically sense. Thus I >>> also understood why populate_vma_page_range() set it. >> >> FOLL_TOUCH doesn't have anything to do with installing the PTE - it >> essentially means "the caller of get_user_pages wants to read/write >> the contents of the returned page, so please do the same things you >> would do if userspace was accessing the page". So in particular, if >> you look up a page via get_user_pages() with FOLL_WRITE|FOLL_TOUCH, >> that tells the MM subsystem "I will be writing into this page directly >> from the kernel, bypassing the userspace page tables, so please mark >> it as dirty now so that it will be properly written back later". Part >> of that is that it marks the page as recently used, which has an >> effect on LRU pageout behavior, I think - as far as I understand, that >> is why populate_vma_page_range() uses FOLL_TOUCH. >> >> If you look at __get_user_pages(), you can see that it is split up >> into two major parts: faultin_page() for creating PTEs, and >> follow_page_mask() for grabbing pages from PTEs. faultin_page() >> ignores FOLL_TOUCH completely; only follow_page_mask() uses it. >> >> In a way I guess maybe you do want the "mark as recently accessed" >> part that FOLL_TOUCH would give you without FOLL_WRITE? But I think >> you very much don't want the dirtying that FOLL_TOUCH|FOLL_WRITE leads >> to. Maybe the ideal approach would be to add a new FOLL flag to say "I >> only want to mark as recently used, I don't want to dirty". Or maybe >> it's enough to just leave out the FOLL_TOUCH entirely, I don't know. > > Any thoughts why populate_vma_page_range() does it?
Sorry, I missed the explanation above - thanks!
-- Thanks,
David / dhildenb
| |