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

Groups > linux.kernel > #1698730 > unrolled thread

Re: Possible race condition in oom-killer

Back to article view | Back to linux.kernel

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

Contents

  Re: Possible race condition in oom-killer Michal Hocko <mhocko@kernel.org> - 2017-07-28 14:40 +0200
    Re: Possible race condition in oom-killer Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2017-07-28 15:10 +0200
      Re: Possible race condition in oom-killer Michal Hocko <mhocko@kernel.org> - 2017-07-28 15:10 +0200
        Re: Possible race condition in oom-killer Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2017-07-28 15:20 +0200
          Re: Possible race condition in oom-killer Michal Hocko <mhocko@kernel.org> - 2017-07-28 15:40 +0200
            Re: Possible race condition in oom-killer Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2017-07-28 16:00 +0200
              Re: Possible race condition in oom-killer Michal Hocko <mhocko@kernel.org> - 2017-07-28 16:10 +0200
                Re: Possible race condition in oom-killer Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2017-07-29 06:40 +0200
    Re: Possible race condition in oom-killer Michal Hocko <mhocko@kernel.org> - 2017-07-28 16:20 +0200

#1698730 — Re: Possible race condition in oom-killer

[CC linux-mm]

On Fri 28-07-17 17:22:25, Manish Jaggi wrote:
> was: Re: [PATCH] mm, oom: allow oom reaper to race with exit_mmap
> 
> Hi Michal,
> On 7/27/2017 2:54 PM, Michal Hocko wrote:
> >On Thu 27-07-17 13:59:09, Manish Jaggi wrote:
> >[...]
> >>With 4.11.6 I was getting random kernel panics (Out of memory - No process left to kill),
> >>  when running LTP oom01 /oom02 ltp tests on our arm64 hardware with ~256G memory and high core count.
> >>The issue experienced was as follows
> >>	that either test (oom01/oom02) selected a pid as victim and waited for the pid to be killed.
> >>	that pid was marked as killed but somewhere there is a race and the process didnt get killed.
> >>	and the oom01/oom02 test started killing further processes, till it panics.
> >>IIUC this issue is quite similar to your patch description. But applying your patch I still see the issue.
> >>If it is not related to this patch, can you please suggest by looking at the log, what could be preventing
> >>the killing of victim.
> >>
> >>Log (https://pastebin.com/hg5iXRj2)
> >>
> >>As a subtest of oom02 starts, it prints out the victim - In this case 4578
> >>
> >>oom02       0  TINFO  :  start OOM testing for mlocked pages.
> >>oom02       0  TINFO  :  expected victim is 4578.
> >>
> >>When oom02 thread invokes oom-killer, it did select 4578  for killing...
> >I will definitely have a look. Can you report it in a separate email
> >thread please? Are you able to reproduce with the current Linus or
> >linux-next trees?
> Yes this issue is visible with linux-next.

Could you provide the full kernel log from this run please? I do not
expect there to be much difference but just to be sure that the code I
am looking at matches logs.

[...]
> >>[  365.283361] oom02:4586 invoked oom-killer: gfp_mask=0x16040c0(GFP_KERNEL|__GFP_COMP|__GFP_NOTRACK), nodemask=1,  order=0, oom_score_adj=0
> >Yes because
> >[  365.283499] Node 1 Normal free:19500kB min:33804kB low:165916kB high:298028kB active_anon:13312kB inactive_anon:172kB active_file:0kB inactive_file:1044kB unevictable:131560064kB writepending:0kB present:134213632kB managed:132113248kB mlocked:131560064kB slab_reclaimable:5748kB slab_unreclaimable:17808kB kernel_stack:2720kB pagetables:254636kB bounce:0kB free_pcp:10476kB local_pcp:144kB free_cma:0kB
> >
> >Although we have killed and reaped oom02 process Node1 is still below
> >min watermark and that is why we have hit the oom killer again. It
> >is not immediatelly clear to me why, that would require a deeper
> >inspection.
> I have a doubt here
> my understanding of oom test: oom() function basically forks itself and
> starts n threads each thread has a loop which allocates and touches memory
> thus will trigger oom-killer and will kill the process. the parent process
> is on a wait() and will print pass/fail.
> 
> So IIUC when 4578 is reaped all the child threads should be terminated,
> which happens in pass case (line 152)
> But even after being killed and reaped,  the oom killer is invoked again
> which doesn't seem right.

As I've said the OOM killer hits because the memory from Node 1 didn't
get freed for some reasov or got immediatally populated.

> Could it be that the process is just marked hidden from oom including its
> threads, thus oom-killer continues.

The whole process should be killed and the OOM reaper should only mark
the victim oom invisible _after_ the address space has been reaped (and
memory freed). You said the patch from
http://lkml.kernel.org/r/20170724072332.31903-1-mhocko@kernel.org didn't
help so it shouldn't be a race with the last __mmput.

Thanks!
-- 
Michal Hocko
SUSE Labs

[toc] | [next] | [standalone]

#1698760

(Oops. Forgot to add CC.)

On 2017/07/28 21:32, Michal Hocko wrote:
> [CC linux-mm]
>
> On Fri 28-07-17 17:22:25, Manish Jaggi wrote:
>> was: Re: [PATCH] mm, oom: allow oom reaper to race with exit_mmap
>>
>> Hi Michal,
>> On 7/27/2017 2:54 PM, Michal Hocko wrote:
>>> On Thu 27-07-17 13:59:09, Manish Jaggi wrote:
>>> [...]
>>>> With 4.11.6 I was getting random kernel panics (Out of memory - No process left to kill),
>>>>  when running LTP oom01 /oom02 ltp tests on our arm64 hardware with ~256G memory and high core count.
>>>> The issue experienced was as follows
>>>> 	that either test (oom01/oom02) selected a pid as victim and waited for the pid to be killed.
>>>> 	that pid was marked as killed but somewhere there is a race and the process didnt get killed.
>>>> 	and the oom01/oom02 test started killing further processes, till it panics.
>>>> IIUC this issue is quite similar to your patch description. But applying your patch I still see the issue.
>>>> If it is not related to this patch, can you please suggest by looking at the log, what could be preventing
>>>> the killing of victim.
>>>>
>>>> Log (https://pastebin.com/hg5iXRj2)
>>>>
>>>> As a subtest of oom02 starts, it prints out the victim - In this case 4578
>>>>
>>>> oom02       0  TINFO  :  start OOM testing for mlocked pages.
>>>> oom02       0  TINFO  :  expected victim is 4578.
>>>>
>>>> When oom02 thread invokes oom-killer, it did select 4578  for killing...
>>> I will definitely have a look. Can you report it in a separate email
>>> thread please? Are you able to reproduce with the current Linus or
>>> linux-next trees?
>> Yes this issue is visible with linux-next.
>
> Could you provide the full kernel log from this run please? I do not
> expect there to be much difference but just to be sure that the code I
> am looking at matches logs.

4578 is consuming memory as mlocked pages. But the OOM reaper cannot reclaim
mlocked pages (i.e. can_madv_dontneed_vma() returns false due to VM_LOCKED), can it?

oom02       0  TINFO  :  start OOM testing for mlocked pages.
oom02       0  TINFO  :  expected victim is 4578.
[  365.267347] oom_reaper: reaped process 4578 (oom02), now anon-rss:131559616kB, file-rss:0kB, shmem-rss:0kB

As a result, MMF_OOM_SKIP is set without reclaiming much memory.
Thus, it is natural that subsequent OOM victims are selected immediately because
almost all memory is still in use. Since 4578 is multi-threaded (isn't it?),
it will take time to call final __mmput() because mm->users are large.
Since there are many threads, it is possible that all OOM killable processes are
killed before final __mmput() of 4578 (which releases mlocked pages) is called.

>
> [...]
>>>> [  365.283361] oom02:4586 invoked oom-killer: gfp_mask=0x16040c0(GFP_KERNEL|__GFP_COMP|__GFP_NOTRACK), nodemask=1,  order=0, oom_score_adj=0
>>> Yes because
>>> [  365.283499] Node 1 Normal free:19500kB min:33804kB low:165916kB high:298028kB active_anon:13312kB inactive_anon:172kB active_file:0kB inactive_file:1044kB unevictable:131560064kB writepending:0kB present:134213632kB managed:132113248kB mlocked:131560064kB slab_reclaimable:5748kB slab_unreclaimable:17808kB kernel_stack:2720kB pagetables:254636kB bounce:0kB free_pcp:10476kB local_pcp:144kB free_cma:0kB
>>>
>>> Although we have killed and reaped oom02 process Node1 is still below
>>> min watermark and that is why we have hit the oom killer again. It
>>> is not immediatelly clear to me why, that would require a deeper
>>> inspection.
>> I have a doubt here
>> my understanding of oom test: oom() function basically forks itself and
>> starts n threads each thread has a loop which allocates and touches memory
>> thus will trigger oom-killer and will kill the process. the parent process
>> is on a wait() and will print pass/fail.
>>
>> So IIUC when 4578 is reaped all the child threads should be terminated,
>> which happens in pass case (line 152)
>> But even after being killed and reaped,  the oom killer is invoked again
>> which doesn't seem right.
>
> As I've said the OOM killer hits because the memory from Node 1 didn't
> get freed for some reasov or got immediatally populated.

Because of mlocked pages by multi threaded process, it will take time to
reclaim mlocked pages.

>
>> Could it be that the process is just marked hidden from oom including its
>> threads, thus oom-killer continues.
>
> The whole process should be killed and the OOM reaper should only mark
> the victim oom invisible _after_ the address space has been reaped (and
> memory freed). You said the patch from
> http://lkml.kernel.org/r/20170724072332.31903-1-mhocko@kernel.org didn't
> help so it shouldn't be a race with the last __mmput.
>
> Thanks!
>

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

#1698762

On Fri 28-07-17 21:59:50, Tetsuo Handa wrote:
> (Oops. Forgot to add CC.)
> 
> On 2017/07/28 21:32, Michal Hocko wrote:
> > [CC linux-mm]
> >
> > On Fri 28-07-17 17:22:25, Manish Jaggi wrote:
> >> was: Re: [PATCH] mm, oom: allow oom reaper to race with exit_mmap
> >>
> >> Hi Michal,
> >> On 7/27/2017 2:54 PM, Michal Hocko wrote:
> >>> On Thu 27-07-17 13:59:09, Manish Jaggi wrote:
> >>> [...]
> >>>> With 4.11.6 I was getting random kernel panics (Out of memory - No process left to kill),
> >>>>  when running LTP oom01 /oom02 ltp tests on our arm64 hardware with ~256G memory and high core count.
> >>>> The issue experienced was as follows
> >>>> 	that either test (oom01/oom02) selected a pid as victim and waited for the pid to be killed.
> >>>> 	that pid was marked as killed but somewhere there is a race and the process didnt get killed.
> >>>> 	and the oom01/oom02 test started killing further processes, till it panics.
> >>>> IIUC this issue is quite similar to your patch description. But applying your patch I still see the issue.
> >>>> If it is not related to this patch, can you please suggest by looking at the log, what could be preventing
> >>>> the killing of victim.
> >>>>
> >>>> Log (https://pastebin.com/hg5iXRj2)
> >>>>
> >>>> As a subtest of oom02 starts, it prints out the victim - In this case 4578
> >>>>
> >>>> oom02       0  TINFO  :  start OOM testing for mlocked pages.
> >>>> oom02       0  TINFO  :  expected victim is 4578.
> >>>>
> >>>> When oom02 thread invokes oom-killer, it did select 4578  for killing...
> >>> I will definitely have a look. Can you report it in a separate email
> >>> thread please? Are you able to reproduce with the current Linus or
> >>> linux-next trees?
> >> Yes this issue is visible with linux-next.
> >
> > Could you provide the full kernel log from this run please? I do not
> > expect there to be much difference but just to be sure that the code I
> > am looking at matches logs.
> 
> 4578 is consuming memory as mlocked pages. But the OOM reaper cannot reclaim
> mlocked pages (i.e. can_madv_dontneed_vma() returns false due to VM_LOCKED), can it?

You are absolutely right. I am pretty sure I've checked mlocked counter
as the first thing but that must be from one of the earlier oom reports.
My fault I haven't checked it in the critical one

[  365.267347] oom_reaper: reaped process 4578 (oom02), now anon-rss:131559616kB, file-rss:0kB, shmem-rss:0kB
[  365.282658] oom_reaper: reaped process 4583 (oom02), now anon-rss:131561664kB, file-rss:0kB, shmem-rss:0kB

and the above screemed about the fact I was just completely blind.

mlock pages handling is on my todo list for quite some time already but
I didn't get around it to implement that. mlock code is very tricky.
-- 
Michal Hocko
SUSE Labs

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

#1698771

Michal Hocko wrote:
> > 4578 is consuming memory as mlocked pages. But the OOM reaper cannot reclaim
> > mlocked pages (i.e. can_madv_dontneed_vma() returns false due to VM_LOCKED), can it?
> 
> You are absolutely right. I am pretty sure I've checked mlocked counter
> as the first thing but that must be from one of the earlier oom reports.
> My fault I haven't checked it in the critical one
> 
> [  365.267347] oom_reaper: reaped process 4578 (oom02), now anon-rss:131559616kB, file-rss:0kB, shmem-rss:0kB
> [  365.282658] oom_reaper: reaped process 4583 (oom02), now anon-rss:131561664kB, file-rss:0kB, shmem-rss:0kB
> 
> and the above screemed about the fact I was just completely blind.
> 
> mlock pages handling is on my todo list for quite some time already but
> I didn't get around it to implement that. mlock code is very tricky.

task_will_free_mem(current) in out_of_memory() returning false due to
MMF_OOM_SKIP already set allowed each thread sharing that mm to select a new
OOM victim. If task_will_free_mem(current) in out_of_memory() did not return
false, threads sharing MMF_OOM_SKIP mm would not have selected new victims
to the level where all OOM killable processes are killed and calls panic().

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

#1698789

On Fri 28-07-17 22:15:01, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > > 4578 is consuming memory as mlocked pages. But the OOM reaper cannot reclaim
> > > mlocked pages (i.e. can_madv_dontneed_vma() returns false due to VM_LOCKED), can it?
> > 
> > You are absolutely right. I am pretty sure I've checked mlocked counter
> > as the first thing but that must be from one of the earlier oom reports.
> > My fault I haven't checked it in the critical one
> > 
> > [  365.267347] oom_reaper: reaped process 4578 (oom02), now anon-rss:131559616kB, file-rss:0kB, shmem-rss:0kB
> > [  365.282658] oom_reaper: reaped process 4583 (oom02), now anon-rss:131561664kB, file-rss:0kB, shmem-rss:0kB
> > 
> > and the above screemed about the fact I was just completely blind.
> > 
> > mlock pages handling is on my todo list for quite some time already but
> > I didn't get around it to implement that. mlock code is very tricky.
> 
> task_will_free_mem(current) in out_of_memory() returning false due to
> MMF_OOM_SKIP already set allowed each thread sharing that mm to select a new
> OOM victim. If task_will_free_mem(current) in out_of_memory() did not return
> false, threads sharing MMF_OOM_SKIP mm would not have selected new victims
> to the level where all OOM killable processes are killed and calls panic().

I am not sure I understand. Do you mean this?
---
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 9e8b4f030c1c..671e4a4107d0 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -779,13 +779,6 @@ static bool task_will_free_mem(struct task_struct *task)
 	if (!__task_will_free_mem(task))
 		return false;
 
-	/*
-	 * This task has already been drained by the oom reaper so there are
-	 * only small chances it will free some more
-	 */
-	if (test_bit(MMF_OOM_SKIP, &mm->flags))
-		return false;
-
 	if (atomic_read(&mm->mm_users) <= 1)
 		return true;
 
If yes I would have to think about this some more because that might
have weird side effects (e.g. oom_victims counting after threads passed
exit_oom_victim).

Anyway the proper fix for this is to allow reaping mlocked pages. Is
something other than the LTP test affected to give this more priority?
Do we have other usecases where something mlocks the whole memory?
-- 
Michal Hocko
SUSE Labs

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

#1698805

Michal Hocko wrote:
> On Fri 28-07-17 22:15:01, Tetsuo Handa wrote:
> > task_will_free_mem(current) in out_of_memory() returning false due to
> > MMF_OOM_SKIP already set allowed each thread sharing that mm to select a new
> > OOM victim. If task_will_free_mem(current) in out_of_memory() did not return
> > false, threads sharing MMF_OOM_SKIP mm would not have selected new victims
> > to the level where all OOM killable processes are killed and calls panic().
> 
> I am not sure I understand. Do you mean this?

Yes.

> ---
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 9e8b4f030c1c..671e4a4107d0 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -779,13 +779,6 @@ static bool task_will_free_mem(struct task_struct *task)
>  	if (!__task_will_free_mem(task))
>  		return false;
>  
> -	/*
> -	 * This task has already been drained by the oom reaper so there are
> -	 * only small chances it will free some more
> -	 */
> -	if (test_bit(MMF_OOM_SKIP, &mm->flags))
> -		return false;
> -
>  	if (atomic_read(&mm->mm_users) <= 1)
>  		return true;
>  
> If yes I would have to think about this some more because that might
> have weird side effects (e.g. oom_victims counting after threads passed
> exit_oom_victim).

But this check should not be removed unconditionally. We should still return
false if returning true was not sufficient to solve the OOM situation, for
we need to select next OOM victim in that case.

> 
> Anyway the proper fix for this is to allow reaping mlocked pages.

Different approach is to set TIF_MEMDIE to all threads sharing the same
memory so that threads sharing MMF_OOM_SKIP mm do not need to call
out_of_memory() in order to get TIF_MEMDIE. 

Yet another apporach is to use __GFP_KILLABLE (we can start it as
best effort basis).

>                                                                   Is
> something other than the LTP test affected to give this more priority?
> Do we have other usecases where something mlocks the whole memory?

This panic was caused by 50 threads sharing MMF_OOM_SKIP mm exceeding
number of OOM killable processes. Whether memory is locked or not isn't
important. If a multi-threaded process which consumes little memory was
selected as an OOM victim (and reaped by the OOM reaper and MMF_OOM_SKIP
was set immediately), it might be still possible to select next OOM victims
needlessly.

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

#1698828

On Fri 28-07-17 22:55:51, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > On Fri 28-07-17 22:15:01, Tetsuo Handa wrote:
> > > task_will_free_mem(current) in out_of_memory() returning false due to
> > > MMF_OOM_SKIP already set allowed each thread sharing that mm to select a new
> > > OOM victim. If task_will_free_mem(current) in out_of_memory() did not return
> > > false, threads sharing MMF_OOM_SKIP mm would not have selected new victims
> > > to the level where all OOM killable processes are killed and calls panic().
> > 
> > I am not sure I understand. Do you mean this?
> 
> Yes.
> 
> > ---
> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index 9e8b4f030c1c..671e4a4107d0 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -779,13 +779,6 @@ static bool task_will_free_mem(struct task_struct *task)
> >  	if (!__task_will_free_mem(task))
> >  		return false;
> >  
> > -	/*
> > -	 * This task has already been drained by the oom reaper so there are
> > -	 * only small chances it will free some more
> > -	 */
> > -	if (test_bit(MMF_OOM_SKIP, &mm->flags))
> > -		return false;
> > -
> >  	if (atomic_read(&mm->mm_users) <= 1)
> >  		return true;
> >  
> > If yes I would have to think about this some more because that might
> > have weird side effects (e.g. oom_victims counting after threads passed
> > exit_oom_victim).
> 
> But this check should not be removed unconditionally. We should still return
> false if returning true was not sufficient to solve the OOM situation, for
> we need to select next OOM victim in that case.
> 
> > 
> > Anyway the proper fix for this is to allow reaping mlocked pages.
> 
> Different approach is to set TIF_MEMDIE to all threads sharing the same
> memory so that threads sharing MMF_OOM_SKIP mm do not need to call
> out_of_memory() in order to get TIF_MEMDIE. 

This is not so simple. If it were we could simply remove TIF_MEMDIE
altogether and rely on tsk_is_oom_victim.

> Yet another apporach is to use __GFP_KILLABLE (we can start it as
> best effort basis).
> 
> >                                                                   Is
> > something other than the LTP test affected to give this more priority?
> > Do we have other usecases where something mlocks the whole memory?
> 
> This panic was caused by 50 threads sharing MMF_OOM_SKIP mm exceeding
> number of OOM killable processes. Whether memory is locked or not isn't
> important.

You are wrong here I believe. The whole problem is that the OOM victim
is consuming basically all the memory (that is what the test case
actually does IIRC) and that memory is mlocked. oom_reaper is much
faster to evaluate the mm of the victim and bail out sooner than the
exit path actually manages to tear down the address space. And so we
have to find other oom victims until we simply kill everything and
panic.

> If a multi-threaded process which consumes little memory was
> selected as an OOM victim (and reaped by the OOM reaper and MMF_OOM_SKIP
> was set immediately), it might be still possible to select next OOM victims
> needlessly.

This would be true if the address space itself only contained a little
amount of memory and the large part of the memory was in page tables or
other resources which oom_reaper cannot work with. This is not a usual
case though.
-- 
Michal Hocko
SUSE Labs

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

#1699214

Michal Hocko wrote:
> On Fri 28-07-17 22:55:51, Tetsuo Handa wrote:
> > Michal Hocko wrote:
> > > On Fri 28-07-17 22:15:01, Tetsuo Handa wrote:
> > > > task_will_free_mem(current) in out_of_memory() returning false due to
> > > > MMF_OOM_SKIP already set allowed each thread sharing that mm to select a new
> > > > OOM victim. If task_will_free_mem(current) in out_of_memory() did not return
> > > > false, threads sharing MMF_OOM_SKIP mm would not have selected new victims
> > > > to the level where all OOM killable processes are killed and calls panic().
> > > 
> > > I am not sure I understand. Do you mean this?
> > 
> > Yes.
> > 
> > > ---
> > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > > index 9e8b4f030c1c..671e4a4107d0 100644
> > > --- a/mm/oom_kill.c
> > > +++ b/mm/oom_kill.c
> > > @@ -779,13 +779,6 @@ static bool task_will_free_mem(struct task_struct *task)
> > >  	if (!__task_will_free_mem(task))
> > >  		return false;
> > >  
> > > -	/*
> > > -	 * This task has already been drained by the oom reaper so there are
> > > -	 * only small chances it will free some more
> > > -	 */
> > > -	if (test_bit(MMF_OOM_SKIP, &mm->flags))
> > > -		return false;
> > > -
> > >  	if (atomic_read(&mm->mm_users) <= 1)
> > >  		return true;
> > >  
> > > If yes I would have to think about this some more because that might
> > > have weird side effects (e.g. oom_victims counting after threads passed
> > > exit_oom_victim).
> > 
> > But this check should not be removed unconditionally. We should still return
> > false if returning true was not sufficient to solve the OOM situation, for
> > we need to select next OOM victim in that case.
> > 

I think that below one can manage this race condition.

---
 include/linux/sched.h |  1 +
 mm/oom_kill.c         | 21 ++++++++++++++-------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 0db4870..3fccf72 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -652,6 +652,7 @@ struct task_struct {
 	/* disallow userland-initiated cgroup migration */
 	unsigned			no_cgroup_migration:1;
 #endif
+	unsigned			oom_kill_free_check_raced:1;
 
 	unsigned long			atomic_flags; /* Flags requiring atomic access. */
 
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 9e8b4f0..a093193 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -779,13 +779,6 @@ static bool task_will_free_mem(struct task_struct *task)
 	if (!__task_will_free_mem(task))
 		return false;
 
-	/*
-	 * This task has already been drained by the oom reaper so there are
-	 * only small chances it will free some more
-	 */
-	if (test_bit(MMF_OOM_SKIP, &mm->flags))
-		return false;
-
 	if (atomic_read(&mm->mm_users) <= 1)
 		return true;
 
@@ -806,6 +799,20 @@ static bool task_will_free_mem(struct task_struct *task)
 	}
 	rcu_read_unlock();
 
+	/*
+	 * It is possible that current thread fails to try allocation from
+	 * memory reserves if the OOM reaper set MMF_OOM_SKIP on this mm before
+	 * current thread calls out_of_memory() in order to get TIF_MEMDIE.
+	 * In that case, allow current thread to try TIF_MEMDIE allocation
+	 * before start selecting next OOM victims.
+	 */
+	if (ret && test_bit(MMF_OOM_SKIP, &mm->flags)) {
+		if (task == current && !task->oom_kill_free_check_raced)
+			task->oom_kill_free_check_raced = true;
+		else
+			ret = false;
+	}
+
 	return ret;
 }
 
-- 
1.8.3.1

What is "oom_victims counting after threads passed exit_oom_victim" ?

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

#1698834

On Fri 28-07-17 19:20:42, Manish Jaggi wrote:
> 
> Hi Michal,
> On 7/28/2017 6:02 PM, Michal Hocko wrote:
> >[CC linux-mm]
> >
> >On Fri 28-07-17 17:22:25, Manish Jaggi wrote:
> >>was: Re: [PATCH] mm, oom: allow oom reaper to race with exit_mmap
> >>
> >>Hi Michal,
> >>On 7/27/2017 2:54 PM, Michal Hocko wrote:
> >>>On Thu 27-07-17 13:59:09, Manish Jaggi wrote:
> >>>[...]
> >>>>With 4.11.6 I was getting random kernel panics (Out of memory - No process left to kill),
> >>>>  when running LTP oom01 /oom02 ltp tests on our arm64 hardware with ~256G memory and high core count.
> >>>>The issue experienced was as follows
> >>>>	that either test (oom01/oom02) selected a pid as victim and waited for the pid to be killed.
> >>>>	that pid was marked as killed but somewhere there is a race and the process didnt get killed.
> >>>>	and the oom01/oom02 test started killing further processes, till it panics.
> >>>>IIUC this issue is quite similar to your patch description. But applying your patch I still see the issue.
> >>>>If it is not related to this patch, can you please suggest by looking at the log, what could be preventing
> >>>>the killing of victim.
> >>>>
> >>>>Log (https://pastebin.com/hg5iXRj2)
> >>>>
> >>>>As a subtest of oom02 starts, it prints out the victim - In this case 4578
> >>>>
> >>>>oom02       0  TINFO  :  start OOM testing for mlocked pages.
> >>>>oom02       0  TINFO  :  expected victim is 4578.
> >>>>
> >>>>When oom02 thread invokes oom-killer, it did select 4578  for killing...
> >>>I will definitely have a look. Can you report it in a separate email
> >>>thread please? Are you able to reproduce with the current Linus or
> >>>linux-next trees?
> >>Yes this issue is visible with linux-next.
> >Could you provide the full kernel log from this run please? I do not
> >expect there to be much difference but just to be sure that the code I
> >am looking at matches logs.
> The log is here: https://pastebin.com/Pmn5ZwEM
> mlocked memory keeps on increasing till panic.

Thank you for retesting. I confirm the issue is that the oom reaper
hides the oom victim too early because the whole address space is
mlocked basically and there is not much to free. As the exit of the test
takes some time a new instance of the test pid 4625 in this case will go
and consume more than the exiting frees and that would go on an on until
we kill other eligible tasks until we panic due to no more eligible
tasks.

This is a bad situation and as I've said elsewhere in the thread the
proper fix is to teach the oom reaper to handle mlocked pages. This is
not a trivial task. We could play some other dirty tricks but I am not
sure it is worth it considering this is rather artificial test. 

Thanks!
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web