linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Shuah Khan <shuah.khan@hp.com>
To: cl@linux.com, penberg@kernel.org, glommer@parallels.com,
	js1304@gmail.com, David Rientjes <rientjes@google.com>
Cc: linux-mm@kvack.org, LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	shuahkhan@gmail.com
Subject: [PATCH v2] mm: Restructure kmem_cache_create() to move debug cache integrity checks into a new function
Date: Mon, 06 Aug 2012 15:13:51 -0600	[thread overview]
Message-ID: <1344287631.2486.57.camel@lorien2> (raw)
In-Reply-To: <1344272614.2486.40.camel@lorien2>

kmem_cache_create() does cache integrity checks when CONFIG_DEBUG_VM
is defined. These checks interspersed with the regular code path has
lead to compile time warnings when compiled without CONFIG_DEBUG_VM
defined. Restructuring the code to move the integrity checks in to a new
function would eliminate the current compile warning problem and also
will allow for future changes to the debug only code to evolve without
introducing new warnings in the regular path. This restructuring work
is based on the discussion in the following thread:

https://lkml.org/lkml/2012/7/13/424

Signed-off-by: Shuah Khan <shuah.khan@hp.com>
---
 mm/slab_common.c |   79 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 43 insertions(+), 36 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 12637ce..67409f7 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -23,6 +23,46 @@ enum slab_state slab_state;
 LIST_HEAD(slab_caches);
 DEFINE_MUTEX(slab_mutex);
 
+#ifdef CONFIG_DEBUG_VM
+static int kmem_cache_sanity_check(const char *name, size_t size)
+{
+	struct kmem_cache *s = NULL;
+
+	list_for_each_entry(s, &slab_caches, list) {
+		char tmp;
+		int res;
+
+		/*
+		 * This happens when the module gets unloaded and doesn't
+		 * destroy its slab cache and no-one else reuses the vmalloc
+		 * area of the module.  Print a warning.
+		 */
+		res = probe_kernel_address(s->name, tmp);
+		if (res) {
+			pr_err("Slab cache with size %d has lost its name\n",
+			       s->object_size);
+			continue;
+		}
+
+		if (!strcmp(s->name, name)) {
+			pr_err("%s (%s): Cache name already exists.\n",
+			       __func__, name);
+			dump_stack();
+			s = NULL;
+			return -EINVAL;
+		}
+	}
+
+	WARN_ON(strchr(name, ' '));	/* It confuses parsers */
+	return 0;
+}
+#else
+static inline int kmem_cache_sanity_check(const char *name, size_t size)
+{
+	return 0;
+}
+#endif
+
 /*
  * kmem_cache_create - Create a cache.
  * @name: A string which is used in /proc/slabinfo to identify this cache.
@@ -53,48 +93,17 @@ struct kmem_cache *kmem_cache_create(const char *name, size_t size, size_t align
 {
 	struct kmem_cache *s = NULL;
 
-#ifdef CONFIG_DEBUG_VM
 	if (!name || in_interrupt() || size < sizeof(void *) ||
 		size > KMALLOC_MAX_SIZE) {
-		printk(KERN_ERR "kmem_cache_create(%s) integrity check"
-			" failed\n", name);
+		pr_err("kmem_cache_create(%s) integrity check failed\n", name);
 		goto out;
 	}
-#endif
 
 	get_online_cpus();
 	mutex_lock(&slab_mutex);
 
-#ifdef CONFIG_DEBUG_VM
-	list_for_each_entry(s, &slab_caches, list) {
-		char tmp;
-		int res;
-
-		/*
-		 * This happens when the module gets unloaded and doesn't
-		 * destroy its slab cache and no-one else reuses the vmalloc
-		 * area of the module.  Print a warning.
-		 */
-		res = probe_kernel_address(s->name, tmp);
-		if (res) {
-			printk(KERN_ERR
-			       "Slab cache with size %d has lost its name\n",
-			       s->object_size);
-			continue;
-		}
-
-		if (!strcmp(s->name, name)) {
-			printk(KERN_ERR "kmem_cache_create(%s): Cache name"
-				" already exists.\n",
-				name);
-			dump_stack();
-			s = NULL;
-			goto oops;
-		}
-	}
-
-	WARN_ON(strchr(name, ' '));	/* It confuses parsers */
-#endif
+	if (kmem_cache_sanity_check(name, size))
+		goto oops;
 
 	s = __kmem_cache_create(name, size, align, flags, ctor);
 
@@ -102,9 +111,7 @@ oops:
 	mutex_unlock(&slab_mutex);
 	put_online_cpus();
 
-#ifdef CONFIG_DEBUG_VM
 out:
-#endif
 	if (!s && (flags & SLAB_PANIC))
 		panic("kmem_cache_create: Failed to create slab '%s'\n", name);
 
-- 
1.7.9.5




  reply	other threads:[~2012-08-06 21:13 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-13 23:12 [PATCH TRIVIAL] mm: Fix build warning in kmem_cache_create() Shuah Khan
2012-07-14  9:18 ` David Rientjes
2012-07-14 12:01   ` Pekka Enberg
2012-07-16  3:04     ` Shuah Khan
2012-07-16  9:58       ` David Rientjes
2012-07-16 14:17         ` Christoph Lameter
2012-07-16 15:56           ` Shuah Khan
2012-07-16 19:58           ` David Rientjes
2012-07-16 20:14             ` Christoph Lameter
2012-07-16 23:48               ` David Rientjes
2012-07-17 14:36                 ` Christoph Lameter
2012-07-17 14:46                   ` Pekka Enberg
2012-07-17 15:11                     ` Christoph Lameter
2012-07-23  7:04                       ` Glauber Costa
2012-07-25 15:28                         ` Christoph Lameter
2012-07-17 16:52                     ` Shuah Khan
2012-07-30 10:18 ` Pekka Enberg
2012-07-30 19:56   ` David Rientjes
2012-07-30 20:41     ` Pekka Enberg
2012-07-31  2:07       ` David Rientjes
2012-07-31  6:05         ` Pekka Enberg
2012-08-06  3:41   ` Shuah Khan
2012-08-06 15:14     ` [PATCH RESEND] mm: Restructure kmem_cache_create() to move debug cache integrity checks into a new function Shuah Khan
2012-08-06 16:49       ` JoonSoo Kim
2012-08-06 17:03         ` Shuah Khan
2012-08-06 21:13           ` Shuah Khan [this message]
2012-08-09 14:06             ` [PATCH v2] " Shuah Khan
2012-08-09 14:13             ` Christoph Lameter (Open Source)
2012-08-09 17:01               ` Shuah Khan
2012-08-09 19:08                 ` Christoph Lameter (Open Source)
2012-08-09 19:33                   ` Shuah Khan
2012-08-12 16:40                     ` [PATCH v3] " Shuah Khan
2012-08-12 17:36                       ` Christoph
2012-08-15 23:53                       ` Andrew Morton
2012-08-16  6:40                         ` Pekka Enberg
2012-08-08 14:14           ` [PATCH RESEND] " Christoph Lameter (Open Source)
2012-08-08 15:13             ` Shuah Khan

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=1344287631.2486.57.camel@lorien2 \
    --to=shuah.khan@hp.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=glommer@parallels.com \
    --cc=js1304@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=shuahkhan@gmail.com \
    --cc=torvalds@linux-foundation.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).