tg3: Increase the PCI MRRS
Subsystems (2)
PATCH 10/13 tg3: Increase the PCI MRRS
Previous devices hardcoded the PCI Maximum Read Request Size to 4K. To better comply with the PCI spec, the hardware now defaults the MRRS to 512 bytes. This will yield poor driver performance if left untouched. This patch increases the MRRS to 4K on driver initialization.
drivers/net/tg3.c +7 -1
@@ -5098,12 +5098,15 @@ static void tg3_restore_pci_state(struct tg3 *tp)pci_write_config_word (tp->pdev,PCI_COMMAND , tp->pci_cmd); -if (!(tp->tg3_flags2 &TG3_FLG2_PCI_EXPRESS )) { +if (tp->tg3_flags2 &TG3_FLG2_PCI_EXPRESS ) +pcie_set_readrq (tp->pdev,4096 ); +else {pci_write_config_byte (tp->pdev,PCI_CACHE_LINE_SIZE , tp->pci_cacheline_sz);pci_write_config_byte (tp->pdev,PCI_LATENCY_TIMER , tp->pci_lat_timer); } +/* Make sure PCI-X relaxed ordering bit is clear. */ if (tp->pcix_cap) {u16 pcix_cmd;
@@ -11215,6 +11218,9 @@ static int __devinit tg3_get_invariants(struct tg3 *tp) pcie_cap =pci_find_capability (tp->pdev,PCI_CAP_ID_EXP );if (pcie_cap !=0 ) { tp->tg3_flags2 |=TG3_FLG2_PCI_EXPRESS ; + +pcie_set_readrq (tp->pdev,4096 ); +if (GET_ASIC_REV (tp->pci_chip_rev_id) ==ASIC_REV_5906 ) {u16 lnkctl;
David Miller
maintainerFrom: "Matt Carlson" <[email protected]>
Date: Fri, 09 Nov 2007 16:39:01 -0800
Quoted 7 lines
Previous devices hardcoded the PCI Maximum Read Request Size to 4K. To better comply with the PCI spec, the hardware now defaults the MRRS to 512 bytes. This will yield poor driver performance if left untouched. This patch increases the MRRS to 4K on driver initialization.
Signed-off-by Matt Carlson <[email protected]>Signed-off-by Michael Chan <[email protected]>
I've applied this patch, but...
I sense that the PCI spec wants devices to use an MRRS value of 512 in order to get better fairness on a PCI-E segment amongst multiple devices.
Quoted 1 line
From that perspective, jacking up the MRRS to 4096 unilaterally seems
like a very bad idea. If this was necessary for good performance, I'm sure the PCI spec folks would have choosen a higher value.
Or is this some tg3 specific performance issue?
Quoted 17 lines · David Miller
From: "Matt Carlson" <[email protected]>
Date: Fri, 09 Nov 2007 16:39:01 -0800Quoted 7 lines
Previous devices hardcoded the PCI Maximum Read Request Size to 4K. To better comply with the PCI spec, the hardware now defaults the MRRS to 512 bytes. This will yield poor driver performance if left untouched. This patch increases the MRRS to 4K on driver initialization.
Signed-off-by Matt Carlson <[email protected]>Signed-off-by Michael Chan <[email protected]>I've applied this patch, but...
I sense that the PCI spec wants devices to use an MRRS value of 512 in order to get better fairness on a PCI-E segment amongst multiple devices.
Quoted 5 lines
From that perspective, jacking up the MRRS to 4096 unilaterally seems like a very bad idea. If this was necessary for good performance, I'm sure the PCI spec folks would have choosen a higher value.
Or is this some tg3 specific performance issue?
Keeping the MRRS at 512 introduces DMA latencies that effectively prevent us from achieving linerate. With a packet size of ~1.5K and the MRRS at 512 bytes, the DMA will be broken into at least 3 DMA reads. Each DMA read takes ~1usec to initiate. It is this overhead that starts to cut into total throughput.
David Miller
maintainerFrom: "Matt Carlson" <[email protected]>
Date: Thu, 15 Nov 2007 14:20:10 -0800
Quoted 22 lines · David Miller
Quoted 21 lines
From: "Matt Carlson" <[email protected]>
Date: Fri, 09 Nov 2007 16:39:01 -0800Quoted 7 lines
Previous devices hardcoded the PCI Maximum Read Request Size to 4K. To better comply with the PCI spec, the hardware now defaults the MRRS to 512 bytes. This will yield poor driver performance if left untouched. This patch increases the MRRS to 4K on driver initialization.
Signed-off-by Matt Carlson <[email protected]>Signed-off-by Michael Chan <[email protected]>I've applied this patch, but...
I sense that the PCI spec wants devices to use an MRRS value of 512 in order to get better fairness on a PCI-E segment amongst multiple devices.
From that perspective, jacking up the MRRS to 4096 unilaterally seems like a very bad idea. If this was necessary for good performance, I'm sure the PCI spec folks would have choosen a higher value.
Quoted 7 lines
Quoted 1 line
Or is this some tg3 specific performance issue?
Keeping the MRRS at 512 introduces DMA latencies that effectively prevent us from achieving linerate. With a packet size of ~1.5K and the MRRS at 512 bytes, the DMA will be broken into at least 3 DMA reads. Each DMA read takes ~1usec to initiate. It is this overhead that starts to cut into total throughput.
Ok, but wouldn't every networking device on PCI need to do this then?
I want to hear what you think about this wrt. what I mentioned about fairness above. What's the point of PCI specifying a limit to comply with if nobody complies with the limit for localized performance reasons?
I think this is an important issue. Someone down the road is going to see bad disk throughput when doing lots of network transfers and wonder why that is. It will be hard to debug, but it won't be difficult for us to do something proactive about this right now to prevent that problem from happening in the first place.
Thanks.
Quoted 10 lines · David Miller
From: "Matt Carlson" <[email protected]>
Date: Thu, 15 Nov 2007 14:20:10 -0800Quoted 6 lines
Keeping the MRRS at 512 introduces DMA latencies that effectively prevent us from achieving linerate. With a packet size of ~1.5K and the MRRS at 512 bytes, the DMA will be broken into at least 3 DMA reads. Each DMA read takes ~1usec to initiate. It is this overhead that starts to cut into total throughput.
Ok, but wouldn't every networking device on PCI need to do this then?
No, it depends on the design. For example, a bigger maximum payload size will alleviate the problem (tg3 hardware is using 128). Multiple DMA read channels to pipeline the multiple read requests will also help.
We don't need to increase the MRRS on bnx2 hardware to get line-rate, for example.
Quoted 5 lines
I want to hear what you think about this wrt. what I mentioned about fairness above. What's the point of PCI specifying a limit to comply with if nobody complies with the limit for localized performance reasons?
Fairness is harder to know because it depends on chipset behavior. PCIE is not shared so there's no fairness issue at the local PCIE. Any fairness issue will be at the bridge.
I don't know what's the exact rationale for defaulting to 512, but I will try to find out.
Quoted 9 lines
I think this is an important issue. Someone down the road is going to see bad disk throughput when doing lots of network transfers and wonder why that is. It will be hard to debug, but it won't be difficult for us to do something proactive about this right now to prevent that problem from happening in the first place.
Thanks.
David Miller
maintainerFrom: "Michael Chan" <[email protected]>
Date: Thu, 15 Nov 2007 15:51:31 -0800
Quoted 2 lines
I don't know what's the exact rationale for defaulting to 512, but I will try to find out.
Thank you.
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
I sense that the PCI spec wants devices to use an MRRS value of 512 in order to get better fairness on a PCI-E segment amongst multiple devices.
Unless there are PCIe switches, there is only ever one device on the PCIe segment yes? Even on the biggest PCI-X or PCIe systems with which I am familiar, each PCIe slot is a separate PCIe bus, and I think that holds for the most recent PCI-X one as well.
Now, "core" I/O may be another matter - for example, on the rx3600 and rx6600 there are some shared PCI-X bus slots (two per) and on the "combo" versions in addition to the two independent PCIe x8's there is one pair of PCIe's on a switched bus.
However, none of the shared bus slots are recommended for "high performance cards" (that wonderfully moving target of a devinition :).
Quoted 11 lines
Quoted 11 lines
Quoted 5 lines
From that perspective, jacking up the MRRS to 4096 unilaterally seems like a very bad idea. If this was necessary for good performance, I'm sure the PCI spec folks would have choosen a higher value.
Or is this some tg3 specific performance issue?
Keeping the MRRS at 512 introduces DMA latencies that effectively prevent us from achieving linerate. With a packet size of ~1.5K and the MRRS at 512 bytes, the DMA will be broken into at least 3 DMA reads. Each DMA read takes ~1usec to initiate. It is this overhead that starts to cut into total throughput.
Reminds me of Tigon2 on original PCI.
Quoted 1 line
Ok, but wouldn't every networking device on PCI need to do this then?
I'm going to get very rapidly out of my PCI depth, but on one or the other (e vs X) isn't is possible from the standpoint of PCI for a device to have multiple transactions outstanding at a time?
If a given device can only have one outstanding at a time and happens to take a while to setup the DMA (perhaps there is firmware down there or something) I could see where it would want to make the transactions as large as it could, but another device, either able to have multiple in flight, or perhaps quicker on the DMA setup draw might not need it so large.
And even if the device itself is reasonably quick on the DMA setup draw, there may be systems, particularly large ones, where the rest of the setup of the DMA isn't instantaneous.
Quoted 4 lines
I want to hear what you think about this wrt. what I mentioned about fairness above. What's the point of PCI specifying a limit to comply with if nobody complies with the limit for localized performance reasons?
I'm _guessing_ that much of those limits were set with shared busses in mind. The "trend" seems to be towards single-slot busses.
Quoted 5 lines
I think this is an important issue. Someone down the road is going to see bad disk throughput when doing lots of network transfers and wonder why that is. It will be hard to debug, but it won't be difficult for us to do something proactive about this right now to prevent that problem from happening in the first place.
Does the current value of the MRRS get displayed in lspci output? It wouldn't be a slam dunk, but if someone were looking at that and saw the value large they might make an educated guess.
rick jones
Quoted 3 lines · Rick Jones
I'm going to get very rapidly out of my PCI depth, but on one or the other (e vs X) isn't is possible from the standpoint of PCI for a device to have multiple transactions outstanding at a time?
Yes, see my other response. Multiple outstanding transactions and a bigger maximum payload size will increase the throughput without the need to increase the MRRS.
Quoted 5 lines
Does the current value of the MRRS get displayed in lspci output? It wouldn't be a slam dunk, but if someone were looking at that and saw the value large they might make an educated guess.
Yes:
Capabilities: [ac] Express Endpoint IRQ 0
Device: Supported: MaxPayload 512 bytes, PhantFunc 0, ExtTag-
Device: Latency L0s <1us, L1 <2us
Device: AtnBtn- AtnInd- PwrInd-
Device: Errors: Correctable- Non-Fatal- Fatal- Unsupported-
Device: RlxdOrd+ ExtTag- PhantFunc- AuxPwr+ NoSnoop+
Device: MaxPayload 256 bytes, MaxReadReq 512 bytes