Patchwork r8169: remove unneeded dirty_rx index

login
register
mail settings
Submitter Timo Teräs
Date Jan. 16, 2013, 7:01 a.m.
Message ID <1358319684-32364-1-git-send-email-timo.teras@iki.fi>
Download mbox | patch
Permalink /patch/212417/
State Accepted
Delegated to: David Miller
Headers show

Comments

Timo Teräs - Jan. 16, 2013, 7:01 a.m.
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(-)
David Miller - Jan. 16, 2013, 9:25 p.m.
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
françois romieu - Jan. 16, 2013, 9:26 p.m.
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.
françois romieu - Jan. 16, 2013, 10:16 p.m.
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]).
David Miller - Jan. 16, 2013, 11:02 p.m.
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

Patch

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;
 }