| Message ID | 1358319684-32364-1-git-send-email-timo.teras@iki.fi |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Timo Teräs <timo.teras@iki.fi> Date: Wed, 16 Jan 2013 09:01:24 +0200 > @@ -6035,10 +6034,8 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, u32 budget > unsigned int count; > > cur_rx = tp->cur_rx; > - rx_left = NUM_RX_DESC + tp->dirty_rx - cur_rx; > - rx_left = min(rx_left, budget); > > - for (; rx_left > 0; rx_left--, cur_rx++) { > + for (rx_left = min(budget, NUM_RX_DESC); rx_left > 0; rx_left--, cur_rx++) { Is this really equivalent? I think cur_rx still needs to be incorporated into whatever you use as the initial rx_left. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David Miller <davem@davemloft.net> : [...] > > @@ -6035,10 +6034,8 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, u32 budget > > unsigned int count; > > > > cur_rx = tp->cur_rx; > > - rx_left = NUM_RX_DESC + tp->dirty_rx - cur_rx; > > - rx_left = min(rx_left, budget); > > > > - for (; rx_left > 0; rx_left--, cur_rx++) { > > + for (rx_left = min(budget, NUM_RX_DESC); rx_left > 0; rx_left--, cur_rx++) { > > Is this really equivalent? I think cur_rx still needs to be > incorporated into whatever you use as the initial rx_left. The rx ring can't contain hole so the code above must be read with "tp->cur_rx == tp->dirty_rx" in mind.
Timo Teräs <timo.teras@iki.fi> : > After commit 6f0333b ("r8169: use 50% less ram for RX ring") the rx > ring buffers are always copied making dirty_rx useless. > > Signed-off-by: Timo Teräs <timo.teras@iki.fi> Acked-by: Francois Romieu <romieu@fr.zoreil.com> Even if it is obviously not -net material, please specify the branch you aim at (eg [PATCH net-next] or [PATCH net]).
From: Francois Romieu <romieu@fr.zoreil.com> Date: Wed, 16 Jan 2013 23:16:40 +0100 > Timo Teräs <timo.teras@iki.fi> : >> After commit 6f0333b ("r8169: use 50% less ram for RX ring") the rx >> ring buffers are always copied making dirty_rx useless. >> >> Signed-off-by: Timo Teräs <timo.teras@iki.fi> > > Acked-by: Francois Romieu <romieu@fr.zoreil.com> Applied to net-next. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c index 89184f3..97fdbb1 100644 --- a/drivers/net/ethernet/realtek/r8169.c +++ b/drivers/net/ethernet/realtek/r8169.c @@ -83,7 +83,7 @@ static const int multicast_filter_limit = 32; #define R8169_REGS_SIZE 256 #define R8169_NAPI_WEIGHT 64 #define NUM_TX_DESC 64 /* Number of Tx descriptor registers */ -#define NUM_RX_DESC 256 /* Number of Rx descriptor registers */ +#define NUM_RX_DESC 256U /* Number of Rx descriptor registers */ #define R8169_TX_RING_BYTES (NUM_TX_DESC * sizeof(struct TxDesc)) #define R8169_RX_RING_BYTES (NUM_RX_DESC * sizeof(struct RxDesc)) @@ -727,7 +727,6 @@ struct rtl8169_private { u16 mac_version; u32 cur_rx; /* Index into the Rx descriptor buffer of next Rx pkt. */ u32 cur_tx; /* Index into the Tx descriptor buffer of next Rx pkt. */ - u32 dirty_rx; u32 dirty_tx; struct rtl8169_stats rx_stats; struct rtl8169_stats tx_stats; @@ -4177,7 +4176,7 @@ static void rtl_init_rxcfg(struct rtl8169_private *tp) static void rtl8169_init_ring_indexes(struct rtl8169_private *tp) { - tp->dirty_tx = tp->dirty_rx = tp->cur_tx = tp->cur_rx = 0; + tp->dirty_tx = tp->cur_tx = tp->cur_rx = 0; } static void rtl_hw_jumbo_enable(struct rtl8169_private *tp) @@ -5920,7 +5919,7 @@ static void rtl8169_pcierr_interrupt(struct net_device *dev) PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_SIG_TARGET_ABORT)); /* The infamous DAC f*ckup only happens at boot time */ - if ((tp->cp_cmd & PCIDAC) && !tp->dirty_rx && !tp->cur_rx) { + if ((tp->cp_cmd & PCIDAC) && !tp->cur_rx) { void __iomem *ioaddr = tp->mmio_addr; netif_info(tp, intr, dev, "disabling PCI DAC\n"); @@ -6035,10 +6034,8 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, u32 budget unsigned int count; cur_rx = tp->cur_rx; - rx_left = NUM_RX_DESC + tp->dirty_rx - cur_rx; - rx_left = min(rx_left, budget); - for (; rx_left > 0; rx_left--, cur_rx++) { + for (rx_left = min(budget, NUM_RX_DESC); rx_left > 0; rx_left--, cur_rx++) { unsigned int entry = cur_rx % NUM_RX_DESC; struct RxDesc *desc = tp->RxDescArray + entry; u32 status; @@ -6123,8 +6120,6 @@ process_pkt: count = cur_rx - tp->cur_rx; tp->cur_rx = cur_rx; - tp->dirty_rx += count; - return count; }
After commit 6f0333b ("r8169: use 50% less ram for RX ring") the rx ring buffers are always copied making dirty_rx useless. Signed-off-by: Timo Teräs <timo.teras@iki.fi> --- drivers/net/ethernet/realtek/r8169.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)