Re: [PATCH v1 07/25] lockdep: Add preemption disabled assertion API

Peter Zijlstra
2020-05-26T08:16:45Z

On Tue, May 26, 2020 at 02:52:31AM +0200, Ahmed S. Darwish wrote:
> Peter Zijlstra <peterz@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.