linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Glauber Costa <glommer@parallels.com>
To: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org, cgroups@vger.kernel.org,
	Mel Gorman <mgorman@suse.de>, Tejun Heo <tj@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Michal Hocko <mhocko@suse.cz>,
	Johannes Weiner <hannes@cmpxchg.org>,
	kamezawa.hiroyu@jp.fujitsu.com, Christoph Lameter <cl@linux.com>,
	David Rientjes <rientjes@google.com>,
	Pekka Enberg <penberg@kernel.org>,
	devel@openvz.org, Glauber Costa <glommer@parallels.com>,
	Pekka Enberg <penberg@cs.helsinki.fi>,
	Suleiman Souhlal <suleiman@google.com>
Subject: [PATCH v5 10/18] sl[au]b: always get the cache from its page in kfree
Date: Fri, 19 Oct 2012 18:20:34 +0400	[thread overview]
Message-ID: <1350656442-1523-11-git-send-email-glommer@parallels.com> (raw)
In-Reply-To: <1350656442-1523-1-git-send-email-glommer@parallels.com>

struct page already have this information. If we start chaining
caches, this information will always be more trustworthy than
whatever is passed into the function

A parent pointer is added to the slub structure, so we can make sure
the freeing comes from either the right slab, or from its rightful
parent.

[ v3: added parent testing with VM_BUG_ON ]
[ v4: make it faster when kmemcg not in use ]

Signed-off-by: Glauber Costa <glommer@parallels.com>
CC: Christoph Lameter <cl@linux.com>
CC: Pekka Enberg <penberg@cs.helsinki.fi>
CC: Christoph Lameter <cl@linux.com>
CC: Pekka Enberg <penberg@cs.helsinki.fi>
CC: Michal Hocko <mhocko@suse.cz>
CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
CC: Johannes Weiner <hannes@cmpxchg.org>
CC: Suleiman Souhlal <suleiman@google.com>
CC: Tejun Heo <tj@kernel.org>
---
 include/linux/memcontrol.h |  4 ++++
 mm/slab.c                  | 17 ++++++++++++++++-
 mm/slab.h                  | 13 +++++++++++++
 mm/slub.c                  | 14 ++++++++++++--
 4 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 92fc47a..f1ecb4f 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -534,6 +534,10 @@ static inline void sock_release_memcg(struct sock *sk)
 {
 }
 
+static inline bool memcg_kmem_enabled(void)
+{
+	return false;
+}
 static inline bool
 memcg_kmem_newpage_charge(gfp_t gfp, struct mem_cgroup **memcg, int order)
 {
diff --git a/mm/slab.c b/mm/slab.c
index 98b3460..6f22067 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3911,9 +3911,24 @@ EXPORT_SYMBOL(__kmalloc);
  * Free an object which was previously allocated from this
  * cache.
  */
-void kmem_cache_free(struct kmem_cache *cachep, void *objp)
+void kmem_cache_free(struct kmem_cache *s, void *objp)
 {
 	unsigned long flags;
+	struct kmem_cache *cachep;
+
+	/*
+	 * When kmemcg is not being used, both assignments should return the
+	 * same value. but we don't want to pay the assignment price in that
+	 * case. If it is not compiled in, the compiler should be smart enough
+	 * to not do even the assignment. In that case, slab_equal_or_root
+	 * will also be a constant.
+	 */
+	if (memcg_kmem_enabled()) {
+		cachep = virt_to_cache(objp);
+		VM_BUG_ON(!slab_equal_or_root(cachep, s));
+	} else
+		cachep = s;
+
 
 	local_irq_save(flags);
 	debug_check_no_locks_freed(objp, cachep->object_size);
diff --git a/mm/slab.h b/mm/slab.h
index c35ecce..b9b5f1f 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -108,6 +108,13 @@ static inline bool cache_match_memcg(struct kmem_cache *cachep,
 	return (is_root_cache(cachep) && !memcg) ||
 		(cachep->memcg_params->memcg == memcg);
 }
+
+static inline bool slab_equal_or_root(struct kmem_cache *s,
+					struct kmem_cache *p)
+{
+	return (p == s) ||
+		(s->memcg_params && (p == s->memcg_params->root_cache));
+}
 #else
 static inline bool is_root_cache(struct kmem_cache *s)
 {
@@ -119,5 +126,11 @@ static inline bool cache_match_memcg(struct kmem_cache *cachep,
 {
 	return true;
 }
+
+static inline bool slab_equal_or_root(struct kmem_cache *s,
+					struct kmem_cache *p)
+{
+	return true;
+}
 #endif
 #endif
diff --git a/mm/slub.c b/mm/slub.c
index 05aefe2..6e1a90f 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2609,9 +2609,19 @@ redo:
 
 void kmem_cache_free(struct kmem_cache *s, void *x)
 {
-	struct page *page;
+	struct page *page = virt_to_head_page(x);
 
-	page = virt_to_head_page(x);
+	/*
+	 * When kmemcg is not being used, both assignments should return the
+	 * same value. but we don't want to pay the assignment price in that
+	 * case. If it is not compiled in, the compiler should be smart enough
+	 * to not do even the assignment. In that case, slab_equal_or_root
+	 * will also be a constant.
+	 */
+	if (memcg_kmem_enabled()) {
+		VM_BUG_ON(!slab_equal_or_root(page->slab, s));
+		s = page->slab;
+	}
 
 	if (kmem_cache_debug(s) && page->slab != s) {
 		pr_err("kmem_cache_free: Wrong slab cache. %s but object"
-- 
1.7.11.7

--
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>

  parent reply	other threads:[~2012-10-19 14:22 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-19 14:20 [PATCH v5 00/18] slab accounting for memcg Glauber Costa
2012-10-19 14:20 ` [PATCH v5 01/18] move slabinfo processing to slab_common.c Glauber Costa
2012-10-24  6:43   ` Pekka Enberg
2012-10-19 14:20 ` [PATCH v5 02/18] move print_slabinfo_header " Glauber Costa
2012-10-19 14:20 ` [PATCH v5 03/18] sl[au]b: process slabinfo_show in common code Glauber Costa
2012-10-19 14:20 ` [PATCH v5 04/18] slab: don't preemptively remove element from list in cache destroy Glauber Costa
2012-10-19 19:34   ` Christoph Lameter
2012-10-22  8:40     ` Glauber Costa
2012-10-24  6:54       ` Pekka Enberg
2012-10-24 16:19         ` Glauber Costa
2012-10-19 14:20 ` [PATCH v5 05/18] slab/slub: struct memcg_params Glauber Costa
2012-10-23 17:25   ` JoonSoo Kim
2012-10-24  8:42     ` Glauber Costa
2012-10-19 14:20 ` [PATCH v5 06/18] consider a memcg parameter in kmem_create_cache Glauber Costa
2012-10-23 17:50   ` JoonSoo Kim
2012-10-24  8:42     ` Glauber Costa
2012-10-25 13:42     ` Glauber Costa
2012-10-19 14:20 ` [PATCH v5 07/18] Allocate memory for memcg caches whenever a new memcg appears Glauber Costa
2012-10-19 14:20 ` [PATCH v5 08/18] memcg: infrastructure to match an allocation to the right cache Glauber Costa
2012-10-24 18:10   ` JoonSoo Kim
2012-10-25 11:05     ` Glauber Costa
2012-10-25 18:06       ` Tejun Heo
2012-10-25 18:08         ` Tejun Heo
2012-10-19 14:20 ` [PATCH v5 09/18] memcg: skip memcg kmem allocations in specified code regions Glauber Costa
2012-10-19 14:20 ` Glauber Costa [this message]
2012-10-19 19:44   ` [PATCH v5 10/18] sl[au]b: always get the cache from its page in kfree Christoph Lameter
2012-10-22 10:13     ` Glauber Costa
2012-10-19 14:20 ` [PATCH v5 11/18] sl[au]b: Allocate objects from memcg cache Glauber Costa
2012-10-19 19:46   ` Christoph Lameter
2012-10-29 15:14   ` JoonSoo Kim
2012-10-29 15:19     ` Glauber Costa
2012-10-19 14:20 ` [PATCH v5 12/18] memcg: destroy memcg caches Glauber Costa
2012-10-19 14:20 ` [PATCH v5 13/18] memcg/sl[au]b Track all the memcg children of a kmem_cache Glauber Costa
2012-10-29 15:26   ` JoonSoo Kim
2012-10-30 11:31     ` Glauber Costa
2012-10-19 14:20 ` [PATCH v5 14/18] memcg/sl[au]b: shrink dead caches Glauber Costa
2012-10-19 19:47   ` Christoph Lameter
2012-10-22  7:37     ` Glauber Costa
2012-10-19 14:20 ` [PATCH v5 15/18] Aggregate memcg cache values in slabinfo Glauber Costa
2012-10-19 19:50   ` Christoph Lameter
2012-10-22 15:11     ` Glauber Costa
2012-10-19 14:20 ` [PATCH v5 16/18] slab: propagate tunables values Glauber Costa
2012-10-19 19:51   ` Christoph Lameter
2012-10-22  7:48     ` Glauber Costa
2012-10-23 20:44       ` Christoph Lameter
2012-10-19 14:20 ` [PATCH v5 17/18] slub: slub-specific propagation changes Glauber Costa
2012-10-19 14:20 ` [PATCH v5 18/18] Add slab-specific documentation about the kmem controller Glauber Costa

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=1350656442-1523-11-git-send-email-glommer@parallels.com \
    --to=glommer@parallels.com \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=cl@linux.com \
    --cc=devel@openvz.org \
    --cc=hannes@cmpxchg.org \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=mhocko@suse.cz \
    --cc=penberg@cs.helsinki.fi \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=suleiman@google.com \
    --cc=tj@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 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).