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


Groups > linux.kernel > #1700951 > unrolled thread

Re: Possible race condition in oom-killer

Started byMichal Hocko <mhocko@kernel.org>
First post2017-08-01 14:20 +0200
Last post2017-08-01 16:50 +0200
Articles 3 — 2 participants

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-08-01 14:20 +0200
    Re: Possible race condition in oom-killer Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> - 2017-08-01 16:20 +0200
      Re: Possible race condition in oom-killer Michal Hocko <mhocko@kernel.org> - 2017-08-01 16:50 +0200

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

FromMichal Hocko <mhocko@kernel.org>
Date2017-08-01 14:20 +0200
SubjectRe: Possible race condition in oom-killer
Message-ID<u9Ejv-3Ut-11@gated-at.bofh.it>
On Sat 29-07-17 13:31:44, Tetsuo Handa wrote:
[...]
> @@ -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;
>  }

I was going to argue that this will not work because we could mark a
former OOM victim again after it passed exit_oom_victim but this seems
impossible because task_will_free_mem checks task->mm and that will be
NULL by that time. This is still an ugly hack and it doesn't provide any
additional guarantee. Once we merge [1] then the oom victim wouldn't
need to get TIF_MEMDIE to access memory reserves.

[1] http://lkml.kernel.org/r/20170727090357.3205-2-mhocko@kernel.org
-- 
Michal Hocko
SUSE Labs

[toc] | [next] | [standalone]


#1701104

FromTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date2017-08-01 16:20 +0200
Message-ID<u9GbE-52o-23@gated-at.bofh.it>
In reply to#1700951
Michal Hocko wrote:
>                       Once we merge [1] then the oom victim wouldn't
> need to get TIF_MEMDIE to access memory reserves.
> 
> [1] http://lkml.kernel.org/r/20170727090357.3205-2-mhocko@kernel.org

False. We are not setting oom_mm to all thread groups (!CLONE_THREAD) sharing
that mm (CLONE_VM). Thus, one thread from each thread group sharing that mm
will have to call out_of_memory() in order to set oom_mm, and they will find
task_will_free_mem() returning false due to MMF_OOM_SKIP already set, and
after all goes to next OOM victim selection.

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


#1701124

FromMichal Hocko <mhocko@kernel.org>
Date2017-08-01 16:50 +0200
Message-ID<u9GEH-5cQ-17@gated-at.bofh.it>
In reply to#1701104
On Tue 01-08-17 23:16:13, Tetsuo Handa wrote:
> Michal Hocko wrote:
> >                       Once we merge [1] then the oom victim wouldn't
> > need to get TIF_MEMDIE to access memory reserves.
> > 
> > [1] http://lkml.kernel.org/r/20170727090357.3205-2-mhocko@kernel.org
> 
> False. We are not setting oom_mm to all thread groups (!CLONE_THREAD) sharing
> that mm (CLONE_VM). Thus, one thread from each thread group sharing that mm
> will have to call out_of_memory() in order to set oom_mm, and they will find
> task_will_free_mem() returning false due to MMF_OOM_SKIP already set, and
> after all goes to next OOM victim selection.

Once the patch is merged we can mark_oom_victim all of them as well.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web