[PATCH] tg3: add amd8131 to "write reorder" chipsets

[PATCH] tg3: add amd8131 to "write reorder" chipsets

[John W. Linville]

Add the AMD 8131 bridge to the list of chipsets that reorder writes.

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---

 drivers/net/tg3.c |    2 ++
 1 files changed, 2 insertions(+)

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index f645921..ce6f3be 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -10078,6 +10078,8 @@ static int __devinit tg3_get_invariants(
 	static struct pci_device_id write_reorder_chipsets[] = {
 		{ PCI_DEVICE(PCI_VENDOR_ID_AMD,
 		             PCI_DEVICE_ID_AMD_FE_GATE_700C) },
+		{ PCI_DEVICE(PCI_VENDOR_ID_AMD,
+		             PCI_DEVICE_ID_AMD_8131_BRIDGE) },
 		{ PCI_DEVICE(PCI_VENDOR_ID_VIA,
 			     PCI_DEVICE_ID_VIA_8385_0) },
 		{ },
-- 
John W. Linville
linville@tuxdriver.com

Re: [PATCH] tg3: add amd8131 to "write reorder" chipsets

[David Miller]

From: "John W. Linville" <linville@tuxdriver.com>
Date: Fri, 7 Jul 2006 14:58:36 -0400

> Add the AMD 8131 bridge to the list of chipsets that reorder writes.
> 
> Signed-off-by: John W. Linville <linville@tuxdriver.com>

What failure report is this chipset blacklist addition based
upon?  I just want to look at the paper trail before adding
this.

Thanks.

Re: [PATCH] tg3: add amd8131 to "write reorder" chipsets

[John W. Linville]

On Fri, Jul 07, 2006 at 12:12:20PM -0700, David Miller wrote:
> From: "John W. Linville" <linville@tuxdriver.com>
> Date: Fri, 7 Jul 2006 14:58:36 -0400
> 
> > Add the AMD 8131 bridge to the list of chipsets that reorder writes.
> > 
> > Signed-off-by: John W. Linville <linville@tuxdriver.com>
> 
> What failure report is this chipset blacklist addition based
> upon?  I just want to look at the paper trail before adding
> this.

What?  Due diligence?  You must be joking!!!  :-)

	https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=174019#c48

Hth!

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [PATCH] tg3: add amd8131 to "write reorder" chipsets

[Auke Kok]

David Miller wrote:
> From: "John W. Linville" <linville@tuxdriver.com>
> Date: Fri, 7 Jul 2006 14:58:36 -0400
> 
>> Add the AMD 8131 bridge to the list of chipsets that reorder writes.
>>
>> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> 
> What failure report is this chipset blacklist addition based
> upon?  I just want to look at the paper trail before adding
> this.

Moreover, since e1000 suffers from reordering/write-combining problems on the 
8132, I'm starting to see an (ugly) pattern here. I didn't know that tg3 has a 
whole blacklist for this, could this affect other network cards as well?

Auke

Re: [PATCH] tg3: add amd8131 to "write reorder" chipsets

[David Miller]

From: Auke Kok <auke-jan.h.kok@intel.com>
Date: Fri, 07 Jul 2006 13:21:38 -0700

> Moreover, since e1000 suffers from reordering/write-combining
> problems on the 8132, I'm starting to see an (ugly) pattern here. I
> didn't know that tg3 has a whole blacklist for this, could this
> affect other network cards as well?

The tg3 is probably more susceptible to this problem because of
how it uses a software structure in memory for interrupt status and
RX/TX descriptor table head/tail indexes.

Whereas the e1000 always has to synchronize with MMIO register reads
to get interrupt status and the indexes.

Re: [PATCH] tg3: add amd8131 to "write reorder" chipsets

[David Miller]

From: "John W. Linville" <linville@tuxdriver.com>
Date: Fri, 7 Jul 2006 14:58:36 -0400

> Add the AMD 8131 bridge to the list of chipsets that reorder writes.
> 
> Signed-off-by: John W. Linville <linville@tuxdriver.com>

Applied, thanks John.

Re: [PATCH] tg3: add amd8131 to "write reorder" chipsets

[Michael Chan]

John W. Linville wrote:

> Add the AMD 8131 bridge to the list of chipsets that reorder writes.
> 
> Signed-off-by: John W. Linville <linville@tuxdriver.com>

Hi John,

The latest tg3 driver in 2.6.18 has a recovery mechanism to detect
and recover from this kind of reordering problem.  I suppose we can
add this chipset now if we're reasonably sure that it is reordering
MMIO.  But it will be nice to actually see the recovery mechanism in
action catching this chipset.

Re: [PATCH] tg3: add amd8131 to "write reorder" chipsets

[John W. Linville]

On Fri, Jul 07, 2006 at 12:51:35PM -0700, Michael Chan wrote:
> John W. Linville wrote:
> 
> > Add the AMD 8131 bridge to the list of chipsets that reorder writes.
> > 
> > Signed-off-by: John W. Linville <linville@tuxdriver.com>
> 
> Hi John,
> 
> The latest tg3 driver in 2.6.18 has a recovery mechanism to detect
> and recover from this kind of reordering problem.  I suppose we can
> add this chipset now if we're reasonably sure that it is reordering
> MMIO.  But it will be nice to actually see the recovery mechanism in
> action catching this chipset.

Obviously it is between you and Dave to decide what is best.  We are
carrying this patch in RHEL4 at the moment, so I didn't want to hold
it back from upstream.  If you think you have a better solution then
"it's on you"... :-)

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [PATCH] tg3: add amd8131 to "write reorder" chipsets

[Michael Chan]

John W. Linville wrote:

> Obviously it is between you and Dave to decide what is best.  We are
> carrying this patch in RHEL4 at the moment, so I didn't want to hold
> it back from upstream.  If you think you have a better solution then
> "it's on you"... :-)
> 

Since 2.6.18 already has the recovery logic in place, we can probably
holdoff merging this patch upstream.

If e1000 is having similar problems with a similar AMD chipset, it
must be a fairly widespread problem.  It might be a good idea to
push this patch to -stable.

Re: [PATCH] tg3: add amd8131 to "write reorder" chipsets

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Fri, 7 Jul 2006 14:22:11 -0700

> John W. Linville wrote:
> 
> > Obviously it is between you and Dave to decide what is best.  We are
> > carrying this patch in RHEL4 at the moment, so I didn't want to hold
> > it back from upstream.  If you think you have a better solution then
> > "it's on you"... :-)
> 
> Since 2.6.18 already has the recovery logic in place, we can probably
> holdoff merging this patch upstream.

I think that since we do have a proof positive report, and well
documented success of the fix, we should add the fix even though
we have the fallback thing to take care of it.

This is how we were going to use the fall-back code, to make the the
chipset blacklist more robust yet still allow the user's machine to
work meanwhile.  Here, we have enough information to add the blacklist
entry safely.

What do you think?

Re: [PATCH] tg3: add amd8131 to "write reorder" chipsets

[Michael Chan]

David Miller wrote:

> I think that since we do have a proof positive report, and well
> documented success of the fix, we should add the fix even though
> we have the fallback thing to take care of it.
> 
> This is how we were going to use the fall-back code, to make the the
> chipset blacklist more robust yet still allow the user's machine to
> work meanwhile.  Here, we have enough information to add the blacklist
> entry safely.
> 
> What do you think?
> 
> 
OK, no objection to merging it now.