Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1626923 > unrolled thread

Re: [PATCH V2] scsi: mpt3sas: remove redundant wmb

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V2] scsi: mpt3sas: remove redundant wmb "Martin K. Petersen" <martin.petersen@oracle.com> - 2017-04-20 04:30 +0200
    Re: [PATCH V2] scsi: mpt3sas: remove redundant wmb Sreekanth Reddy <sreekanth.reddy@broadcom.com> - 2017-04-21 10:00 +0200
      Re: [PATCH V2] scsi: mpt3sas: remove redundant wmb Sinan Kaya <okaya@codeaurora.org> - 2017-04-21 15:50 +0200
      Re: [PATCH V2] scsi: mpt3sas: remove redundant wmb Brian King <brking@linux.vnet.ibm.com> - 2017-04-21 21:10 +0200

#1626923 — Re: [PATCH V2] scsi: mpt3sas: remove redundant wmb

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!

-- 
Martin K. Petersen	Oracle Linux Engineering

[toc] | [next] | [standalone]

#1628021

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

[toc] | [prev] | [next] | [standalone]

#1628265

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.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

[toc] | [prev] | [next] | [standalone]

#1628519

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

-- 
Brian King
Power Linux I/O
IBM Linux Technology Center

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web