linux-kernel - Re: [PATCH] target: Use WARNON_NON_RT(!irqs

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]

Message-ID: <CA+55aFxVkuntSektVmwXktzvfEezdDTAM_zj4DJdp3qJxWf8aA@mail.gmail.com>
Date:   Fri, 23 Mar 2018 09:37:40 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Bart Van Assche <Bart.VanAssche@....com>
Cc:     "bigeasy@...utronix.de" <bigeasy@...utronix.de>,
        "acme@...nel.org" <acme@...nel.org>,
        "nab@...ux-iscsi.org" <nab@...ux-iscsi.org>,
        "daniel@...stot.me" <daniel@...stot.me>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "rostedt@...dmis.org" <rostedt@...dmis.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "williams@...hat.com" <williams@...hat.com>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "lclaudio@...hat.com" <lclaudio@...hat.com>,
        "target-devel@...r.kernel.org" <target-devel@...r.kernel.org>,
        "linux-rt-users@...r.kernel.org" <linux-rt-users@...r.kernel.org>
Subject: Re: [PATCH] target: Use WARNON_NON_RT(!irqs_disabled())

On Fri, Mar 23, 2018 at 9:25 AM, Bart Van Assche <Bart.VanAssche@....com> wrote:
>
> Have you considered to delete the WARN_ON_ONCE(!irqs_disabled()) statement?
> I think that check duplicates functionality that already exists in lockdep
> since lockdep is already able to detect spinlock use inconsistencies.

Please just delete both lines.

There is exactly two callers of that static function, and both of them do

        spin_lock_irq(&cmd->t_state_lock);

right above the call.

It's not like this is some function that is exported to random users,
and we should check that the calling convention is right.

So honestly, even lockdep annotations look like you don't need them.

This looks like "it may have been useful during coding to document
things, but it's not useful long-term".

Sure, the annotation is not wrong, but even if you go "verification is
good", you should ask yourself whether there are maybe better places
that would catch more relevant problems, than putting verification
into some static function with two trivially correct callers wrt this
verification?

              Linus