| Message ID | 1491591978-17880-1-git-send-email-okaya@codeaurora.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
Sinan Kaya <okaya@codeaurora.org> writes: > Due to relaxed ordering requirements on multiple architectures, > drivers are required to use wmb/rmb/mb combinations when they need to > guarantee observability between the memory and the HW. > > The mpt3sas driver is already using wmb() for this purpose. However, > it issues a writel following wmb(). writel() function on arm/arm64 > arhictectures have an embedded wmb() call inside. > > This results in unnecessary performance loss and code duplication. > > writel already guarantees ordering for both cpu and bus. we don't need > additional wmb() Broadcom folks, please review!
On Thu, Apr 20, 2017 at 7:58 AM, Martin K. Petersen <martin.petersen@oracle.com> wrote: > Sinan Kaya <okaya@codeaurora.org> writes: > >> Due to relaxed ordering requirements on multiple architectures, >> drivers are required to use wmb/rmb/mb combinations when they need to >> guarantee observability between the memory and the HW. >> >> The mpt3sas driver is already using wmb() for this purpose. However, >> it issues a writel following wmb(). writel() function on arm/arm64 >> arhictectures have an embedded wmb() call inside. [Sreekanth] Whether same thing applicable for SPARC & POWER architectures. If yes then we are fine with this patch changes. >> >> This results in unnecessary performance loss and code duplication. >> >> writel already guarantees ordering for both cpu and bus. we don't need >> additional wmb() > > Broadcom folks, please review! > > -- > Martin K. Petersen Oracle Linux Engineering
On 4/21/2017 3:56 AM, Sreekanth Reddy wrote: > [Sreekanth] Whether same thing applicable for SPARC & POWER > architectures. If yes then we are fine with this patch changes. This behavior is common for all architectures according to this document. Who would be the best person to comment on SPARC and POWER architectures in specific? James and I exchanged some comments on the first version. James? can you comment on POWER behavior. https://www.kernel.org/doc/Documentation/memory-barriers.txt Inside of the Linux kernel, I/O should be done through the appropriate accessor routines - such as inb() or writel() - which know how to make such accesses appropriately sequential. "Whilst this, for the most part, renders the explicit use of memory barriers unnecessary", there are a couple of situations where they might be needed: (1) On some systems, I/O stores are not strongly ordered across all CPUs, and so for _all_ general drivers locks should be used and mmiowb() must be issued prior to unlocking the critical section. (2) If the accessor functions are used to refer to an I/O memory window with relaxed memory access properties, then _mandatory_ memory barriers are required to enforce ordering.
On 04/21/2017 02:56 AM, Sreekanth Reddy wrote: > On Thu, Apr 20, 2017 at 7:58 AM, Martin K. Petersen > <martin.petersen@oracle.com> wrote: >> Sinan Kaya <okaya@codeaurora.org> writes: >> >>> Due to relaxed ordering requirements on multiple architectures, >>> drivers are required to use wmb/rmb/mb combinations when they need to >>> guarantee observability between the memory and the HW. >>> >>> The mpt3sas driver is already using wmb() for this purpose. However, >>> it issues a writel following wmb(). writel() function on arm/arm64 >>> arhictectures have an embedded wmb() call inside. > > [Sreekanth] Whether same thing applicable for SPARC & POWER > architectures. If yes then we are fine with this patch changes. This is also true for Power. Reviewed-by: Brian King <brking@linux.vnet.ibm.com> -Brian
Sinan, > Due to relaxed ordering requirements on multiple architectures, > drivers are required to use wmb/rmb/mb combinations when they need to > guarantee observability between the memory and the HW. Applied to 4.12/scsi-queue, thanks!
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c index 5b7aec5..18039bb 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_base.c +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c @@ -1025,7 +1025,6 @@ static int mpt3sas_remove_dead_ioc_func(void *arg) 0 : ioc->reply_free_host_index + 1; ioc->reply_free[ioc->reply_free_host_index] = cpu_to_le32(reply); - wmb(); writel(ioc->reply_free_host_index, &ioc->chip->ReplyFreeHostIndex); } @@ -1074,7 +1073,6 @@ static int mpt3sas_remove_dead_ioc_func(void *arg) return IRQ_NONE; } - wmb(); if (ioc->is_warpdrive) { writel(reply_q->reply_post_host_index, ioc->reply_post_host_index[msix_index]);
Due to relaxed ordering requirements on multiple architectures, drivers are required to use wmb/rmb/mb combinations when they need to guarantee observability between the memory and the HW. The mpt3sas driver is already using wmb() for this purpose. However, it issues a writel following wmb(). writel() function on arm/arm64 arhictectures have an embedded wmb() call inside. This results in unnecessary performance loss and code duplication. writel already guarantees ordering for both cpu and bus. we don't need additional wmb() Signed-off-by: Sinan Kaya <okaya@codeaurora.org> --- drivers/scsi/mpt3sas/mpt3sas_base.c | 2 -- 1 file changed, 2 deletions(-)