On 30/05/2018 11:16, Sebastian Andrzej Siewior wrote:
On 2018-05-30 10:34:12 [+0100], John Garry wrote:
Sorry, but personally I don't see much value in this change. I think it's
better for safety to be consistent in how we lock & unlock the spinlock,
i.e. use irqsave variant (or similar).
The lock should do irqsave() and unlock irqrestore(). This
local_irqsave() + unlock() is not correct.
Aren't they the same, i.e. local_irq_save()+spin_lock() =
spin_lock_irqsave()? Both give state of lock held, interrupts and
preemption disabled.
And even if we have audited the callers to know the irqstate when this
function is called, since sas_ata_qc_issue() is a callback for
ata_port_operations.qc_issue, then it should be documented that interrupts
should always be disabled for this callback.
If the lock has been acquired without disabling the interrupts then it
was already wrong. This is just duct tape and does not fix the
problematic caller. Lockdep will yell.
John
Sebastian
.
