linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <brouer@redhat.com>
To: linux-mm@kvack.org
Cc: vdavydov@virtuozzo.com, Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Christoph Lameter <cl@linux.com>,
	Jesper Dangaard Brouer <brouer@redhat.com>
Subject: [PATCH V3 1/2] slub: fix kmem cgroup bug in kmem_cache_alloc_bulk
Date: Mon, 09 Nov 2015 19:17:31 +0100	[thread overview]
Message-ID: <20151109181703.8231.66384.stgit@firesoul> (raw)
In-Reply-To: <20151109181604.8231.22983.stgit@firesoul>

The call slab_pre_alloc_hook() interacts with kmemgc and is not
allowed to be called several times inside the bulk alloc for loop,
due to the call to memcg_kmem_get_cache().

This would result in hitting the VM_BUG_ON in __memcg_kmem_get_cache.

As suggested by Vladimir Davydov, change slab_post_alloc_hook()
to be able to handle an array of objects.

A subtle detail is, loop iterator "i" in slab_post_alloc_hook()
must have same type (size_t) as size argument.  This helps the
compiler to easier realize that it can remove the loop, when all
debug statements inside loop evaluates to nothing.
 Note, this is only an issue because the kernel is compiled with
GCC option: -fno-strict-overflow

Reported-by: Vladimir Davydov <vdavydov@virtuozzo.com>
Suggested-by: Vladimir Davydov <vdavydov@virtuozzo.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>

---
V3:
 Addressed input and used suggetions from Vladimir

 mm/slub.c |   35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 9be12ffae9fc..8e6b929d06d6 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1292,14 +1292,21 @@ static inline struct kmem_cache *slab_pre_alloc_hook(struct kmem_cache *s,
 	return memcg_kmem_get_cache(s, flags);
 }
 
-static inline void slab_post_alloc_hook(struct kmem_cache *s,
-					gfp_t flags, void *object)
+static inline void slab_post_alloc_hook(struct kmem_cache *s, gfp_t flags,
+					size_t size, void **p)
 {
+	size_t i;
+
 	flags &= gfp_allowed_mask;
-	kmemcheck_slab_alloc(s, flags, object, slab_ksize(s));
-	kmemleak_alloc_recursive(object, s->object_size, 1, s->flags, flags);
+	for (i = 0; i < size; i++) {
+		void *object = p[i];
+
+		kmemcheck_slab_alloc(s, flags, object, slab_ksize(s));
+		kmemleak_alloc_recursive(object, s->object_size, 1,
+					 s->flags, flags);
+		kasan_slab_alloc(s, object);
+	}
 	memcg_kmem_put_cache(s);
-	kasan_slab_alloc(s, object);
 }
 
 static inline void slab_free_hook(struct kmem_cache *s, void *x)
@@ -2556,7 +2563,7 @@ redo:
 	if (unlikely(gfpflags & __GFP_ZERO) && object)
 		memset(object, 0, s->object_size);
 
-	slab_post_alloc_hook(s, gfpflags, object);
+	slab_post_alloc_hook(s, gfpflags, 1, object);
 
 	return object;
 }
@@ -2906,6 +2913,11 @@ bool kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
 	struct kmem_cache_cpu *c;
 	int i;
 
+	/* memcg and kmem_cache debug support */
+	s = slab_pre_alloc_hook(s, flags);
+	if (unlikely(!s))
+		return false;
+
 	/*
 	 * Drain objects in the per cpu slab, while disabling local
 	 * IRQs, which protects against PREEMPT and interrupts
@@ -2931,16 +2943,9 @@ bool kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
 			continue; /* goto for-loop */
 		}
 
-		/* kmem_cache debug support */
-		s = slab_pre_alloc_hook(s, flags);
-		if (unlikely(!s))
-			goto error;
-
 		c->freelist = get_freepointer(s, object);
 		p[i] = object;
 
-		/* kmem_cache debug support */
-		slab_post_alloc_hook(s, flags, object);
 	}
 	c->tid = next_tid(c->tid);
 	local_irq_enable();
@@ -2953,11 +2958,15 @@ bool kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
 			memset(p[j], 0, s->object_size);
 	}
 
+	/* memcg and kmem_cache debug support */
+	slab_post_alloc_hook(s, flags, size, p);
+
 	return true;
 
 error:
 	__kmem_cache_free_bulk(s, i, p);
 	local_irq_enable();
+	memcg_kmem_put_cache(s);
 	return false;
 }
 EXPORT_SYMBOL(kmem_cache_alloc_bulk);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2015-11-09 18:17 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-05 15:37 [PATCH V2 0/2] SLUB bulk API interactions with kmem cgroup Jesper Dangaard Brouer
2015-11-05 15:37 ` [PATCH V2 1/2] slub: fix kmem cgroup bug in kmem_cache_alloc_bulk Jesper Dangaard Brouer
2015-11-05 16:18   ` Vladimir Davydov
2015-11-07 16:40     ` Jesper Dangaard Brouer
2015-11-05 15:38 ` [PATCH V2 2/2] slub: add missing kmem cgroup support to kmem_cache_free_bulk Jesper Dangaard Brouer
2015-11-05 16:25   ` Vladimir Davydov
2015-11-07 16:53     ` Jesper Dangaard Brouer
2015-11-07 20:25       ` Vladimir Davydov
2015-11-07 20:55         ` Christoph Lameter
2015-11-09 16:39         ` Jesper Dangaard Brouer
2015-11-09 18:38           ` Vladimir Davydov
2015-11-05 16:10 ` [PATCH V2 0/2] SLUB bulk API interactions with kmem cgroup Vladimir Davydov
2015-11-09 18:16   ` [PATCH V3 " Jesper Dangaard Brouer
2015-11-09 18:17     ` Jesper Dangaard Brouer [this message]
2015-11-09 19:13       ` [PATCH V3 1/2] slub: fix kmem cgroup bug in kmem_cache_alloc_bulk Vladimir Davydov
2015-11-09 20:25         ` Jesper Dangaard Brouer
2015-11-10  8:46           ` Vladimir Davydov
2015-11-10 15:55             ` Jesper Dangaard Brouer
2015-11-10 16:17               ` Christoph Lameter
2015-11-10 18:32               ` Vladimir Davydov
2015-11-11 15:28                 ` Jesper Dangaard Brouer
2015-11-11 18:30                   ` Jesper Dangaard Brouer
2015-11-11 18:56                     ` Vladimir Davydov
2015-11-12 14:27                       ` Jesper Dangaard Brouer
2015-11-09 22:04         ` Christoph Lameter
2015-11-10  8:30           ` Vladimir Davydov
2015-11-10 15:23             ` Christoph Lameter
2015-11-09 18:17     ` [PATCH V3 2/2] slub: add missing kmem cgroup support to kmem_cache_free_bulk Jesper Dangaard Brouer
2015-11-09 18:56       ` Vladimir Davydov
2015-11-13 10:57   ` [PATCH V4 0/2] SLUB bulk API interactions with kmem cgroup Jesper Dangaard Brouer
2015-11-13 10:57     ` [PATCH V4 1/2] slub: fix kmem cgroup bug in kmem_cache_alloc_bulk Jesper Dangaard Brouer
2015-11-14 11:04       ` Vladimir Davydov
2015-11-13 10:57     ` [PATCH V4 2/2] slub: add missing kmem cgroup support to kmem_cache_free_bulk Jesper Dangaard Brouer

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=20151109181703.8231.66384.stgit@firesoul \
    --to=brouer@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-mm@kvack.org \
    --cc=vdavydov@virtuozzo.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).