Re: [PATCH] [RFC] TAINT_PERFORMANCE
From: Laura Abbott
Date: Tue Aug 19 2014 - 19:47:15 EST
On 8/19/2014 2:26 PM, Dave Hansen wrote:
> From: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
>
> 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.
>
> 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.
>
> 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.
>
> The dump_stack() is really just to be loud.
>
> For anybody that *really* cares, I put the whole thing under
> #ifdef CONFIG_DEBUG_KERNEL.
>
> The messages look like this:
>
> [ 2.534574] CONFIG_LOCKDEP enabled
> [ 2.536392] Do not use this kernel for performance measurement.
> [ 2.547189] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.16.0-10473-gc8d6637-dirty #800
> [ 2.558075] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> [ 2.564483] 0000000080000000 ffff88009c70be78 ffffffff817ce318 0000000000000000
> [ 2.582505] ffffffff81dca5b6 ffff88009c70be88 ffffffff81dca5e2 ffff88009c70bef8
> [ 2.588589] ffffffff81000377 0000000000000000 0007000700000142 ffffffff81b78968
> [ 2.592638] Call Trace:
> [ 2.593762] [<ffffffff817ce318>] dump_stack+0x4e/0x68
> [ 2.597742] [<ffffffff81dca5b6>] ? oops_setup+0x2e/0x2e
> [ 2.601247] [<ffffffff81dca5e2>] performance_taint+0x2c/0x3c
> [ 2.603498] [<ffffffff81000377>] do_one_initcall+0xe7/0x290
> [ 2.606556] [<ffffffff81db3215>] kernel_init_freeable+0x106/0x19a
> [ 2.609718] [<ffffffff81db29e8>] ? do_early_param+0x86/0x86
> [ 2.613772] [<ffffffff817bcfc0>] ? rest_init+0x150/0x150
> [ 2.617333] [<ffffffff817bcfce>] kernel_init+0xe/0xf0
> [ 2.620840] [<ffffffff817dbc7c>] ret_from_fork+0x7c/0xb0
> [ 2.624718] [<ffffffff817bcfc0>] ? rest_init+0x150/0x150
>
> Signed-off-by: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: ak@xxxxxxxxxxxxxxx
> Cc: tim.c.chen@xxxxxxxxxxxxxxx
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: Christoph Lameter <cl@xxxxxxxxx>
> Cc: Pekka Enberg <penberg@xxxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Cc: linux-mm@xxxxxxxxx
> ---
>
> b/include/linux/kernel.h | 1 +
> b/kernel/panic.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 41 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-19 14:14:23.444983711 -0700
> @@ -225,6 +225,7 @@ static const struct tnt tnts[] = {
> { TAINT_OOT_MODULE, 'O', ' ' },
> { TAINT_UNSIGNED_MODULE, 'E', ' ' },
> { TAINT_SOFTLOCKUP, 'L', ' ' },
> + { TAINT_PERFORMANCE, 'Q', ' ' },
> };
>
> /**
> @@ -501,3 +502,42 @@ static int __init oops_setup(char *s)
> return 0;
> }
> early_param("oops", oops_setup);
> +
> +#ifdef CONFIG_DEBUG_KERNEL
> +#define TAINT_PERF_IF(x) do { \
> + if (IS_ENABLED(CONFIG_##x)) { \
> + do_taint = 1; \
> + pr_warn("CONFIG_%s enabled\n", __stringify(x));\
> + } \
> + } while (0)
> +
> +static int __init performance_taint(void)
> +{
> + int do_taint = 0;
> +
> + /*
> + * This should list any kernel options that can substantially
> + * affect performance. This is intended to give a big, fat
> + * warning during bootup so that folks have a fighting chance
> + * of noticing these things.
> + */
> + TAINT_PERF_IF(LOCKDEP);
> + TAINT_PERF_IF(LOCK_STAT);
> + TAINT_PERF_IF(DEBUG_VM);
> + TAINT_PERF_IF(DEBUG_VM_VMACACHE);
> + TAINT_PERF_IF(DEBUG_VM_RB);
> + TAINT_PERF_IF(DEBUG_SLAB);
> + TAINT_PERF_IF(DEBUG_OBJECTS_FREE);
> + TAINT_PERF_IF(DEBUG_KMEMLEAK);
> + TAINT_PERF_IF(SCHEDSTATS);
> +
I nominate CONFIG_DEBUG_PAGEALLOC, CONFIG_SLUB_DEBUG,
CONFIG_SLUB_DEBUG_ON as well since I've wasted days debugging
supposed performance issues where those were on.
Thanks,
Laura
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/