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

From: Thomas Gleixner
Date: Thu Mar 22 2018 - 05:15:26 EST


On Wed, 21 Mar 2018, Linus Torvalds wrote:
> [ 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?

We are working on replacing the _NONRT _RT variants with proper lockdep
mechnisms which are aware of the RT vs. non-RT magic under the hood. Just
not there yet.

Thanks,

tglx