[patch] mm, hugetlb: abort __get_user_pages if current has been oom killed

[patch] mm, hugetlb: abort __get_user_pages if current has been oom killed

[David Rientjes]

If __get_user_pages() is faulting a significant number of hugetlb pages,
usually as the result of mmap(MAP_LOCKED), it can potentially allocate a
very large amount of memory.

If the process has been oom killed, this will cause a lot of memory to
be overcharged to its memcg since it has access to memory reserves or
could potentially deplete all system memory reserves.

In the same way that commit 4779280d1ea4 ("mm: make get_user_pages() 
interruptible") aborted for pending SIGKILLs when faulting non-hugetlb
memory, based on the premise of commit 462e00cc7151 ("oom: stop
allocating user memory if TIF_MEMDIE is set"), hugetlb page faults now
terminate when the process has been oom killed.

Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 mm/gup.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/gup.c b/mm/gup.c
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -457,6 +457,8 @@ long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
 			if (!vma || check_vma_flags(vma, gup_flags))
 				return i ? : -EFAULT;
 			if (is_vm_hugetlb_page(vma)) {
+				if (unlikely(fatal_signal_pending(current)))
+					return i ? : -ERESTARTSYS;
 				i = follow_hugetlb_page(mm, vma, pages, vmas,
 						&start, &nr_pages, i,
 						gup_flags);

Re: [patch] mm, hugetlb: abort __get_user_pages if current has been oom killed

[Kirill A. Shutemov]

On Sun, Mar 08, 2015 at 04:12:12PM -0700, David Rientjes wrote:
> If __get_user_pages() is faulting a significant number of hugetlb pages,
> usually as the result of mmap(MAP_LOCKED), it can potentially allocate a
> very large amount of memory.
> 
> If the process has been oom killed, this will cause a lot of memory to
> be overcharged to its memcg since it has access to memory reserves or
> could potentially deplete all system memory reserves.
> 
> In the same way that commit 4779280d1ea4 ("mm: make get_user_pages() 
> interruptible") aborted for pending SIGKILLs when faulting non-hugetlb
> memory, based on the premise of commit 462e00cc7151 ("oom: stop
> allocating user memory if TIF_MEMDIE is set"), hugetlb page faults now
> terminate when the process has been oom killed.
> 
> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  mm/gup.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/gup.c b/mm/gup.c
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -457,6 +457,8 @@ long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
>  			if (!vma || check_vma_flags(vma, gup_flags))
>  				return i ? : -EFAULT;
>  			if (is_vm_hugetlb_page(vma)) {
> +				if (unlikely(fatal_signal_pending(current)))
> +					return i ? : -ERESTARTSYS;
>  				i = follow_hugetlb_page(mm, vma, pages, vmas,
>  						&start, &nr_pages, i,
>  						gup_flags);

Shouldn't the check be inside loop in follow_hugetlb_page()?
IIUC, it wouldn't help much if nr_pages and hugetlb vma are big enough.

-- 
 Kirill A. Shutemov

[patch v2] mm, hugetlb: abort __get_user_pages if current has been oom killed

[David Rientjes]

If __get_user_pages() is faulting a significant number of hugetlb pages,
usually as the result of mmap(MAP_LOCKED), it can potentially allocate a
very large amount of memory.

If the process has been oom killed, this will cause a lot of memory to
be overcharged to its memcg since it has access to memory reserves or
could potentially deplete all system memory reserves.

In the same way that commit 4779280d1ea4 ("mm: make get_user_pages() 
interruptible") aborted for pending SIGKILLs when faulting non-hugetlb
memory, based on the premise of commit 462e00cc7151 ("oom: stop
allocating user memory if TIF_MEMDIE is set"), hugetlb page faults now
terminate when the process has been oom killed.

Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 v2: check signal inside follow_huegtlb_page() loop per Kirill

 mm/hugetlb.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3276,6 +3276,15 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,
 		struct page *page;
 
 		/*
+		 * If we have a pending SIGKILL, don't keep faulting pages and
+		 * potentially allocating memory.
+		 */
+		if (unlikely(fatal_signal_pending(current))) {
+			remainder = 0;
+			break;
+		}
+
+		/*
 		 * Some archs (sparc64, sh*) have multiple pte_ts to
 		 * each hugepage.  We have to make sure we get the
 		 * first, for the page indexing below to work.

Re: [patch v2] mm, hugetlb: abort __get_user_pages if current has been oom killed

[Kirill A. Shutemov]

On Mon, Mar 09, 2015 at 12:42:15AM -0700, David Rientjes wrote:
> If __get_user_pages() is faulting a significant number of hugetlb pages,
> usually as the result of mmap(MAP_LOCKED), it can potentially allocate a
> very large amount of memory.
> 
> If the process has been oom killed, this will cause a lot of memory to
> be overcharged to its memcg since it has access to memory reserves or
> could potentially deplete all system memory reserves.
> 
> In the same way that commit 4779280d1ea4 ("mm: make get_user_pages() 
> interruptible") aborted for pending SIGKILLs when faulting non-hugetlb
> memory, based on the premise of commit 462e00cc7151 ("oom: stop
> allocating user memory if TIF_MEMDIE is set"), hugetlb page faults now
> terminate when the process has been oom killed.
> 
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
> Signed-off-by: David Rientjes <rientjes@google.com>

Acked-by: Kirill A. Shutemo <kirill.shutemov@linux.intel.com>

-- 
 Kirill A. Shutemov

Re: [patch v2] mm, hugetlb: abort __get_user_pages if current has been oom killed

[Greg Thelen]

On Mon, Mar 09 2015, David Rientjes wrote:

> If __get_user_pages() is faulting a significant number of hugetlb pages,
> usually as the result of mmap(MAP_LOCKED), it can potentially allocate a
> very large amount of memory.
>
> If the process has been oom killed, this will cause a lot of memory to
> be overcharged to its memcg since it has access to memory reserves or
> could potentially deplete all system memory reserves.

s/memcg/hugetlb_cgroup/ but I don't think hugetlb has any
fatal_signal_pending() based overcharging.  I no objection to the patch,
but this doesn't seems like a cgroup thing, so the commit log could
stand a tweak.

> In the same way that commit 4779280d1ea4 ("mm: make get_user_pages() 
> interruptible") aborted for pending SIGKILLs when faulting non-hugetlb
> memory, based on the premise of commit 462e00cc7151 ("oom: stop
> allocating user memory if TIF_MEMDIE is set"), hugetlb page faults now
> terminate when the process has been oom killed.
>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  v2: check signal inside follow_huegtlb_page() loop per Kirill
>
>  mm/hugetlb.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3276,6 +3276,15 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,
>  		struct page *page;
>  
>  		/*
> +		 * If we have a pending SIGKILL, don't keep faulting pages and
> +		 * potentially allocating memory.
> +		 */
> +		if (unlikely(fatal_signal_pending(current))) {
> +			remainder = 0;
> +			break;
> +		}
> +
> +		/*
>  		 * Some archs (sparc64, sh*) have multiple pte_ts to
>  		 * each hugepage.  We have to make sure we get the
>  		 * first, for the page indexing below to work.

[patch v3] mm, hugetlb: abort __get_user_pages if current has been oom killed

[David Rientjes]

If __get_user_pages() is faulting a significant number of hugetlb pages,
usually as the result of mmap(MAP_LOCKED), it can potentially allocate a
very large amount of memory.

If the process has been oom killed, this will cause a lot of memory to
potentially deplete memory reserves.

In the same way that commit 4779280d1ea4 ("mm: make get_user_pages() 
interruptible") aborted for pending SIGKILLs when faulting non-hugetlb
memory, based on the premise of commit 462e00cc7151 ("oom: stop
allocating user memory if TIF_MEMDIE is set"), hugetlb page faults now
terminate when the process has been oom killed.

Cc: Greg Thelen <gthelen@google.com>
Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Acked-by: "Kirill A. Shutemov" <kirill@shutemov.name>
Signed-off-by: David Rientjes <rientjes@google.com>
---
 v3: tweak changelog per Greg

 mm/hugetlb.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3276,6 +3276,15 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,
 		struct page *page;
 
 		/*
+		 * If we have a pending SIGKILL, don't keep faulting pages and
+		 * potentially allocating memory.
+		 */
+		if (unlikely(fatal_signal_pending(current))) {
+			remainder = 0;
+			break;
+		}
+
+		/*
 		 * Some archs (sparc64, sh*) have multiple pte_ts to
 		 * each hugepage.  We have to make sure we get the
 		 * first, for the page indexing below to work.

Re: [patch v3] mm, hugetlb: abort __get_user_pages if current has been oom killed

[Rik van Riel]

On 03/09/2015 04:07 PM, David Rientjes wrote:
> If __get_user_pages() is faulting a significant number of hugetlb pages,
> usually as the result of mmap(MAP_LOCKED), it can potentially allocate a
> very large amount of memory.
>
> If the process has been oom killed, this will cause a lot of memory to
> potentially deplete memory reserves.
>
> In the same way that commit 4779280d1ea4 ("mm: make get_user_pages()
> interruptible") aborted for pending SIGKILLs when faulting non-hugetlb
> memory, based on the premise of commit 462e00cc7151 ("oom: stop
> allocating user memory if TIF_MEMDIE is set"), hugetlb page faults now
> terminate when the process has been oom killed.
>
> Cc: Greg Thelen <gthelen@google.com>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Acked-by: "Kirill A. Shutemov" <kirill@shutemov.name>
> Signed-off-by: David Rientjes <rientjes@google.com>

Acked-by: Rik van Riel <riel@redhat.com>

Re: [patch v3] mm, hugetlb: abort __get_user_pages if current has been oom killed

[Greg Thelen]

On Mon, Mar 09 2015, David Rientjes wrote:

> If __get_user_pages() is faulting a significant number of hugetlb pages,
> usually as the result of mmap(MAP_LOCKED), it can potentially allocate a
> very large amount of memory.
>
> If the process has been oom killed, this will cause a lot of memory to
> potentially deplete memory reserves.
>
> In the same way that commit 4779280d1ea4 ("mm: make get_user_pages() 
> interruptible") aborted for pending SIGKILLs when faulting non-hugetlb
> memory, based on the premise of commit 462e00cc7151 ("oom: stop
> allocating user memory if TIF_MEMDIE is set"), hugetlb page faults now
> terminate when the process has been oom killed.
>
> Cc: Greg Thelen <gthelen@google.com>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Acked-by: "Kirill A. Shutemov" <kirill@shutemov.name>
> Signed-off-by: David Rientjes <rientjes@google.com>

Looks good.

Acked-by: "Greg Thelen" <gthelen@google.com>

Re: [patch v3] mm, hugetlb: abort __get_user_pages if current has been oom killed

[Davidlohr Bueso]

On Mon, 2015-03-09 at 13:07 -0700, David Rientjes wrote:
> If __get_user_pages() is faulting a significant number of hugetlb pages,
> usually as the result of mmap(MAP_LOCKED), it can potentially allocate a
> very large amount of memory.
> 
> If the process has been oom killed, this will cause a lot of memory to
> potentially deplete memory reserves.
> 
> In the same way that commit 4779280d1ea4 ("mm: make get_user_pages() 
> interruptible") aborted for pending SIGKILLs when faulting non-hugetlb
> memory, based on the premise of commit 462e00cc7151 ("oom: stop
> allocating user memory if TIF_MEMDIE is set"), hugetlb page faults now
> terminate when the process has been oom killed.
> 
> Cc: Greg Thelen <gthelen@google.com>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Acked-by: "Kirill A. Shutemov" <kirill@shutemov.name>
> Signed-off-by: David Rientjes <rientjes@google.com>

Makes sense.

Acked-by: Davidlohr Bueso <dave@stgolabs.net>