linux-kernel - Re: [PATCH 1/6] mm, oom: do not loop over all tasks if there are no external tasks sharing mm

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]

Date:	Fri, 27 May 2016 10:03:20 +0200
From:	Michal Hocko <mhocko@...nel.org>
To:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Cc:	linux-mm@...ck.org, rientjes@...gle.com, oleg@...hat.com,
	vdavydov@...allels.com, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] mm, oom: do not loop over all tasks if there are no
 external tasks sharing mm

On Fri 27-05-16 09:15:07, Michal Hocko wrote:
> On Fri 27-05-16 08:45:10, Michal Hocko wrote:
> [...]
> > It is still an operation which is not needed for 99% of situations. So
> > if we do not need it for correctness then I do not think this is worth
> > bothering.
> 
> Since you have pointed out exit_mm vs. __exit_signal race yesterday I
> was thinking how to make the check reliable. Even
> atomic_read(mm->mm_users) > get_nr_threads() is not reliable and we can
> miss other tasks just because the current thread group is mostly past
> exit_mm. So far I couldn't find a way to tweak this around though.

Just for the record I was playing with the following yesterday but I
couldn't convince myself that this is safe and reasonable in the first
place (I do not like it to be honest).
---
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 1685890d424e..db027eca8be5 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -123,6 +123,35 @@ struct task_struct *find_lock_task_mm(struct task_struct *p)
 	return t;
 }
 
+bool task_has_external_users(struct task_struct *p)
+{
+	struct mm_struct *mm = NULL;
+	struct task_struct *t;
+	int active_threads = 0;
+	bool ret = true;	/* be pessimistic */
+
+	rcu_read_lock();
+	for_each_thread(p, t) {
+		task_lock(t);
+		if (likely(t->mm)) {
+			active_threads++;
+			if (!mm) {
+				mm = t->mm;
+				atomic_inc(&mm->mm_count);
+			}
+		}
+		task_unlock(t);
+	}
+	rcu_read_unlock();
+
+	if (mm) {
+		if (atomic_read(&mm->mm_users) <= active_threads)
+			ret = false;
+		mmdrop(mm);
+	}
+	return ret;
+}
+
 /*
  * order == -1 means the oom kill is required by sysrq, otherwise only
  * for display purposes.
-- 
Michal Hocko
SUSE Labs