Skip to contentSkip to conversation

via-rhine receive buffers rework

PATCH In review Francois Romieu 6 participants 19 messages Apr 6, 2015 – Apr 8, 2015

Subsystems (3)
Networking Network drivers Via rhine network driver
lore
Share & tools
lore

Francois Romieu

author
#0 lore raw

cover via-rhine receive buffers rework

This is an hopefully readable rework of my patch from two days ago. It's different enough that it imho requires someone's Tested-by.

Patches #1..#3 are mostly there to slim patch #4. Patch #1 may make some sense as a standalone fix though.

The series applies without error against v3.19 - what Nix uses - and davem-next as of 5888b93b750609680735d6b8b737703083ef40ff ("Merge branch 'nf-hook-compress'").

Please test and review.

Francois Romieu (4):
via-rhine: commit receive buffer address before descriptor status update.
via-rhine: add allocation helpers.
via-rhine: gotoize rhine_open error path.
via-rhine: forbid holes in the receive descriptor ring.

 drivers/net/ethernet/via/via-rhine.c | 163 +++++++++++++++++++++--------------
 1 file changed, 100 insertions(+), 63 deletions(-)
Signature · 1 line
2.1.0
author
#1 lore raw

PATCH 1/4 via-rhine: commit receive buffer address before descriptor status update.

Signed-off-by Francois Romieu <[email protected]>
---
 drivers/net/ethernet/via/via-rhine.c | 1 +
 1 file changed, 1 insertion(+)
drivers/net/ethernet/via/via-rhine.c +1 -0
index a191afc..00fea3d 100644
@@ -2063,6 +2063,7 @@ static int rhine_rx(struct net_device *dev, int limit)
 				break;
 			}
 			rp->rx_ring[entry].addr = cpu_to_le32(rp->rx_skbuff_dma[entry]);
+			wmb();
 		}
 		rp->rx_ring[entry].rx_status = cpu_to_le32(DescOwn);
 	}
Signature · 1 line
2.1.0
maintainer
#2 lore raw

Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update.

From: Francois Romieu <[email protected]>
Date: Mon, 6 Apr 2015 20:01:49 +0200

Quoted 10 lines
Signed-off-by Francois Romieu <[email protected]>
---
 drivers/net/ethernet/via/via-rhine.c | 1 +
 1 file changed, 1 insertion(+)
drivers/net/ethernet/via/via-rhine.c +0 -0
index a191afc..00fea3d 100644

drivers/net/ethernet/via/via-rhine.c static int rhine_rx(struct net_device *dev, int limit) · line 2066

Quoted 4 lines
drivers/net/ethernet/via/via-rhine.c +1 -0
@@ -2063,6 +2063,7 @@ static int rhine_rx(struct net_device *dev, int limit)
 				break;
 			}
 			rp->rx_ring[entry].addr = cpu_to_le32(rp->rx_skbuff_dma[entry]);
+			wmb();

dma_wmb() perhaps? I think this is exactly the situation that interface was added for.

author
#3 lore raw

Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update.

David Miller <[email protected]> :

Quoted 1 line

From: Francois Romieu <[email protected]>

[...]

Quoted 8 lines

drivers/net/ethernet/via/via-rhine.c static int rhine_rx(struct net_device *dev, int limit) · line 2066

Quoted 5 lines
drivers/net/ethernet/via/via-rhine.c +1 -0
@@ -2063,6 +2063,7 @@ static int rhine_rx(struct net_device *dev, int limit)
 				break;
 			}
 			rp->rx_ring[entry].addr = cpu_to_le32(rp->rx_skbuff_dma[entry]);
+			wmb();

dma_wmb() perhaps? I think this is exactly the situation that interface was added for.

I need the buffer address to be written in the receive descriptor before the descriptor status is. The cpu does W1, W2 and the nic mustn't see W2, W1.

Signature · 1 line
Ueimor
#4 lore raw

Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update.

Quoted 10 lines · Francois Romieu

David Miller <[email protected]> :

Quoted 1 line

From: Francois Romieu <[email protected]>

[...]

Quoted 7 lines

drivers/net/ethernet/via/via-rhine.c static int rhine_rx(struct net_device *dev, int limit) · line 2066

Quoted 5 lines
drivers/net/ethernet/via/via-rhine.c +1 -0
@@ -2063,6 +2063,7 @@ static int rhine_rx(struct net_device *dev, int limit)
  				break;
  			}
  			rp->rx_ring[entry].addr = cpu_to_le32(rp->rx_skbuff_dma[entry]);
+			wmb();

dma_wmb() perhaps? I think this is exactly the situation that interface was added for.

Quoted 2 lines

I need the buffer address to be written in the receive descriptor before the descriptor status is. The cpu does W1, W2 and the nic mustn't see W2, W1.

That is the point of the dma_wmb(). If you are writing both W1 and W2 to system memory then dma_wmb should be enough, if W1 is system memory and W2 is device memory (MMIO) then you need wmb().

You can think of dma_wmb as being something similar to smp_wmb w/o the SMP processor requirement. On architectures that are strong ordered such as most x86 the wmb() translates to a barrier. On other architectures it is something usually a little lighter than a full wmb() so for example on PowerPC wmb is sync() while dma_wmb() is lwsync().

- Alex

maintainer
#5 lore raw

Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update.

In reply to Francois Romieu

From: Francois Romieu <[email protected]>
Date: Tue, 7 Apr 2015 23:02:48 +0200

Quoted 12 lines

David Miller <[email protected]> :

Quoted 1 line

From: Francois Romieu <[email protected]>

[...]

Quoted 8 lines

drivers/net/ethernet/via/via-rhine.c static int rhine_rx(struct net_device *dev, int limit) · line 2066

Quoted 5 lines
drivers/net/ethernet/via/via-rhine.c +1 -0
@@ -2063,6 +2063,7 @@ static int rhine_rx(struct net_device *dev, int limit)
 				break;
 			}
 			rp->rx_ring[entry].addr = cpu_to_le32(rp->rx_skbuff_dma[entry]);
+			wmb();

dma_wmb() perhaps? I think this is exactly the situation that interface was added for.

Quoted 2 lines

I need the buffer address to be written in the receive descriptor before the descriptor status is. The cpu does W1, W2 and the nic mustn't see W2, W1.

That's exactly dma_wmb().

It barriers cpu writes so that the device sees things in a certain order.

It's what all the most common ethernet chip drivers use in their descriptor handling routines now.

#6 lore raw

Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update.

Quoted 7 lines · David Miller

That's exactly dma_wmb().

It barriers cpu writes so that the device sees things in a certain order.

It's what all the most common ethernet chip drivers use in their descriptor handling routines now.

To be fair, only 2 drivers currently use dma_wmb()

#7 lore raw

Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update.

Quoted 10 lines · Eric Dumazet
Quoted 7 lines · David Miller

That's exactly dma_wmb().

It barriers cpu writes so that the device sees things in a certain order.

It's what all the most common ethernet chip drivers use in their descriptor handling routines now.

To be fair, only 2 drivers currently use dma_wmb()

Hey, I got at least 4.. :-) I only got around to patching 3 Intel drivers and one RealTek since that is what I had to test with. I was honestly hoping there would be more interest from other developers to pick this up and update their drivers to avoid unnecessary barriers but it doesn't look like I have had much luck on that front.

Maybe what I can do is submit a set of patches over the next couple of weeks to try and update all the spots that either need to have a barrier added, such as what is being addressed here, or can have a barrier weakened as I have already done for a few other drivers.

- Alex

#8 lore raw

Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update.

Quoted 9 lines · Alexander Duyck
Quoted 1 line · Eric Dumazet

To be fair, only 2 drivers currently use dma_wmb()

Hey, I got at least 4.. :-) I only got around to patching 3 Intel drivers and one RealTek since that is what I had to test with. I was honestly hoping there would be more interest from other developers to pick this up and update their drivers to avoid unnecessary barriers but it doesn't look like I have had much luck on that front.

Strange I see only 2 drivers here, and no Intel ones :

# git grep -l dma_wmb drivers/net
drivers/net/ethernet/amd/xgbe/xgbe-dev.c
drivers/net/ethernet/realtek/r8169.c

I probably missed something ;)

#9 lore raw

Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update.

Quoted 8 lines · Eric Dumazet
Quoted 7 lines · Alexander Duyck
Quoted 1 line · Eric Dumazet

To be fair, only 2 drivers currently use dma_wmb()

Hey, I got at least 4.. :-) I only got around to patching 3 Intel drivers and one RealTek since that is what I had to test with. I was honestly hoping there would be more interest from other developers to pick this up and update their drivers to avoid unnecessary barriers but it doesn't look like I have had much luck on that front.

Quoted 7 lines

Strange I see only 2 drivers here, and no Intel ones :

# git grep -l dma_wmb drivers/net
drivers/net/ethernet/amd/xgbe/xgbe-dev.c
drivers/net/ethernet/realtek/r8169.c

I probably missed something ;)

Yeah, I was thinking of the dma_rmb more than the dma_wmb, they are essentially just two sides to the same coin.

dma_rmb - read descriptor status for ownership, then dma_rmb, then if we own it process rest of descriptor dma_wmb - write descriptor fields, then dma_wmb, then update status bit to release ownership

A wmb is still needed if a PIO doorbell must be used to notify the device of additional descriptors.

- Alex

#10 lore raw

RE: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update.

From: Alexander Duyck

Quoted 1 line

Sent: 08 April 2015 00:21

...

Quoted 5 lines

dma_wmb - write descriptor fields, then dma_wmb, then update status bit to release ownership

A wmb is still needed if a PIO doorbell must be used to notify the device of additional descriptors.

That is needed after the write that releases ownership. So you are likely to need a dma_wmb() before the status bit update and a full wmb() just after it.

If you are adding a lot of frames then you should be able to update the status for the first fragment last - and only need the barriers on the final update.

David

#11 lore raw

Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update.

In reply to Francois Romieu
Quoted 10 lines · Francois Romieu
Signed-off-by Francois Romieu <[email protected]>
---
  drivers/net/ethernet/via/via-rhine.c | 1 +
  1 file changed, 1 insertion(+)
drivers/net/ethernet/via/via-rhine.c +0 -0
index a191afc..00fea3d 100644

drivers/net/ethernet/via/via-rhine.c static int rhine_rx(struct net_device *dev, int limit) · line 2069

Quoted 7 lines
drivers/net/ethernet/via/via-rhine.c +1 -0
@@ -2063,6 +2063,7 @@ static int rhine_rx(struct net_device *dev, int limit)
  				break;
  			}
  			rp->rx_ring[entry].addr = cpu_to_le32(rp->rx_skbuff_dma[entry]);
+			wmb();
  		}
  		rp->rx_ring[entry].rx_status = cpu_to_le32(DescOwn);
  	}

Do you need a full wmb() here or would a dma_wmb() be enough? It could make a difference on some processor architectures.

- Alex

author
#12 lore raw

PATCH 2/4 via-rhine: add allocation helpers.

In reply to Francois Romieu
Signed-off-by Francois Romieu <[email protected]>
---
 drivers/net/ethernet/via/via-rhine.c | 57 +++++++++++++++++++++++++++---------
 1 file changed, 43 insertions(+), 14 deletions(-)
drivers/net/ethernet/via/via-rhine.c +43 -14
index 00fea3d..cf8dfcd 100644
@@ -1211,12 +1211,47 @@ static void free_ring(struct net_device* dev)
 
 }
 
-static void alloc_rbufs(struct net_device *dev)
+struct rhine_skb_dma {
+	struct sk_buff *skb;
+	dma_addr_t dma;
+};
+
+static inline int rhine_skb_dma_init(struct net_device *dev,
+				     struct rhine_skb_dma *sd)
 {
 	struct rhine_private *rp = netdev_priv(dev);
 	struct device *hwdev = dev->dev.parent;
+	const int size = rp->rx_buf_sz;
+
+	sd->skb = netdev_alloc_skb(dev, size);
+	if (!sd->skb)
+		return -ENOMEM;
+
+	sd->dma = dma_map_single(hwdev, sd->skb->data, size, DMA_FROM_DEVICE);
+	if (unlikely(dma_mapping_error(hwdev, sd->dma))) {
+		netif_err(rp, drv, dev, "Rx DMA mapping failure\n");
+		dev_kfree_skb_any(sd->skb);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static inline void rhine_skb_dma_nic_store(struct rhine_private *rp,
+					   struct rhine_skb_dma *sd, int entry)
+{
+	rp->rx_skbuff_dma[entry] = sd->dma;
+	rp->rx_skbuff[entry] = sd->skb;
+
+	rp->rx_ring[entry].addr = cpu_to_le32(sd->dma);
+	wmb();
+}
+
+static void alloc_rbufs(struct net_device *dev)
+{
+	struct rhine_private *rp = netdev_priv(dev);
 	dma_addr_t next;
-	int i;
+	int rc, i;
 
 	rp->dirty_rx = rp->cur_rx = 0;
 

@@ -1237,20 +1272,14 @@ static void alloc_rbufs(struct net_device *dev)
 
 	/* Fill in the Rx buffers.  Handle allocation failure gracefully. */
 	for (i = 0; i < RX_RING_SIZE; i++) {
-		struct sk_buff *skb = netdev_alloc_skb(dev, rp->rx_buf_sz);
-		rp->rx_skbuff[i] = skb;
-		if (skb == NULL)
-			break;
+		struct rhine_skb_dma sd;
 
-		rp->rx_skbuff_dma[i] =
-			dma_map_single(hwdev, skb->data, rp->rx_buf_sz,
-				       DMA_FROM_DEVICE);
-		if (dma_mapping_error(hwdev, rp->rx_skbuff_dma[i])) {
-			rp->rx_skbuff_dma[i] = 0;
-			dev_kfree_skb(skb);
+		rc = rhine_skb_dma_init(dev, &sd);
+		if (rc < 0)
 			break;
-		}
-		rp->rx_ring[i].addr = cpu_to_le32(rp->rx_skbuff_dma[i]);
+
+		rhine_skb_dma_nic_store(rp, &sd, i);
+
 		rp->rx_ring[i].rx_status = cpu_to_le32(DescOwn);
 	}
 	rp->dirty_rx = (unsigned int)(i - RX_RING_SIZE);
Signature · 1 line
2.1.0
author
#13 lore raw

PATCH 3/4 via-rhine: gotoize rhine_open error path.

In reply to Francois Romieu
Signed-off-by Francois Romieu <[email protected]>
---
 drivers/net/ethernet/via/via-rhine.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
drivers/net/ethernet/via/via-rhine.c +10 -6
index cf8dfcd..ae1ab6b 100644
@@ -1684,15 +1684,14 @@ static int rhine_open(struct net_device *dev)
 
 	rc = request_irq(rp->irq, rhine_interrupt, IRQF_SHARED, dev->name, dev);
 	if (rc)
-		return rc;
+		goto out;
 
 	netif_dbg(rp, ifup, dev, "%s() irq %d\n", __func__, rp->irq);
 
 	rc = alloc_ring(dev);
-	if (rc) {
-		free_irq(rp->irq, dev);
-		return rc;
-	}
+	if (rc < 0)
+		goto out_free_irq;
+
 	alloc_rbufs(dev);
 	alloc_tbufs(dev);
 	rhine_chip_reset(dev);

@@ -1705,7 +1704,12 @@ static int rhine_open(struct net_device *dev)
 
 	netif_start_queue(dev);
 
-	return 0;
+out:
+	return rc;
+
+out_free_irq:
+	free_irq(rp->irq, dev);
+	goto out;
 }
 
 static void rhine_reset_task(struct work_struct *work)
Signature · 1 line
2.1.0
author
#14 lore raw

PATCH 4/4 via-rhine: forbid holes in the receive descriptor ring.

In reply to Francois Romieu

Rationales:
- throttle work under memory pressure;
- lower receive descriptor recycling latency for the network adapter;
- lower the maintenance burden of uncommon paths.

The patch is twofold:
1. it fails early if the receive buffer ring can't be completely initialized at dev->open() time
2. it drops packets on the floor in the napi receive handler so as to keep the ring full.

Signed-off-by Francois Romieu <[email protected]>
---
 drivers/net/ethernet/via/via-rhine.c | 101 ++++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 49 deletions(-)
drivers/net/ethernet/via/via-rhine.c +52 -49
index ae1ab6b..94729b1 100644
@@ -471,7 +471,7 @@ struct rhine_private {
 	/* Frequently used values: keep some adjacent for cache effect. */
 	u32 quirks;
 	struct rx_desc *rx_head_desc;
-	unsigned int cur_rx, dirty_rx;	/* Producer/consumer ring indices */
+	unsigned int cur_rx;
 	unsigned int cur_tx, dirty_tx;
 	unsigned int rx_buf_sz;		/* Based on MTU+slack. */
 	struct rhine_stats rx_stats;

@@ -1237,6 +1237,16 @@ static inline int rhine_skb_dma_init(struct net_device *dev,
 	return 0;
 }
 
+static void rhine_reset_rbufs(struct rhine_private *rp)
+{
+	int i;
+
+	rp->cur_rx = 0;
+
+	for (i = 0; i < RX_RING_SIZE; i++)
+		rp->rx_ring[i].rx_status = cpu_to_le32(DescOwn);
+}
+
 static inline void rhine_skb_dma_nic_store(struct rhine_private *rp,
 					   struct rhine_skb_dma *sd, int entry)
 {

@@ -1247,14 +1257,14 @@ static inline void rhine_skb_dma_nic_store(struct rhine_private *rp,
 	wmb();
 }
 
-static void alloc_rbufs(struct net_device *dev)
+static void free_rbufs(struct net_device* dev);
+
+static int alloc_rbufs(struct net_device *dev)
 {
 	struct rhine_private *rp = netdev_priv(dev);
 	dma_addr_t next;
 	int rc, i;
 
-	rp->dirty_rx = rp->cur_rx = 0;
-
 	rp->rx_buf_sz = (dev->mtu <= 1500 ? PKT_BUF_SZ : dev->mtu + 32);
 	rp->rx_head_desc = &rp->rx_ring[0];
 	next = rp->rx_ring_dma;

@@ -1275,14 +1285,17 @@ static void alloc_rbufs(struct net_device *dev)
 		struct rhine_skb_dma sd;
 
 		rc = rhine_skb_dma_init(dev, &sd);
-		if (rc < 0)
-			break;
+		if (rc < 0) {
+			free_rbufs(dev);
+			goto out;
+		}
 
 		rhine_skb_dma_nic_store(rp, &sd, i);
-
-		rp->rx_ring[i].rx_status = cpu_to_le32(DescOwn);
 	}
-	rp->dirty_rx = (unsigned int)(i - RX_RING_SIZE);
+
+	rhine_reset_rbufs(rp);
+out:
+	return rc;
 }
 
 static void free_rbufs(struct net_device* dev)

@@ -1692,7 +1705,10 @@ static int rhine_open(struct net_device *dev)
 	if (rc < 0)
 		goto out_free_irq;
 
-	alloc_rbufs(dev);
+	rc = alloc_rbufs(dev);
+	if (rc < 0)
+		goto out_free_ring;
+
 	alloc_tbufs(dev);
 	rhine_chip_reset(dev);
 	rhine_task_enable(rp);

@@ -1707,6 +1723,8 @@ static int rhine_open(struct net_device *dev)
 out:
 	return rc;
 
+out_free_ring:
+	free_ring(dev);
 out_free_irq:
 	free_irq(rp->irq, dev);
 	goto out;

@@ -1729,9 +1747,9 @@ static void rhine_reset_task(struct work_struct *work)
 
 	/* clear all descriptors */
 	free_tbufs(dev);
-	free_rbufs(dev);
 	alloc_tbufs(dev);
-	alloc_rbufs(dev);
+
+	rhine_reset_rbufs(rp);
 
 	/* Reinitialize the hardware. */
 	rhine_chip_reset(dev);

@@ -2021,16 +2039,18 @@ static int rhine_rx(struct net_device *dev, int limit)
 				}
 			}
 		} else {
-			struct sk_buff *skb = NULL;
 			/* Length should omit the CRC */
 			int pkt_len = data_size - 4;
+			struct sk_buff *skb;
 			u16 vlan_tci = 0;
 
 			/* Check if the packet is long enough to accept without
 			   copying to a minimally-sized skbuff. */
-			if (pkt_len < rx_copybreak)
+			if (pkt_len < rx_copybreak) {
 				skb = netdev_alloc_skb_ip_align(dev, pkt_len);
-			if (skb) {
+				if (unlikely(!skb))
+					goto drop;
+
 				dma_sync_single_for_cpu(hwdev,
 							rp->rx_skbuff_dma[entry],
 							rp->rx_buf_sz,

@@ -2039,25 +2059,28 @@ static int rhine_rx(struct net_device *dev, int limit)
 				skb_copy_to_linear_data(skb,
 						 rp->rx_skbuff[entry]->data,
 						 pkt_len);
-				skb_put(skb, pkt_len);
+
 				dma_sync_single_for_device(hwdev,
 							   rp->rx_skbuff_dma[entry],
 							   rp->rx_buf_sz,
 							   DMA_FROM_DEVICE);
 			} else {
+				struct rhine_skb_dma sd;
+
+				if (unlikely(rhine_skb_dma_init(dev, &sd) < 0))
+					goto drop;
+
 				skb = rp->rx_skbuff[entry];
-				if (skb == NULL) {
-					netdev_err(dev, "Inconsistent Rx descriptor chain\n");
-					break;
-				}
-				rp->rx_skbuff[entry] = NULL;
-				skb_put(skb, pkt_len);
+
 				dma_unmap_single(hwdev,
 						 rp->rx_skbuff_dma[entry],
 						 rp->rx_buf_sz,
 						 DMA_FROM_DEVICE);
+				rhine_skb_dma_nic_store(rp, &sd, entry);
 			}
 
+			skb_put(skb, pkt_len);
+
 			if (unlikely(desc_length & DescTag))
 				vlan_tci = rhine_get_vlan_tci(skb, data_size);
 

@@ -2072,36 +2095,17 @@ static int rhine_rx(struct net_device *dev, int limit)
 			rp->rx_stats.packets++;
 			u64_stats_update_end(&rp->rx_stats.syncp);
 		}
+give_descriptor_to_nic:
+		rp->rx_ring[entry].rx_status = cpu_to_le32(DescOwn);
 		entry = (++rp->cur_rx) % RX_RING_SIZE;
 		rp->rx_head_desc = &rp->rx_ring[entry];
 	}
 
-	/* Refill the Rx ring buffers. */
-	for (; rp->cur_rx - rp->dirty_rx > 0; rp->dirty_rx++) {
-		struct sk_buff *skb;
-		entry = rp->dirty_rx % RX_RING_SIZE;
-		if (rp->rx_skbuff[entry] == NULL) {
-			skb = netdev_alloc_skb(dev, rp->rx_buf_sz);
-			rp->rx_skbuff[entry] = skb;
-			if (skb == NULL)
-				break;	/* Better luck next round. */
-			rp->rx_skbuff_dma[entry] =
-				dma_map_single(hwdev, skb->data,
-					       rp->rx_buf_sz,
-					       DMA_FROM_DEVICE);
-			if (dma_mapping_error(hwdev,
-					      rp->rx_skbuff_dma[entry])) {
-				dev_kfree_skb(skb);
-				rp->rx_skbuff_dma[entry] = 0;
-				break;
-			}
-			rp->rx_ring[entry].addr = cpu_to_le32(rp->rx_skbuff_dma[entry]);
-			wmb();
-		}
-		rp->rx_ring[entry].rx_status = cpu_to_le32(DescOwn);
-	}
-
 	return count;
+
+drop:
+	dev->stats.rx_dropped++;
+	goto give_descriptor_to_nic;
 }
 
 static void rhine_restart_tx(struct net_device *dev) {

@@ -2506,9 +2510,8 @@ static int rhine_resume(struct device *device)
 	enable_mmio(rp->pioaddr, rp->quirks);
 	rhine_power_init(dev);
 	free_tbufs(dev);
-	free_rbufs(dev);
 	alloc_tbufs(dev);
-	alloc_rbufs(dev);
+	rhine_reset_rbufs(rp);
 	rhine_task_enable(rp);
 	spin_lock_bh(&rp->lock);
 	init_registers(dev);
Signature · 1 line
2.1.0
#15 lore raw
In reply to Francois Romieu

On 6 Apr 2015, Francois Romieu outgrape:

Quoted 2 lines

This is an hopefully readable rework of my patch from two days ago. It's different enough that it imho requires someone's Tested-by.

I'll give it a torturing tomorrow.

Signature · 1 line
NULL && (void)
#16 lore raw

On 6 Apr 2015, [email protected] verbalised:

Quoted 6 lines

On 6 Apr 2015, Francois Romieu outgrape:

Quoted 2 lines

This is an hopefully readable rework of my patch from two days ago. It's different enough that it imho requires someone's Tested-by.

I'll give it a torturing tomorrow.

Tormented by routing a simultaneous iperf and make-over-nfs through it (with the make using ccache, and the ccache cache directory *also* accessed via nfs, to hit it really hard :) ).

No crashes in several hours of running: looks good!

Signature · 1 line
NULL && (void)
author
#17 lore raw

Nix <[email protected]> :
[...]

Quoted 1 line

No crashes in several hours of running: looks good!

Mildly. :o/

You may never notice if you don't resume and the reset work queue doesn't run but patch #4 lacks the bits below.

(rx_head_desc should go away)

drivers/net/ethernet/via/via-rhine.c +1 -1
index a54a9f4..7f31c3a 100644
@@ -1244,6 +1244,7 @@ static void rhine_reset_rbufs(struct rhine_private *rp)
 	int i;
 
 	rp->cur_rx = 0;
+	rp->rx_head_desc = rp->rx_ring;
 
 	for (i = 0; i < RX_RING_SIZE; i++)
 		rp->rx_ring[i].rx_status = cpu_to_le32(DescOwn);

@@ -1268,7 +1269,6 @@ static int alloc_rbufs(struct net_device *dev)
 	int rc, i;
 
 	rp->rx_buf_sz = (dev->mtu <= 1500 ? PKT_BUF_SZ : dev->mtu + 32);
-	rp->rx_head_desc = &rp->rx_ring[0];
 	next = rp->rx_ring_dma;
 
 	/* Init the ring entries */
#18 lore raw

On 7 Apr 2015, Francois Romieu stated:

Quoted 8 lines

Nix <[email protected]> :
[...]

Quoted 1 line

No crashes in several hours of running: looks good!

Mildly. :o/

You may never notice if you don't resume and the reset work queue doesn't run but patch #4 lacks the bits below.

I don't suspend this box, no, it uses only 5W of power and it's my link to the internet so suspending it would be a bad move :)

Quoted 1 line

(rx_head_desc should go away)

I'll test this out on top of the other stuff tomorrow...

Signature · 1 line
NULL && (void)