From: Robert Stonehouse <rstonehouse@solarflare.com>
To: David Miller <davem@davemloft.net>
Cc: <sshah@solarflare.com>, <netdev@vger.kernel.org>,
<linux-net-drivers@solarflare.com>
Subject: Re: [PATCH net v4 1/2] sfc: use 64-bit writes for PIO.
Date: Thu, 5 Jun 2014 18:08:48 +0100 [thread overview]
Message-ID: <5390A420.9070903@solarflare.com> (raw)
In-Reply-To: <20140603.155807.2227032324368753328.davem@davemloft.net>
On 03/06/14 23:58, David Miller wrote:
> Did you do any functional testing of this on such a machine?
>
> I'm extremely disappointed in this patch submission, because you
> didn't even_compile_ test this in the environment where you claim
> the problem exists.
The next patch posted in the series would have meant that this change
was limited to x86_64 systems.
I am sorry that by posting the patches in an incorrect order/not merging
them we gave the impression that we do not test these patches (but hands
up, patch 1/2 did break 32 bit compiles and certainly did need more
work. Thanks to Sergei, Joe and yourself for comments)
For x86_64 machines we checked writeq() implementations in io.h and
disassembled the module to ensure that a single 64bit access was used.
I would like to get an improved bug fix upstream, as I know this rare
hardware issue has been hit in the wild and with TX checksum offload
enabled corrupted data can get to the application.
The reason for limiting to x86_64 is that in this code path there is an
IO write to a write-combined mapping. It was a very hard case to hit
(believed to depend on the timing of accesses and resulting
write-combining behaviour), but we never reproduced it in our lab.
Limiting this IO to WC mappings to x86_64 only was a safety measure
given the amount of hours of testing we have on x86_64 vs.other 64 bit
platforms that we test with (PPC64)
This feature gives a small latency win on this hardware family for small
packets.
Unless there are more comments to consider the next post will be a
merged patch, addressing all points raised so far.
Regards
Rob
next prev parent reply other threads:[~2014-06-05 17:08 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-03 10:02 [PATCH net v4 0/2] sfc: Restrict PIO for 64bit arch in order to avoid data corruption Shradha Shah
2014-06-03 10:04 ` [PATCH net v4 1/2] sfc: use 64-bit writes for PIO Shradha Shah
2014-06-03 16:45 ` Sergei Shtylyov
2014-06-03 17:55 ` Joe Perches
2014-06-03 22:58 ` David Miller
2014-06-05 17:08 ` Robert Stonehouse [this message]
2014-06-03 10:05 ` [PATCH net v4 2/2] sfc: Restrict PIO to 64-bit architectures Shradha Shah
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=5390A420.9070903@solarflare.com \
--to=rstonehouse@solarflare.com \
--cc=davem@davemloft.net \
--cc=linux-net-drivers@solarflare.com \
--cc=netdev@vger.kernel.org \
--cc=sshah@solarflare.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.