Message-Id: <20140822205625.657E9890@viggo.jf.intel.com>
Date:	Fri, 22 Aug 2014 13:56:25 -0700
From:	Dave Hansen <dave@...1.net>
To:	linux-kernel@...r.kernel.org
Cc:	Dave Hansen <dave@...1.net>, dave.hansen@...ux.intel.com,
	peterz@...radead.org, mingo@...hat.com, ak@...ux.intel.com,
	tim.c.chen@...ux.intel.com, akpm@...ux-foundation.org,
	cl@...ux.com, penberg@...nel.org, linux-mm@...ck.org,
	kirill@...temov.name, lauraa@...eaurora.org, davej@...hat.com
Subject: [PATCH] [v4] warn on performance-impacting configs aka. TAINT_PERFORMANCE


From: Dave Hansen <dave.hansen@...ux.intel.com>

Changes from v3:
 * remove vestiges of TAINT_PERFORMANCE
 * change filename to check-configs
 * fix typos in description
 * print out CONFIG_FOO=y
 * tone down warning message
 * add KMEMCHECK and GCOV
 * add PROVE_LOCKING, but keep LOCKDEP since _anything_ selecting
   it will cause scalaing issues at least.  But, move LOCKDEP
   below LOCK_STAT and PROVE_LOCKING.
 * no more perfo-mance (missing an 'r')
 * temporary variable in lieu of multiple ARRAY_SIZE()
 * break early out of snprintf() loop

Changes from v2:
 * remove tainting and stack track
 * add debugfs file
 * added a little text to guide folks who want to add more
   options

Changes from v1:
 * remove schedstats
 * add DEBUG_PAGEALLOC and SLUB_DEBUG_ON

--

I have more than once myself been the victim of an accidentally-
enabled kernel config option being mistaken for a true
performance problem.

I'm sure I've also taken profiles or performance measurements
and assumed they were real-world when really I was measuring the
performance with an option that nobody turns on in production.

A warning like this late in boot will help remind folks when
these kinds of things are enabled.  We can also teach tooling to
look for and capture /sys/kernel/debug/config_debug .

As for the patch...

I originally wanted this for CONFIG_DEBUG_VM, but I think it also
applies to things like lockdep and slab debugging.  See the patch
for the list of offending config options.  I'm open to adding
more, but this seemed like a good list to start.

The compiler is smart enough to really trim down the code when
the array is empty.  An objdump -d looks like this:

	lib/check-configs.o:     file format elf64-x86-64

	Disassembly of section .init.text:

	000000000000000 <check_configs>:
	  0:	55                   	push   %rbp
	  1:	31 c0                	xor    %eax,%eax
	  3:	48 89 e5             	mov    %rsp,%rbp
	  6:	5d                   	pop    %rbp
	  7:	c3                   	retq

This could be done with Kconfig and an #ifdef to save us 8 bytes
of text and the entry in the late_initcall() section.  Doing it
this way lets us keep the list of these things in one spot, and
also gives us a convenient way to dump out the name of the
offending option.

For anybody that *really* cares, I put the whole thing under
CONFIG_DEBUG_KERNEL in the Makefile.

The messages look like this:

[    3.865297] INFO: Be careful when using this kernel for performance measurement.
[    3.868776] INFO: Potentially performance-altering options enabled:
[    3.871558] 	CONFIG_LOCKDEP=y
[    3.873326] 	CONFIG_SLUB_DEBUG_ON=y

Signed-off-by: Dave Hansen <dave.hansen@...ux.intel.com>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Ingo Molnar <mingo@...hat.com>
Cc: ak@...ux.intel.com
Cc: tim.c.chen@...ux.intel.com
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Christoph Lameter <cl@...ux.com>
Cc: Pekka Enberg <penberg@...nel.org>
Cc: linux-kernel@...r.kernel.org
Cc: linux-mm@...ck.org
Cc: kirill@...temov.name
Cc: lauraa@...eaurora.org
Cc: davej@...hat.com
---

 b/kernel/Makefile        |    1 
 b/kernel/check-configs.c |  128 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 129 insertions(+)

diff -puN kernel/Makefile~taint-performance kernel/Makefile
--- a/kernel/Makefile~taint-performance	2014-08-22 09:29:43.009121391 -0700
+++ b/kernel/Makefile	2014-08-22 09:29:43.014121620 -0700
@@ -96,6 +96,7 @@ obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
 obj-$(CONFIG_JUMP_LABEL) += jump_label.o
 obj-$(CONFIG_CONTEXT_TRACKING) += context_tracking.o
 obj-$(CONFIG_TORTURE_TEST) += torture.o
+obj-$(CONFIG_DEBUG_KERNEL) += check-configs.o
 
 $(obj)/configs.o: $(obj)/config_data.h
 
diff -puN /dev/null kernel/check-configs.c
--- /dev/null	2014-04-10 11:28:14.066815724 -0700
+++ b/kernel/check-configs.c	2014-08-22 11:20:14.679463265 -0700
@@ -0,0 +1,128 @@
+#include <linux/bug.h>
+#include <linux/debugfs.h>
+#include <linux/gfp.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+
+/*
+ * This should list any kernel options that can substantially
+ * affect performance.  This is intended to give a loud
+ * warning during bootup so that folks have a fighting chance
+ * of noticing these things.
+ *
+ * This is fairly subjective, but a good rule of thumb for these
+ * is: if it is enabled widely in production, then it does not
+ * belong here.  If a major enterprise kernel enables a feature
+ * for a non-debug kernel, it _really_ does not belong.
+ */
+static const char * const performance_killing_configs[] = {
+#ifdef CONFIG_PROVE_LOCKING
+	"PROVE_LOCKING",
+#endif
+#ifdef CONFIG_LOCK_STAT
+	"LOCK_STAT",
+#endif
+#ifdef CONFIG_LOCKDEP
+	"LOCKDEP",
+#endif
+#ifdef CONFIG_DEBUG_VM
+	"DEBUG_VM",
+#endif
+#ifdef CONFIG_DEBUG_VM_VMACACHE
+	"DEBUG_VM_VMACACHE",
+#endif
+#ifdef CONFIG_DEBUG_VM_RB
+	"DEBUG_VM_RB",
+#endif
+#ifdef CONFIG_DEBUG_SLAB
+	"DEBUG_SLAB",
+#endif
+#ifdef CONFIG_SLUB_DEBUG_ON
+	"SLUB_DEBUG_ON",
+#endif
+#ifdef CONFIG_DEBUG_KMEMLEAK
+	"DEBUG_KMEMLEAK",
+#endif
+#ifdef CONFIG_DEBUG_PAGEALLOC
+	"DEBUG_PAGEALLOC",
+#endif
+#ifdef CONFIG_KMEMCHECK
+	"DEBUG_KMEMCHECK",
+#endif
+#ifdef CONFIG_DEBUG_OBJECTS_ENABLE_DEFAULT
+	"DEBUG_OBJECTS_ENABLE_DEFAULT",
+#endif
+#ifdef CONFIG_GCOV_KERNEL
+	"GCOV_KERNEL",
+#endif
+};
+
+static const char config_prefix[] = "CONFIG_";
+static const char config_postfix[] = "=y\n";
+/*
+ * Dump out the list of the offending config options to a file
+ * in debugfs so that tooling can look for and capture it.
+ */
+static ssize_t performance_check_read(struct file *file, char __user *user_buf,
+			size_t count, loff_t *ppos)
+{
+	int i;
+	int ret;
+	char *buf;
+	size_t buf_written = 0;
+	size_t buf_left;
+	size_t buf_len;
+	int nr_configs = ARRAY_SIZE(performance_killing_configs);
+
+	if (!nr_configs)
+		return 0;
+
+	buf_len = 1;
+	for (i = 0; i < nr_configs; i++)
+		buf_len += strlen(config_prefix) +
+			   strlen(performance_killing_configs[i]);
+	/* Add space for the end of the line for each entry */
+	buf_len += nr_configs * strlen(config_postfix);
+
+	buf = kmalloc(buf_len, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	buf_left = buf_len;
+	for (i = 0; i < nr_configs; i++) {
+		buf_written += snprintf(buf + buf_written, buf_left,
+					"%s%s%s", config_prefix,
+					performance_killing_configs[i],
+					config_postfix);
+		buf_left = buf_len - buf_written;
+		if (buf_left <= 0)
+			break;
+	}
+	ret = simple_read_from_buffer(user_buf, buf_written, ppos, buf, buf_len);
+	kfree(buf);
+	return ret;
+}
+
+static const struct file_operations fops_performance_check = {
+	.read = performance_check_read,
+	.llseek = default_llseek,
+};
+
+static int __init check_configs(void)
+{
+	int i;
+
+	if (!ARRAY_SIZE(performance_killing_configs))
+		return 0;
+
+	pr_info("INFO: Be careful when using this kernel for performance measurement.\n");
+	pr_info("INFO: Potentially performance-altering options present:\n");
+	for (i = 0; i < ARRAY_SIZE(performance_killing_configs); i++) {
+		pr_warn("\t%s%s%s", config_prefix,
+				performance_killing_configs[i], config_postfix);
+	}
+	debugfs_create_file("config_debug", S_IRUSR | S_IWUSR,
+				NULL, NULL, &fops_performance_check);
+	return 0;
+}
+late_initcall(check_configs);
_
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists