On Tue, May 26, 2020 at 02:52:31AM +0200, Ahmed S. Darwish wrote:
> Peter Zijlstra <pet...@infradead.org> wrote:
> > +#define lockdep_assert_irqs_enabled()
> > \
> > +do {
> > \
> > + WARN_ON_ONCE(debug_locks && !this_cpu_read(hardirqs_enabled)); \
> > +} while (0)
> >
>
> Given that lockdep_off() is defined at lockdep.c as:
>
> void lockdep_off(void)
> {
> current->lockdep_recursion += LOCKDEP_OFF;
> }
>
> This would imply that all of the macros:
>
> - lockdep_assert_irqs_enabled()
> - lockdep_assert_irqs_disabled()
> - lockdep_assert_in_irq()
> - lockdep_assert_preemption_disabled()
> - lockdep_assert_preemption_enabled()
>
> will do the lockdep checks *even if* lockdep_off() was called.
>
> This doesn't sound right. Even if all of the above macros call sites
> didn't care about lockdep_off()/on(), it is semantically incoherent.
lockdep_off() is an abomination and really should not exist.
That dm-cache-target.c thing, for example, is atrocious shite that will
explode on -rt. Whoever wrote that needs a 'medal'.
People using it deserve all the pain they get.
Also; IRQ state _should_ be tracked irrespective of tracking lock
dependencies -- I see that that currently isn't entirely the case, lemme
go fix that.
