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

Brian King
2009-06-12T16:17:26Z

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