All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH v7 0/7] netdev: intel: Eliminate duplicate barriers on weakly-ordered archs
Date: Tue, 27 Mar 2018 09:54:17 -0700	[thread overview]
Message-ID: <1522169657.6503.1.camel@intel.com> (raw)
In-Reply-To: <e3ada376-52a5-573b-33f1-9aa84af75f0d@codeaurora.org>

On Tue, 2018-03-27 at 08:42 -0400, Sinan Kaya wrote:
> On 3/23/2018 10:34 PM, okaya at codeaurora.org wrote:
> > On 2018-03-23 19:58, Jeff Kirsher wrote:
> > > On Fri, 2018-03-23 at 14:53 -0700, Alexander Duyck wrote:
> > > > On Fri, Mar 23, 2018 at 11:52 AM, Sinan Kaya <okaya@codeaurora.
> > > > org>
> > > > wrote:
> > > > > 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.
> > > > 
> > > > I looked over the set and they seem good.
> > > > 
> > > > Reviewed-by: Alexander Duyck <alexander.h.duyck@intel.com>
> > > 
> > > Grrr, patch 1 does not apply cleanly to my next-queue tree (dev-
> > > queue
> > > branch).  I will deal with this series in a day or two, after I
> > > have dealt
> > > with my driver pull requests.
> > 
> > Sorry, you will have to replace the ones you took from me.
> 
> Double sorry now.
> 
> 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 V8 or is it
> too late?

Currently I do not have any of your patches applied to my next-queue
tree (dev-queue branch).  So feel free to do any revisions you need to
do and to re-submit to intel-wired-lan at lists.osuosl.org (IWL) mailing
list.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20180327/a20aa26b/attachment.asc>

WARNING: multiple messages have this Message-ID (diff)
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
To: Sinan Kaya <okaya@codeaurora.org>
Cc: sulrich@codeaurora.org, Netdev <netdev@vger.kernel.org>,
	Timur Tabi <timur@codeaurora.org>,
	Alexander Duyck <alexander.duyck@gmail.com>,
	intel-wired-lan <intel-wired-lan@lists.osuosl.org>,
	linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v7 0/7] netdev: intel: Eliminate duplicate barriers on weakly-ordered archs
Date: Tue, 27 Mar 2018 09:54:17 -0700	[thread overview]
Message-ID: <1522169657.6503.1.camel@intel.com> (raw)
In-Reply-To: <e3ada376-52a5-573b-33f1-9aa84af75f0d@codeaurora.org>


[-- Attachment #1.1: Type: text/plain, Size: 3057 bytes --]

On Tue, 2018-03-27 at 08:42 -0400, Sinan Kaya wrote:
> On 3/23/2018 10:34 PM, okaya@codeaurora.org wrote:
> > On 2018-03-23 19:58, Jeff Kirsher wrote:
> > > On Fri, 2018-03-23 at 14:53 -0700, Alexander Duyck wrote:
> > > > On Fri, Mar 23, 2018 at 11:52 AM, Sinan Kaya <okaya@codeaurora.
> > > > org>
> > > > wrote:
> > > > > 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.
> > > > 
> > > > I looked over the set and they seem good.
> > > > 
> > > > Reviewed-by: Alexander Duyck <alexander.h.duyck@intel.com>
> > > 
> > > Grrr, patch 1 does not apply cleanly to my next-queue tree (dev-
> > > queue
> > > branch).  I will deal with this series in a day or two, after I
> > > have dealt
> > > with my driver pull requests.
> > 
> > Sorry, you will have to replace the ones you took from me.
> 
> Double sorry now.
> 
> 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 V8 or is it
> too late?

Currently I do not have any of your patches applied to my next-queue
tree (dev-queue branch).  So feel free to do any revisions you need to
do and to re-submit to intel-wired-lan@lists.osuosl.org (IWL) mailing
list.

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: jeffrey.t.kirsher@intel.com (Jeff Kirsher)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v7 0/7] netdev: intel: Eliminate duplicate barriers on weakly-ordered archs
Date: Tue, 27 Mar 2018 09:54:17 -0700	[thread overview]
Message-ID: <1522169657.6503.1.camel@intel.com> (raw)
In-Reply-To: <e3ada376-52a5-573b-33f1-9aa84af75f0d@codeaurora.org>

On Tue, 2018-03-27 at 08:42 -0400, Sinan Kaya wrote:
> On 3/23/2018 10:34 PM, okaya at codeaurora.org wrote:
> > On 2018-03-23 19:58, Jeff Kirsher wrote:
> > > On Fri, 2018-03-23 at 14:53 -0700, Alexander Duyck wrote:
> > > > On Fri, Mar 23, 2018 at 11:52 AM, Sinan Kaya <okaya@codeaurora.
> > > > org>
> > > > wrote:
> > > > > 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.
> > > > 
> > > > I looked over the set and they seem good.
> > > > 
> > > > Reviewed-by: Alexander Duyck <alexander.h.duyck@intel.com>
> > > 
> > > Grrr, patch 1 does not apply cleanly to my next-queue tree (dev-
> > > queue
> > > branch).  I will deal with this series in a day or two, after I
> > > have dealt
> > > with my driver pull requests.
> > 
> > Sorry, you will have to replace the ones you took from me.
> 
> Double sorry now.
> 
> 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 V8 or is it
> too late?

Currently I do not have any of your patches applied to my next-queue
tree (dev-queue branch).  So feel free to do any revisions you need to
do and to re-submit to intel-wired-lan at lists.osuosl.org (IWL) mailing
list.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180327/a20aa26b/attachment.sig>

  parent reply	other threads:[~2018-03-27 16:54 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-23 18:52 [PATCH v7 0/7] netdev: intel: Eliminate duplicate barriers on weakly-ordered archs Sinan Kaya
2018-03-23 18:52 ` Sinan Kaya
2018-03-23 18:52 ` [Intel-wired-lan] [PATCH v7 1/7] i40e/i40evf: " Sinan Kaya
2018-03-23 18:52   ` Sinan Kaya
2018-03-23 18:52   ` Sinan Kaya
2018-03-23 18:52 ` [Intel-wired-lan] [PATCH v7 2/7] ixgbe: eliminate " Sinan Kaya
2018-03-23 18:52   ` Sinan Kaya
2018-03-23 18:52   ` Sinan Kaya
2018-03-23 18:52   ` Sinan Kaya
2018-03-23 18:52 ` [Intel-wired-lan] [PATCH v7 3/7] igbvf: " Sinan Kaya
2018-03-23 18:52   ` Sinan Kaya
2018-03-23 18:52   ` Sinan Kaya
2018-03-23 18:52   ` Sinan Kaya
2018-03-23 18:52 ` [Intel-wired-lan] [PATCH v7 4/7] igb: " Sinan Kaya
2018-03-23 18:52   ` Sinan Kaya
2018-03-23 18:52   ` Sinan Kaya
2018-03-23 18:52 ` [Intel-wired-lan] [PATCH v7 5/7] fm10k: Eliminate " Sinan Kaya
2018-03-23 18:52   ` Sinan Kaya
2018-03-23 18:52   ` Sinan Kaya
2018-03-23 18:52 ` [Intel-wired-lan] [PATCH v7 6/7] ixgbevf: keep writel() closer to wmb() Sinan Kaya
2018-03-23 18:52   ` Sinan Kaya
2018-03-23 18:52   ` Sinan Kaya
2018-03-23 18:53 ` [Intel-wired-lan] [PATCH v7 7/7] ixgbevf: eliminate duplicate barriers on weakly-ordered archs Sinan Kaya
2018-03-23 18:53   ` Sinan Kaya
2018-03-23 18:53   ` Sinan Kaya
2018-03-23 21:53 ` [Intel-wired-lan] [PATCH v7 0/7] netdev: intel: Eliminate " Alexander Duyck
2018-03-23 21:53   ` Alexander Duyck
2018-03-23 21:53   ` Alexander Duyck
2018-03-23 23:58   ` [Intel-wired-lan] " Jeff Kirsher
2018-03-23 23:58     ` Jeff Kirsher
2018-03-23 23:58     ` Jeff Kirsher
2018-03-24  2:34     ` [Intel-wired-lan] " okaya
2018-03-24  2:34       ` okaya at codeaurora.org
2018-03-24  2:34       ` okaya
2018-03-27 12:42       ` [Intel-wired-lan] " Sinan Kaya
2018-03-27 12:42         ` Sinan Kaya
2018-03-27 12:42         ` Sinan Kaya
2018-03-27 14:04         ` [Intel-wired-lan] " Lino Sanfilippo
2018-03-27 14:04           ` Lino Sanfilippo
2018-03-27 14:04           ` Lino Sanfilippo
2018-03-27 14:23           ` [Intel-wired-lan] " Sinan Kaya
2018-03-27 14:23             ` Sinan Kaya
2018-03-27 14:23             ` Sinan Kaya
2018-03-27 14:33             ` [Intel-wired-lan] " Lino Sanfilippo
2018-03-27 14:33               ` Aw: " Lino Sanfilippo
2018-03-27 14:33               ` Lino Sanfilippo
2018-03-27 14:38             ` [Intel-wired-lan] " Alexander Duyck
2018-03-27 14:38               ` Alexander Duyck
2018-03-27 14:38               ` Alexander Duyck
2018-03-27 14:48               ` [Intel-wired-lan] " Sinan Kaya
2018-03-27 14:48                 ` Sinan Kaya
2018-03-27 14:48                 ` Sinan Kaya
2018-03-27 16:54         ` Jeff Kirsher [this message]
2018-03-27 16:54           ` Jeff Kirsher
2018-03-27 16:54           ` Jeff Kirsher
2018-03-27 17:33           ` [Intel-wired-lan] " Sinan Kaya
2018-03-27 17:33             ` Sinan Kaya
2018-03-27 17:33             ` Sinan Kaya

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1522169657.6503.1.camel@intel.com \
    --to=jeffrey.t.kirsher@intel.com \
    --cc=intel-wired-lan@osuosl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.