All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: David Laight <David.Laight@ACULAB.COM>,
	"'Dylan Hung'" <dylan_hung@aspeedtech.com>,
	Jakub Kicinski <kuba@kernel.org>, Joel Stanley <joel@jms.id.au>
Cc: "David S . Miller" <davem@davemloft.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Po-Yu Chuang <ratbert@faraday-tech.com>,
	linux-aspeed <linux-aspeed@lists.ozlabs.org>,
	OpenBMC Maillist <openbmc@lists.ozlabs.org>,
	BMC-SW <BMC-SW@aspeedtech.com>
Subject: Re: [PATCH] net: ftgmac100: Fix missing TX-poll issue
Date: Wed, 21 Oct 2020 09:05:34 +1100	[thread overview]
Message-ID: <7b44608bed9eccad80457cbfdfcca9043aae56f2.camel@kernel.crashing.org> (raw)
In-Reply-To: <f75555e09d47476a871669ffe017c4f8@AcuMS.aculab.com>

On Tue, 2020-10-20 at 13:15 +0000, David Laight wrote:
> That rather depends where the data is 'stuck'.
> 
> An old sparc cpu would flush the cpu store buffer before the read.
> But a modern x86 cpu will satisfy the read from the store buffer
> for cached data.
> 
> If the write is 'posted' on a PCI(e) bus then the read can't overtake it.
> But that is a memory access so shouldn't be to a PCI(e) address.
> 
> Shouldn't dma_wb() actually force your 'cpu to dram' queue be flushed?
> In which case you need one after writing the ring descriptor and
> before the poke of the mac engine.
> 
> The barrier before the descriptor write only needs to guarantee
> ordering of the writes - it can probably be a lighter barrier?
> 
> It might be that your dma_wmb() needs to do a write+read of
> an uncached DRAM location in order to empty the cpu to dram queue.

This is a specific bug with how a specific IP block is hooked up in
those SOCs, I wouldn't bloat the global dma_wmb for that. The read back
in the driver with appropriate comment should be enough.

Cheers,
Ben.



WARNING: multiple messages have this Message-ID (diff)
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: David Laight <David.Laight@ACULAB.COM>,
	"'Dylan Hung'" <dylan_hung@aspeedtech.com>,
	Jakub Kicinski <kuba@kernel.org>, Joel Stanley <joel@jms.id.au>
Cc: BMC-SW <BMC-SW@aspeedtech.com>,
	Po-Yu Chuang <ratbert@faraday-tech.com>,
	linux-aspeed <linux-aspeed@lists.ozlabs.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	OpenBMC Maillist <openbmc@lists.ozlabs.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"David S . Miller" <davem@davemloft.net>
Subject: Re: [PATCH] net: ftgmac100: Fix missing TX-poll issue
Date: Wed, 21 Oct 2020 09:05:34 +1100	[thread overview]
Message-ID: <7b44608bed9eccad80457cbfdfcca9043aae56f2.camel@kernel.crashing.org> (raw)
In-Reply-To: <f75555e09d47476a871669ffe017c4f8@AcuMS.aculab.com>

On Tue, 2020-10-20 at 13:15 +0000, David Laight wrote:
> That rather depends where the data is 'stuck'.
> 
> An old sparc cpu would flush the cpu store buffer before the read.
> But a modern x86 cpu will satisfy the read from the store buffer
> for cached data.
> 
> If the write is 'posted' on a PCI(e) bus then the read can't overtake it.
> But that is a memory access so shouldn't be to a PCI(e) address.
> 
> Shouldn't dma_wb() actually force your 'cpu to dram' queue be flushed?
> In which case you need one after writing the ring descriptor and
> before the poke of the mac engine.
> 
> The barrier before the descriptor write only needs to guarantee
> ordering of the writes - it can probably be a lighter barrier?
> 
> It might be that your dma_wmb() needs to do a write+read of
> an uncached DRAM location in order to empty the cpu to dram queue.

This is a specific bug with how a specific IP block is hooked up in
those SOCs, I wouldn't bloat the global dma_wmb for that. The read back
in the driver with appropriate comment should be enough.

Cheers,
Ben.



  reply	other threads:[~2020-10-20 22:06 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-19  7:39 [PATCH] net: ftgmac100: Fix missing TX-poll issue Dylan Hung
2020-10-19  7:39 ` Dylan Hung
2020-10-19  8:57 ` Joel Stanley
2020-10-19  8:57   ` Joel Stanley
2020-10-19  9:19   ` Dylan Hung
2020-10-19  9:19     ` Dylan Hung
2020-10-19 19:00   ` Jakub Kicinski
2020-10-19 19:00     ` Jakub Kicinski
2020-10-19 23:23     ` Benjamin Herrenschmidt
2020-10-19 23:23       ` Benjamin Herrenschmidt
2020-10-20  2:57       ` Jakub Kicinski
2020-10-20  2:57         ` Jakub Kicinski
2020-10-20  6:15         ` Benjamin Herrenschmidt
2020-10-20  6:15           ` Benjamin Herrenschmidt
2020-10-20 17:24           ` Jakub Kicinski
2020-10-20 17:24             ` Jakub Kicinski
2020-10-20  6:14     ` Dylan Hung
2020-10-20  6:14       ` Dylan Hung
2020-10-20 13:15       ` David Laight
2020-10-20 13:15         ` David Laight
2020-10-20 22:05         ` Benjamin Herrenschmidt [this message]
2020-10-20 22:05           ` Benjamin Herrenschmidt
2020-10-20 19:49       ` Arnd Bergmann
2020-10-20 22:10         ` Benjamin Herrenschmidt
2020-10-20 22:10           ` Benjamin Herrenschmidt
2020-10-20 22:25           ` Andrew Jeffery
2020-10-23 13:08             ` Dylan Hung
2020-10-26 22:21               ` Benjamin Herrenschmidt
2020-10-27  2:18                 ` Joel Stanley
2020-10-27  2:18                   ` Joel Stanley
2020-10-21  7:16           ` Arnd Bergmann
2020-10-21 12:11             ` Arnd Bergmann
2020-10-21 12:11               ` Arnd Bergmann
2020-10-22  7:40               ` Benjamin Herrenschmidt
2020-10-22  7:40                 ` Benjamin Herrenschmidt
2020-10-23  8:39                 ` Arnd Bergmann
2020-10-23  8:39                   ` Arnd Bergmann

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=7b44608bed9eccad80457cbfdfcca9043aae56f2.camel@kernel.crashing.org \
    --to=benh@kernel.crashing.org \
    --cc=BMC-SW@aspeedtech.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=davem@davemloft.net \
    --cc=dylan_hung@aspeedtech.com \
    --cc=joel@jms.id.au \
    --cc=kuba@kernel.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=openbmc@lists.ozlabs.org \
    --cc=ratbert@faraday-tech.com \
    /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.