diff mbox series

net-next: ag71xx: Rearrange ag711xx struct to remove holes

Message ID 20190723034309.16492-1-rosenp@gmail.com
State Changes Requested
Delegated to: David Miller
Headers show
Series net-next: ag71xx: Rearrange ag711xx struct to remove holes | expand

Commit Message

Rosen Penev July 23, 2019, 3:43 a.m. UTC 
Removed ____cacheline_aligned attribute to ring structs. This actually
causes holes in the ag71xx struc as well as lower performance.

Rearranged struct members to fall within respective cachelines. The RX
ring struct now does not share a cacheline with the TX ring. The NAPI
atruct 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@gmail.com>
---
 drivers/net/ethernet/atheros/ag71xx.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

Comments

David Miller July 25, 2019, 6:21 p.m. UTC | #1 
From: Rosen Penev <rosenp@gmail.com>
Date: Mon, 22 Jul 2019 20:43:09 -0700

> Removed ____cacheline_aligned attribute to ring structs. This actually
> causes holes in the ag71xx struc as well as lower performance.

If you are legitimizing a change because of performance you must provide
detailed performance results that support this reason.

I'm not applying this patch until you respin it with the required
information.

Thank you.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
index 8f450a03a885..4699feb4ee31 100644
--- a/drivers/net/ethernet/atheros/ag71xx.c
+++ b/drivers/net/ethernet/atheros/ag71xx.c
@@ -295,17 +295,16 @@  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;
 
 	u16 rx_buf_size;
-	u8 rx_buf_offset;
+	u16 rx_buf_offset;
 
+	const struct ag71xx_dcfg *dcfg;
 	struct net_device *ndev;
 	struct platform_device *pdev;
 	struct napi_struct napi;
-	u32 msg_enable;
-	const struct ag71xx_dcfg *dcfg;
 
 	/* From this point onwards we're not looking at per-packet fields. */
 	void __iomem *mac_base;
@@ -313,20 +312,18 @@  struct ag71xx {
 	struct ag71xx_desc *stop_desc;
 	dma_addr_t stop_desc_dma;
 
-	int phy_if_mode;
-
-	struct delayed_work restart_work;
-	struct timer_list oom_timer;
-
-	struct reset_control *mac_reset;
-
 	u32 fifodata[3];
 	int mac_idx;
+	int phy_if_mode;
+	u32 msg_enable;
 
 	struct reset_control *mdio_reset;
 	struct mii_bus *mii_bus;
 	struct clk *clk_mdio;
 	struct clk *clk_eth;
+	struct reset_control *mac_reset;
+	struct delayed_work restart_work;
+	struct timer_list oom_timer;
 };
 
 static int ag71xx_desc_empty(struct ag71xx_desc *desc)