On 2018/5/24 15:52, Sebastian Andrzej Siewior wrote:
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 …".
I mean the "TODO" comment above the local_irq_save().
Thanks.
Sebastian
.
