On 2018-05-24 15:38:51 [+0800], Jason Yan wrote: > > 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. yes, correct. But if the lock is acquired with interrupts enabled or the interrupts are enabled while the lock is held then lockdep will notify you. So you ensure that the lock is held at this point then and lockdep will do the validation (however, no need to do so in sas_ata_qc_issue() because lockdep do the evaluation during unlock/lock). > Because you are trying to delete the irq-save code which will make the > comment hard to understand, I'm just giving an advise. It's welcome > if you have a better way. I am removing local_irq_save() prior an unlock of a lock which has to be taken with disabled interrupts. Therefore the interrupts are disabling as part of the locking procedure and the additional disabling is a nop. What comment are you talking about? sas_ata_qc_issue() has no comments except for "If the device fell …". > Thanks. > Sebastian
