Re: PATCH: PMC-Sierra MaxRAID driver to support 6Gb/s SAS RAID controller

[Brian King <brking@linux.vnet.ibm.com>]

Brian King wrote:
>>>> +     spin_unlock_irqrestore(&pinstance->pending_pool_lock, lock_flags);
>>>> +
>>>> +     /* Mulitple paths (IO path, control path) may be submitting IOARCBs,
>>>> +      * hence it is necessary to protect writes to IOA's ioarrin register.
>>>> +      * All writes to IOA ioarrin are synchronized with host_lock
>>>> +      */
>>>> +     if (lock)
>>>> +             spin_lock_irqsave(pinstance->host->host_lock,
>>>> +                               pinstance->host_lock_flags);
>>>> +
>>>> +     /* apply memory barrier */
>>>> +     mb();
>>>> +     /* driver writes lower 32-bit value of IOARCB address only */
>>>> +     write64(cmd->ioa_cb->ioarcb.ioarcb_bus_addr, pinstance->ioarrin);
>>>> +
>>>> +     if (lock)
>>>> +             spin_unlock_irqrestore(pinstance->host->host_lock,
>>>> +                                    pinstance->host_lock_flags);
>>> Any way to get rid of this lock flag getting passed in?
>> And I believe due to spinlock/unlock, the mb() is not needed.
>> Spin locks imply memory barriers.
> 
> Incorrect. The memory barrier here ensures that the command being
> constructed for the adapter is in a consistent state and that all
> the writes to the command buffer are flushed to memory before
> the write64 happens, which will trigger the adapter to DMA the
> command buffer and start executing the command.

After a chat on IRC with Grant and others, it looks like I was
looking at the wrong spin_unlock. The spin_unlock of the
pending_pool_lock should be sufficient to guarantee cache coherency
of the command buffer wrt to DMA.

-Brian

-- 
Brian King
Linux on Power Virtualization
IBM Linux Technology Center