| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1408725157.4347.14.camel@schen9-DESK>
Date: Fri, 22 Aug 2014 09:32:37 -0700
From: Tim Chen <tim.c.chen@...ux.intel.com>
To: Dave Hansen <dave@...1.net>
Cc: linux-kernel@...r.kernel.org, dave.hansen@...ux.intel.com,
peterz@...radead.org, mingo@...hat.com, ak@...ux.intel.com,
akpm@...ux-foundation.org, cl@...ux.com, penberg@...nel.org,
linux-mm@...ck.org, kirill@...temov.name, lauraa@...eaurora.org
Subject: Re: [PATCH] [v3] warn on performance-impacting configs aka.
TAINT_PERFORMANCE
On Thu, 2014-08-21 at 13:24 -0700, Dave Hansen wrote:
> From: Dave Hansen <dave.hansen@...ux.intel.com>
>
> 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 measuing 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/perf-configs.o: file format elf64-x86-64
>
> Disassembly of section .init.text:
>
> 0000000000000000 <performance_taint>:
> 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] WARNING: Do not use this kernel for performance measurement.
> [ 3.868776] WARNING: Potentially performance-altering options:
> [ 3.871558] CONFIG_LOCKDEP enabled
> [ 3.873326] CONFIG_SLUB_DEBUG_ON enabled
>
> 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
> ---
>
> b/include/linux/kernel.h | 1
> b/kernel/panic.c | 1
> b/lib/Makefile | 1
> b/lib/perf-configs.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 117 insertions(+)
>
> diff -puN include/linux/kernel.h~taint-performance include/linux/kernel.h
> --- a/include/linux/kernel.h~taint-performance 2014-08-19 11:38:07.424005355 -0700
> +++ b/include/linux/kernel.h 2014-08-19 11:38:20.960615904 -0700
> @@ -471,6 +471,7 @@ extern enum system_states {
> #define TAINT_OOT_MODULE 12
> #define TAINT_UNSIGNED_MODULE 13
> #define TAINT_SOFTLOCKUP 14
> +#define TAINT_PERFORMANCE 15
>
> extern const char hex_asc[];
> #define hex_asc_lo(x) hex_asc[((x) & 0x0f)]
> diff -puN kernel/panic.c~taint-performance kernel/panic.c
> --- a/kernel/panic.c~taint-performance 2014-08-19 11:38:28.928975233 -0700
> +++ b/kernel/panic.c 2014-08-20 09:56:29.528471033 -0700
> @@ -225,6 +225,7 @@ static const struct tnt tnts[] = {
> { TAINT_OOT_MODULE, 'O', ' ' },
> { TAINT_UNSIGNED_MODULE, 'E', ' ' },
> { TAINT_SOFTLOCKUP, 'L', ' ' },
> + { TAINT_PERFORMANCE, 'Q', ' ' },
> };
>
> /**
> diff -puN /dev/null lib/perf-configs.c
> --- /dev/null 2014-04-10 11:28:14.066815724 -0700
> +++ b/lib/perf-configs.c 2014-08-21 13:22:25.586598278 -0700
> @@ -0,0 +1,114 @@
> +#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 perfomance_killing_configs[] = {
> +#ifdef CONFIG_LOCKDEP
> + "LOCKDEP",
> +#endif
> +#ifdef CONFIG_LOCK_STAT
> + "LOCK_STAT",
> +#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_OBJECTS_FREE
> + "DEBUG_OBJECTS_FREE",
> +#endif
> +#ifdef CONFIG_DEBUG_KMEMLEAK
> + "DEBUG_KMEMLEAK",
> +#endif
> +#ifdef CONFIG_DEBUG_PAGEALLOC
> + "DEBUG_PAGEALLOC",
I think coverage profiling also impact performance.
So I sould also put CONFIG_GCOV_KERNEL in the list.
Tim
Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)
Powered by blists - more mailing lists