Re: [PATCH] target: Use WARNON_NON_RT(!irqs_disabled())

From: Linus Torvalds
Date: Wed Mar 21 2018 - 14:44:08 EST


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

On Wed, Mar 21, 2018 at 8:38 AM, Arnaldo Carvalho de Melo
<acme@xxxxxxxxxx> 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