All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lee Schermerhorn <Lee.Schermerhorn@hp.com>
To: linux-mm <linux-mm@kvack.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Ying Han <yinghan@google.com>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Oleg Nesterov <oleg@redhat.com>,
	Pekka Enberg <penberg@cs.helsinki.fi>,
	Paul Menage <menage@google.com>,
	Rohit Seth <rohitseth@google.com>
Subject: [PATCH] mmotm:  ignore sigkill in get_user_pages during munlock
Date: Wed, 03 Dec 2008 15:01:31 -0500	[thread overview]
Message-ID: <1228334491.6693.82.camel@lts-notebook> (raw)
In-Reply-To: <604427e00812022117x6538553w8ceb24e6fa7f3a30@mail.gmail.com>

PATCH ignore sigkill in get_user_pages during munlock

Against:  2.6.28-rc7-mmotm-081203-0150

Fixes:  make-get_user_pages-interruptible.patch

An unfortunate side effect of "make-get_user_pages-interruptible"
is that it prevents a SIGKILL'd task from munlock-ing pages that it
had mlocked, resulting in freeing of mlocked pages.  Freeing of mlocked
pages, in itself, is not so bad.  We just count them now--altho' I
had hoped to remove this stat and add PG_MLOCKED to the free pages
flags check.

However, consider pages in shared libraries mapped by more than one
task that a task mlocked--e.g., via mlockall().  If the task that
mlocked the pages exits via SIGKILL, these pages would be left mlocked
and unevictable.

Proposed fix:

Add another GUP flag to ignore sigkill when calling get_user_pages
from munlock()--similar to Kosaki Motohiro's 'IGNORE_VMA_PERMISSIONS
flag for the same purpose.  We are not actually allocating memory in
this case, which "make-get_user_pages-interruptible" intends to avoid.
We're just munlocking pages that are already resident and mapped, and
we're reusing get_user_pages() to access those pages.

?? Maybe we should combine 'IGNORE_VMA_PERMISSIONS and '_IGNORE_SIGKILL
into a single flag:  GUP_FLAGS_MUNLOCK ???

Signed-off-by: Lee Schermerhorn <lee.schermerhorn@hp.com>

 mm/internal.h |    1 +
 mm/memory.c   |   11 ++++++++---
 mm/mlock.c    |    9 +++++----
 3 files changed, 14 insertions(+), 7 deletions(-)

Index: linux-2.6.28-rc7-mmotm-081203/mm/internal.h
===================================================================
--- linux-2.6.28-rc7-mmotm-081203.orig/mm/internal.h	2008-12-03 14:32:06.000000000 -0500
+++ linux-2.6.28-rc7-mmotm-081203/mm/internal.h	2008-12-03 14:32:08.000000000 -0500
@@ -276,6 +276,7 @@ static inline void mminit_validate_memmo
 #define GUP_FLAGS_WRITE                  0x1
 #define GUP_FLAGS_FORCE                  0x2
 #define GUP_FLAGS_IGNORE_VMA_PERMISSIONS 0x4
+#define GUP_FLAGS_IGNORE_SIGKILL         0x8
 
 int __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
 		     unsigned long start, int len, int flags,
Index: linux-2.6.28-rc7-mmotm-081203/mm/memory.c
===================================================================
--- linux-2.6.28-rc7-mmotm-081203.orig/mm/memory.c	2008-12-03 14:32:06.000000000 -0500
+++ linux-2.6.28-rc7-mmotm-081203/mm/memory.c	2008-12-03 14:33:46.000000000 -0500
@@ -1197,6 +1197,7 @@ int __get_user_pages(struct task_struct 
 	int write = !!(flags & GUP_FLAGS_WRITE);
 	int force = !!(flags & GUP_FLAGS_FORCE);
 	int ignore = !!(flags & GUP_FLAGS_IGNORE_VMA_PERMISSIONS);
+	int ignore_sigkill = !!(flags & GUP_FLAGS_IGNORE_SIGKILL);
 
 	if (len <= 0)
 		return 0;
@@ -1275,10 +1276,14 @@ int __get_user_pages(struct task_struct 
 			struct page *page;
 
 			/*
-			 * If we have a pending SIGKILL, don't keep
-			 * allocating memory.
+			 * If we have a pending SIGKILL, don't keep faulting
+			 * pages and potentially allocating memory, unless
+			 * current is handling munlock--e.g., on exit. In
+			 * that case, we are not allocating memory.  Rather,
+			 * we're only unlocking already resident/mapped pages.
 			 */
-			if (unlikely(fatal_signal_pending(current)))
+			if (unlikely(!ignore_sigkill &&
+					fatal_signal_pending(current)))
 				return i ? i : -ERESTARTSYS;
 
 			if (write)
Index: linux-2.6.28-rc7-mmotm-081203/mm/mlock.c
===================================================================
--- linux-2.6.28-rc7-mmotm-081203.orig/mm/mlock.c	2008-12-03 14:32:06.000000000 -0500
+++ linux-2.6.28-rc7-mmotm-081203/mm/mlock.c	2008-12-03 14:32:08.000000000 -0500
@@ -173,12 +173,13 @@ static long __mlock_vma_pages_range(stru
 		  (atomic_read(&mm->mm_users) != 0));
 
 	/*
-	 * mlock:   don't page populate if page has PROT_NONE permission.
-	 * munlock: the pages always do munlock althrough
-	 *          its has PROT_NONE permission.
+	 * mlock:   don't page populate if vma has PROT_NONE permission.
+	 * munlock: always do munlock although the vma has PROT_NONE
+	 *          permission, or SIGKILL is pending.
 	 */
 	if (!mlock)
-		gup_flags |= GUP_FLAGS_IGNORE_VMA_PERMISSIONS;
+		gup_flags |= GUP_FLAGS_IGNORE_VMA_PERMISSIONS |
+			     GUP_FLAGS_IGNORE_SIGKILL;
 
 	if (vma->vm_flags & VM_WRITE)
 		gup_flags |= GUP_FLAGS_WRITE;


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2008-12-03 20:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-03  5:17 [PATCH][V7]make get_user_pages interruptible Ying Han
2008-12-03  7:19 ` KOSAKI Motohiro
2008-12-03  8:21   ` Pekka Enberg
2008-12-03 15:03 ` Lee Schermerhorn
2008-12-03 20:25   ` Ying Han
2008-12-03 20:36     ` Lee Schermerhorn
2008-12-03 20:01 ` Lee Schermerhorn [this message]
2008-12-04  0:30   ` [PATCH] mmotm: ignore sigkill in get_user_pages during munlock KOSAKI Motohiro
2008-12-04  1:19     ` Ying Han
2008-12-04  1:49     ` Lee Schermerhorn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1228334491.6693.82.camel@lts-notebook \
    --to=lee.schermerhorn@hp.com \
    --cc=akpm@linux-foundation.org \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-mm@kvack.org \
    --cc=menage@google.com \
    --cc=oleg@redhat.com \
    --cc=penberg@cs.helsinki.fi \
    --cc=rohitseth@google.com \
    --cc=yinghan@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.