All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: David Rientjes <rientjes@google.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Mandeep Singh Baines <msb@chromium.org>,
	"Ma, Xindong" <xindong.ma@intel.com>,
	Michal Hocko <mhocko@suse.cz>, Sameer Nanda <snanda@chromium.org>,
	Sergey Dyasly <dserrg@gmail.com>,
	"Tu, Xiaobing" <xiaobing.tu@intel.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2 3/4] oom_kill: has_intersects_mems_allowed() needs rcu_read_lock()
Date: Wed, 4 Dec 2013 14:04:16 +0100	[thread overview]
Message-ID: <20131204130416.GA6014@redhat.com> (raw)
In-Reply-To: <20131204130351.GA5984@redhat.com>

At least out_of_memory() calls has_intersects_mems_allowed()
without even rcu_read_lock(), this is obviously buggy.

Add the necessary rcu_read_lock(). This means that we can not
simply return from the loop, we need "bool ret" and "break".

While at it, swap the names of task_struct's (the argument and
the local). This cleanups the code a little bit and avoids the
unnecessary initialization.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-and-Tested-by: Sergey Dyasly <dserrg@gmail.com>
Reviewed-by: Sameer Nanda <snanda@chromium.org>
---
 mm/oom_kill.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 96d7945..0d8ad1e 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -47,18 +47,20 @@ static DEFINE_SPINLOCK(zone_scan_lock);
 #ifdef CONFIG_NUMA
 /**
  * has_intersects_mems_allowed() - check task eligiblity for kill
- * @tsk: task struct of which task to consider
+ * @start: task struct of which task to consider
  * @mask: nodemask passed to page allocator for mempolicy ooms
  *
  * Task eligibility is determined by whether or not a candidate task, @tsk,
  * shares the same mempolicy nodes as current if it is bound by such a policy
  * and whether or not it has the same set of allowed cpuset nodes.
  */
-static bool has_intersects_mems_allowed(struct task_struct *tsk,
+static bool has_intersects_mems_allowed(struct task_struct *start,
 					const nodemask_t *mask)
 {
-	struct task_struct *start = tsk;
+	struct task_struct *tsk;
+	bool ret = false;
 
+	rcu_read_lock();
 	for_each_thread(start, tsk) {
 		if (mask) {
 			/*
@@ -67,19 +69,20 @@ static bool has_intersects_mems_allowed(struct task_struct *tsk,
 			 * mempolicy intersects current, otherwise it may be
 			 * needlessly killed.
 			 */
-			if (mempolicy_nodemask_intersects(tsk, mask))
-				return true;
+			ret = mempolicy_nodemask_intersects(tsk, mask);
 		} else {
 			/*
 			 * This is not a mempolicy constrained oom, so only
 			 * check the mems of tsk's cpuset.
 			 */
-			if (cpuset_mems_allowed_intersects(current, tsk))
-				return true;
+			ret = cpuset_mems_allowed_intersects(current, tsk);
 		}
+		if (ret)
+			break;
 	}
+	rcu_read_unlock();
 
-	return false;
+	return ret;
 }
 #else
 static bool has_intersects_mems_allowed(struct task_struct *tsk,
-- 
1.5.5.1


  parent reply	other threads:[~2013-12-04 13:03 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-04 13:03 [PATCH v2 0/4] initial while_each_thread() fixes Oleg Nesterov
2013-12-04 13:04 ` [PATCH v2 1/4] introduce for_each_thread() to replace the buggy while_each_thread() Oleg Nesterov
2013-12-04 13:28   ` Frederic Weisbecker
2013-12-04 13:49     ` Oleg Nesterov
2013-12-04 14:17       ` Frederic Weisbecker
2013-12-04 15:27         ` Oleg Nesterov
2013-12-05  0:58   ` David Rientjes
2013-12-05 18:16     ` Oleg Nesterov
2013-12-05 23:23       ` David Rientjes
2013-12-04 13:04 ` [PATCH v2 2/4] oom_kill: change oom_kill.c to use for_each_thread() Oleg Nesterov
2013-12-04 15:37   ` Michal Hocko
2013-12-05  0:39   ` David Rientjes
2013-12-04 13:04 ` Oleg Nesterov [this message]
2013-12-04 15:37   ` [PATCH v2 3/4] oom_kill: has_intersects_mems_allowed() needs rcu_read_lock() Michal Hocko
2013-12-05  0:41   ` David Rientjes
2013-12-04 13:04 ` [PATCH v2 4/4] oom_kill: add rcu_read_lock() into find_lock_task_mm() Oleg Nesterov
2013-12-04 15:40   ` Michal Hocko
2013-12-05  0:42   ` David Rientjes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131204130416.GA6014@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=dserrg@gmail.com \
    --cc=ebiederm@xmission.com \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhocko@suse.cz \
    --cc=msb@chromium.org \
    --cc=rientjes@google.com \
    --cc=snanda@chromium.org \
    --cc=xiaobing.tu@intel.com \
    --cc=xindong.ma@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.