Hi, Please consider this patch as Acked-by: Chaitra P B <chaitra.basappa@xxxxxxxxxxxx> Thanks, Chaitra -----Original Message----- From: Tomas Henzl [mailto:thenzl@xxxxxxxxxx] Sent: Thursday, May 24, 2018 9:19 PM To: James Bottomley; linux-scsi@xxxxxxxxxxxxxxx Cc: chaitra.basappa@xxxxxxxxxxxx; sreekanth.reddy@xxxxxxxxxxxx; Sathya.Prakash@xxxxxxxxxxxx Subject: Re: [PATCH] mpt3sas: Add an i/o barrier On 05/24/2018 05:33 PM, James Bottomley wrote: > On Thu, 2018-05-24 at 17:31 +0200, Tomas Henzl wrote: >> On 05/24/2018 05:19 PM, James Bottomley wrote: >>> On Thu, 2018-05-24 at 17:12 +0200, Tomas Henzl wrote: >>>> A barrier should be added to ensure proper ordering of memory >>>> mapped writes. >>>> >>>> Signed-off-by: Tomas Henzl <thenzl@xxxxxxxxxx> >>>> --- >>>> drivers/scsi/mpt3sas/mpt3sas_base.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c >>>> b/drivers/scsi/mpt3sas/mpt3sas_base.c >>>> index bf04fa90f..569392d0d 100644 >>>> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c >>>> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c >>>> @@ -3348,6 +3348,7 @@ _base_mpi_ep_writeq(__u64 b, volatile void >>>> __iomem *addr, >>>> spin_lock_irqsave(writeq_lock, flags); >>>> writel((u32)(data_out), addr); >>>> writel((u32)(data_out >> 32), (addr + 4)); >>>> + mmiowb(); >>>> spin_unlock_irqrestore(writeq_lock, flags); >>>> } >>> I thought, assuming mpt3sas has this right, that this construction >>> is only used on 32 bit platforms that don't have a writeq >>> instruction? I don't believe there's any overlap with the NUMA >>> systems that need io and memory domain synchronization, so either >>> this problem is purely theoretical or mpt3sas doesn't have the use >>> of writeq correct and if the latter case it should be fixed >>> correctly. >> The _base_mpi_ep_writeq is used regardless to 32/64 bit arch for >> example in _base_put_smid_mpi_ep_scsi_io, >> mpt3sas_base_put_smid_hi_priority and so on. > So it's the latter ... but my point is that that should be fixed > rather than adding barriers to what should be a corner case work around. I think that the hw is for some reason not able to handle a 64 write, the patch in e5747439366c1079257083f231f5dd9a84bf0fd7 "scsi: mpt3sas: Introduce function to clone mpi request" states that it is intentional. So adding the write barrier still makes sense for me. > > James >
