[PATCH] ath79/ag71xx: rearrange ag71xx structs to remove holes

Rui Salvaterra rsalvaterra at gmail.com
Sun Sep 19 03:38:39 PDT 2021


From: Rosen Penev <rosenp at gmail.com>

Remove ____cacheline_aligned attribute to ring structs. This actually causes
holes in the ag71xx struct as well as lower performance.

Rearrange struct members to fall within respective cachelines. The RX ring
struct now does not share a cacheline with the TX ring. The NAPI struct now
takes up its own cachelines and does not share.

According to pahole -C ag71xx -c 32

Before:

struct ag71xx {
	/* size: 384, cachelines: 12, members: 22 */
	/* sum members: 375, holes: 2, sum holes: 9 */

After:

struct ag71xx {
	/* size: 376, cachelines: 12, members: 22 */
	/* last cacheline: 24 bytes */

Signed-off-by: Rosen Penev <rosenp at gmail.com>
[Fix typos in the patch description]
Signed-off-by: Rui Salvaterra <rsalvaterra at gmail.com>
---
The ag71xx_ring changes are already part of the upstream driver. However, since
we're not using it at all yet, let's apply this to our driver for the time
being, as David explicitly requests performance numbers before applying it
upstream [1] (and rightly so, in my opinion).

[1] https://lore.kernel.org/netdev/20190725.112108.2287417619951369896.davem@davemloft.net/

 .../net/ethernet/atheros/ag71xx/ag71xx.h      | 32 ++++++++++---------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h b/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
index 5ff9439f0d..6ed1f78459 100644
--- a/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
+++ b/target/linux/ath79/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
@@ -107,13 +107,16 @@ struct ag71xx_buf {
 };
 
 struct ag71xx_ring {
-	struct ag71xx_buf	*buf;
-	u8			*descs_cpu;
-	dma_addr_t		descs_dma;
-	u16			desc_split;
-	u16			order;
+	/* "Hot" fields in the data path. */
 	unsigned int		curr;
 	unsigned int		dirty;
+
+	/* "Cold" fields - not used in the data path. */
+	struct ag71xx_buf	*buf;
+	u16			order;
+	u16			desc_split;
+	dma_addr_t		descs_dma;
+	u8			*descs_cpu;
 };
 
 struct ag71xx_int_stats {
@@ -151,21 +154,20 @@ 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;
+	u16			rx_buf_size;
+	u16			rx_buf_offset;
+	u32			msg_enable;
 
+	struct ag71xx_ring	tx_ring;
 	int			mac_idx;
-
 	u16			desc_pktlen_mask;
-	u16			rx_buf_size;
-	u8			rx_buf_offset;
 	u8			tx_hang_workaround:1;
 
+	struct napi_struct	napi;
 	struct net_device	*dev;
 	struct platform_device  *pdev;
 	spinlock_t		lock;
-	struct napi_struct	napi;
-	u32			msg_enable;
 
 	/*
 	 * From this point onwards we're not looking at per-packet fields.
@@ -188,12 +190,12 @@ struct ag71xx {
 	unsigned int		speed;
 	int			duplex;
 
-	struct delayed_work	restart_work;
-	struct timer_list	oom_timer;
-
 	struct reset_control *mac_reset;
 	struct reset_control *mdio_reset;
 
+	struct delayed_work	restart_work;
+	struct timer_list	oom_timer;
+
 	u32			fifodata[3];
 	u32			plldata[3];
 	u32			pllreg[3];
-- 
2.33.0




More information about the openwrt-devel mailing list