linux-kernel - Re: [PATCH] firmware: dmi_scan: Drop dmi

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:   Mon, 25 Sep 2017 11:24:17 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Jean Delvare <jdelvare@...e.de>
Cc:     Ingo Molnar <mingo@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] firmware: dmi_scan: Drop dmi_initialized

On Mon, Sep 25, 2017 at 11:00:11AM +0200, Jean Delvare wrote:
> I can find 169 occurrences of "(must|has to|should) be called
> (before|after)" in the kernel source tree, plus 19 occurrences of "call
> this function (before|after)" so apparently I'm not the only fool who
> thinks documenting such ordering requirements is worthwhile.

Documenting things is good, assuming people read it is another. In fact
its provably incorrect, people simply don't read.

I've been slowly converting all those occurrences of "should be called
with 'foo' lock held" into:

	lockdep_assert_held(&foo);

simply because people do _not_ read. And even if they did read, its
still easy to forget and make a mistake.

Heck, I sometimes get it wrong on code I wrote.

Documentation good, runtime checks also good.

Because when you do get it wrong p(someone getting it wrong at some
point) = 1, a WARN_ON_ONCE() triggering that has a comment that says:

 /*
  * if you trigger this; you violated rule #123
  */
 WARN_ON_ONCE(!invariant_cond_123);

saves ever so much more time than debugging weird and wonderful splats
much later in the code that rely on our rule #123.