linux-kernel - Re: [PATCH] [v2] TAINT

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 20 Aug 2014 10:29:49 -0400
From:	Josh Boyer <jwboyer@...oraproject.org>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Dave Hansen <dave@...1.net>,
	"Linux-Kernel@...r. Kernel. Org" <linux-kernel@...r.kernel.org>,
	dave.hansen@...ux.intel.com, Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	Andi Kleen <ak@...ux.intel.com>, tim.c.chen@...ux.intel.com,
	Andrew Morton <akpm@...ux-foundation.org>,
	Christoph Lameter <cl@...ux.com>, penberg@...nel.org,
	Linux-MM <linux-mm@...ck.org>, kirill@...temov.name,
	lauraa@...eaurora.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] [v2] TAINT_PERFORMANCE

On Wed, Aug 20, 2014 at 4:11 AM, Ingo Molnar <mingo@...nel.org> wrote:
>
> * Dave Hansen <dave@...1.net> wrote:
>
>>
>> From: Dave Hansen <dave.hansen@...ux.intel.com>
>>
>> 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.

It will also immediately cause any software like ABRT to file
pointless bugs on kernels built with those options.  Please don't do
this.

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

I agree.

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