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 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
