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

Groups > linux.kernel > #1325396 > unrolled thread

[PATCH 0/5] oom reaper v5

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] oom reaper v5 Michal Hocko <mhocko@kernel.org> - 2016-02-03 14:20 +0100
    [PATCH 2/5] oom reaper: handle mlocked pages Michal Hocko <mhocko@kernel.org> - 2016-02-03 14:20 +0100
      Re: [PATCH 2/5] oom reaper: handle mlocked pages David Rientjes <rientjes@google.com> - 2016-02-04 01:00 +0100
    [PATCH 4/5] mm, oom_reaper: report success/failure Michal Hocko <mhocko@kernel.org> - 2016-02-03 14:20 +0100
      Re: [PATCH 4/5] mm, oom_reaper: report success/failure David Rientjes <rientjes@google.com> - 2016-02-04 00:20 +0100
        Re: [PATCH 4/5] mm, oom_reaper: report success/failure Michal Hocko <mhocko@kernel.org> - 2016-02-04 07:50 +0100
          Re: [PATCH 4/5] mm, oom_reaper: report success/failure David Rientjes <rientjes@google.com> - 2016-02-04 23:40 +0100
            Re: [PATCH 4/5] mm, oom_reaper: report success/failure Michal Hocko <mhocko@kernel.org> - 2016-02-05 10:30 +0100
              Re: [PATCH 4/5] mm, oom_reaper: report success/failure Michal Hocko <mhocko@kernel.org> - 2016-02-06 07:40 +0100
    [PATCH 1/5] mm, oom: introduce oom reaper Michal Hocko <mhocko@kernel.org> - 2016-02-03 14:20 +0100
      Re: [PATCH 1/5] mm, oom: introduce oom reaper David Rientjes <rientjes@google.com> - 2016-02-04 00:50 +0100
        Re: [PATCH 1/5] mm, oom: introduce oom reaper Michal Hocko <mhocko@kernel.org> - 2016-02-04 07:50 +0100
      Re: [PATCH 1/5] mm, oom: introduce oom reaper Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-02-06 14:30 +0100
    [PATCH 5/5] mm, oom_reaper: implement OOM victims queuing Michal Hocko <mhocko@kernel.org> - 2016-02-03 14:20 +0100
      Re: [PATCH 5/5] mm, oom_reaper: implement OOM victims queuing Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-02-04 11:50 +0100
        Re: [PATCH 5/5] mm, oom_reaper: implement OOM victims queuing Michal Hocko <mhocko@kernel.org> - 2016-02-04 16:00 +0100
          Re: [PATCH 5/5] mm, oom_reaper: implement OOM victims queuing Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-02-06 07:00 +0100
            Re: [PATCH 5/5] mm, oom_reaper: implement OOM victims queuing Michal Hocko <mhocko@kernel.org> - 2016-02-06 09:40 +0100
              Re: [PATCH 5/5] mm, oom_reaper: implement OOM victims queuing Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-02-06 16:40 +0100
    [PATCH 3/5] oom: clear TIF_MEMDIE after oom_reaper managed to unmap the address space Michal Hocko <mhocko@kernel.org> - 2016-02-03 14:20 +0100
      Re: [PATCH 3/5] oom: clear TIF_MEMDIE after oom_reaper managed to unmap the address space Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-02-04 15:40 +0100
        Re: [PATCH 3/5] oom: clear TIF_MEMDIE after oom_reaper managed to  unmap the address space Michal Hocko <mhocko@kernel.org> - 2016-02-04 15:50 +0100
          Re: [PATCH 3/5] oom: clear TIF_MEMDIE after oom_reaper managed to unmap the address space Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-02-04 16:10 +0100
            Re: [PATCH 3/5] oom: clear TIF_MEMDIE after oom_reaper managed to  unmap the address space Michal Hocko <mhocko@kernel.org> - 2016-02-04 17:40 +0100
              Re: [PATCH 3/5] oom: clear TIF_MEMDIE after oom_reaper managed to unmap the address space Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-02-05 12:20 +0100
                Re: [PATCH 3/5] oom: clear TIF_MEMDIE after oom_reaper managed to  unmap the address space Michal Hocko <mhocko@kernel.org> - 2016-02-06 09:40 +0100
                  Re: [PATCH 3/5] oom: clear TIF_MEMDIE after oom_reaper managed to unmap the address space Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-02-06 12:30 +0100
          Re: [PATCH 3/5] oom: clear TIF_MEMDIE after oom_reaper managed to  unmap the address space Michal Hocko <mhocko@kernel.org> - 2016-02-06 07:50 +0100
            Re: [PATCH 3/5] oom: clear TIF_MEMDIE after oom_reaper managed to unmap the address space Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2016-02-06 15:40 +0100

Page 1 of 2  [1] 2  Next page →

#1325396 — [PATCH 0/5] oom reaper v5

Hi,
I am reposting the whole patchset on top of mmotm with the previous
version of the patchset reverted for an easier review. The series
applies cleanly on top of the current Linus tree as well.

The previous version was posted http://lkml.kernel.org/r/1452094975-551-1-git-send-email-mhocko@kernel.org
I have tried to address most of the feedback. There was some push
for extending the current implementation further but I do not feel
comfortable to do that right now. I believe that we should start
as easy as possible and add extensions on top. That shouldn't be
hard with the current architecture.

Wrt. the previous version, I have added patches 4 and 5. Patch4 reports
success/failures to reap a task which is useful to see how the reaper
operates. Patch 5 is implementing a more robust API between the oom
killer and the oom reaper. We allow more tasks to be queued for the
reaper at the same time rather than the original signle task mode.

Patch 1 also dropped oom_reaper thread priority handling as per
David.  I ended up keeping vma filtering code inside __oom_reap_task.
I still believe this is a better fit because the rules are a single
fit for the reaper. They cannot be shared with a larger code base.

In the meantime I have prepared down_write_killable rw_semaphore
variant http://lkml.kernel.org/r/1454444369-2146-1-git-send-email-mhocko@kernel.org
and also have a tentative patch to convert some users of mmap_sem for
write to use the killable version. This needs more checking though but
I guess I will have something ready in 2 weeks or so (I will be on
vacation next week).

For the general description of the oom_reaper functionality, please
refer to Patch1.

I would be really greatful if we could postpone any
functional/semantical enhancements for later discussion and focus on the
correctness of these particular patches as there were no fundamental
objectios to the current approach.

Thanks!

[toc] | [next] | [standalone]

#1325402 — [PATCH 2/5] oom reaper: handle mlocked pages

From: Michal Hocko <mhocko@suse.com>

__oom_reap_vmas current skips over all mlocked vmas because they need a
special treatment before they are unmapped. This is primarily done for
simplicity. There is no reason to skip over them and reduce the amount
of reclaimed memory. This is safe from the semantic point of view
because try_to_unmap_one during rmap walk would keep tell the reclaim
to cull the page back and mlock it again.

munlock_vma_pages_all is also safe to be called from the oom reaper
context because it doesn't sit on any locks but mmap_sem (for read).

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 mm/oom_kill.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 9a0e4e5f50b4..840e03986497 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -443,13 +443,6 @@ static bool __oom_reap_vmas(struct mm_struct *mm)
 			continue;
 
 		/*
-		 * mlocked VMAs require explicit munlocking before unmap.
-		 * Let's keep it simple here and skip such VMAs.
-		 */
-		if (vma->vm_flags & VM_LOCKED)
-			continue;
-
-		/*
 		 * Only anonymous pages have a good chance to be dropped
 		 * without additional steps which we cannot afford as we
 		 * are OOM already.
@@ -459,9 +452,12 @@ static bool __oom_reap_vmas(struct mm_struct *mm)
 		 * we do not want to block exit_mmap by keeping mm ref
 		 * count elevated without a good reason.
 		 */
-		if (vma_is_anonymous(vma) || !(vma->vm_flags & VM_SHARED))
+		if (vma_is_anonymous(vma) || !(vma->vm_flags & VM_SHARED)) {
+			if (vma->vm_flags & VM_LOCKED)
+				munlock_vma_pages_all(vma);
 			unmap_page_range(&tlb, vma, vma->vm_start, vma->vm_end,
 					 &details);
+		}
 	}
 	tlb_finish_mmu(&tlb, 0, -1);
 	up_read(&mm->mmap_sem);
-- 
2.7.0

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

#1326296 — Re: [PATCH 2/5] oom reaper: handle mlocked pages

On Wed, 3 Feb 2016, Michal Hocko wrote:

> From: Michal Hocko <mhocko@suse.com>
> 
> __oom_reap_vmas current skips over all mlocked vmas because they need a
> special treatment before they are unmapped. This is primarily done for
> simplicity. There is no reason to skip over them and reduce the amount
> of reclaimed memory. This is safe from the semantic point of view
> because try_to_unmap_one during rmap walk would keep tell the reclaim
> to cull the page back and mlock it again.
> 
> munlock_vma_pages_all is also safe to be called from the oom reaper
> context because it doesn't sit on any locks but mmap_sem (for read).
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>

Acked-by: David Rientjes <rientjes@google.com>

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

#1325403 — [PATCH 4/5] mm, oom_reaper: report success/failure

From: Michal Hocko <mhocko@suse.com>

Inform about the successful/failed oom_reaper attempts and dump all the
held locks to tell us more who is blocking the progress.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 mm/oom_kill.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 8e345126d73e..b87acdca2a41 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -420,6 +420,7 @@ static struct task_struct *oom_reaper_th;
 static struct task_struct *task_to_reap;
 static DECLARE_WAIT_QUEUE_HEAD(oom_reaper_wait);
 
+#define K(x) ((x) << (PAGE_SHIFT-10))
 static bool __oom_reap_task(struct task_struct *tsk)
 {
 	struct mmu_gather tlb;
@@ -476,6 +477,11 @@ static bool __oom_reap_task(struct task_struct *tsk)
 		}
 	}
 	tlb_finish_mmu(&tlb, 0, -1);
+	pr_info("oom_reaper: reaped process :%d (%s) anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lulB\n",
+			task_pid_nr(tsk), tsk->comm,
+			K(get_mm_counter(mm, MM_ANONPAGES)),
+			K(get_mm_counter(mm, MM_FILEPAGES)),
+			K(get_mm_counter(mm, MM_SHMEMPAGES)));
 	up_read(&mm->mmap_sem);
 
 	/*
@@ -492,14 +498,21 @@ static bool __oom_reap_task(struct task_struct *tsk)
 	return ret;
 }
 
+#define MAX_OOM_REAP_RETRIES 10
 static void oom_reap_task(struct task_struct *tsk)
 {
 	int attempts = 0;
 
 	/* Retry the down_read_trylock(mmap_sem) a few times */
-	while (attempts++ < 10 && !__oom_reap_task(tsk))
+	while (attempts++ < MAX_OOM_REAP_RETRIES && !__oom_reap_task(tsk))
 		schedule_timeout_idle(HZ/10);
 
+	if (attempts > MAX_OOM_REAP_RETRIES) {
+		pr_info("oom_reaper: unable to reap pid:%d (%s)\n",
+				task_pid_nr(tsk), tsk->comm);
+		debug_show_all_locks();
+	}
+
 	/* Drop a reference taken by wake_oom_reaper */
 	put_task_struct(tsk);
 }
@@ -646,7 +659,6 @@ static bool process_shares_mm(struct task_struct *p, struct mm_struct *mm)
 	return false;
 }
 
-#define K(x) ((x) << (PAGE_SHIFT-10))
 /*
  * Must be called while holding a reference to p, which will be released upon
  * returning.
-- 
2.7.0

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

#1326172 — Re: [PATCH 4/5] mm, oom_reaper: report success/failure

On Wed, 3 Feb 2016, Michal Hocko wrote:

> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 8e345126d73e..b87acdca2a41 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -420,6 +420,7 @@ static struct task_struct *oom_reaper_th;
>  static struct task_struct *task_to_reap;
>  static DECLARE_WAIT_QUEUE_HEAD(oom_reaper_wait);
>  
> +#define K(x) ((x) << (PAGE_SHIFT-10))
>  static bool __oom_reap_task(struct task_struct *tsk)
>  {
>  	struct mmu_gather tlb;
> @@ -476,6 +477,11 @@ static bool __oom_reap_task(struct task_struct *tsk)
>  		}
>  	}
>  	tlb_finish_mmu(&tlb, 0, -1);
> +	pr_info("oom_reaper: reaped process :%d (%s) anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lulB\n",
> +			task_pid_nr(tsk), tsk->comm,
> +			K(get_mm_counter(mm, MM_ANONPAGES)),
> +			K(get_mm_counter(mm, MM_FILEPAGES)),
> +			K(get_mm_counter(mm, MM_SHMEMPAGES)));
>  	up_read(&mm->mmap_sem);
>  
>  	/*

This is a bit misleading, it would appear that the rss values are what was 
reaped when in fact they represent just the values of the mm being reaped.  
We have already printed these values as an artifact in the kernel log.

I think it would be helpful to show anon-rss after reaping, however, so we 
can compare to the previous anon-rss that was reported.  And, I agree that 
leaving behind a message in the kernel log that reaping has been 
successful is worthwhile.  So this line should just show what anon-rss is 
after reaping and make it clear that this is not the memory reaped.

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

#1326442 — Re: [PATCH 4/5] mm, oom_reaper: report success/failure

On Wed 03-02-16 15:10:57, David Rientjes wrote:
> On Wed, 3 Feb 2016, Michal Hocko wrote:
> 
> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index 8e345126d73e..b87acdca2a41 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -420,6 +420,7 @@ static struct task_struct *oom_reaper_th;
> >  static struct task_struct *task_to_reap;
> >  static DECLARE_WAIT_QUEUE_HEAD(oom_reaper_wait);
> >  
> > +#define K(x) ((x) << (PAGE_SHIFT-10))
> >  static bool __oom_reap_task(struct task_struct *tsk)
> >  {
> >  	struct mmu_gather tlb;
> > @@ -476,6 +477,11 @@ static bool __oom_reap_task(struct task_struct *tsk)
> >  		}
> >  	}
> >  	tlb_finish_mmu(&tlb, 0, -1);
> > +	pr_info("oom_reaper: reaped process :%d (%s) anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lulB\n",
> > +			task_pid_nr(tsk), tsk->comm,
> > +			K(get_mm_counter(mm, MM_ANONPAGES)),
> > +			K(get_mm_counter(mm, MM_FILEPAGES)),
> > +			K(get_mm_counter(mm, MM_SHMEMPAGES)));
> >  	up_read(&mm->mmap_sem);
> >  
> >  	/*
> 
> This is a bit misleading, it would appear that the rss values are what was 
> reaped when in fact they represent just the values of the mm being reaped.  
> We have already printed these values as an artifact in the kernel log.

Yes and the idea was to provide the after state to compare before and
after. That's why I have kept the similar format. Just dropped the
virtual memory size because that doesn't make any sense in this context
now.

> I think it would be helpful to show anon-rss after reaping, however, so we 
> can compare to the previous anon-rss that was reported.  And, I agree that 
> leaving behind a message in the kernel log that reaping has been 
> successful is worthwhile.  So this line should just show what anon-rss is 
> after reaping and make it clear that this is not the memory reaped.

Does
"oom_reaper: reaped process %d (%s) current memory anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB "

sound any better?
-- 
Michal Hocko
SUSE Labs

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

#1327246 — Re: [PATCH 4/5] mm, oom_reaper: report success/failure

On Thu, 4 Feb 2016, Michal Hocko wrote:

> > I think it would be helpful to show anon-rss after reaping, however, so we 
> > can compare to the previous anon-rss that was reported.  And, I agree that 
> > leaving behind a message in the kernel log that reaping has been 
> > successful is worthwhile.  So this line should just show what anon-rss is 
> > after reaping and make it clear that this is not the memory reaped.
> 
> Does
> "oom_reaper: reaped process %d (%s) current memory anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB "
> 
> sound any better?

oom_reaper: reaped process %d (%s), now anon-rss:%lukB

would probably be better until additional support is added to do other 
kinds of reaping other than just primarily heap.  This should help to 
quantify the exact amount of memory that could be reaped (or otherwise 
unmapped) iff oom_reaper has to get involved rather than fluctations that 
have nothing to do with it.

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

#1327555 — Re: [PATCH 4/5] mm, oom_reaper: report success/failure

On Thu 04-02-16 14:31:26, David Rientjes wrote:
> On Thu, 4 Feb 2016, Michal Hocko wrote:
> 
> > > I think it would be helpful to show anon-rss after reaping, however, so we 
> > > can compare to the previous anon-rss that was reported.  And, I agree that 
> > > leaving behind a message in the kernel log that reaping has been 
> > > successful is worthwhile.  So this line should just show what anon-rss is 
> > > after reaping and make it clear that this is not the memory reaped.
> > 
> > Does
> > "oom_reaper: reaped process %d (%s) current memory anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB "
> > 
> > sound any better?
> 
> oom_reaper: reaped process %d (%s), now anon-rss:%lukB
> 
> would probably be better until additional support is added to do other 
> kinds of reaping other than just primarily heap.  This should help to 
> quantify the exact amount of memory that could be reaped (or otherwise 
> unmapped) iff oom_reaper has to get involved rather than fluctations that 
> have nothing to do with it.

---
From 402090df64de7f80d7d045b0b17e860220837fa6 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Fri, 5 Feb 2016 10:24:23 +0100
Subject: [PATCH] mm-oom_reaper-report-success-failure-fix

update the log message to be more specific

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 mm/oom_kill.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 87d644c97ac9..ca61e6cfae52 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -479,7 +479,7 @@ static bool __oom_reap_task(struct task_struct *tsk)
 		}
 	}
 	tlb_finish_mmu(&tlb, 0, -1);
-	pr_info("oom_reaper: reaped process :%d (%s) anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lulB\n",
+	pr_info("oom_reaper: reaped process %d (%s), now anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lulB\n",
 			task_pid_nr(tsk), tsk->comm,
 			K(get_mm_counter(mm, MM_ANONPAGES)),
 			K(get_mm_counter(mm, MM_FILEPAGES)),
-- 
2.7.0


-- 
Michal Hocko
SUSE Labs

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

#1328231 — Re: [PATCH 4/5] mm, oom_reaper: report success/failure

On Fri 05-02-16 10:26:40, Michal Hocko wrote:
[...]
> From 402090df64de7f80d7d045b0b17e860220837fa6 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Fri, 5 Feb 2016 10:24:23 +0100
> Subject: [PATCH] mm-oom_reaper-report-success-failure-fix
> 
> update the log message to be more specific
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
>  mm/oom_kill.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 87d644c97ac9..ca61e6cfae52 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -479,7 +479,7 @@ static bool __oom_reap_task(struct task_struct *tsk)
>  		}
>  	}
>  	tlb_finish_mmu(&tlb, 0, -1);
> -	pr_info("oom_reaper: reaped process :%d (%s) anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lulB\n",
> +	pr_info("oom_reaper: reaped process %d (%s), now anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lulB\n",

Dohh, s@lulB@ulkB@

>  			task_pid_nr(tsk), tsk->comm,
>  			K(get_mm_counter(mm, MM_ANONPAGES)),
>  			K(get_mm_counter(mm, MM_FILEPAGES)),
> -- 
> 2.7.0
> 
> 
> -- 
> Michal Hocko
> SUSE Labs

-- 
Michal Hocko
SUSE Labs

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

#1325404 — [PATCH 1/5] mm, oom: introduce oom reaper

From: Michal Hocko <mhocko@suse.com>

This is based on the idea from Mel Gorman discussed during LSFMM 2015 and
independently brought up by Oleg Nesterov.

The OOM killer currently allows to kill only a single task in a good
hope that the task will terminate in a reasonable time and frees up its
memory.  Such a task (oom victim) will get an access to memory reserves
via mark_oom_victim to allow a forward progress should there be a need
for additional memory during exit path.

It has been shown (e.g. by Tetsuo Handa) that it is not that hard to
construct workloads which break the core assumption mentioned above and
the OOM victim might take unbounded amount of time to exit because it
might be blocked in the uninterruptible state waiting for an event
(e.g. lock) which is blocked by another task looping in the page
allocator.

This patch reduces the probability of such a lockup by introducing a
specialized kernel thread (oom_reaper) which tries to reclaim additional
memory by preemptively reaping the anonymous or swapped out memory
owned by the oom victim under an assumption that such a memory won't
be needed when its owner is killed and kicked from the userspace anyway.
There is one notable exception to this, though, if the OOM victim was
in the process of coredumping the result would be incomplete. This is
considered a reasonable constrain because the overall system health is
more important than debugability of a particular application.

A kernel thread has been chosen because we need a reliable way of
invocation so workqueue context is not appropriate because all the
workers might be busy (e.g. allocating memory). Kswapd which sounds
like another good fit is not appropriate as well because it might get
blocked on locks during reclaim as well.

oom_reaper has to take mmap_sem on the target task for reading so the
solution is not 100% because the semaphore might be held or blocked for
write but the probability is reduced considerably wrt. basically any
lock blocking forward progress as described above. In order to prevent
from blocking on the lock without any forward progress we are using only
a trylock and retry 10 times with a short sleep in between.
Users of mmap_sem which need it for write should be carefully reviewed
to use _killable waiting as much as possible and reduce allocations
requests done with the lock held to absolute minimum to reduce the risk
even further.

The API between oom killer and oom reaper is quite trivial. wake_oom_reaper
updates mm_to_reap with cmpxchg to guarantee only NULL->mm transition
and oom_reaper clear this atomically once it is done with the work. This
means that only a single mm_struct can be reaped at the time. As the
operation is potentially disruptive we are trying to limit it to the
ncessary minimum and the reaper blocks any updates while it operates on
an mm. mm_struct is pinned by mm_count to allow parallel exit_mmap and a
race is detected by atomic_inc_not_zero(mm_users).

Chnages since v4
- drop MAX_RT_PRIO-1 as per David - memcg/cpuset/mempolicy OOM killing
  might interfere with the rest of the system
Changes since v3
- many style/compile fixups by Andrew
- unmap_mapping_range_tree needs full initialization of zap_details
  to prevent from missing unmaps and follow up BUG_ON during truncate
  resp. misaccounting - Kirill/Andrew
- exclude mlocked pages because they need an explicit munlock by Kirill
- use subsys_initcall instead of module_init - Paul Gortmaker
- do not tear down mm if it is shared with the global init because this
  could lead to SEGV and panic - Tetsuo
Changes since v2
- fix mm_count refernce leak reported by Tetsuo
- make sure oom_reaper_th is NULL after kthread_run fails - Tetsuo
- use wait_event_freezable rather than open coded wait loop - suggested
  by Tetsuo
Changes since v1
- fix the screwed up detail->check_swap_entries - Johannes
- do not use kthread_should_stop because that would need a cleanup
  and we do not have anybody to stop us - Tetsuo
- move wake_oom_reaper to oom_kill_process because we have to wait
  for all tasks sharing the same mm to get killed - Tetsuo
- do not reap mm structs which are shared with unkillable tasks - Tetsuo

Suggested-by: Oleg Nesterov <oleg@redhat.com>
Suggested-by: Mel Gorman <mgorman@suse.de>
Acked-by: Mel Gorman <mgorman@suse.de>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/mm.h |   2 +
 mm/internal.h      |   5 ++
 mm/memory.c        |  17 +++---
 mm/oom_kill.c      | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 4 files changed, 162 insertions(+), 13 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 05b9fbbceb01..8a67ea2a6323 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1111,6 +1111,8 @@ struct zap_details {
 	struct address_space *check_mapping;	/* Check page->mapping if set */
 	pgoff_t	first_index;			/* Lowest page->index to unmap */
 	pgoff_t last_index;			/* Highest page->index to unmap */
+	bool ignore_dirty;			/* Ignore dirty pages */
+	bool check_swap_entries;		/* Check also swap entries */
 };
 
 struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
diff --git a/mm/internal.h b/mm/internal.h
index ed90298c12db..cac6eb458727 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -42,6 +42,11 @@ extern int do_swap_page(struct mm_struct *mm, struct vm_area_struct *vma,
 void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *start_vma,
 		unsigned long floor, unsigned long ceiling);
 
+void unmap_page_range(struct mmu_gather *tlb,
+			     struct vm_area_struct *vma,
+			     unsigned long addr, unsigned long end,
+			     struct zap_details *details);
+
 static inline void set_page_count(struct page *page, int v)
 {
 	atomic_set(&page->_count, v);
diff --git a/mm/memory.c b/mm/memory.c
index 42d5bec9bb91..c158dc53ca3d 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1105,6 +1105,12 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
 
 			if (!PageAnon(page)) {
 				if (pte_dirty(ptent)) {
+					/*
+					 * oom_reaper cannot tear down dirty
+					 * pages
+					 */
+					if (unlikely(details && details->ignore_dirty))
+						continue;
 					force_flush = 1;
 					set_page_dirty(page);
 				}
@@ -1123,8 +1129,8 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
 			}
 			continue;
 		}
-		/* If details->check_mapping, we leave swap entries. */
-		if (unlikely(details))
+		/* only check swap_entries if explicitly asked for in details */
+		if (unlikely(details && !details->check_swap_entries))
 			continue;
 
 		entry = pte_to_swp_entry(ptent);
@@ -1229,7 +1235,7 @@ static inline unsigned long zap_pud_range(struct mmu_gather *tlb,
 	return addr;
 }
 
-static void unmap_page_range(struct mmu_gather *tlb,
+void unmap_page_range(struct mmu_gather *tlb,
 			     struct vm_area_struct *vma,
 			     unsigned long addr, unsigned long end,
 			     struct zap_details *details)
@@ -1237,9 +1243,6 @@ static void unmap_page_range(struct mmu_gather *tlb,
 	pgd_t *pgd;
 	unsigned long next;
 
-	if (details && !details->check_mapping)
-		details = NULL;
-
 	BUG_ON(addr >= end);
 	tlb_start_vma(tlb, vma);
 	pgd = pgd_offset(vma->vm_mm, addr);
@@ -2419,7 +2422,7 @@ static inline void unmap_mapping_range_tree(struct rb_root *root,
 void unmap_mapping_range(struct address_space *mapping,
 		loff_t const holebegin, loff_t const holelen, int even_cows)
 {
-	struct zap_details details;
+	struct zap_details details = { };
 	pgoff_t hba = holebegin >> PAGE_SHIFT;
 	pgoff_t hlen = (holelen + PAGE_SIZE - 1) >> PAGE_SHIFT;
 
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index e3ab892903ee..9a0e4e5f50b4 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -35,6 +35,11 @@
 #include <linux/freezer.h>
 #include <linux/ftrace.h>
 #include <linux/ratelimit.h>
+#include <linux/kthread.h>
+#include <linux/init.h>
+
+#include <asm/tlb.h>
+#include "internal.h"
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/oom.h>
@@ -406,6 +411,133 @@ static DECLARE_WAIT_QUEUE_HEAD(oom_victims_wait);
 
 bool oom_killer_disabled __read_mostly;
 
+#ifdef CONFIG_MMU
+/*
+ * OOM Reaper kernel thread which tries to reap the memory used by the OOM
+ * victim (if that is possible) to help the OOM killer to move on.
+ */
+static struct task_struct *oom_reaper_th;
+static struct mm_struct *mm_to_reap;
+static DECLARE_WAIT_QUEUE_HEAD(oom_reaper_wait);
+
+static bool __oom_reap_vmas(struct mm_struct *mm)
+{
+	struct mmu_gather tlb;
+	struct vm_area_struct *vma;
+	struct zap_details details = {.check_swap_entries = true,
+				      .ignore_dirty = true};
+	bool ret = true;
+
+	/* We might have raced with exit path */
+	if (!atomic_inc_not_zero(&mm->mm_users))
+		return true;
+
+	if (!down_read_trylock(&mm->mmap_sem)) {
+		ret = false;
+		goto out;
+	}
+
+	tlb_gather_mmu(&tlb, mm, 0, -1);
+	for (vma = mm->mmap ; vma; vma = vma->vm_next) {
+		if (is_vm_hugetlb_page(vma))
+			continue;
+
+		/*
+		 * mlocked VMAs require explicit munlocking before unmap.
+		 * Let's keep it simple here and skip such VMAs.
+		 */
+		if (vma->vm_flags & VM_LOCKED)
+			continue;
+
+		/*
+		 * Only anonymous pages have a good chance to be dropped
+		 * without additional steps which we cannot afford as we
+		 * are OOM already.
+		 *
+		 * We do not even care about fs backed pages because all
+		 * which are reclaimable have already been reclaimed and
+		 * we do not want to block exit_mmap by keeping mm ref
+		 * count elevated without a good reason.
+		 */
+		if (vma_is_anonymous(vma) || !(vma->vm_flags & VM_SHARED))
+			unmap_page_range(&tlb, vma, vma->vm_start, vma->vm_end,
+					 &details);
+	}
+	tlb_finish_mmu(&tlb, 0, -1);
+	up_read(&mm->mmap_sem);
+out:
+	mmput(mm);
+	return ret;
+}
+
+static void oom_reap_vmas(struct mm_struct *mm)
+{
+	int attempts = 0;
+
+	/* Retry the down_read_trylock(mmap_sem) a few times */
+	while (attempts++ < 10 && !__oom_reap_vmas(mm))
+		schedule_timeout_idle(HZ/10);
+
+	/* Drop a reference taken by wake_oom_reaper */
+	mmdrop(mm);
+}
+
+static int oom_reaper(void *unused)
+{
+	while (true) {
+		struct mm_struct *mm;
+
+		wait_event_freezable(oom_reaper_wait,
+				     (mm = READ_ONCE(mm_to_reap)));
+		oom_reap_vmas(mm);
+		WRITE_ONCE(mm_to_reap, NULL);
+	}
+
+	return 0;
+}
+
+static void wake_oom_reaper(struct mm_struct *mm)
+{
+	struct mm_struct *old_mm;
+
+	if (!oom_reaper_th)
+		return;
+
+	/*
+	 * Pin the given mm. Use mm_count instead of mm_users because
+	 * we do not want to delay the address space tear down.
+	 */
+	atomic_inc(&mm->mm_count);
+
+	/*
+	 * Make sure that only a single mm is ever queued for the reaper
+	 * because multiple are not necessary and the operation might be
+	 * disruptive so better reduce it to the bare minimum.
+	 */
+	old_mm = cmpxchg(&mm_to_reap, NULL, mm);
+	if (!old_mm)
+		wake_up(&oom_reaper_wait);
+	else
+		mmdrop(mm);
+}
+
+static int __init oom_init(void)
+{
+	oom_reaper_th = kthread_run(oom_reaper, NULL, "oom_reaper");
+	if (IS_ERR(oom_reaper_th)) {
+		pr_err("Unable to start OOM reaper %ld. Continuing regardless\n",
+				PTR_ERR(oom_reaper_th));
+		oom_reaper_th = NULL;
+	}
+	return 0;
+}
+subsys_initcall(oom_init)
+#else
+static void wake_oom_reaper(struct mm_struct *mm)
+{
+}
+#endif
+
 /**
  * mark_oom_victim - mark the given task as OOM victim
  * @tsk: task to mark
@@ -511,6 +643,7 @@ void oom_kill_process(struct oom_control *oc, struct task_struct *p,
 	unsigned int victim_points = 0;
 	static DEFINE_RATELIMIT_STATE(oom_rs, DEFAULT_RATELIMIT_INTERVAL,
 					      DEFAULT_RATELIMIT_BURST);
+	bool can_oom_reap = true;
 
 	/*
 	 * If the task is already exiting, don't alarm the sysadmin or kill
@@ -601,17 +734,23 @@ void oom_kill_process(struct oom_control *oc, struct task_struct *p,
 			continue;
 		if (same_thread_group(p, victim))
 			continue;
-		if (unlikely(p->flags & PF_KTHREAD))
-			continue;
-		if (is_global_init(p))
-			continue;
-		if (p->signal->oom_score_adj == OOM_SCORE_ADJ_MIN)
+		if (unlikely(p->flags & PF_KTHREAD) || is_global_init(p) ||
+		    p->signal->oom_score_adj == OOM_SCORE_ADJ_MIN) {
+			/*
+			 * We cannot use oom_reaper for the mm shared by this
+			 * process because it wouldn't get killed and so the
+			 * memory might be still used.
+			 */
+			can_oom_reap = false;
 			continue;
-
+		}
 		do_send_sig_info(SIGKILL, SEND_SIG_FORCED, p, true);
 	}
 	rcu_read_unlock();
 
+	if (can_oom_reap)
+		wake_oom_reaper(mm);
+
 	mmdrop(mm);
 	put_task_struct(victim);
 }
-- 
2.7.0

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

#1326247 — Re: [PATCH 1/5] mm, oom: introduce oom reaper

On Wed, 3 Feb 2016, Michal Hocko wrote:

> From: Michal Hocko <mhocko@suse.com>
> 
> This is based on the idea from Mel Gorman discussed during LSFMM 2015 and
> independently brought up by Oleg Nesterov.
> 
> The OOM killer currently allows to kill only a single task in a good
> hope that the task will terminate in a reasonable time and frees up its
> memory.  Such a task (oom victim) will get an access to memory reserves
> via mark_oom_victim to allow a forward progress should there be a need
> for additional memory during exit path.
> 
> It has been shown (e.g. by Tetsuo Handa) that it is not that hard to
> construct workloads which break the core assumption mentioned above and
> the OOM victim might take unbounded amount of time to exit because it
> might be blocked in the uninterruptible state waiting for an event
> (e.g. lock) which is blocked by another task looping in the page
> allocator.
> 
> This patch reduces the probability of such a lockup by introducing a
> specialized kernel thread (oom_reaper) which tries to reclaim additional
> memory by preemptively reaping the anonymous or swapped out memory
> owned by the oom victim under an assumption that such a memory won't
> be needed when its owner is killed and kicked from the userspace anyway.
> There is one notable exception to this, though, if the OOM victim was
> in the process of coredumping the result would be incomplete. This is
> considered a reasonable constrain because the overall system health is
> more important than debugability of a particular application.
> 
> A kernel thread has been chosen because we need a reliable way of
> invocation so workqueue context is not appropriate because all the
> workers might be busy (e.g. allocating memory). Kswapd which sounds
> like another good fit is not appropriate as well because it might get
> blocked on locks during reclaim as well.
> 
> oom_reaper has to take mmap_sem on the target task for reading so the
> solution is not 100% because the semaphore might be held or blocked for
> write but the probability is reduced considerably wrt. basically any
> lock blocking forward progress as described above. In order to prevent
> from blocking on the lock without any forward progress we are using only
> a trylock and retry 10 times with a short sleep in between.
> Users of mmap_sem which need it for write should be carefully reviewed
> to use _killable waiting as much as possible and reduce allocations
> requests done with the lock held to absolute minimum to reduce the risk
> even further.
> 
> The API between oom killer and oom reaper is quite trivial. wake_oom_reaper
> updates mm_to_reap with cmpxchg to guarantee only NULL->mm transition
> and oom_reaper clear this atomically once it is done with the work. This
> means that only a single mm_struct can be reaped at the time. As the
> operation is potentially disruptive we are trying to limit it to the
> ncessary minimum and the reaper blocks any updates while it operates on
> an mm. mm_struct is pinned by mm_count to allow parallel exit_mmap and a
> race is detected by atomic_inc_not_zero(mm_users).
> 
> Chnages since v4
> - drop MAX_RT_PRIO-1 as per David - memcg/cpuset/mempolicy OOM killing
>   might interfere with the rest of the system
> Changes since v3
> - many style/compile fixups by Andrew
> - unmap_mapping_range_tree needs full initialization of zap_details
>   to prevent from missing unmaps and follow up BUG_ON during truncate
>   resp. misaccounting - Kirill/Andrew
> - exclude mlocked pages because they need an explicit munlock by Kirill
> - use subsys_initcall instead of module_init - Paul Gortmaker
> - do not tear down mm if it is shared with the global init because this
>   could lead to SEGV and panic - Tetsuo
> Changes since v2
> - fix mm_count refernce leak reported by Tetsuo
> - make sure oom_reaper_th is NULL after kthread_run fails - Tetsuo
> - use wait_event_freezable rather than open coded wait loop - suggested
>   by Tetsuo
> Changes since v1
> - fix the screwed up detail->check_swap_entries - Johannes
> - do not use kthread_should_stop because that would need a cleanup
>   and we do not have anybody to stop us - Tetsuo
> - move wake_oom_reaper to oom_kill_process because we have to wait
>   for all tasks sharing the same mm to get killed - Tetsuo
> - do not reap mm structs which are shared with unkillable tasks - Tetsuo
> 
> Suggested-by: Oleg Nesterov <oleg@redhat.com>
> Suggested-by: Mel Gorman <mgorman@suse.de>
> Acked-by: Mel Gorman <mgorman@suse.de>
> Signed-off-by: Michal Hocko <mhocko@suse.com>

Acked-by: David Rientjes <rientjes@google.com>

I think all the patches could really have been squashed together because 
subsequent patches just overwrite already added code.  I was going to 
suggest not doing atomic_inc(&mm->mm_count) in wake_oom_reaper() and 
change oom_kill_process() to do

	if (can_oom_reap)
		wake_oom_reaper(mm);
	else
		mmdrop(mm);

but I see that we don't even touch mm->mm_count after the third patch.

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

#1326445 — Re: [PATCH 1/5] mm, oom: introduce oom reaper

On Wed 03-02-16 15:48:18, David Rientjes wrote:
> On Wed, 3 Feb 2016, Michal Hocko wrote:
> 
> > From: Michal Hocko <mhocko@suse.com>
> > 
> > This is based on the idea from Mel Gorman discussed during LSFMM 2015 and
> > independently brought up by Oleg Nesterov.
> > 
> > The OOM killer currently allows to kill only a single task in a good
> > hope that the task will terminate in a reasonable time and frees up its
> > memory.  Such a task (oom victim) will get an access to memory reserves
> > via mark_oom_victim to allow a forward progress should there be a need
> > for additional memory during exit path.
> > 
> > It has been shown (e.g. by Tetsuo Handa) that it is not that hard to
> > construct workloads which break the core assumption mentioned above and
> > the OOM victim might take unbounded amount of time to exit because it
> > might be blocked in the uninterruptible state waiting for an event
> > (e.g. lock) which is blocked by another task looping in the page
> > allocator.
> > 
> > This patch reduces the probability of such a lockup by introducing a
> > specialized kernel thread (oom_reaper) which tries to reclaim additional
> > memory by preemptively reaping the anonymous or swapped out memory
> > owned by the oom victim under an assumption that such a memory won't
> > be needed when its owner is killed and kicked from the userspace anyway.
> > There is one notable exception to this, though, if the OOM victim was
> > in the process of coredumping the result would be incomplete. This is
> > considered a reasonable constrain because the overall system health is
> > more important than debugability of a particular application.
> > 
> > A kernel thread has been chosen because we need a reliable way of
> > invocation so workqueue context is not appropriate because all the
> > workers might be busy (e.g. allocating memory). Kswapd which sounds
> > like another good fit is not appropriate as well because it might get
> > blocked on locks during reclaim as well.
> > 
> > oom_reaper has to take mmap_sem on the target task for reading so the
> > solution is not 100% because the semaphore might be held or blocked for
> > write but the probability is reduced considerably wrt. basically any
> > lock blocking forward progress as described above. In order to prevent
> > from blocking on the lock without any forward progress we are using only
> > a trylock and retry 10 times with a short sleep in between.
> > Users of mmap_sem which need it for write should be carefully reviewed
> > to use _killable waiting as much as possible and reduce allocations
> > requests done with the lock held to absolute minimum to reduce the risk
> > even further.
> > 
> > The API between oom killer and oom reaper is quite trivial. wake_oom_reaper
> > updates mm_to_reap with cmpxchg to guarantee only NULL->mm transition
> > and oom_reaper clear this atomically once it is done with the work. This
> > means that only a single mm_struct can be reaped at the time. As the
> > operation is potentially disruptive we are trying to limit it to the
> > ncessary minimum and the reaper blocks any updates while it operates on
> > an mm. mm_struct is pinned by mm_count to allow parallel exit_mmap and a
> > race is detected by atomic_inc_not_zero(mm_users).
> > 
> > Chnages since v4
> > - drop MAX_RT_PRIO-1 as per David - memcg/cpuset/mempolicy OOM killing
> >   might interfere with the rest of the system
> > Changes since v3
> > - many style/compile fixups by Andrew
> > - unmap_mapping_range_tree needs full initialization of zap_details
> >   to prevent from missing unmaps and follow up BUG_ON during truncate
> >   resp. misaccounting - Kirill/Andrew
> > - exclude mlocked pages because they need an explicit munlock by Kirill
> > - use subsys_initcall instead of module_init - Paul Gortmaker
> > - do not tear down mm if it is shared with the global init because this
> >   could lead to SEGV and panic - Tetsuo
> > Changes since v2
> > - fix mm_count refernce leak reported by Tetsuo
> > - make sure oom_reaper_th is NULL after kthread_run fails - Tetsuo
> > - use wait_event_freezable rather than open coded wait loop - suggested
> >   by Tetsuo
> > Changes since v1
> > - fix the screwed up detail->check_swap_entries - Johannes
> > - do not use kthread_should_stop because that would need a cleanup
> >   and we do not have anybody to stop us - Tetsuo
> > - move wake_oom_reaper to oom_kill_process because we have to wait
> >   for all tasks sharing the same mm to get killed - Tetsuo
> > - do not reap mm structs which are shared with unkillable tasks - Tetsuo
> > 
> > Suggested-by: Oleg Nesterov <oleg@redhat.com>
> > Suggested-by: Mel Gorman <mgorman@suse.de>
> > Acked-by: Mel Gorman <mgorman@suse.de>
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
> 
> Acked-by: David Rientjes <rientjes@google.com>

Thanks!
 
> I think all the patches could really have been squashed together because 
> subsequent patches just overwrite already added code. 

The primary reason is a better bisectability and incremental nature of
changes.

> I was going to 
> suggest not doing atomic_inc(&mm->mm_count) in wake_oom_reaper() and 
> change oom_kill_process() to do

I found it easier to follow the reference counting that way (pin the mm
at the place when I hand over it to the async thread).

> 
> 	if (can_oom_reap)
> 		wake_oom_reaper(mm);
> 	else
> 		mmdrop(mm);
> 
> but I see that we don't even touch mm->mm_count after the third patch.

-- 
Michal Hocko
SUSE Labs

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

#1328301 — Re: [PATCH 1/5] mm, oom: introduce oom reaper

Michal Hocko wrote:
> There is one notable exception to this, though, if the OOM victim was
> in the process of coredumping the result would be incomplete. This is
> considered a reasonable constrain because the overall system health is
> more important than debugability of a particular application.

Is it possible to clarify what "the result would be incomplete" mean?

  (1) The size of coredump file becomes smaller than it should be, and
      data in reaped pages is not included into the file.

  (2) The size of coredump file does not change, and data in reaped pages
      is included into the file as NUL byte.

  (3) The size of coredump file does not change, and data in reaped pages
      is included into the file as-is (i.e. information leak security risk).

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

#1325405 — [PATCH 5/5] mm, oom_reaper: implement OOM victims queuing

From: Michal Hocko <mhocko@suse.com>

wake_oom_reaper has allowed only 1 oom victim to be queued. The main
reason for that was the simplicity as other solutions would require
some way of queuing. The current approach is racy and that was deemed
sufficient as the oom_reaper is considered a best effort approach
to help with oom handling when the OOM victim cannot terminate in a
reasonable time. The race could lead to missing an oom victim which can
get stuck

out_of_memory
  wake_oom_reaper
    cmpxchg // OK
    			oom_reaper
			  oom_reap_task
			    __oom_reap_task
oom_victim terminates
			      atomic_inc_not_zero // fail
out_of_memory
  wake_oom_reaper
    cmpxchg // fails
			  task_to_reap = NULL

This race requires 2 OOM invocations in a short time period which is not
very likely but certainly not impossible. E.g. the original victim might
have not released a lot of memory for some reason.

The situation would improve considerably if wake_oom_reaper used a more
robust queuing. This is what this patch implements. This means adding
oom_reaper_list list_head into task_struct (eat a hole before embeded
thread_struct for that purpose) and a oom_reaper_lock spinlock for
queuing synchronization. wake_oom_reaper will then add the task on the
queue and oom_reaper will dequeue it.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/sched.h |  3 +++
 mm/oom_kill.c         | 36 +++++++++++++++++++-----------------
 2 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index a9cdd032b988..c25996c336de 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1814,6 +1814,9 @@ struct task_struct {
 	unsigned long	task_state_change;
 #endif
 	int pagefault_disabled;
+#ifdef CONFIG_MMU
+	struct list_head oom_reaper_list;
+#endif
 /* CPU-specific state of this task */
 	struct thread_struct thread;
 /*
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index b87acdca2a41..87d644c97ac9 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -417,8 +417,10 @@ bool oom_killer_disabled __read_mostly;
  * victim (if that is possible) to help the OOM killer to move on.
  */
 static struct task_struct *oom_reaper_th;
-static struct task_struct *task_to_reap;
 static DECLARE_WAIT_QUEUE_HEAD(oom_reaper_wait);
+static LIST_HEAD(oom_reaper_list);
+static DEFINE_SPINLOCK(oom_reaper_lock);
+
 
 #define K(x) ((x) << (PAGE_SHIFT-10))
 static bool __oom_reap_task(struct task_struct *tsk)
@@ -520,12 +522,20 @@ static void oom_reap_task(struct task_struct *tsk)
 static int oom_reaper(void *unused)
 {
 	while (true) {
-		struct task_struct *tsk;
+		struct task_struct *tsk = NULL;
 
 		wait_event_freezable(oom_reaper_wait,
-				     (tsk = READ_ONCE(task_to_reap)));
-		oom_reap_task(tsk);
-		WRITE_ONCE(task_to_reap, NULL);
+				     (!list_empty(&oom_reaper_list)));
+		spin_lock(&oom_reaper_lock);
+		if (!list_empty(&oom_reaper_list)) {
+			tsk = list_first_entry(&oom_reaper_list,
+					struct task_struct, oom_reaper_list);
+			list_del(&tsk->oom_reaper_list);
+		}
+		spin_unlock(&oom_reaper_lock);
+
+		if (tsk)
+			oom_reap_task(tsk);
 	}
 
 	return 0;
@@ -533,23 +543,15 @@ static int oom_reaper(void *unused)
 
 static void wake_oom_reaper(struct task_struct *tsk)
 {
-	struct task_struct *old_tsk;
-
 	if (!oom_reaper_th)
 		return;
 
 	get_task_struct(tsk);
 
-	/*
-	 * Make sure that only a single mm is ever queued for the reaper
-	 * because multiple are not necessary and the operation might be
-	 * disruptive so better reduce it to the bare minimum.
-	 */
-	old_tsk = cmpxchg(&task_to_reap, NULL, tsk);
-	if (!old_tsk)
-		wake_up(&oom_reaper_wait);
-	else
-		put_task_struct(tsk);
+	spin_lock(&oom_reaper_lock);
+	list_add(&tsk->oom_reaper_list, &oom_reaper_list);
+	spin_unlock(&oom_reaper_lock);
+	wake_up(&oom_reaper_wait);
 }
 
 static int __init oom_init(void)
-- 
2.7.0

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

#1326639 — Re: [PATCH 5/5] mm, oom_reaper: implement OOM victims queuing

Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
> 
> wake_oom_reaper has allowed only 1 oom victim to be queued. The main
> reason for that was the simplicity as other solutions would require
> some way of queuing. The current approach is racy and that was deemed
> sufficient as the oom_reaper is considered a best effort approach
> to help with oom handling when the OOM victim cannot terminate in a
> reasonable time. The race could lead to missing an oom victim which can
> get stuck
> 
> out_of_memory
>   wake_oom_reaper
>     cmpxchg // OK
>     			oom_reaper
> 			  oom_reap_task
> 			    __oom_reap_task
> oom_victim terminates
> 			      atomic_inc_not_zero // fail
> out_of_memory
>   wake_oom_reaper
>     cmpxchg // fails
> 			  task_to_reap = NULL
> 
> This race requires 2 OOM invocations in a short time period which is not
> very likely but certainly not impossible. E.g. the original victim might
> have not released a lot of memory for some reason.
> 
> The situation would improve considerably if wake_oom_reaper used a more
> robust queuing. This is what this patch implements. This means adding
> oom_reaper_list list_head into task_struct (eat a hole before embeded
> thread_struct for that purpose) and a oom_reaper_lock spinlock for
> queuing synchronization. wake_oom_reaper will then add the task on the
> queue and oom_reaper will dequeue it.
> 

I think we want to rewrite this patch's description from a different point
of view.

As of "[PATCH 1/5] mm, oom: introduce oom reaper", we assumed that we try to
manage OOM livelock caused by system-wide OOM events using the OOM reaper.
Therefore, the OOM reaper had high scheduling priority and we considered side
effect of the OOM reaper as a reasonable constraint.

But as the discussion went by, we started to try to manage OOM livelock
caused by non system-wide OOM events (e.g. memcg OOM) using the OOM reaper.
Therefore, the OOM reaper now has normal scheduling priority. For non
system-wide OOM events, side effect of the OOM reaper might not be a
reasonable constraint. Some administrator might expect that the OOM reaper
does not break coredumping unless the system is under system-wide OOM events.

The race described in this patch's description sounds as if 2 OOM invocations
are by system-wide OOM events. If we consider only system-wide OOM events,
there is no need to keep task_to_reap != NULL after the OOM reaper found
a task to reap (shown below) because existing victim will prevent the OOM
killer from calling wake_oom_reaper().

----------------------------------------
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 012dd6f..c919ddb 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1835,9 +1835,6 @@ struct task_struct {
 	unsigned long	task_state_change;
 #endif
 	int pagefault_disabled;
-#ifdef CONFIG_MMU
-	struct list_head oom_reaper_list;
-#endif
 /* CPU-specific state of this task */
 	struct thread_struct thread;
 /*
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index b42c6bc..fa6a302 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -422,10 +422,8 @@ bool oom_killer_disabled __read_mostly;
  * victim (if that is possible) to help the OOM killer to move on.
  */
 static struct task_struct *oom_reaper_th;
+static struct task_struct *task_to_reap;
 static DECLARE_WAIT_QUEUE_HEAD(oom_reaper_wait);
-static LIST_HEAD(oom_reaper_list);
-static DEFINE_SPINLOCK(oom_reaper_lock);
-
 
 static bool __oom_reap_task(struct task_struct *tsk)
 {
@@ -526,20 +524,11 @@ static void oom_reap_task(struct task_struct *tsk)
 static int oom_reaper(void *unused)
 {
 	while (true) {
-		struct task_struct *tsk = NULL;
+		struct task_struct *tsk;
 
 		wait_event_freezable(oom_reaper_wait,
-				     (!list_empty(&oom_reaper_list)));
-		spin_lock(&oom_reaper_lock);
-		if (!list_empty(&oom_reaper_list)) {
-			tsk = list_first_entry(&oom_reaper_list,
-					struct task_struct, oom_reaper_list);
-			list_del(&tsk->oom_reaper_list);
-		}
-		spin_unlock(&oom_reaper_lock);
-
-		if (tsk)
-			oom_reap_task(tsk);
+				     (tsk = xchg(&task_to_reap, NULL)));
+		oom_reap_task(tsk);
 	}
 
 	return 0;
@@ -551,11 +540,11 @@ static void wake_oom_reaper(struct task_struct *tsk)
 		return;
 
 	get_task_struct(tsk);
-
-	spin_lock(&oom_reaper_lock);
-	list_add(&tsk->oom_reaper_list, &oom_reaper_list);
-	spin_unlock(&oom_reaper_lock);
-	wake_up(&oom_reaper_wait);
+	tsk = xchg(&task_to_reap, tsk);
+	if (!tsk)
+		wake_up(&oom_reaper_wait);
+	else
+		put_task_struct(tsk);
 }
 
 static int __init oom_init(void)
----------------------------------------

But if we consider non system-wide OOM events, it is not very unlikely to hit
this race. This queue is useful for situations where memcg1 and memcg2 hit
memcg OOM at the same time and victim1 in memcg1 cannot terminate immediately.

I expect parallel reaping (shown below) because there is no need to serialize
victim tasks (e.g. wait for reaping victim1 in memcg1 which can take up to
1 second to complete before start reaping victim2 in memcg2) if we implement
this queue.

----------------------------------------
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index b42c6bc..c2d6472 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -427,7 +427,7 @@ static LIST_HEAD(oom_reaper_list);
 static DEFINE_SPINLOCK(oom_reaper_lock);
 
 
-static bool __oom_reap_task(struct task_struct *tsk)
+static bool oom_reap_task(struct task_struct *tsk)
 {
 	struct mmu_gather tlb;
 	struct vm_area_struct *vma;
@@ -504,42 +504,42 @@ out:
 	return ret;
 }
 
-#define MAX_OOM_REAP_RETRIES 10
-static void oom_reap_task(struct task_struct *tsk)
-{
-	int attempts = 0;
-
-	/* Retry the down_read_trylock(mmap_sem) a few times */
-	while (attempts++ < MAX_OOM_REAP_RETRIES && !__oom_reap_task(tsk))
-		schedule_timeout_idle(HZ/10);
-
-	if (attempts > MAX_OOM_REAP_RETRIES) {
-		pr_info("oom_reaper: unable to reap pid:%d (%s)\n",
-				task_pid_nr(tsk), tsk->comm);
-		debug_show_all_locks();
-	}
-
-	/* Drop a reference taken by wake_oom_reaper */
-	put_task_struct(tsk);
-}
-
 static int oom_reaper(void *unused)
 {
 	while (true) {
-		struct task_struct *tsk = NULL;
-
+		struct task_struct *tsk;
+		struct task_struct *t;
+		LIST_HEAD(list);
+		int i;
+
 		wait_event_freezable(oom_reaper_wait,
 				     (!list_empty(&oom_reaper_list)));
 		spin_lock(&oom_reaper_lock);
-		if (!list_empty(&oom_reaper_list)) {
-			tsk = list_first_entry(&oom_reaper_list,
-					struct task_struct, oom_reaper_list);
-			list_del(&tsk->oom_reaper_list);
-		}
+		list_splice(&oom_reaper_list, &list);
+		INIT_LIST_HEAD(&oom_reaper_list);
 		spin_unlock(&oom_reaper_lock);
-
-		if (tsk)
-			oom_reap_task(tsk);
+		/* Retry the down_read_trylock(mmap_sem) a few times */
+		for (i = 0; i < 10; i++) {
+			list_for_each_entry_safe(tsk, t, &list,
+						 oom_reaper_list) {
+				if (!oom_reap_task(tsk))
+					continue;
+				list_del(&tsk->oom_reaper_list);
+				/* Drop a reference taken by wake_oom_reaper */
+				put_task_struct(tsk);
+			}
+			if (list_empty(&list))
+				break;
+			schedule_timeout_idle(HZ/10);
+		}
+		if (list_empty(&list))
+			continue;
+		list_for_each_entry(tsk, &list, oom_reaper_list) {
+			pr_info("oom_reaper: unable to reap pid:%d (%s)\n",
+				task_pid_nr(tsk), tsk->comm);
+			put_task_struct(tsk);
+		}
+		debug_show_all_locks();
 	}
 
 	return 0;
----------------------------------------

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

#1326893 — Re: [PATCH 5/5] mm, oom_reaper: implement OOM victims queuing

On Thu 04-02-16 19:49:29, Tetsuo Handa wrote:
[...]
> I think we want to rewrite this patch's description from a different point
> of view.
> 
> As of "[PATCH 1/5] mm, oom: introduce oom reaper", we assumed that we try to
> manage OOM livelock caused by system-wide OOM events using the OOM reaper.
> Therefore, the OOM reaper had high scheduling priority and we considered side
> effect of the OOM reaper as a reasonable constraint.
> 
> But as the discussion went by, we started to try to manage OOM livelock
> caused by non system-wide OOM events (e.g. memcg OOM) using the OOM reaper.
> Therefore, the OOM reaper now has normal scheduling priority. For non
> system-wide OOM events, side effect of the OOM reaper might not be a
> reasonable constraint. Some administrator might expect that the OOM reaper
> does not break coredumping unless the system is under system-wide OOM events.

I am willing to discuss this as an option after we actually hear about a
_real_ usecase.

[...]

> But if we consider non system-wide OOM events, it is not very unlikely to hit
> this race. This queue is useful for situations where memcg1 and memcg2 hit
> memcg OOM at the same time and victim1 in memcg1 cannot terminate immediately.

This can happen of course but the likelihood is _much_ smaller without
the global OOM because the memcg OOM killer is invoked from a lockless
context so the oom context cannot block the victim to proceed.

> I expect parallel reaping (shown below) because there is no need to serialize
> victim tasks (e.g. wait for reaping victim1 in memcg1 which can take up to
> 1 second to complete before start reaping victim2 in memcg2) if we implement
> this queue.

I would really prefer to go a simpler way first and extend the code when
we see the current approach insufficient for real life loads. Please do
not get me wrong, of course the code can be enhanced in many different
ways and optimize for lots of pathological cases but I really believe
that we should start with correctness first and only later care about
optimizing corner cases. Realistically, who really cares about
oom_reaper acting on an Nth completely stuck tasks after N seconds?
For now, my target is to guarantee that oom_reaper will _eventually_
process a queued task and reap its memory if the target hasn't exited
yet. I do not think this is an unreasonable goal...

Thanks!
-- 
Michal Hocko
SUSE Labs

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

#1328230 — Re: [PATCH 5/5] mm, oom_reaper: implement OOM victims queuing

Michal Hocko wrote:
> > But if we consider non system-wide OOM events, it is not very unlikely to hit
> > this race. This queue is useful for situations where memcg1 and memcg2 hit
> > memcg OOM at the same time and victim1 in memcg1 cannot terminate immediately.
> 
> This can happen of course but the likelihood is _much_ smaller without
> the global OOM because the memcg OOM killer is invoked from a lockless
> context so the oom context cannot block the victim to proceed.

Suppose mem_cgroup_out_of_memory() is called from a lockless context via
mem_cgroup_oom_synchronize() called from pagefault_out_of_memory(), that
"lockless" is talking about only current thread, doesn't it?

Since oom_kill_process() sets TIF_MEMDIE on first mm!=NULL thread of a
victim process, it is possible that non-first mm!=NULL thread triggers
pagefault_out_of_memory() and first mm!=NULL thread gets TIF_MEMDIE,
isn't it?

Then, where is the guarantee that victim1 (first mm!=NULL thread in memcg1
which got TIF_MEMDIE) is not waiting at down_read(&victim2->mm->mmap_sem)
when victim2 (first mm!=NULL thread in memcg2 which got TIF_MEMDIE) is
waiting at down_write(&victim2->mm->mmap_sem) or both victim1 and victim2
are waiting on a lock somewhere in memory reclaim path (e.g.
mutex_lock(&inode->i_mutex))?

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

#1328261 — Re: [PATCH 5/5] mm, oom_reaper: implement OOM victims queuing

On Sat 06-02-16 14:54:24, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > > But if we consider non system-wide OOM events, it is not very unlikely to hit
> > > this race. This queue is useful for situations where memcg1 and memcg2 hit
> > > memcg OOM at the same time and victim1 in memcg1 cannot terminate immediately.
> > 
> > This can happen of course but the likelihood is _much_ smaller without
> > the global OOM because the memcg OOM killer is invoked from a lockless
> > context so the oom context cannot block the victim to proceed.
> 
> Suppose mem_cgroup_out_of_memory() is called from a lockless context via
> mem_cgroup_oom_synchronize() called from pagefault_out_of_memory(), that
> "lockless" is talking about only current thread, doesn't it?

Yes and you need the OOM context to sit on the same lock as the victim
to form a deadlock. So while the victim might be blocked somewhere it is
much less likely it would be deadlocked.

> Since oom_kill_process() sets TIF_MEMDIE on first mm!=NULL thread of a
> victim process, it is possible that non-first mm!=NULL thread triggers
> pagefault_out_of_memory() and first mm!=NULL thread gets TIF_MEMDIE,
> isn't it?

I got lost here completely. Maybe it is your usage of thread terminology
again.
 
> Then, where is the guarantee that victim1 (first mm!=NULL thread in memcg1
> which got TIF_MEMDIE) is not waiting at down_read(&victim2->mm->mmap_sem)
> when victim2 (first mm!=NULL thread in memcg2 which got TIF_MEMDIE) is
> waiting at down_write(&victim2->mm->mmap_sem)

All threads/processes sharing the same mm are in fact in the same memory
cgroup. That is the reason we have owner in the task_struct

> or both victim1 and victim2
> are waiting on a lock somewhere in memory reclaim path (e.g.
> mutex_lock(&inode->i_mutex))?

Such waiting has to make a forward progress at some point in time
because the lock itself cannot be deadlocked by the memcg OOM context.


-- 
Michal Hocko
SUSE Labs

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

#1328320 — Re: [PATCH 5/5] mm, oom_reaper: implement OOM victims queuing

Michal Hocko wrote:
> On Sat 06-02-16 14:54:24, Tetsuo Handa wrote:
> > Michal Hocko wrote:
> > > > But if we consider non system-wide OOM events, it is not very unlikely to hit
> > > > this race. This queue is useful for situations where memcg1 and memcg2 hit
> > > > memcg OOM at the same time and victim1 in memcg1 cannot terminate immediately.
> > > 
> > > This can happen of course but the likelihood is _much_ smaller without
> > > the global OOM because the memcg OOM killer is invoked from a lockless
> > > context so the oom context cannot block the victim to proceed.
> > 
> > Suppose mem_cgroup_out_of_memory() is called from a lockless context via
> > mem_cgroup_oom_synchronize() called from pagefault_out_of_memory(), that
> > "lockless" is talking about only current thread, doesn't it?
> 
> Yes and you need the OOM context to sit on the same lock as the victim
> to form a deadlock. So while the victim might be blocked somewhere it is
> much less likely it would be deadlocked.
> 
> > Since oom_kill_process() sets TIF_MEMDIE on first mm!=NULL thread of a
> > victim process, it is possible that non-first mm!=NULL thread triggers
> > pagefault_out_of_memory() and first mm!=NULL thread gets TIF_MEMDIE,
> > isn't it?
> 
> I got lost here completely. Maybe it is your usage of thread terminology
> again.

I'm using "process" == "thread group" which contains at least one "thread",
and "thread" == "struct task_struct".
My assumption is

   (1) app1 process has two threads named app1t1 and app1t2
   (2) app2 process has two threads named app2t1 and app2t2
   (3) app1t1->mm == app1t2->mm != NULL and app2t1->mm == app2t2->mm != NULL
   (4) app1 is in memcg1 and app2 is in memcg2

and sequence is

   (1) app1t2 triggers pagefault_out_of_memory()
   (2) app1t2 calls mem_cgroup_out_of_memory() via mem_cgroup_oom_synchronize()
   (3) oom_scan_process_thread() selects app1 as an OOM victim process
   (4) find_lock_task_mm() selects app1t1 as an OOM victim thread
   (5) app1t1 gets TIF_MEMDIE
   (6) app2t2 triggers pagefault_out_of_memory()
   (7) app2t2 calls mem_cgroup_out_of_memory() via mem_cgroup_oom_synchronize()
   (8) oom_scan_process_thread() selects app2 as an OOM victim process
   (9) find_lock_task_mm() selects app2t1 as an OOM victim thread
   (10) app2t1 gets TIF_MEMDIE

.

I'm talking about situation where app1t1 is blocked at down_write(&app1t1->mm->mmap_sem)
because somebody else is already waiting at down_read(&app1t1->mm->mmap_sem) or is
doing memory allocation between down_read(&app1t1->mm->mmap_sem) and
up_read(&app1t1->mm->mmap_sem). In this case, this [PATCH 5/5] helps the OOM reaper to
reap app2t1->mm after giving up waiting for down_read(&app1t1->mm->mmap_sem) to succeed.

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

#1325408 — [PATCH 3/5] oom: clear TIF_MEMDIE after oom_reaper managed to unmap the address space

From: Michal Hocko <mhocko@suse.com>

When oom_reaper manages to unmap all the eligible vmas there shouldn't
be much of the freable memory held by the oom victim left anymore so it
makes sense to clear the TIF_MEMDIE flag for the victim and allow the
OOM killer to select another task.

The lack of TIF_MEMDIE also means that the victim cannot access memory
reserves anymore but that shouldn't be a problem because it would get
the access again if it needs to allocate and hits the OOM killer again
due to the fatal_signal_pending resp. PF_EXITING check. We can safely
hide the task from the OOM killer because it is clearly not a good
candidate anymore as everyhing reclaimable has been torn down already.

This patch will allow to cap the time an OOM victim can keep TIF_MEMDIE
and thus hold off further global OOM killer actions granted the oom
reaper is able to take mmap_sem for the associated mm struct. This is
not guaranteed now but further steps should make sure that mmap_sem
for write should be blocked killable which will help to reduce such a
lock contention. This is not done by this patch.

Note that exit_oom_victim might be called on a remote task from
__oom_reap_task now so we have to check and clear the flag atomically
otherwise we might race and underflow oom_victims or wake up
waiters too early.

Suggested-by: Johannes Weiner <hannes@cmpxchg.org>
Suggested-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/oom.h |  2 +-
 kernel/exit.c       |  2 +-
 mm/oom_kill.c       | 73 +++++++++++++++++++++++++++++++++++------------------
 3 files changed, 50 insertions(+), 27 deletions(-)

diff --git a/include/linux/oom.h b/include/linux/oom.h
index 03e6257321f0..45993b840ed6 100644
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -91,7 +91,7 @@ extern enum oom_scan_t oom_scan_process_thread(struct oom_control *oc,
 
 extern bool out_of_memory(struct oom_control *oc);
 
-extern void exit_oom_victim(void);
+extern void exit_oom_victim(struct task_struct *tsk);
 
 extern int register_oom_notifier(struct notifier_block *nb);
 extern int unregister_oom_notifier(struct notifier_block *nb);
diff --git a/kernel/exit.c b/kernel/exit.c
index 07110c6020a0..4b1c6e2658bd 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -436,7 +436,7 @@ static void exit_mm(struct task_struct *tsk)
 	mm_update_next_owner(mm);
 	mmput(mm);
 	if (test_thread_flag(TIF_MEMDIE))
-		exit_oom_victim();
+		exit_oom_victim(tsk);
 }
 
 static struct task_struct *find_alive_thread(struct task_struct *p)
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 840e03986497..8e345126d73e 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -417,20 +417,36 @@ bool oom_killer_disabled __read_mostly;
  * victim (if that is possible) to help the OOM killer to move on.
  */
 static struct task_struct *oom_reaper_th;
-static struct mm_struct *mm_to_reap;
+static struct task_struct *task_to_reap;
 static DECLARE_WAIT_QUEUE_HEAD(oom_reaper_wait);
 
-static bool __oom_reap_vmas(struct mm_struct *mm)
+static bool __oom_reap_task(struct task_struct *tsk)
 {
 	struct mmu_gather tlb;
 	struct vm_area_struct *vma;
+	struct mm_struct *mm;
+	struct task_struct *p;
 	struct zap_details details = {.check_swap_entries = true,
 				      .ignore_dirty = true};
 	bool ret = true;
 
-	/* We might have raced with exit path */
-	if (!atomic_inc_not_zero(&mm->mm_users))
+	/*
+	 * Make sure we find the associated mm_struct even when the particular
+	 * thread has already terminated and cleared its mm.
+	 * We might have race with exit path so consider our work done if there
+	 * is no mm.
+	 */
+	p = find_lock_task_mm(tsk);
+	if (!p)
+		return true;
+
+	mm = p->mm;
+	if (!atomic_inc_not_zero(&mm->mm_users)) {
+		task_unlock(p);
 		return true;
+	}
+
+	task_unlock(p);
 
 	if (!down_read_trylock(&mm->mmap_sem)) {
 		ret = false;
@@ -461,60 +477,66 @@ static bool __oom_reap_vmas(struct mm_struct *mm)
 	}
 	tlb_finish_mmu(&tlb, 0, -1);
 	up_read(&mm->mmap_sem);
+
+	/*
+	 * Clear TIF_MEMDIE because the task shouldn't be sitting on a
+	 * reasonably reclaimable memory anymore. OOM killer can continue
+	 * by selecting other victim if unmapping hasn't led to any
+	 * improvements. This also means that selecting this task doesn't
+	 * make any sense.
+	 */
+	tsk->signal->oom_score_adj = OOM_SCORE_ADJ_MIN;
+	exit_oom_victim(tsk);
 out:
 	mmput(mm);
 	return ret;
 }
 
-static void oom_reap_vmas(struct mm_struct *mm)
+static void oom_reap_task(struct task_struct *tsk)
 {
 	int attempts = 0;
 
 	/* Retry the down_read_trylock(mmap_sem) a few times */
-	while (attempts++ < 10 && !__oom_reap_vmas(mm))
+	while (attempts++ < 10 && !__oom_reap_task(tsk))
 		schedule_timeout_idle(HZ/10);
 
 	/* Drop a reference taken by wake_oom_reaper */
-	mmdrop(mm);
+	put_task_struct(tsk);
 }
 
 static int oom_reaper(void *unused)
 {
 	while (true) {
-		struct mm_struct *mm;
+		struct task_struct *tsk;
 
 		wait_event_freezable(oom_reaper_wait,
-				     (mm = READ_ONCE(mm_to_reap)));
-		oom_reap_vmas(mm);
-		WRITE_ONCE(mm_to_reap, NULL);
+				     (tsk = READ_ONCE(task_to_reap)));
+		oom_reap_task(tsk);
+		WRITE_ONCE(task_to_reap, NULL);
 	}
 
 	return 0;
 }
 
-static void wake_oom_reaper(struct mm_struct *mm)
+static void wake_oom_reaper(struct task_struct *tsk)
 {
-	struct mm_struct *old_mm;
+	struct task_struct *old_tsk;
 
 	if (!oom_reaper_th)
 		return;
 
-	/*
-	 * Pin the given mm. Use mm_count instead of mm_users because
-	 * we do not want to delay the address space tear down.
-	 */
-	atomic_inc(&mm->mm_count);
+	get_task_struct(tsk);
 
 	/*
 	 * Make sure that only a single mm is ever queued for the reaper
 	 * because multiple are not necessary and the operation might be
 	 * disruptive so better reduce it to the bare minimum.
 	 */
-	old_mm = cmpxchg(&mm_to_reap, NULL, mm);
-	if (!old_mm)
+	old_tsk = cmpxchg(&task_to_reap, NULL, tsk);
+	if (!old_tsk)
 		wake_up(&oom_reaper_wait);
 	else
-		mmdrop(mm);
+		put_task_struct(tsk);
 }
 
 static int __init oom_init(void)
@@ -529,7 +551,7 @@ static int __init oom_init(void)
 }
 subsys_initcall(oom_init)
 #else
-static void wake_oom_reaper(struct mm_struct *mm)
+static void wake_oom_reaper(struct task_struct *mm)
 {
 }
 #endif
@@ -560,9 +582,10 @@ void mark_oom_victim(struct task_struct *tsk)
 /**
  * exit_oom_victim - note the exit of an OOM victim
  */
-void exit_oom_victim(void)
+void exit_oom_victim(struct task_struct *tsk)
 {
-	clear_thread_flag(TIF_MEMDIE);
+	if (!test_and_clear_tsk_thread_flag(tsk, TIF_MEMDIE))
+		return;
 
 	if (!atomic_dec_return(&oom_victims))
 		wake_up_all(&oom_victims_wait);
@@ -745,7 +768,7 @@ void oom_kill_process(struct oom_control *oc, struct task_struct *p,
 	rcu_read_unlock();
 
 	if (can_oom_reap)
-		wake_oom_reaper(mm);
+		wake_oom_reaper(victim);
 
 	mmdrop(mm);
 	put_task_struct(victim);
-- 
2.7.0

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

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web