All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andi Kleen <andi@firstfloor.org>
To: linux-kernel@vger.kernel.org
Cc: Andi Kleen <ak@linux.intel.com>, penberg@kernel.org, cl@linux.com
Subject: [PATCH] slab/mempolicy: always use local policy from interrupt context v3
Date: Wed, 30 May 2012 21:34:04 -0700	[thread overview]
Message-ID: <1338438844-5022-1-git-send-email-andi@firstfloor.org> (raw)

From: Andi Kleen <ak@linux.intel.com>

slab_node() could access current->mempolicy from interrupt context.
However there's a race condition during exit where the mempolicy
is first freed and then the pointer zeroed.

Using this from interrupts seems bogus anyways. The interrupt
will interrupt a random process and therefore get a random
mempolicy. Many times, this will be idle's, which noone can change.

Just disable this here and always use local for slab
from interrupts. I also cleaned up the callers of slab_node a bit
which always passed the same argument.

I believe the original mempolicy code did that in fact,
so it's likely a regression.

v2: send version with correct logic
v3: simplify. fix typo.
Reported-by: Arun Sharma <asharma@fb.com>
Cc: penberg@kernel.org
Cc: cl@linux.com
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 include/linux/mempolicy.h |    2 +-
 mm/mempolicy.c            |    6 ++++--
 mm/slab.c                 |    4 ++--
 mm/slub.c                 |    2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/include/linux/mempolicy.h b/include/linux/mempolicy.h
index 7c727a9..7106786 100644
--- a/include/linux/mempolicy.h
+++ b/include/linux/mempolicy.h
@@ -215,7 +215,7 @@ extern struct zonelist *huge_zonelist(struct vm_area_struct *vma,
 extern bool init_nodemask_of_mempolicy(nodemask_t *mask);
 extern bool mempolicy_nodemask_intersects(struct task_struct *tsk,
 				const nodemask_t *mask);
-extern unsigned slab_node(struct mempolicy *policy);
+extern unsigned slab_node(void);
 
 extern enum zone_type policy_zone;
 
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index cfb6c86..b65eb06 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1586,9 +1586,11 @@ static unsigned interleave_nodes(struct mempolicy *policy)
  * task can change it's policy.  The system default policy requires no
  * such protection.
  */
-unsigned slab_node(struct mempolicy *policy)
+unsigned slab_node(void)
 {
-	if (!policy || policy->flags & MPOL_F_LOCAL)
+	struct mempolicy *policy = current->mempolicy;
+
+	if (in_interrupt() || !policy || policy->flags & MPOL_F_LOCAL)
 		return numa_node_id();
 
 	switch (policy->mode) {
diff --git a/mm/slab.c b/mm/slab.c
index e901a36..af3b405 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3336,7 +3336,7 @@ static void *alternate_node_alloc(struct kmem_cache *cachep, gfp_t flags)
 	if (cpuset_do_slab_mem_spread() && (cachep->flags & SLAB_MEM_SPREAD))
 		nid_alloc = cpuset_slab_spread_node();
 	else if (current->mempolicy)
-		nid_alloc = slab_node(current->mempolicy);
+		nid_alloc = slab_node();
 	if (nid_alloc != nid_here)
 		return ____cache_alloc_node(cachep, flags, nid_alloc);
 	return NULL;
@@ -3368,7 +3368,7 @@ static void *fallback_alloc(struct kmem_cache *cache, gfp_t flags)
 
 retry_cpuset:
 	cpuset_mems_cookie = get_mems_allowed();
-	zonelist = node_zonelist(slab_node(current->mempolicy), flags);
+	zonelist = node_zonelist(slab_node(), flags);
 
 retry:
 	/*
diff --git a/mm/slub.c b/mm/slub.c
index ffe13fd..ef936f3 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1614,7 +1614,7 @@ static struct page *get_any_partial(struct kmem_cache *s, gfp_t flags,
 
 	do {
 		cpuset_mems_cookie = get_mems_allowed();
-		zonelist = node_zonelist(slab_node(current->mempolicy), flags);
+		zonelist = node_zonelist(slab_node(), flags);
 		for_each_zone_zonelist(zone, z, zonelist, high_zoneidx) {
 			struct kmem_cache_node *n;
 
-- 
1.7.7.6


             reply	other threads:[~2012-05-31  4:34 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-31  4:34 Andi Kleen [this message]
2012-05-31 14:13 ` [PATCH] slab/mempolicy: always use local policy from interrupt context v3 Christoph Lameter
2012-05-31 20:45 ` David Rientjes
2012-06-01  6:41   ` Pekka Enberg
2012-06-01 14:10     ` Christoph Lameter
2012-06-09  9:40 ` [PATCH v5] slab/mempolicy: always use local policy from interrupt context David Mackey
2012-06-09  9:40   ` David Mackey
2012-06-10  2:19   ` David Rientjes
2012-06-10  2:19     ` David Rientjes
2012-06-17  1:11     ` David Rientjes
2012-06-17  1:11       ` David Rientjes
2012-06-17 10:37       ` Pekka Enberg
2012-06-17 10:37         ` Pekka Enberg
2012-06-11 15:05   ` Christoph Lameter
2012-06-11 15:05     ` Christoph Lameter
2012-06-18  8:20   ` KOSAKI Motohiro
2012-06-18  8:20     ` KOSAKI Motohiro
2012-06-20  7:02     ` Pekka Enberg
2012-06-20  7:02       ` Pekka Enberg
  -- strict thread matches above, loose matches on Subject: below --
2012-05-07 22:55 [PATCH] slab/mempolicy: always use local policy from interrupt context v3 Andi Kleen
2012-05-07 23:47 ` KOSAKI Motohiro
2012-05-09  4:59   ` David Rientjes
2012-05-09  8:03     ` KOSAKI Motohiro
2012-05-30  6:47     ` Pekka Enberg
2012-05-30 15:40       ` Christoph Lameter
2012-05-30 16:32         ` Andi Kleen
2012-05-30 21:30           ` David Rientjes
2012-05-31  7:43             ` Pekka Enberg
2012-05-08 14:03 ` Christoph Lameter

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=1338438844-5022-1-git-send-email-andi@firstfloor.org \
    --to=andi@firstfloor.org \
    --cc=ak@linux.intel.com \
    --cc=cl@linux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=penberg@kernel.org \
    /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.