make get_user_pages interruptible

From: Ying Han <[email protected]>

make get_user_pages interruptible
The initial implementation of checking TIF_MEMDIE covers the cases of OOM killing. If the process has been OOM killed, the TIF_MEMDIE is set and it return immediately. This patch includes:

1. add the case that the SIGKILL is sent by user processes. The process can try to get_user_pages() unlimited memory even if a user process has sent a SIGKILL to it(maybe a monitor find the process exceed its memory limit and try to kill it). In the old implementation, the SIGKILL won't be handled until the get_user_pages() returns.

2. change the return value to be ERESTARTSYS. It makes no sense to return ENOMEM if the get_user_pages returned by getting a SIGKILL signal. Considering the general convention for a system call interrupted by a signal is ERESTARTNOSYS, so the current return value is consistant to that.

Signed-off-by Paul Menage <[email protected]>

Signed-off-by Ying Han <[email protected]>

Quoted 18 lines

From: Ying Han <[email protected]>

make get_user_pages interruptible
The initial implementation of checking TIF_MEMDIE covers the cases of OOM killing. If the process has been OOM killed, the TIF_MEMDIE is set and it return immediately. This patch includes:

1. add the case that the SIGKILL is sent by user processes. The process can try to get_user_pages() unlimited memory even if a user process has sent a SIGKILL to it(maybe a monitor find the process exceed its memory limit and try to kill it). In the old implementation, the SIGKILL won't be handled until the get_user_pages() returns.

2. change the return value to be ERESTARTSYS. It makes no sense to return ENOMEM if the get_user_pages returned by getting a SIGKILL signal. Considering the general convention for a system call interrupted by a signal is ERESTARTNOSYS, so the current return value is consistant to that.

Quoted 2 lines

Signed-off-by Paul Menage <[email protected]>

Signed-off-by Ying Han <[email protected]>

looks good to me.

Reviewed-by KOSAKI Motohiro <[email protected]>

From: Ying Han <[email protected]>

make get_user_pages interruptible
The initial implementation of checking TIF_MEMDIE covers the cases of OOM killing. If the process has been OOM killed, the TIF_MEMDIE is set and it return immediately. This patch includes:

1. add the case that the SIGKILL is sent by user processes. The process can try to get_user_pages() unlimited memory even if a user process has sent a SIGKILL to it(maybe a monitor find the process exceed its memory limit and try to kill it). In the old implementation, the SIGKILL won't be handled until the get_user_pages() returns.

2. change the return value to be ERESTARTSYS. It makes no sense to return ENOMEM if the get_user_pages returned by getting a SIGKILL signal. Considering the general convention for a system call interrupted by a signal is ERESTARTNOSYS, so the current return value is consistant to that.

Signed-off-by Paul Menage <[email protected]>

Signed-off-by Ying Han <[email protected]>

Quoted 21 lines · Ying Han

From: Ying Han <[email protected]>

make get_user_pages interruptible
The initial implementation of checking TIF_MEMDIE covers the cases of OOM killing. If the process has been OOM killed, the TIF_MEMDIE is set and it return immediately. This patch includes:

1. add the case that the SIGKILL is sent by user processes. The process can try to get_user_pages() unlimited memory even if a user process has sent a SIGKILL to it(maybe a monitor find the process exceed its memory limit and try to kill it). In the old implementation, the SIGKILL won't be handled until the get_user_pages() returns.

2. change the return value to be ERESTARTSYS. It makes no sense to return ENOMEM if the get_user_pages returned by getting a SIGKILL signal. Considering the general convention for a system call interrupted by a signal is ERESTARTNOSYS, so the current return value is consistant to that.

Signed-off-by Paul Menage <[email protected]>

Signed-off-by Ying Han <[email protected]>

<snip>

Couple of things:

* I tested your previous patch [that was "just too ugly to live with" :)] overnight with my swap/unevictable-lru/mlocked-pages stress test on both x86_64 and ia64. I replaced the two patches in mmotm 081201 with the "ugly one". Both systems ran for ~16:40 [hh:mm] without error, before I stopped the tests.

* Your patch--bailing out of get_user_pages() when current has SIGKILL pending--breaks munlock on exit when SIGKILL is pending. This results in freeing of mlocked pages [not so bad, I guess] and possibly leaving, e.g., shared library pages mlocked and unevictable after last VM_LOCKED vma is removed. I noticed this because SIGKILL is how the test harness kills off the running tests. I have a patch that fixes this. The overnight runs included this patch. I'll post it after rebasing and a quick retest [he says optimistically] on mmotm-081203.

Lee

Quoted 26 lines · Lee Schermerhorn

Quoted 13 lines · Ying Han

From: Ying Han <[email protected]>

make get_user_pages interruptible
The initial implementation of checking TIF_MEMDIE covers the cases of OOM killing. If the process has been OOM killed, the TIF_MEMDIE is set and it return immediately. This patch includes:

1. add the case that the SIGKILL is sent by user processes. The process can try to get_user_pages() unlimited memory even if a user process has sent a SIGKILL to it(maybe a monitor find the process exceed its memory limit and try to kill it). In the old implementation, the SIGKILL won't be handled until the get_user_pages() returns.

Quoted 8 lines

2. change the return value to be ERESTARTSYS. It makes no sense to return ENOMEM if the get_user_pages returned by getting a SIGKILL signal. Considering the general convention for a system call interrupted by a signal is ERESTARTNOSYS, so the current return value is consistant to that.

Signed-off-by Paul Menage <[email protected]>

Signed-off-by Ying Han <[email protected]>

<snip>

Couple of things:

Quoted 5 lines

* I tested your previous patch [that was "just too ugly to live with" :)] overnight with my swap/unevictable-lru/mlocked-pages stress test on both x86_64 and ia64. I replaced the two patches in mmotm 081201 with the "ugly one". Both systems ran for ~16:40 [hh:mm] without error, before I stopped the tests.

thanks Lee and the "swap/unevictable-lru/mlocked-pages" tests is somewhere i can access? just curious.

Quoted 9 lines

* Your patch--bailing out of get_user_pages() when current has SIGKILL pending--breaks munlock on exit when SIGKILL is pending. This results in freeing of mlocked pages [not so bad, I guess] and possibly leaving, e.g., shared library pages mlocked and unevictable after last VM_LOCKED vma is removed. I noticed this because SIGKILL is how the test harness kills off the running tests. I have a patch that fixes this. The overnight runs included this patch. I'll post it after rebasing and a quick retest [he says optimistically] on mmotm-081203.

sorry not get exactly what you mean it breaks the munlock. :-)

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 ???

Quoted 13 lines

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.

Quoted 4 lines

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.

Indeed!
Thank your for clarification!

Ying, I'd like to explain unevictable lru design for you a bit more.

__get_user_pages() also called exit(2) path.

do_exit()
exit_mm()
mmput()

     exit_mmap()
       munlock_vma_pages_all()
         munlock_vma_pages_range()
           __mlock_vma_pages_range()
             __get_user_pages()

__mlock_vma_pages_range() process
(1) grab mlock related pages by __get_user_pages()
(2) isolate the page from lru
(3) the page move to evictable list if possible

if (1) is interupptible, the page left unevictable lru although the page is not mlocked already.

this feature was introduced 2.6.28-rc1. So I should noticed at last review, very sorry.

this patch is definitly needed.

Reviewed-by KOSAKI Motohiro <[email protected]>

Quoted 10 lines

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.

Quoted 2 lines

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

In my personal feeling, I like current two flags :)

Quoted 13 lines

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.

Quoted 4 lines

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.

Indeed!
Thank your for clarification!

Ying, I'd like to explain unevictable lru design for you a bit more.

__get_user_pages() also called exit(2) path.

do_exit()
exit_mm()

    mmput()
      exit_mmap()
        munlock_vma_pages_all()
          munlock_vma_pages_range()
            __mlock_vma_pages_range()
              __get_user_pages()

__mlock_vma_pages_range() process
(1) grab mlock related pages by __get_user_pages()
(2) isolate the page from lru
(3) the page move to evictable list if possible

if (1) is interupptible, the page left unevictable lru although the page is not mlocked already.

this feature was introduced 2.6.28-rc1. So I should noticed at last review, very sorry.

this patch is definitly needed.

Reviewed-by KOSAKI Motohiro <[email protected]>

Quoted 10 lines

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.

Quoted 2 lines

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

In my personal feeling, I like current two flags :)