linux-kernel - Re: [PATCH 2/3] oom, oom_reaper: Try to reap tasks which skip regular OOM killer path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201604072038.CHC51027.MSJOFVLHOFFtQO@I-love.SAKURA.ne.jp>
Date:	Thu, 7 Apr 2016 20:38:43 +0900
From:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To:	mhocko@...nel.org, linux-mm@...ck.org
Cc:	rientjes@...gle.com, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org, mhocko@...e.com, oleg@...hat.com
Subject: Re: [PATCH 2/3] oom, oom_reaper: Try to reap tasks which skip regular OOM killer path

Michal Hocko wrote:
> @@ -563,6 +582,53 @@ static void wake_oom_reaper(struct task_struct *tsk)
>  	wake_up(&oom_reaper_wait);
>  }
>  
> +/* Check if we can reap the given task. This has to be called with stable
> + * tsk->mm
> + */
> +static void try_oom_reaper(struct task_struct *tsk)
> +{
> +	struct mm_struct *mm = tsk->mm;
> +	struct task_struct *p;
> +
> +	if (!mm)
> +		return;
> +
> +	/*
> +	 * There might be other threads/processes which are either not
> +	 * dying or even not killable.
> +	 */
> +	if (atomic_read(&mm->mm_users) > 1) {
> +		rcu_read_lock();
> +		for_each_process(p) {
> +			bool exiting;
> +
> +			if (!process_shares_mm(p, mm))
> +				continue;
> +			if (same_thread_group(p, tsk))
> +				continue;
> +			if (fatal_signal_pending(p))
> +				continue;
> +
> +			/*
> +			 * If the task is exiting make sure the whole thread group
> +			 * is exiting and cannot acces mm anymore.
> +			 */
> +			spin_lock_irq(&p->sighand->siglock);
> +			exiting = signal_group_exit(p->signal);
> +			spin_unlock_irq(&p->sighand->siglock);
> +			if (exiting)
> +				continue;
> +
> +			/* Give up */
> +			rcu_read_unlock();
> +			return;
> +		}
> +		rcu_read_unlock();
> +	}
> +
> +	wake_oom_reaper(tsk);
> +}
> +

I think you want to change "try_oom_reaper() without wake_oom_reaper()"
as mm_is_reapable() and use it from oom_kill_process() in order to skip
p->signal->oom_score_adj == OOM_SCORE_ADJ_MIN test which needlessly makes
can_oom_reap false.

> @@ -694,6 +746,7 @@ void oom_kill_process(struct oom_control *oc, struct task_struct *p,
>  	task_lock(p);
>  	if (p->mm && task_will_free_mem(p)) {
>  		mark_oom_victim(p);
> +		try_oom_reaper(p);
>  		task_unlock(p);
>  		put_task_struct(p);
>  		return;
> @@ -873,6 +926,7 @@ bool out_of_memory(struct oom_control *oc)
>  	if (current->mm &&
>  	    (fatal_signal_pending(current) || task_will_free_mem(current))) {
>  		mark_oom_victim(current);
> +		try_oom_reaper(current);
>  		return true;
>  	}
>  

Why don't you call try_oom_reaper() from the shortcuts in
mem_cgroup_out_of_memory() as well?

Why don't you embed try_oom_reaper() into mark_oom_victim() like I did at
http://lkml.kernel.org/r/201602052014.HBG52666.HFMOQVLFOSFJtO@I-love.SAKURA.ne.jp ?