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

From: Linus Torvalds
Date: Fri Mar 23 2018 - 12:37:49 EST


On Fri, Mar 23, 2018 at 9:25 AM, Bart Van Assche <Bart.VanAssche@xxxxxxx> wrote:
>
> Have you considered to delete the WARN_ON_ONCE(!irqs_disabled()) statement?
> I think that check duplicates functionality that already exists in lockdep
> since lockdep is already able to detect spinlock use inconsistencies.

Please just delete both lines.

There is exactly two callers of that static function, and both of them do

spin_lock_irq(&cmd->t_state_lock);

right above the call.

It's not like this is some function that is exported to random users,
and we should check that the calling convention is right.

So honestly, even lockdep annotations look like you don't need them.

This looks like "it may have been useful during coding to document
things, but it's not useful long-term".

Sure, the annotation is not wrong, but even if you go "verification is
good", you should ask yourself whether there are maybe better places
that would catch more relevant problems, than putting verification
into some static function with two trivially correct callers wrt this
verification?

Linus