[tip:core/locking] lockdep, bug: Exclude TAINT_FIRMWARE_WORKAROUND from disabling lockdep

[tip:core/locking] lockdep, bug: Exclude TAINT_FIRMWARE_WORKAROUND from disabling lockdep

[tip-bot for Peter Zijlstra]

Commit-ID:  df754e6af2f237a6c020c0daff55a1a609338e31
Gitweb:     http://git.kernel.org/tip/df754e6af2f237a6c020c0daff55a1a609338e31
Author:     Peter Zijlstra <a.p.zijlstra@chello.nl>
AuthorDate: Mon, 14 Nov 2011 13:13:49 +0100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 6 Dec 2011 08:16:47 +0100

lockdep, bug: Exclude TAINT_FIRMWARE_WORKAROUND from disabling lockdep

It's unlikely that TAINT_FIRMWARE_WORKAROUND causes false
lockdep messages, so do not disable lockdep in that case.
We still want to keep lockdep disabled in the
TAINT_OOT_MODULE case:

  - bin-only modules can cause various instabilities in
    their and in unrelated kernel code

  - they are impossible to debug for kernel developers

  - they also typically do not have the copyright license
    permission to link to the GPL-ed lockdep code.

Suggested-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/n/tip-xopopjjens57r0i13qnyh2yo@git.kernel.org
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/panic.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/kernel/panic.c b/kernel/panic.c
index b2659360..1b83fd8 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -240,8 +240,16 @@ void add_taint(unsigned flag)
 	 * Also we want to keep up lockdep for staging development and
 	 * post-warning case.
 	 */
-	if (flag != TAINT_CRAP && flag != TAINT_WARN && __debug_locks_off())
-		printk(KERN_WARNING "Disabling lock debugging due to kernel taint\n");
+	switch (flag) {
+	case TAINT_CRAP:
+	case TAINT_WARN:
+	case TAINT_FIRMWARE_WORKAROUND:
+		break;
+
+	default:
+		if (__debug_locks_off())
+			printk(KERN_WARNING "Disabling lock debugging due to kernel taint\n");
+	}
 
 	set_bit(flag, &tainted_mask);
 }

Re: [tip:core/locking] lockdep, bug: Exclude TAINT_FIRMWARE_WORKAROUND from disabling lockdep

[Nick Bowler]

On 2011-12-06 01:38 -0800, tip-bot for Peter Zijlstra wrote:
> Commit-ID:  df754e6af2f237a6c020c0daff55a1a609338e31
> Gitweb:     http://git.kernel.org/tip/df754e6af2f237a6c020c0daff55a1a609338e31
> Author:     Peter Zijlstra <a.p.zijlstra@chello.nl>
> AuthorDate: Mon, 14 Nov 2011 13:13:49 +0100
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Tue, 6 Dec 2011 08:16:47 +0100
> 
> lockdep, bug: Exclude TAINT_FIRMWARE_WORKAROUND from disabling lockdep
> 
> It's unlikely that TAINT_FIRMWARE_WORKAROUND causes false
> lockdep messages, so do not disable lockdep in that case.
> We still want to keep lockdep disabled in the
> TAINT_OOT_MODULE case:
> 
>   - bin-only modules can cause various instabilities in
>     their and in unrelated kernel code
> 
>   - they are impossible to debug for kernel developers
> 
>   - they also typically do not have the copyright license
>     permission to link to the GPL-ed lockdep code.

This is all bogus.  We have a proprietary module taint for the above.
Out of tree does not mean "bin-only", "non-GPL" or anything else like
that.  If the intent is to tell out of tree, GPL module developers to
fuck off and die, then just go all the way and delete support for out
of tree modules from Kbuild.  May as well go further and just prevent
the kernel from loading such modules outright, too.

Don't gag developers by arbitrarily disabling kernel debugging
functionality: functionality that they can use to debug their
*GPL* modules.

-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

Re: [tip:core/locking] lockdep, bug: Exclude TAINT_FIRMWARE_WORKAROUND from disabling lockdep

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]

On Tue, 2011-12-06 at 10:13 -0500, Nick Bowler wrote:
> On 2011-12-06 01:38 -0800, tip-bot for Peter Zijlstra wrote:
> > Commit-ID:  df754e6af2f237a6c020c0daff55a1a609338e31
> > Gitweb:     http://git.kernel.org/tip/df754e6af2f237a6c020c0daff55a1a609338e31
> > Author:     Peter Zijlstra <a.p.zijlstra@chello.nl>
> > AuthorDate: Mon, 14 Nov 2011 13:13:49 +0100
> > Committer:  Ingo Molnar <mingo@elte.hu>
> > CommitDate: Tue, 6 Dec 2011 08:16:47 +0100
> > 
> > lockdep, bug: Exclude TAINT_FIRMWARE_WORKAROUND from disabling lockdep
> > 
> > It's unlikely that TAINT_FIRMWARE_WORKAROUND causes false
> > lockdep messages, so do not disable lockdep in that case.
> > We still want to keep lockdep disabled in the
> > TAINT_OOT_MODULE case:
> > 
> >   - bin-only modules can cause various instabilities in
> >     their and in unrelated kernel code
> > 
> >   - they are impossible to debug for kernel developers
> > 
> >   - they also typically do not have the copyright license
> >     permission to link to the GPL-ed lockdep code.
> 
> This is all bogus.  We have a proprietary module taint for the above.
> Out of tree does not mean "bin-only", "non-GPL" or anything else like
> that.  If the intent is to tell out of tree, GPL module developers to
> fuck off and die, then just go all the way and delete support for out
> of tree modules from Kbuild.  May as well go further and just prevent
> the kernel from loading such modules outright, too.
> 
> Don't gag developers by arbitrarily disabling kernel debugging
> functionality: functionality that they can use to debug their
> *GPL* modules.

I totally agree; *I* wanted to keep lockdep enabled for (GPL-compatible)
OOT modules.

Ben.

-- 
Ben Hutchings
Absolutum obsoletum. (If it works, it's out of date.) - Stafford Beer

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [tip:core/locking] lockdep, bug: Exclude TAINT_FIRMWARE_WORKAROUND from disabling lockdep

[Alan Cox]

On Tue, 6 Dec 2011 10:13:45 -0500
Nick Bowler <nbowler@elliptictech.com> wrote:

> On 2011-12-06 01:38 -0800, tip-bot for Peter Zijlstra wrote:
> > Commit-ID:  df754e6af2f237a6c020c0daff55a1a609338e31
> > Gitweb:     http://git.kernel.org/tip/df754e6af2f237a6c020c0daff55a1a609338e31
> > Author:     Peter Zijlstra <a.p.zijlstra@chello.nl>
> > AuthorDate: Mon, 14 Nov 2011 13:13:49 +0100
> > Committer:  Ingo Molnar <mingo@elte.hu>
> > CommitDate: Tue, 6 Dec 2011 08:16:47 +0100
> > 
> > lockdep, bug: Exclude TAINT_FIRMWARE_WORKAROUND from disabling lockdep
> > 
> > It's unlikely that TAINT_FIRMWARE_WORKAROUND causes false
> > lockdep messages, so do not disable lockdep in that case.
> > We still want to keep lockdep disabled in the
> > TAINT_OOT_MODULE case:
> > 
> >   - bin-only modules can cause various instabilities in
> >     their and in unrelated kernel code
> > 
> >   - they are impossible to debug for kernel developers
> > 
> >   - they also typically do not have the copyright license
> >     permission to link to the GPL-ed lockdep code.
> 
> This is all bogus.  We have a proprietary module taint for the above.
> Out of tree does not mean "bin-only", "non-GPL" or anything else like
> that.  

It also stops people developing drivers of their own who are not
building them in the kernel tree currently (eg when I'm building tests
of the GMA500 graphics driver). This is a ludicrous patch and should be
reverted

Alan

Re: [tip:core/locking] lockdep, bug: Exclude TAINT_FIRMWARE_WORKAROUND from disabling lockdep

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 1932 bytes --]

On Tue, 2011-12-06 at 15:42 +0000, Alan Cox wrote:
> On Tue, 6 Dec 2011 10:13:45 -0500
> Nick Bowler <nbowler@elliptictech.com> wrote:
> 
> > On 2011-12-06 01:38 -0800, tip-bot for Peter Zijlstra wrote:
> > > Commit-ID:  df754e6af2f237a6c020c0daff55a1a609338e31
> > > Gitweb:     http://git.kernel.org/tip/df754e6af2f237a6c020c0daff55a1a609338e31
> > > Author:     Peter Zijlstra <a.p.zijlstra@chello.nl>
> > > AuthorDate: Mon, 14 Nov 2011 13:13:49 +0100
> > > Committer:  Ingo Molnar <mingo@elte.hu>
> > > CommitDate: Tue, 6 Dec 2011 08:16:47 +0100
> > > 
> > > lockdep, bug: Exclude TAINT_FIRMWARE_WORKAROUND from disabling lockdep
> > > 
> > > It's unlikely that TAINT_FIRMWARE_WORKAROUND causes false
> > > lockdep messages, so do not disable lockdep in that case.
> > > We still want to keep lockdep disabled in the
> > > TAINT_OOT_MODULE case:
> > > 
> > >   - bin-only modules can cause various instabilities in
> > >     their and in unrelated kernel code
> > > 
> > >   - they are impossible to debug for kernel developers
> > > 
> > >   - they also typically do not have the copyright license
> > >     permission to link to the GPL-ed lockdep code.
> > 
> > This is all bogus.  We have a proprietary module taint for the above.
> > Out of tree does not mean "bin-only", "non-GPL" or anything else like
> > that.  
> 
> It also stops people developing drivers of their own who are not
> building them in the kernel tree currently (eg when I'm building tests
> of the GMA500 graphics driver). This is a ludicrous patch and should be
> reverted

My change to set TAINT_OOT_MODULE had the (unintentional) effect of
disabling lockdep for OOT modules.  I subsequently proposed to change
that (while adding taint flags to lockdep output).  This patch doesn't
make any change either way.

Ben.

-- 
Ben Hutchings
Absolutum obsoletum. (If it works, it's out of date.) - Stafford Beer

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [tip:core/locking] lockdep, bug: Exclude TAINT_FIRMWARE_WORKAROUND from disabling lockdep

[Ingo Molnar]

* Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> On Tue, 6 Dec 2011 10:13:45 -0500
> Nick Bowler <nbowler@elliptictech.com> wrote:
> 
> > On 2011-12-06 01:38 -0800, tip-bot for Peter Zijlstra wrote:
> > > Commit-ID:  df754e6af2f237a6c020c0daff55a1a609338e31
> > > Gitweb:     http://git.kernel.org/tip/df754e6af2f237a6c020c0daff55a1a609338e31
> > > Author:     Peter Zijlstra <a.p.zijlstra@chello.nl>
> > > AuthorDate: Mon, 14 Nov 2011 13:13:49 +0100
> > > Committer:  Ingo Molnar <mingo@elte.hu>
> > > CommitDate: Tue, 6 Dec 2011 08:16:47 +0100
> > > 
> > > lockdep, bug: Exclude TAINT_FIRMWARE_WORKAROUND from disabling lockdep
> > > 
> > > It's unlikely that TAINT_FIRMWARE_WORKAROUND causes false
> > > lockdep messages, so do not disable lockdep in that case.
> > > We still want to keep lockdep disabled in the
> > > TAINT_OOT_MODULE case:
> > > 
> > >   - bin-only modules can cause various instabilities in
> > >     their and in unrelated kernel code
> > > 
> > >   - they are impossible to debug for kernel developers
> > > 
> > >   - they also typically do not have the copyright license
> > >     permission to link to the GPL-ed lockdep code.
> > 
> > This is all bogus.  We have a proprietary module taint for the above.
> > Out of tree does not mean "bin-only", "non-GPL" or anything else like
> > that.  
> 
> It also stops people developing drivers of their own who are 
> not building them in the kernel tree currently (eg when I'm 
> building tests of the GMA500 graphics driver). This is a 
> ludicrous patch and should be reverted

That patch is already upstream, it was done via the module tree:

 2449b8ba0745: module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

lockdep disabling itself defensively is an effect of that 
change. The commit you replied to here does not change that 
aspect in any way.

My primary worry is to not have lockdep active when there's 
binary modules in a system - can TAINT_OOT_MODULE be set but 
TAINT_PROPRIETARY_MODULE not set for non-GPL modules?

If not, and if TAINT_OOT_MODULE set and TAINT_PROPRIETARY_MODULE 
cleared guarantees the GPL-ness of the module then i have no 
problem with keeping lockdep active in that case.

Thanks,

	Ingo

Re: [tip:core/locking] lockdep, bug: Exclude TAINT_FIRMWARE_WORKAROUND from disabling lockdep

[Alan Cox]

> My primary worry is to not have lockdep active when there's 
> binary modules in a system - can TAINT_OOT_MODULE be set but 
> TAINT_PROPRIETARY_MODULE not set for non-GPL modules?

Yes. I imagine anyone wanting to use lockdep with binary modules
would just lie anyway.

> If not, and if TAINT_OOT_MODULE set and TAINT_PROPRIETARY_MODULE 
> cleared guarantees the GPL-ness of the module then i have no 
> problem with keeping lockdep active in that case.

Insofar as nobody is making their code line about licenses.

Alan

Re: [tip:core/locking] lockdep, bug: Exclude TAINT_FIRMWARE_WORKAROUND from disabling lockdep

[Ingo Molnar]

* Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > My primary worry is to not have lockdep active when there's 
> > binary modules in a system - can TAINT_OOT_MODULE be set but 
> > TAINT_PROPRIETARY_MODULE not set for non-GPL modules?
> 
> Yes. I imagine anyone wanting to use lockdep with binary modules
> would just lie anyway.
> 
> > If not, and if TAINT_OOT_MODULE set and TAINT_PROPRIETARY_MODULE 
> > cleared guarantees the GPL-ness of the module then i have no 
> > problem with keeping lockdep active in that case.
> 
> Insofar as nobody is making their code line about licenses.

Fair enough - so i agree that we can allow OOT_MODULE's with 
lockdep and thus revert the lockdep-disabling effect of:

 2449b8ba0745: module,bug: Add TAINT_OOT_MODULE flag for modules not built in-tree

Thanks,

	Ingo