Re: [PATCH] [v2] TAINT_PERFORMANCE

[Ingo Molnar]

* Dave Hansen <dave@xxxxxxxx> wrote:

> 
> From: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> 
> 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.

Most of these options already announce themselves in the 
syslog.

> 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.

> [    2.534574] CONFIG_LOCKDEP enabled
> [    2.536392] Do not use this kernel for performance measurement.

This is workload dependent: for many kernel workloads this is 
indeed true. For many user-space workloads it will add very 
little overhead.

> [    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

Generating a stack dump that tells us nothing isn't really 
useful.

>  	{ TAINT_SOFTLOCKUP,		'L', ' ' },
> +	{ TAINT_PERFORMANCE,		'Q', ' ' },

Also this looks like a slight abuse of the taint flag: we taint 
the kernel if there's a problem with it. But even for many 
types of performance measurements, a debug kernel is just fine. 
For other types of performance measurements, even a non-debug 
kernel option can have big impact.

A better option might be to declare known performance killers 
in /proc/config_debug or so, and maybe print them once at the 
end of the bootup, with a 'WARNING:' or 'INFO:' prefix. That 
way tooling (benchmarks, profilers, etc.) can print them, but 
it's also present in the syslog, just in case.

/proc/config_debug is different from /proc/config.gz IKCONFIG, 
because it would always be present when performance impacting 
options are enabled. So tools would only have to check the 
existence of this file, for the simplest test.

In any case I don't think it's a good idea to abuse existing 
facilities just to gain attention: you'll get the extra 
attention, but the abuse dillutes the utility of those only 
tangentially related facilities.

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/