[LEDE-DEV,2/3] ag71xx: Remove ___cacheline_aligned from ring structs.

Message ID 1517956050-32412-2-git-send-email-rosenp@gmail.com
State Deferred
Headers show
Series
  • [LEDE-DEV,1/3] ag71xx: Remove timestamp struct member and use trans_start instead
Related show

Commit Message

Rosen Penev Feb. 6, 2018, 10:27 p.m.
Qualcomm's struct members and inner workings of their driver are all different.
While this might make sense for their driver, it seems to hurt here. In
iperf3, i've seen inconsistent results including a drop of 100mbps on an
Archer C7v4. This patch keeps the results high and relatively consistent.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 .../linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Florian Fainelli Feb. 7, 2018, 4:27 a.m. | #1
On February 6, 2018 2:27:29 PM PST, Rosen Penev <rosenp@gmail.com> wrote:
>Qualcomm's struct members and inner workings of their driver are all
>different.
>While this might make sense for their driver, it seems to hurt here. In
>iperf3, i've seen inconsistent results including a drop of 100mbps on
>an
>Archer C7v4. This patch keeps the results high and relatively
>consistent.
>
>Signed-off-by: Rosen Penev <rosenp@gmail.com>

Do you use pahole to get a view of how the structure members are aligned and if there is room for reducing the holes and/or moving hot/frequently accessed members to a cache line aligned offset within the structure? If not, I would highly recommend using it and possibly providing the output before and after to show the improvement.
Rosen Penev Feb. 7, 2018, 5:07 a.m. | #2
On Tue, Feb 6, 2018 at 8:27 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On February 6, 2018 2:27:29 PM PST, Rosen Penev <rosenp@gmail.com> wrote:
>>Qualcomm's struct members and inner workings of their driver are all
>>different.
>>While this might make sense for their driver, it seems to hurt here. In
>>iperf3, i've seen inconsistent results including a drop of 100mbps on
>>an
>>Archer C7v4. This patch keeps the results high and relatively
>>consistent.
>>
>>Signed-off-by: Rosen Penev <rosenp@gmail.com>
>
> Do you use pahole to get a view of how the structure members are aligned and if there is room for reducing the holes and/or moving hot/frequently accessed members to a cache line aligned offset within the structure? If not, I would highly recommend using it and possibly providing the output before and after to show the improvement.
Never heard of pahole. I'm just backporting QCA patches since their
driver has some useful stuff.
>
> --
> Florian

Patch

diff --git a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
index d7897a9..8d7976e 100644
--- a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
+++ b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
@@ -159,8 +159,8 @@  struct ag71xx {
 	 * Critical data related to the per-packet data path are clustered
 	 * early in this structure to help improve the D-cache footprint.
 	 */
-	struct ag71xx_ring	rx_ring ____cacheline_aligned;
-	struct ag71xx_ring	tx_ring ____cacheline_aligned;
+	struct ag71xx_ring	rx_ring;
+	struct ag71xx_ring	tx_ring;
 
 	unsigned int            max_frame_len;
 	unsigned int            desc_pktlen_mask;