Messages in this thread | | | Date | Tue, 26 May 2020 10:13:50 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v1 07/25] lockdep: Add preemption disabled assertion API |
| |
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.
| |