Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1214107 > unrolled thread

[PATCH v8 3/6] mm: Introduce VM_LOCKONFAULT

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v8 3/6] mm: Introduce VM_LOCKONFAULT Eric B Munson <emunson@akamai.com> - 2015-08-26 20:30 +0200
    Re: [PATCH v8 3/6] mm: Introduce VM_LOCKONFAULT Michal Hocko <mhocko@kernel.org> - 2015-08-28 16:20 +0200
      Re: [PATCH v8 3/6] mm: Introduce VM_LOCKONFAULT Eric B Munson <emunson@akamai.com> - 2015-08-28 21:40 +0200
        Re: [PATCH v8 3/6] mm: Introduce VM_LOCKONFAULT Michal Hocko <mhocko@kernel.org> - 2015-08-31 11:00 +0200
      Re: [PATCH v8 3/6] mm: Introduce VM_LOCKONFAULT Eric B Munson <emunson@mgebm.net> - 2015-09-01 15:00 +0200
    Re: [PATCH v8 3/6] mm: Introduce VM_LOCKONFAULT Vlastimil Babka <vbabka@suse.cz> - 2015-08-28 17:00 +0200

#1214107 — [PATCH v8 3/6] mm: Introduce VM_LOCKONFAULT

The cost of faulting in all memory to be locked can be very high when
working with large mappings.  If only portions of the mapping will be
used this can incur a high penalty for locking.

For the example of a large file, this is the usage pattern for a large
statical language model (probably applies to other statical or graphical
models as well).  For the security example, any application transacting
in data that cannot be swapped out (credit card data, medical records,
etc).

This patch introduces the ability to request that pages are not
pre-faulted, but are placed on the unevictable LRU when they are finally
faulted in.  The VM_LOCKONFAULT flag will be used together with
VM_LOCKED and has no effect when set without VM_LOCKED.  Setting the
VM_LOCKONFAULT flag for a VMA will cause pages faulted into that VMA to
be added to the unevictable LRU when they are faulted or if they are
already present, but will not cause any missing pages to be faulted in.

Exposing this new lock state means that we cannot overload the meaning
of the FOLL_POPULATE flag any longer.  Prior to this patch it was used
to mean that the VMA for a fault was locked.  This means we need the
new FOLL_MLOCK flag to communicate the locked state of a VMA.
FOLL_POPULATE will now only control if the VMA should be populated and
in the case of VM_LOCKONFAULT, it will not be set.

Signed-off-by: Eric B Munson <emunson@akamai.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: linux-kernel@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: linux-api@vger.kernel.org
---
Changes from v7:
*Drop entries in smaps and dri code to avoid exposing VM_LOCKONFAULT to
 userspace.  VM_LOCKONFAULT is still exposed via mm/debug.c
*Create VM_LOCKED_CLEAR_MASK to be used anywhere we want to clear all
 flags relating to locked VMAs

 include/linux/mm.h |  5 +++++
 kernel/fork.c      |  2 +-
 mm/debug.c         |  1 +
 mm/gup.c           | 10 ++++++++--
 mm/huge_memory.c   |  2 +-
 mm/hugetlb.c       |  4 ++--
 mm/mlock.c         |  2 +-
 mm/mmap.c          |  2 +-
 mm/rmap.c          |  6 ++++--
 9 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index bf6f117..47ff851 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -136,6 +136,7 @@ extern unsigned int kobjsize(const void *objp);
 
 #define VM_DONTCOPY	0x00020000      /* Do not copy this vma on fork */
 #define VM_DONTEXPAND	0x00040000	/* Cannot expand with mremap() */
+#define VM_LOCKONFAULT	0x00080000	/* Lock the pages covered when they are faulted in */
 #define VM_ACCOUNT	0x00100000	/* Is a VM accounted object */
 #define VM_NORESERVE	0x00200000	/* should the VM suppress accounting */
 #define VM_HUGETLB	0x00400000	/* Huge TLB Page VM */
@@ -199,6 +200,9 @@ extern unsigned int kobjsize(const void *objp);
 /* This mask defines which mm->def_flags a process can inherit its parent */
 #define VM_INIT_DEF_MASK	VM_NOHUGEPAGE
 
+/* This mask is used to clear all the VMA flags used by mlock */
+#define VM_LOCKED_CLEAR_MASK	(~(VM_LOCKED | VM_LOCKONFAULT))
+
 /*
  * mapping from the currently active vm_flags protection bits (the
  * low four bits) to a page protection mask..
@@ -2071,6 +2075,7 @@ static inline struct page *follow_page(struct vm_area_struct *vma,
 #define FOLL_NUMA	0x200	/* force NUMA hinting page fault */
 #define FOLL_MIGRATION	0x400	/* wait for page to replace migration entry */
 #define FOLL_TRIED	0x800	/* a retry, previous pass started an IO */
+#define FOLL_MLOCK	0x1000	/* lock present pages */
 
 typedef int (*pte_fn_t)(pte_t *pte, pgtable_t token, unsigned long addr,
 			void *data);
diff --git a/kernel/fork.c b/kernel/fork.c
index dbd9b8d..eee1cc5 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -454,7 +454,7 @@ static int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
 		tmp->vm_mm = mm;
 		if (anon_vma_fork(tmp, mpnt))
 			goto fail_nomem_anon_vma_fork;
-		tmp->vm_flags &= ~VM_LOCKED;
+		tmp->vm_flags &= VM_LOCKED_CLEAR_MASK;
 		tmp->vm_next = tmp->vm_prev = NULL;
 		file = tmp->vm_file;
 		if (file) {
diff --git a/mm/debug.c b/mm/debug.c
index 76089dd..25176bb 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -121,6 +121,7 @@ static const struct trace_print_flags vmaflags_names[] = {
 	{VM_GROWSDOWN,			"growsdown"	},
 	{VM_PFNMAP,			"pfnmap"	},
 	{VM_DENYWRITE,			"denywrite"	},
+	{VM_LOCKONFAULT,		"lockonfault"	},
 	{VM_LOCKED,			"locked"	},
 	{VM_IO,				"io"		},
 	{VM_SEQ_READ,			"seqread"	},
diff --git a/mm/gup.c b/mm/gup.c
index 6297f6b..dce6ccd 100644
--- 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_POPULATE | FOLL_MLOCK;
+	if (vma->vm_flags & VM_LOCKONFAULT)
+		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
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 097c7a4..cba783e 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1238,7 +1238,7 @@ struct page *follow_trans_huge_pmd(struct vm_area_struct *vma,
 					  pmd, _pmd,  1))
 			update_mmu_cache_pmd(vma, addr, pmd);
 	}
-	if ((flags & FOLL_POPULATE) && (vma->vm_flags & VM_LOCKED)) {
+	if ((flags & FOLL_MLOCK) && (vma->vm_flags & VM_LOCKED)) {
 		if (page->mapping && trylock_page(page)) {
 			lru_add_drain();
 			if (page->mapping)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index a8c3087..a01ccc3 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3764,8 +3764,8 @@ static unsigned long page_table_shareable(struct vm_area_struct *svma,
 	unsigned long s_end = sbase + PUD_SIZE;
 
 	/* Allow segments to share if only one is marked locked */
-	unsigned long vm_flags = vma->vm_flags & ~VM_LOCKED;
-	unsigned long svm_flags = svma->vm_flags & ~VM_LOCKED;
+	unsigned long vm_flags = vma->vm_flags & VM_LOCKED_CLEAR_MASK;
+	unsigned long svm_flags = svma->vm_flags & VM_LOCKED_CLEAR_MASK;
 
 	/*
 	 * match the virtual addresses, permission and the alignment of the
diff --git a/mm/mlock.c b/mm/mlock.c
index 3094f27..7efe27d 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -422,7 +422,7 @@ static unsigned long __munlock_pagevec_fill(struct pagevec *pvec,
 void munlock_vma_pages_range(struct vm_area_struct *vma,
 			     unsigned long start, unsigned long end)
 {
-	vma->vm_flags &= ~VM_LOCKED;
+	vma->vm_flags &= VM_LOCKED_CLEAR_MASK;
 
 	while (start < end) {
 		struct page *page = NULL;
diff --git a/mm/mmap.c b/mm/mmap.c
index aa632ad..019dcd6 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1651,7 +1651,7 @@ out:
 					vma == get_gate_vma(current->mm)))
 			mm->locked_vm += (len >> PAGE_SHIFT);
 		else
-			vma->vm_flags &= ~VM_LOCKED;
+			vma->vm_flags &= VM_LOCKED_CLEAR_MASK;
 	}
 
 	if (file)
diff --git a/mm/rmap.c b/mm/rmap.c
index 171b687..14ce002 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -744,7 +744,8 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
 
 		if (vma->vm_flags & VM_LOCKED) {
 			spin_unlock(ptl);
-			pra->vm_flags |= VM_LOCKED;
+			pra->vm_flags |=
+				(vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT));
 			return SWAP_FAIL; /* To break the loop */
 		}
 
@@ -765,7 +766,8 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
 
 		if (vma->vm_flags & VM_LOCKED) {
 			pte_unmap_unlock(pte, ptl);
-			pra->vm_flags |= VM_LOCKED;
+			pra->vm_flags |=
+				(vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT));
 			return SWAP_FAIL; /* To break the loop */
 		}
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1215359

On Wed 26-08-15 14:24:22, Eric B Munson wrote:
> The cost of faulting in all memory to be locked can be very high when
> working with large mappings.  If only portions of the mapping will be
> used this can incur a high penalty for locking.
> 
> For the example of a large file, this is the usage pattern for a large
> statical language model (probably applies to other statical or graphical
> models as well).  For the security example, any application transacting
> in data that cannot be swapped out (credit card data, medical records,
> etc).
> 
> This patch introduces the ability to request that pages are not
> pre-faulted, but are placed on the unevictable LRU when they are finally
> faulted in.  The VM_LOCKONFAULT flag will be used together with
> VM_LOCKED and has no effect when set without VM_LOCKED.  Setting the
> VM_LOCKONFAULT flag for a VMA will cause pages faulted into that VMA to
> be added to the unevictable LRU when they are faulted or if they are
> already present, but will not cause any missing pages to be faulted in.

OK, I can live with this. Thank you for removing the part which exports
the flag to the userspace.
 
> Exposing this new lock state means that we cannot overload the meaning
> of the FOLL_POPULATE flag any longer.  Prior to this patch it was used
> to mean that the VMA for a fault was locked.  This means we need the
> new FOLL_MLOCK flag to communicate the locked state of a VMA.
> FOLL_POPULATE will now only control if the VMA should be populated and
> in the case of VM_LOCKONFAULT, it will not be set.

I thinking that this part is really unnecessary. populate_vma_page_range
could have simply returned without calling gup for VM_LOCKONFAULT
vmas. You would save the pte walk and the currently mapped pages would
be still protected from the reclaim. The side effect would be that they
would litter the regular LRUs and mlock/unevictable counters wouldn't be
updated until those pages are encountered during the reclaim and culled
to unevictable list.

I would expect that mlock with this flag would be typically called
on mostly unpopulated mappings so the side effects would be barely
noticeable while the lack of pte walk would be really nice (especially
for the large mappings).

This would be a nice optimization and minor code reduction but I am not
going to insist on it. I will leave the decision to you.

> Signed-off-by: Eric B Munson <emunson@akamai.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-mm@kvack.org
> Cc: linux-api@vger.kernel.org

Acked-by: Michal Hocko <mhocko@suse.com>

One note below:

> ---
> Changes from v7:
> *Drop entries in smaps and dri code to avoid exposing VM_LOCKONFAULT to
>  userspace.  VM_LOCKONFAULT is still exposed via mm/debug.c
> *Create VM_LOCKED_CLEAR_MASK to be used anywhere we want to clear all
>  flags relating to locked VMAs
> 
>  include/linux/mm.h |  5 +++++
>  kernel/fork.c      |  2 +-
>  mm/debug.c         |  1 +
>  mm/gup.c           | 10 ++++++++--
>  mm/huge_memory.c   |  2 +-
>  mm/hugetlb.c       |  4 ++--
>  mm/mlock.c         |  2 +-
>  mm/mmap.c          |  2 +-
>  mm/rmap.c          |  6 ++++--
>  9 files changed, 24 insertions(+), 10 deletions(-)
[...]
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 171b687..14ce002 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -744,7 +744,8 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
>  
>  		if (vma->vm_flags & VM_LOCKED) {
>  			spin_unlock(ptl);
> -			pra->vm_flags |= VM_LOCKED;
> +			pra->vm_flags |=
> +				(vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT));
>  			return SWAP_FAIL; /* To break the loop */
>  		}
>  
> @@ -765,7 +766,8 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
>  
>  		if (vma->vm_flags & VM_LOCKED) {
>  			pte_unmap_unlock(pte, ptl);
> -			pra->vm_flags |= VM_LOCKED;
> +			pra->vm_flags |=
> +				(vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT));
>  			return SWAP_FAIL; /* To break the loop */
>  		}

Why do we need to export this? Neither of the consumers care and should
care. VM_LOCKONFAULT should never be set without VM_LOCKED which is the
only thing that we should care about.
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1215531

[Multipart message — attachments visible in raw view] — view raw

On Fri, 28 Aug 2015, Michal Hocko wrote:

> On Wed 26-08-15 14:24:22, Eric B Munson wrote:
> > The cost of faulting in all memory to be locked can be very high when
> > working with large mappings.  If only portions of the mapping will be
> > used this can incur a high penalty for locking.
> > 
> > For the example of a large file, this is the usage pattern for a large
> > statical language model (probably applies to other statical or graphical
> > models as well).  For the security example, any application transacting
> > in data that cannot be swapped out (credit card data, medical records,
> > etc).
> > 
> > This patch introduces the ability to request that pages are not
> > pre-faulted, but are placed on the unevictable LRU when they are finally
> > faulted in.  The VM_LOCKONFAULT flag will be used together with
> > VM_LOCKED and has no effect when set without VM_LOCKED.  Setting the
> > VM_LOCKONFAULT flag for a VMA will cause pages faulted into that VMA to
> > be added to the unevictable LRU when they are faulted or if they are
> > already present, but will not cause any missing pages to be faulted in.
> 
> OK, I can live with this. Thank you for removing the part which exports
> the flag to the userspace.
>  
> > Exposing this new lock state means that we cannot overload the meaning
> > of the FOLL_POPULATE flag any longer.  Prior to this patch it was used
> > to mean that the VMA for a fault was locked.  This means we need the
> > new FOLL_MLOCK flag to communicate the locked state of a VMA.
> > FOLL_POPULATE will now only control if the VMA should be populated and
> > in the case of VM_LOCKONFAULT, it will not be set.
> 
> I thinking that this part is really unnecessary. populate_vma_page_range
> could have simply returned without calling gup for VM_LOCKONFAULT
> vmas. You would save the pte walk and the currently mapped pages would
> be still protected from the reclaim. The side effect would be that they
> would litter the regular LRUs and mlock/unevictable counters wouldn't be
> updated until those pages are encountered during the reclaim and culled
> to unevictable list.
> 
> I would expect that mlock with this flag would be typically called
> on mostly unpopulated mappings so the side effects would be barely
> noticeable while the lack of pte walk would be really nice (especially
> for the large mappings).
> 
> This would be a nice optimization and minor code reduction but I am not
> going to insist on it. I will leave the decision to you.

If I am understanding you correctly, this is how the lock on fault set
started.  Jon Corbet pointed out that this would leave pages which were
present when mlock2(MLOCK_ONFAULT) was called in an unlocked state, only
locking them after they were reclaimed and then refaulted.

Even if this was never the case, we scan the entire range for a call to
mlock() and will lock the pages which are present.  Why would we pay the
cost of getting the accounting right on the present pages for mlock, but
not lock on fault?

> 
> > Signed-off-by: Eric B Munson <emunson@akamai.com>
> > Cc: Michal Hocko <mhocko@suse.cz>
> > Cc: Vlastimil Babka <vbabka@suse.cz>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-mm@kvack.org
> > Cc: linux-api@vger.kernel.org
> 
> Acked-by: Michal Hocko <mhocko@suse.com>
> 
> One note below:
> 
> > ---
> > Changes from v7:
> > *Drop entries in smaps and dri code to avoid exposing VM_LOCKONFAULT to
> >  userspace.  VM_LOCKONFAULT is still exposed via mm/debug.c
> > *Create VM_LOCKED_CLEAR_MASK to be used anywhere we want to clear all
> >  flags relating to locked VMAs
> > 
> >  include/linux/mm.h |  5 +++++
> >  kernel/fork.c      |  2 +-
> >  mm/debug.c         |  1 +
> >  mm/gup.c           | 10 ++++++++--
> >  mm/huge_memory.c   |  2 +-
> >  mm/hugetlb.c       |  4 ++--
> >  mm/mlock.c         |  2 +-
> >  mm/mmap.c          |  2 +-
> >  mm/rmap.c          |  6 ++++--
> >  9 files changed, 24 insertions(+), 10 deletions(-)
> [...]
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > index 171b687..14ce002 100644
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -744,7 +744,8 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
> >  
> >  		if (vma->vm_flags & VM_LOCKED) {
> >  			spin_unlock(ptl);
> > -			pra->vm_flags |= VM_LOCKED;
> > +			pra->vm_flags |=
> > +				(vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT));
> >  			return SWAP_FAIL; /* To break the loop */
> >  		}
> >  
> > @@ -765,7 +766,8 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
> >  
> >  		if (vma->vm_flags & VM_LOCKED) {
> >  			pte_unmap_unlock(pte, ptl);
> > -			pra->vm_flags |= VM_LOCKED;
> > +			pra->vm_flags |=
> > +				(vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT));
> >  			return SWAP_FAIL; /* To break the loop */
> >  		}
> 
> Why do we need to export this? Neither of the consumers care and should
> care. VM_LOCKONFAULT should never be set without VM_LOCKED which is the
> only thing that we should care about.

I exported VM_LOCKONFAULT because this is an internal interface and I
saw no harm in doing so.  I do not have a use case for it at the moment,
so I would be fine dropping this hunk.

[toc] | [prev] | [next] | [standalone]

#1216112

On Fri 28-08-15 15:34:54, Eric B Munson wrote:
> On Fri, 28 Aug 2015, Michal Hocko wrote:
> 
> > On Wed 26-08-15 14:24:22, Eric B Munson wrote:
> > > The cost of faulting in all memory to be locked can be very high when
> > > working with large mappings.  If only portions of the mapping will be
> > > used this can incur a high penalty for locking.
> > > 
> > > For the example of a large file, this is the usage pattern for a large
> > > statical language model (probably applies to other statical or graphical
> > > models as well).  For the security example, any application transacting
> > > in data that cannot be swapped out (credit card data, medical records,
> > > etc).
> > > 
> > > This patch introduces the ability to request that pages are not
> > > pre-faulted, but are placed on the unevictable LRU when they are finally
> > > faulted in.  The VM_LOCKONFAULT flag will be used together with
> > > VM_LOCKED and has no effect when set without VM_LOCKED.  Setting the
> > > VM_LOCKONFAULT flag for a VMA will cause pages faulted into that VMA to
> > > be added to the unevictable LRU when they are faulted or if they are
> > > already present, but will not cause any missing pages to be faulted in.
> > 
> > OK, I can live with this. Thank you for removing the part which exports
> > the flag to the userspace.
> >  
> > > Exposing this new lock state means that we cannot overload the meaning
> > > of the FOLL_POPULATE flag any longer.  Prior to this patch it was used
> > > to mean that the VMA for a fault was locked.  This means we need the
> > > new FOLL_MLOCK flag to communicate the locked state of a VMA.
> > > FOLL_POPULATE will now only control if the VMA should be populated and
> > > in the case of VM_LOCKONFAULT, it will not be set.
> > 
> > I thinking that this part is really unnecessary. populate_vma_page_range
> > could have simply returned without calling gup for VM_LOCKONFAULT
> > vmas. You would save the pte walk and the currently mapped pages would
> > be still protected from the reclaim. The side effect would be that they
> > would litter the regular LRUs and mlock/unevictable counters wouldn't be
> > updated until those pages are encountered during the reclaim and culled
> > to unevictable list.
> > 
> > I would expect that mlock with this flag would be typically called
> > on mostly unpopulated mappings so the side effects would be barely
> > noticeable while the lack of pte walk would be really nice (especially
> > for the large mappings).
> > 
> > This would be a nice optimization and minor code reduction but I am not
> > going to insist on it. I will leave the decision to you.
> 
> If I am understanding you correctly, this is how the lock on fault set
> started.  Jon Corbet pointed out that this would leave pages which were
> present when mlock2(MLOCK_ONFAULT) was called in an unlocked state, only
> locking them after they were reclaimed and then refaulted.

Not really. They would be lazily locked during the reclaim. Have a look
at try_to_unmap -> try_to_unmap_one path. So those pages will be
effectively locked - just not accounted for that fact yet. 

> Even if this was never the case, we scan the entire range for a call to
> mlock() and will lock the pages which are present.  Why would we pay the
> cost of getting the accounting right on the present pages for mlock, but
> not lock on fault?

Because mlock() has a different semantic and you _have_ to walk the whole
range just to pre-fault memory. Mlocking the already present pages is
not really adding much on top. Situation is different with lock on
fault because pre-faulting doesn't happen and crawling the whole range
just to find present pages sounds like a wasted time when the same can
be handled lazily.

But as I've said, I will not insist...

> > > Signed-off-by: Eric B Munson <emunson@akamai.com>
> > > Cc: Michal Hocko <mhocko@suse.cz>
> > > Cc: Vlastimil Babka <vbabka@suse.cz>
> > > Cc: Jonathan Corbet <corbet@lwn.net>
> > > Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
> > > Cc: linux-kernel@vger.kernel.org
> > > Cc: linux-mm@kvack.org
> > > Cc: linux-api@vger.kernel.org
> > 
> > Acked-by: Michal Hocko <mhocko@suse.com>
> > 
> > One note below:
> > 
> > > ---
> > > Changes from v7:
> > > *Drop entries in smaps and dri code to avoid exposing VM_LOCKONFAULT to
> > >  userspace.  VM_LOCKONFAULT is still exposed via mm/debug.c
> > > *Create VM_LOCKED_CLEAR_MASK to be used anywhere we want to clear all
> > >  flags relating to locked VMAs
> > > 
> > >  include/linux/mm.h |  5 +++++
> > >  kernel/fork.c      |  2 +-
> > >  mm/debug.c         |  1 +
> > >  mm/gup.c           | 10 ++++++++--
> > >  mm/huge_memory.c   |  2 +-
> > >  mm/hugetlb.c       |  4 ++--
> > >  mm/mlock.c         |  2 +-
> > >  mm/mmap.c          |  2 +-
> > >  mm/rmap.c          |  6 ++++--
> > >  9 files changed, 24 insertions(+), 10 deletions(-)
> > [...]
> > > diff --git a/mm/rmap.c b/mm/rmap.c
> > > index 171b687..14ce002 100644
> > > --- a/mm/rmap.c
> > > +++ b/mm/rmap.c
> > > @@ -744,7 +744,8 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
> > >  
> > >  		if (vma->vm_flags & VM_LOCKED) {
> > >  			spin_unlock(ptl);
> > > -			pra->vm_flags |= VM_LOCKED;
> > > +			pra->vm_flags |=
> > > +				(vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT));
> > >  			return SWAP_FAIL; /* To break the loop */
> > >  		}
> > >  
> > > @@ -765,7 +766,8 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
> > >  
> > >  		if (vma->vm_flags & VM_LOCKED) {
> > >  			pte_unmap_unlock(pte, ptl);
> > > -			pra->vm_flags |= VM_LOCKED;
> > > +			pra->vm_flags |=
> > > +				(vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT));
> > >  			return SWAP_FAIL; /* To break the loop */
> > >  		}
> > 
> > Why do we need to export this? Neither of the consumers care and should
> > care. VM_LOCKONFAULT should never be set without VM_LOCKED which is the
> > only thing that we should care about.
> 
> I exported VM_LOCKONFAULT because this is an internal interface and I
> saw no harm in doing so.  I do not have a use case for it at the moment,
> so I would be fine dropping this hunk.
 
I was objecting because nobody except for the population path should
really care about this flag. The real locking semantic is already
described by VM_LOCKED. If there ever is a user of VM_LOCKONFAULT from
those paths it should be added explicitly. So please drop these two.
The fewer instances of VM_LOCKONFAULT we have the easier this will be to
maintain.
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1216825

On August 28, 2015 10:18:30 AM EDT, Michal Hocko <mhocko@kernel.org> wrote:

>
>Why do we need to export this? Neither of the consumers care and should
>care. VM_LOCKONFAULT should never be set without VM_LOCKED which is the
>only thing that we should care about.

I am out of the office and I don't know if I will be back in time to respin this series without this hunk.  Andrew, can you remove the export of VM_LOCKONFAULT to the rmap code?

Thank,
Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1215383

On 08/26/2015 08:24 PM, Eric B Munson wrote:
> The cost of faulting in all memory to be locked can be very high when
> working with large mappings.  If only portions of the mapping will be
> used this can incur a high penalty for locking.
>
> For the example of a large file, this is the usage pattern for a large
> statical language model (probably applies to other statical or graphical
> models as well).  For the security example, any application transacting
> in data that cannot be swapped out (credit card data, medical records,
> etc).
>
> This patch introduces the ability to request that pages are not
> pre-faulted, but are placed on the unevictable LRU when they are finally
> faulted in.  The VM_LOCKONFAULT flag will be used together with
> VM_LOCKED and has no effect when set without VM_LOCKED.  Setting the
> VM_LOCKONFAULT flag for a VMA will cause pages faulted into that VMA to
> be added to the unevictable LRU when they are faulted or if they are
> already present, but will not cause any missing pages to be faulted in.
>
> Exposing this new lock state means that we cannot overload the meaning
> of the FOLL_POPULATE flag any longer.  Prior to this patch it was used
> to mean that the VMA for a fault was locked.  This means we need the
> new FOLL_MLOCK flag to communicate the locked state of a VMA.
> FOLL_POPULATE will now only control if the VMA should be populated and
> in the case of VM_LOCKONFAULT, it will not be set.
>
> Signed-off-by: Eric B Munson <emunson@akamai.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-mm@kvack.org
> Cc: linux-api@vger.kernel.org

Acked-by: Vlastimil Babka <vbabka@suse.cz>

I just wonder if the call to populate_vma_page_range from mprotect_fixup 
is just an potentially expensive no-op for VM_LOCKONFAULT vma's? It 
might find many cow candidates but faultin_page() won't do anything. And 
it shouldn't find any existing pages to put on the unevictable list from 
this context.

But it's a corner case and preventing it would mean putting in another 
VM_LOCKONFAULT check so maybe we can leave it like this.
-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web