Re: Full hostlock pushdown available

Jeff Garzik
2010-11-02T18:13:20Z

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