On 2018/5/24 15:50, Johannes Thumshirn wrote:
On Thu, May 24, 2018 at 03:38:51PM +0800, Jason Yan wrote:
On 2018/5/24 15:15, Sebastian Andrzej Siewior wrote:
On 2018-05-24 10:58:44 [+0800], Jason Yan wrote:
I think it's fine to delete this irq save code. As for the "TODO"
comment, I think we can add:
BUG_ON(!irqs_disabled());
or
WARN_ON_ONCE(!irqs_disabled());
no, please don't do this. Please add instead
lockdep_assert_held()
on the lock in question and let lockdep to its work. Lockdep has way
better coverage than your irqs_disabled() check which also breaks RT.
lockdep_assert_held() cannot detect the irq state, it can only detect
whether we have held the lock.
I think Sebastian wanted to say lockdep_assert_irqs_disabled().
OK, good idea.
Either way, please never ever use BUG_ON() (even WARN_ON() is
questionable as some people actually use panic_on_oops). It causes
nasty bugs at customer sites.
It's true that BUG_ON() is not recommended. But WARN_ON_ONCE() is
a possible way to replace it, panic_on_oops have no effect on it.
Actually lockdep_assert_irqs_disabled() uses WARN_ONCE() too.
Byte,
Johannes
