Re: [PATCH v7 0/7] netdev: Eliminate duplicate barriers on weakly-ordered archs

[Sinan Kaya]

On 3/27/2018 10:00 AM, David Miller wrote:
> From: Sinan Kaya 
> Date: Tue, 27 Mar 2018 08:40:41 -0400
> 
>> Are you able to drop the applied ones so that I can post V7 or is it
>> too late?
> 
> I cannot "drop" changes from my tree since my tree is used by thousands
> of people and therefore immutable.
> 
> You must therefore send me relative fixes or reverts.
> 

Thanks, I'll send fixes. Just wanted to see whether it got merged or if it was
sitting on a branch.

-- 
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.

Re: [PATCH v7 0/7] netdev: Eliminate duplicate barriers on weakly-ordered archs

[David Miller]

From: Sinan Kaya 
Date: Tue, 27 Mar 2018 08:40:41 -0400

> Are you able to drop the applied ones so that I can post V7 or is it
> too late?

I cannot "drop" changes from my tree since my tree is used by thousands
of people and therefore immutable.

You must therefore send me relative fixes or reverts.

Re: [PATCH v7 0/7] netdev: Eliminate duplicate barriers on weakly-ordered archs

[Sinan Kaya]

Dave,

On 3/26/2018 12:48 PM, David Miller wrote:
> From: Sinan Kaya 
> Date: Sun, 25 Mar 2018 10:39:14 -0400
> 
>> Code includes wmb() followed by writel() in multiple places. writel()
>> already has a barrier on some architectures like arm64.
>>
>> This ends up CPU observing two barriers back to back before executing the
>> register write.
>>
>> Since code already has an explicit barrier call, changing writel() to
>> writel_relaxed().
>>
>> I did a regex search for wmb() followed by writel() in each drivers
>> directory.
>> I scrubbed the ones I care about in this series.
>>
>> I considered "ease of change", "popular usage" and "performance critical
>> path" as the determining criteria for my filtering.
>>
>> We used relaxed API heavily on ARM for a long time but
>> it did not exist on other architectures. For this reason, relaxed
>> architectures have been paying double penalty in order to use the common
>> drivers.
>>
>> Now that relaxed API is present on all architectures, we can go and scrub
>> all drivers to see what needs to change and what can remain.
>>
>> We start with mostly used ones and hope to increase the coverage over time.
>> It will take a while to cover all drivers.
>>
>> Feel free to apply patches individually.
>>
>> Changes since v6:
>> - bring back amazon ena and add mmiowb, remove
>>   ena_com_write_sq_doorbell_rel(). 
>> - remove extra mmiowb in bnx2x
>> - correct spelling mistake in  bnx2x: Replace doorbell barrier() with wmb()
> 
> Series applied, thank you.
> 

I don't know if you have been following "RFC on writel and writel_relaxed" 
thread
or not but there are some new developments about wmb() requirement. 

Basically, wmb() should never be used before writel() as writel() seem to
provide coherency and observability guarantee.

wmb()+writel_relaxed() is slower on some architectures than plain writel()

I'll have to rework these patches to have writel() only. 

Are you able to drop the applied ones so that I can post V7 or is it too late?

Sinan

-- 
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.

Re: [PATCH v7 0/7] netdev: Eliminate duplicate barriers on weakly-ordered archs

[David Miller]

From: Sinan Kaya 
Date: Sun, 25 Mar 2018 10:39:14 -0400

> Code includes wmb() followed by writel() in multiple places. writel()
> already has a barrier on some architectures like arm64.
> 
> This ends up CPU observing two barriers back to back before executing the
> register write.
> 
> Since code already has an explicit barrier call, changing writel() to
> writel_relaxed().
> 
> I did a regex search for wmb() followed by writel() in each drivers
> directory.
> I scrubbed the ones I care about in this series.
> 
> I considered "ease of change", "popular usage" and "performance critical
> path" as the determining criteria for my filtering.
> 
> We used relaxed API heavily on ARM for a long time but
> it did not exist on other architectures. For this reason, relaxed
> architectures have been paying double penalty in order to use the common
> drivers.
> 
> Now that relaxed API is present on all architectures, we can go and scrub
> all drivers to see what needs to change and what can remain.
> 
> We start with mostly used ones and hope to increase the coverage over time.
> It will take a while to cover all drivers.
> 
> Feel free to apply patches individually.
> 
> Changes since v6:
> - bring back amazon ena and add mmiowb, remove
>   ena_com_write_sq_doorbell_rel(). 
> - remove extra mmiowb in bnx2x
> - correct spelling mistake in  bnx2x: Replace doorbell barrier() with wmb()

Series applied, thank you.

[PATCH v7 0/7] netdev: Eliminate duplicate barriers on weakly-ordered archs

[Sinan Kaya]

Code includes wmb() followed by writel() in multiple places. writel()
already has a barrier on some architectures like arm64.

This ends up CPU observing two barriers back to back before executing the
register write.

Since code already has an explicit barrier call, changing writel() to
writel_relaxed().

I did a regex search for wmb() followed by writel() in each drivers
directory.
I scrubbed the ones I care about in this series.

I considered "ease of change", "popular usage" and "performance critical
path" as the determining criteria for my filtering.

We used relaxed API heavily on ARM for a long time but
it did not exist on other architectures. For this reason, relaxed
architectures have been paying double penalty in order to use the common
drivers.

Now that relaxed API is present on all architectures, we can go and scrub
all drivers to see what needs to change and what can remain.

We start with mostly used ones and hope to increase the coverage over time.
It will take a while to cover all drivers.

Feel free to apply patches individually.

Changes since v6:
- bring back amazon ena and add mmiowb, remove
  ena_com_write_sq_doorbell_rel(). 
- remove extra mmiowb in bnx2x
- correct spelling mistake in  bnx2x: Replace doorbell barrier() with wmb()


Sinan Kaya (7):
  net: qla3xxx: Eliminate duplicate barriers on weakly-ordered archs
  qlcnic: Eliminate duplicate barriers on weakly-ordered archs
  bnx2x: Replace doorbell barrier() with wmb()
  bnx2x: Eliminate duplicate barriers on weakly-ordered archs
  net: qlge: Eliminate duplicate barriers on weakly-ordered archs
  bnxt_en: Eliminate duplicate barriers on weakly-ordered archs
  net: ena: Eliminate duplicate barriers on weakly-ordered archs

 drivers/net/ethernet/amazon/ena/ena_com.c   |  8 ++--
 drivers/net/ethernet/amazon/ena/ena_eth_com.h   |  8 ++--
 drivers/net/ethernet/amazon/ena/ena_netdev.c|  5 +++--
 drivers/net/ethernet/broadcom/bnx2x/bnx2x.h | 12 
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c |  5 +++--
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h |  4 ++--
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c |  5 +++--
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c|  4 ++--
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c|  4 +++-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c   |  2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h   |  9 +
 drivers/net/ethernet/qlogic/qla3xxx.c   |  5 +++--
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c |  2 +-
 drivers/net/ethernet/qlogic/qlge/qlge.h | 16 
 drivers/net/ethernet/qlogic/qlge/qlge_main.c|  3 ++-
 15 files changed, 68 insertions(+), 24 deletions(-)

-- 
2.7.4