Re: [PATCH] [v4] warn on performance-impacting configs aka. TAINT_PERFORMANCE

[Ingo Molnar]

* Dave Hansen <dave@xxxxxxxx> wrote:

> 
> From: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> 
> 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@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
> Cc: kirill@xxxxxxxxxxxxx
> Cc: lauraa@xxxxxxxxxxxxxx
> Cc: davej@xxxxxxxxxx

Looks good to me in principle. (haven't tested it)

Reviewed-by: Ingo Molnar <mingo@xxxxxxxxxx>

Thanks,

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