Skip to content

Commit

Permalink
net: enetc: take the MDIO lock only once per NAPI poll cycle
Browse files Browse the repository at this point in the history
The workaround for the ENETC MDIO erratum caused a performance
degradation of 82 Kpps (seen with IP forwarding of two 1Gbps streams of
64B packets). This is due to excessive locking and unlocking in the fast
path, which can be avoided.

By taking the MDIO read-side lock only once per NAPI poll cycle, we are
able to regain 54 Kpps (65%) of the performance hit. The rest of the
performance degradation comes from the TX data path, but unfortunately
it doesn't look like we can optimize that away easily, even with
netdev_xmit_more(), there just isn't any skb batching done, to help with
taking the MDIO lock less often than once per packet.

We need to change the register accessor type for enetc_get_tx_tstamp,
because it now runs under the enetc_lock_mdio as per the new call path
detailed below:

enetc_msix
-> napi_schedule
   -> enetc_poll
      -> enetc_lock_mdio
      -> enetc_clean_tx_ring
         -> enetc_get_tx_tstamp
      -> enetc_clean_rx_ring
      -> enetc_unlock_mdio

Fixes: fd5736b ("enetc: Workaround for MDIO register access issue")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Vladimir Oltean authored and David S. Miller committed Mar 1, 2021
1 parent 3222b5b commit 6d36ecd
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 22 deletions.
31 changes: 9 additions & 22 deletions drivers/net/ethernet/freescale/enetc/enetc.c
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,8 @@ static int enetc_poll(struct napi_struct *napi, int budget)
int work_done;
int i;

enetc_lock_mdio();

for (i = 0; i < v->count_tx_rings; i++)
if (!enetc_clean_tx_ring(&v->tx_ring[i], budget))
complete = false;
Expand All @@ -291,8 +293,10 @@ static int enetc_poll(struct napi_struct *napi, int budget)
if (work_done)
v->rx_napi_work = true;

if (!complete)
if (!complete) {
enetc_unlock_mdio();
return budget;
}

napi_complete_done(napi, work_done);

Expand All @@ -301,8 +305,6 @@ static int enetc_poll(struct napi_struct *napi, int budget)

v->rx_napi_work = false;

enetc_lock_mdio();

/* enable interrupts */
enetc_wr_reg_hot(v->rbier, ENETC_RBIER_RXTIE);

Expand All @@ -327,8 +329,8 @@ static void enetc_get_tx_tstamp(struct enetc_hw *hw, union enetc_tx_bd *txbd,
{
u32 lo, hi, tstamp_lo;

lo = enetc_rd(hw, ENETC_SICTR0);
hi = enetc_rd(hw, ENETC_SICTR1);
lo = enetc_rd_hot(hw, ENETC_SICTR0);
hi = enetc_rd_hot(hw, ENETC_SICTR1);
tstamp_lo = le32_to_cpu(txbd->wb.tstamp);
if (lo <= tstamp_lo)
hi -= 1;
Expand Down Expand Up @@ -358,9 +360,7 @@ static bool enetc_clean_tx_ring(struct enetc_bdr *tx_ring, int napi_budget)
i = tx_ring->next_to_clean;
tx_swbd = &tx_ring->tx_swbd[i];

enetc_lock_mdio();
bds_to_clean = enetc_bd_ready_count(tx_ring, i);
enetc_unlock_mdio();

do_tstamp = false;

Expand Down Expand Up @@ -403,8 +403,6 @@ static bool enetc_clean_tx_ring(struct enetc_bdr *tx_ring, int napi_budget)
tx_swbd = tx_ring->tx_swbd;
}

enetc_lock_mdio();

/* BD iteration loop end */
if (is_eof) {
tx_frm_cnt++;
Expand All @@ -415,8 +413,6 @@ static bool enetc_clean_tx_ring(struct enetc_bdr *tx_ring, int napi_budget)

if (unlikely(!bds_to_clean))
bds_to_clean = enetc_bd_ready_count(tx_ring, i);

enetc_unlock_mdio();
}

tx_ring->next_to_clean = i;
Expand Down Expand Up @@ -660,8 +656,6 @@ static int enetc_clean_rx_ring(struct enetc_bdr *rx_ring,
u32 bd_status;
u16 size;

enetc_lock_mdio();

if (cleaned_cnt >= ENETC_RXBD_BUNDLE) {
int count = enetc_refill_rx_ring(rx_ring, cleaned_cnt);

Expand All @@ -672,19 +666,15 @@ static int enetc_clean_rx_ring(struct enetc_bdr *rx_ring,

rxbd = enetc_rxbd(rx_ring, i);
bd_status = le32_to_cpu(rxbd->r.lstatus);
if (!bd_status) {
enetc_unlock_mdio();
if (!bd_status)
break;
}

enetc_wr_reg_hot(rx_ring->idr, BIT(rx_ring->index));
dma_rmb(); /* for reading other rxbd fields */
size = le16_to_cpu(rxbd->r.buf_len);
skb = enetc_map_rx_buff_to_skb(rx_ring, i, size);
if (!skb) {
enetc_unlock_mdio();
if (!skb)
break;
}

enetc_get_offloads(rx_ring, rxbd, skb);

Expand All @@ -696,7 +686,6 @@ static int enetc_clean_rx_ring(struct enetc_bdr *rx_ring,

if (unlikely(bd_status &
ENETC_RXBD_LSTATUS(ENETC_RXBD_ERR_MASK))) {
enetc_unlock_mdio();
dev_kfree_skb(skb);
while (!(bd_status & ENETC_RXBD_LSTATUS_F)) {
dma_rmb();
Expand Down Expand Up @@ -736,8 +725,6 @@ static int enetc_clean_rx_ring(struct enetc_bdr *rx_ring,

enetc_process_skb(rx_ring, skb);

enetc_unlock_mdio();

napi_gro_receive(napi, skb);

rx_frm_cnt++;
Expand Down
2 changes: 2 additions & 0 deletions drivers/net/ethernet/freescale/enetc/enetc_hw.h
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,8 @@ static inline u64 _enetc_rd_reg64_wa(void __iomem *reg)
#define enetc_wr_reg(reg, val) _enetc_wr_reg_wa((reg), (val))
#define enetc_rd(hw, off) enetc_rd_reg((hw)->reg + (off))
#define enetc_wr(hw, off, val) enetc_wr_reg((hw)->reg + (off), val)
#define enetc_rd_hot(hw, off) enetc_rd_reg_hot((hw)->reg + (off))
#define enetc_wr_hot(hw, off, val) enetc_wr_reg_hot((hw)->reg + (off), val)
#define enetc_rd64(hw, off) _enetc_rd_reg64_wa((hw)->reg + (off))
/* port register accessors - PF only */
#define enetc_port_rd(hw, off) enetc_rd_reg((hw)->port + (off))
Expand Down

0 comments on commit 6d36ecd

Please sign in to comment.