On 11/02/2010 01:53 PM, Andi Kleen wrote:
Boaz' approach is OBVIOUSLY mechanical, correct and bisectable.
Yours is not.
Sorry Jeff, but my patch has 100% the same locking order as Boaz.
The only difference is in which function it is.
Incorrect. Please re-read your own code.
Your code eliminates the drop+reacquire of the host_lock, choosing
instead a brand new, untested, unreviewing locking scheme of holding the
host_lock for the entirety of libata's queuecommand run.
In contrast, Boaz' proposed pattern of adding stub functions such as
int queuecmd_unlocked(scsi_cmd cmd, callback done)
{
lock_irqsave
get serial
call driver's existing queuecommand
unlock_irqrestore
}
does not change libata's locking at all, because it does not modify a
driver's queuecommand at all. It is obviously correct.
Or to restate another way,
Current libata locking
----------------------
spin_lock_irqsave(host_lock)
spin_unlock(host_lock)
spin_lock(ap lock)
...
spin_unlock(ap lock)
spin_lock(host_lock)
spin_unlock_irqrestore(host_lock)
Andi's brand new locking scheme
(missing the release of host lock)
----------------------------------
spin_lock_irqsave(host_lock)
spin_lock(ap lock)
...
spin_unlock(ap lock)
spin_unlock_irqrestore(host_lock)
The two versions are quite obviously NOT equivalent in any way, because
you have ADDED the holding of host_lock for the duration of libata's
queuecommand.
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
