Re: [PATCH V6 3/6] mm: Introduce VM_LOCKONFAULT

[Eric B Munson]

On Thu, 06 Aug 2015, Vlastimil Babka wrote:

...
> >
> >diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> >index ca1e091..38d69fc 100644
> >--- a/fs/proc/task_mmu.c
> >+++ b/fs/proc/task_mmu.c
> >@@ -579,6 +579,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
> 
> This function has the following comment:
> 
> Don't forget to update Documentation/ on changes.
> 
> [...]
> 
> >--- a/mm/gup.c
> >+++ b/mm/gup.c
> >@@ -92,7 +92,7 @@ retry:
> >  		 */
> >  		mark_page_accessed(page);
> >  	}
> >-	if ((flags & FOLL_POPULATE) && (vma->vm_flags & VM_LOCKED)) {
> >+	if ((flags & FOLL_MLOCK) && (vma->vm_flags & VM_LOCKED)) {
> >  		/*
> >  		 * The preliminary mapping check is mainly to avoid the
> >  		 * pointless overhead of lock_page on the ZERO_PAGE
> >@@ -265,6 +265,9 @@ static int faultin_page(struct task_struct *tsk, struct vm_area_struct *vma,
> >  	unsigned int fault_flags = 0;
> >  	int ret;
> >
> >+	/* mlock all present pages, but do not fault in new pages */
> >+	if ((*flags & (FOLL_POPULATE | FOLL_MLOCK)) == FOLL_MLOCK)
> >+		return -ENOENT;
> >  	/* For mm_populate(), just skip the stack guard page. */
> >  	if ((*flags & FOLL_POPULATE) &&
> >  			(stack_guard_page_start(vma, address) ||
> >@@ -850,7 +853,10 @@ long populate_vma_page_range(struct vm_area_struct *vma,
> >  	VM_BUG_ON_VMA(end   > vma->vm_end, vma);
> >  	VM_BUG_ON_MM(!rwsem_is_locked(&mm->mmap_sem), mm);
> >
> >-	gup_flags = FOLL_TOUCH | FOLL_POPULATE;
> >+	gup_flags = FOLL_TOUCH | FOLL_MLOCK;
> >+	if ((vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT)) == VM_LOCKED)
> >+		gup_flags |= FOLL_POPULATE;
> >+
> >  	/*
> >  	 * We want to touch writable mappings with a write fault in order
> >  	 * to break COW, except for shared mappings because these don't COW
> 
> I think this might be breaking the populate part of
> mmap(MAP_POPULATE & ~MAP_LOCKED) case, if I follow the execution
> correctly (it's far from simple...)
> 
> SYSCALL_DEFINE6(mmap_pgoff... with MAP_POPULATE
>   vm_mmap_pgoff(..., MAP_POPULATE...)
>     do_mmap_pgoff(...MAP_POPULATE... &populate) -> populate == TRUE
>     mm_populate()
>       __mm_populate()
>         populate_vma_page_range()
> 
> Previously, this path would have FOLL_POPULATE in gup_flags and
> continue with __get_user_pages() and faultin_page() (actually
> regardless of FOLL_POPULATE) which would fault in the pages.
> 
> After your patch, populate_vma_page_range() will set FOLL_MLOCK, but
> since VM_LOCKED is not set, FOLL_POPULATE won't be set either.
> Then faultin_page() will return on the new check:
> 
> 	flags & (FOLL_POPULATE | FOLL_MLOCK)) == FOLL_MLOCK
> 
> 

I am on vacation atm but I will try and get to respin this series after
making sure there aren't any more FOLL flag issues.

Thanks for keeping with these :)

Eric

Attachment: signature.asc
Description: Digital signature