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+55aFwKN_JijmCt=0S4jF8KzGPmoKNbuYF59va7U4ziJLLryA@mail.gmail.com>
Date:   Wed, 21 Mar 2018 11:43:58 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>
Cc:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        "Nicholas A. Bellinger" <nab@...ux-iscsi.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-rt-users <linux-rt-users@...r.kernel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Linux SCSI List <linux-scsi@...r.kernel.org>,
        Daniel Bristot de Oliveira <daniel@...stot.me>,
        Luis Claudio R. Gonçalves <lclaudio@...hat.com>,
        Clark Williams <williams@...hat.com>,
        target-devel <target-devel@...r.kernel.org>
Subject: Re: [PATCH] target: Use WARNON_NON_RT(!irqs_disabled())

[ Adding PeterZ to participants due to query about lockdep_assert() ]

On Wed, Mar 21, 2018 at 8:38 AM, Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
>
>         assert_spin_locked(&cmd->t_state_lock);
> -       WARN_ON_ONCE(!irqs_disabled());
> +       WARN_ON_ONCE_NONRT(!irqs_disabled());

Ugh.

Can't we just replace both of those with a lockdep annotation?

Does "lockdep_assert_held()" already verify the irq contextr, or do we
need lockdep_assert_irqs_disabled() too?

Honestly, the old-fashioned way of doing verification of state by hand
is understandable, but it's legacy and kind of pointless when we have
much better tools these days.

I'm perfectly willing to leave old assertions in place, but if they
need fixing anyway, I'd damn well want to fix them *right* instead of
starting to just add more piles of hacks on top of the old model.

Because when the details of the locking rules depend on RT vs non-RT,
I want the checks to make sense.  And presumably lockdep is the thing
that really knows what the status of a lock is, no?

                  Linus