On 18.11.2016 17:48, Kashyap Desai wrote: >> -----Original Message----- >> From: [email protected] [mailto:linux-scsi- >> [email protected]] On Behalf Of Tomas Henzl >> Sent: Friday, November 18, 2016 9:23 PM >> To: Hannes Reinecke; Martin K. Petersen >> Cc: Christoph Hellwig; James Bottomley; Sumit Saxena; linux- >> [email protected]; Hannes Reinecke >> Subject: Re: [PATCH 5/5] megaraid_sas: add mmio barrier after register > writes >> On 11.11.2016 10:44, Hannes Reinecke wrote: >>> The megaraid_sas HBA only has a single register for I/O submission, >>> which will be hit pretty hard with scsi-mq. To ensure that the PCI >>> writes have made it across we need to add a mmio barrier after each >>> write; otherwise I've been seeing spurious command completions and I/O >>> stalls. >> Why is it needed that the PCI write reaches the hw exactly at this > point? >> Is it possible that this is a hw deficiency like that the hw can't > handle >> communication without tiny pauses, and so possible to remove in next >> generation? >> Thanks, >> Tomas > I think this is good to have mmiowb as we are already doing for writel() > version of megasas_return_cmd_fusion. > May be not for x86, but for some other CPU arch it is useful. I think it > become more evident while scs-mq support for more than one submission > queue patch of Hannes expose this issue. > > Probably this patch is good. Intermediate PCI device (PCI bridge ?) may > cache PCI packet and eventually out of order PCI packet to MegaRAID HBA > can cause this type of spurious completion.
Usually drivers do not add a write barrier after every pci write, unless like here in megasas_fire_cmd_fusion in the 32bit part where are two paired writes and it must be ensured that the pair arrives without any other write in between. Why is it wrong when a pci write is overtaken by another write or when happens a bit later and if it is wrong - don't we need an additional locking too ? The execution of megasas_fire_cmd_fusion might be interrupted and a delay can happen at any time. tomash > >>> Signed-off-by: Hannes Reinecke <[email protected]> >>> --- >>> drivers/scsi/megaraid/megaraid_sas_fusion.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c >>> b/drivers/scsi/megaraid/megaraid_sas_fusion.c >>> index aba53c0..729a654 100644 >>> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c >>> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c >>> @@ -196,6 +196,7 @@ inline void megasas_return_cmd_fusion(struct >> megasas_instance *instance, >>> le32_to_cpu(req_desc->u.low)); >>> >>> writeq(req_data, &instance->reg_set->inbound_low_queue_port); >>> + mmiowb(); >>> #else >>> unsigned long flags; >>> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of >> a message to [email protected] More majordomo info at >> http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
