* [PATCH RFT net-next 0/4] via-rhine receive buffers rework
@ 2015-04-06 18:01 Francois Romieu
2015-04-06 18:01 ` [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update Francois Romieu
` (4 more replies)
0 siblings, 5 replies; 19+ messages in thread
From: Francois Romieu @ 2015-04-06 18:01 UTC (permalink / raw)
To: netdev; +Cc: Nix, David S. Miller, rl, Bjarke Istrup Pedersen
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(-)
--
2.1.0
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update. 2015-04-06 18:01 [PATCH RFT net-next 0/4] via-rhine receive buffers rework Francois Romieu @ 2015-04-06 18:01 ` Francois Romieu 2015-04-07 19:52 ` David Miller 2015-04-07 19:54 ` Alexander Duyck 2015-04-06 18:01 ` [PATCH net-next 2/4] via-rhine: add allocation helpers Francois Romieu ` (3 subsequent siblings) 4 siblings, 2 replies; 19+ messages in thread From: Francois Romieu @ 2015-04-06 18:01 UTC (permalink / raw) To: netdev; +Cc: Nix, David S. Miller, rl, Bjarke Istrup Pedersen Signed-off-by: Francois Romieu <romieu@fr.zoreil.com> --- drivers/net/ethernet/via/via-rhine.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c index a191afc..00fea3d 100644 --- a/drivers/net/ethernet/via/via-rhine.c +++ b/drivers/net/ethernet/via/via-rhine.c @@ -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); } -- 2.1.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update. 2015-04-06 18:01 ` [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update Francois Romieu @ 2015-04-07 19:52 ` David Miller 2015-04-07 21:02 ` Francois Romieu 2015-04-07 19:54 ` Alexander Duyck 1 sibling, 1 reply; 19+ messages in thread From: David Miller @ 2015-04-07 19:52 UTC (permalink / raw) To: romieu; +Cc: netdev, nix, rl, gurligebis From: Francois Romieu <romieu@fr.zoreil.com> Date: Mon, 6 Apr 2015 20:01:49 +0200 > Signed-off-by: Francois Romieu <romieu@fr.zoreil.com> > --- > drivers/net/ethernet/via/via-rhine.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c > index a191afc..00fea3d 100644 > --- a/drivers/net/ethernet/via/via-rhine.c > +++ b/drivers/net/ethernet/via/via-rhine.c > @@ -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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update. 2015-04-07 19:52 ` David Miller @ 2015-04-07 21:02 ` Francois Romieu 2015-04-07 21:19 ` Alexander Duyck 2015-04-07 21:27 ` David Miller 0 siblings, 2 replies; 19+ messages in thread From: Francois Romieu @ 2015-04-07 21:02 UTC (permalink / raw) To: David Miller; +Cc: netdev, nix, rl, gurligebis, Alexander Duyck David Miller <davem@davemloft.net> : > From: Francois Romieu <romieu@fr.zoreil.com> [...] > > @@ -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. -- Ueimor ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update. 2015-04-07 21:02 ` Francois Romieu @ 2015-04-07 21:19 ` Alexander Duyck 2015-04-07 21:27 ` David Miller 1 sibling, 0 replies; 19+ messages in thread From: Alexander Duyck @ 2015-04-07 21:19 UTC (permalink / raw) To: Francois Romieu, David Miller; +Cc: netdev, nix, rl, gurligebis On 04/07/2015 02:02 PM, Francois Romieu wrote: > David Miller <davem@davemloft.net> : >> From: Francois Romieu <romieu@fr.zoreil.com> > [...] >>> @@ -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. 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update. 2015-04-07 21:02 ` Francois Romieu 2015-04-07 21:19 ` Alexander Duyck @ 2015-04-07 21:27 ` David Miller 2015-04-07 21:54 ` Eric Dumazet 1 sibling, 1 reply; 19+ messages in thread From: David Miller @ 2015-04-07 21:27 UTC (permalink / raw) To: romieu; +Cc: netdev, nix, rl, gurligebis, alexander.h.duyck From: Francois Romieu <romieu@fr.zoreil.com> Date: Tue, 7 Apr 2015 23:02:48 +0200 > David Miller <davem@davemloft.net> : >> From: Francois Romieu <romieu@fr.zoreil.com> > [...] >> > @@ -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. 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update. 2015-04-07 21:27 ` David Miller @ 2015-04-07 21:54 ` Eric Dumazet 2015-04-07 22:12 ` Alexander Duyck 0 siblings, 1 reply; 19+ messages in thread From: Eric Dumazet @ 2015-04-07 21:54 UTC (permalink / raw) To: David Miller; +Cc: romieu, netdev, nix, rl, gurligebis, alexander.h.duyck On Tue, 2015-04-07 at 17:27 -0400, David Miller wrote: > 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() ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update. 2015-04-07 21:54 ` Eric Dumazet @ 2015-04-07 22:12 ` Alexander Duyck 2015-04-07 22:22 ` Eric Dumazet 0 siblings, 1 reply; 19+ messages in thread From: Alexander Duyck @ 2015-04-07 22:12 UTC (permalink / raw) To: Eric Dumazet, David Miller; +Cc: romieu, netdev, nix, rl, gurligebis On 04/07/2015 02:54 PM, Eric Dumazet wrote: > On Tue, 2015-04-07 at 17:27 -0400, David Miller wrote: > >> 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update. 2015-04-07 22:12 ` Alexander Duyck @ 2015-04-07 22:22 ` Eric Dumazet 2015-04-07 23:20 ` Alexander Duyck 0 siblings, 1 reply; 19+ messages in thread From: Eric Dumazet @ 2015-04-07 22:22 UTC (permalink / raw) To: Alexander Duyck; +Cc: David Miller, romieu, netdev, nix, rl, gurligebis On Tue, 2015-04-07 at 15:12 -0700, Alexander Duyck wrote: > On 04/07/2015 02:54 PM, Eric Dumazet wrote: > > 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 ;) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update. 2015-04-07 22:22 ` Eric Dumazet @ 2015-04-07 23:20 ` Alexander Duyck 2015-04-08 11:40 ` David Laight 0 siblings, 1 reply; 19+ messages in thread From: Alexander Duyck @ 2015-04-07 23:20 UTC (permalink / raw) To: Eric Dumazet; +Cc: David Miller, romieu, netdev, nix, rl, gurligebis On 04/07/2015 03:22 PM, Eric Dumazet wrote: > On Tue, 2015-04-07 at 15:12 -0700, Alexander Duyck wrote: >> On 04/07/2015 02:54 PM, Eric Dumazet wrote: >>> 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 ;) 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* RE: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update. 2015-04-07 23:20 ` Alexander Duyck @ 2015-04-08 11:40 ` David Laight 0 siblings, 0 replies; 19+ messages in thread From: David Laight @ 2015-04-08 11:40 UTC (permalink / raw) To: 'Alexander Duyck', Eric Dumazet Cc: David Miller, romieu, netdev, nix, rl, gurligebis From: Alexander Duyck > Sent: 08 April 2015 00:21 ... > 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update. 2015-04-06 18:01 ` [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update Francois Romieu 2015-04-07 19:52 ` David Miller @ 2015-04-07 19:54 ` Alexander Duyck 1 sibling, 0 replies; 19+ messages in thread From: Alexander Duyck @ 2015-04-07 19:54 UTC (permalink / raw) To: Francois Romieu, netdev; +Cc: Nix, David S. Miller, rl, Bjarke Istrup Pedersen On 04/06/2015 11:01 AM, Francois Romieu wrote: > Signed-off-by: Francois Romieu <romieu@fr.zoreil.com> > --- > drivers/net/ethernet/via/via-rhine.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c > index a191afc..00fea3d 100644 > --- a/drivers/net/ethernet/via/via-rhine.c > +++ b/drivers/net/ethernet/via/via-rhine.c > @@ -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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH net-next 2/4] via-rhine: add allocation helpers. 2015-04-06 18:01 [PATCH RFT net-next 0/4] via-rhine receive buffers rework Francois Romieu 2015-04-06 18:01 ` [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update Francois Romieu @ 2015-04-06 18:01 ` Francois Romieu 2015-04-06 18:01 ` [PATCH net-next 3/4] via-rhine: gotoize rhine_open error path Francois Romieu ` (2 subsequent siblings) 4 siblings, 0 replies; 19+ messages in thread From: Francois Romieu @ 2015-04-06 18:01 UTC (permalink / raw) To: netdev; +Cc: Nix, David S. Miller, rl, Bjarke Istrup Pedersen Signed-off-by: Francois Romieu <romieu@fr.zoreil.com> --- drivers/net/ethernet/via/via-rhine.c | 57 +++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c index 00fea3d..cf8dfcd 100644 --- a/drivers/net/ethernet/via/via-rhine.c +++ b/drivers/net/ethernet/via/via-rhine.c @@ -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); -- 2.1.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH net-next 3/4] via-rhine: gotoize rhine_open error path. 2015-04-06 18:01 [PATCH RFT net-next 0/4] via-rhine receive buffers rework Francois Romieu 2015-04-06 18:01 ` [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update Francois Romieu 2015-04-06 18:01 ` [PATCH net-next 2/4] via-rhine: add allocation helpers Francois Romieu @ 2015-04-06 18:01 ` Francois Romieu 2015-04-06 18:01 ` [PATCH net-next 4/4] via-rhine: forbid holes in the receive descriptor ring Francois Romieu 2015-04-06 21:46 ` [PATCH RFT net-next 0/4] via-rhine receive buffers rework Nix 4 siblings, 0 replies; 19+ messages in thread From: Francois Romieu @ 2015-04-06 18:01 UTC (permalink / raw) To: netdev; +Cc: Nix, David S. Miller, rl, Bjarke Istrup Pedersen Signed-off-by: Francois Romieu <romieu@fr.zoreil.com> --- drivers/net/ethernet/via/via-rhine.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c index cf8dfcd..ae1ab6b 100644 --- a/drivers/net/ethernet/via/via-rhine.c +++ b/drivers/net/ethernet/via/via-rhine.c @@ -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) -- 2.1.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH net-next 4/4] via-rhine: forbid holes in the receive descriptor ring. 2015-04-06 18:01 [PATCH RFT net-next 0/4] via-rhine receive buffers rework Francois Romieu ` (2 preceding siblings ...) 2015-04-06 18:01 ` [PATCH net-next 3/4] via-rhine: gotoize rhine_open error path Francois Romieu @ 2015-04-06 18:01 ` Francois Romieu 2015-04-06 21:46 ` [PATCH RFT net-next 0/4] via-rhine receive buffers rework Nix 4 siblings, 0 replies; 19+ messages in thread From: Francois Romieu @ 2015-04-06 18:01 UTC (permalink / raw) To: netdev; +Cc: Nix, David S. Miller, rl, Bjarke Istrup Pedersen 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 <romieu@fr.zoreil.com> --- drivers/net/ethernet/via/via-rhine.c | 101 ++++++++++++++++++----------------- 1 file changed, 52 insertions(+), 49 deletions(-) diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c index ae1ab6b..94729b1 100644 --- a/drivers/net/ethernet/via/via-rhine.c +++ b/drivers/net/ethernet/via/via-rhine.c @@ -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); -- 2.1.0 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH RFT net-next 0/4] via-rhine receive buffers rework 2015-04-06 18:01 [PATCH RFT net-next 0/4] via-rhine receive buffers rework Francois Romieu ` (3 preceding siblings ...) 2015-04-06 18:01 ` [PATCH net-next 4/4] via-rhine: forbid holes in the receive descriptor ring Francois Romieu @ 2015-04-06 21:46 ` Nix 2015-04-07 10:46 ` Nix 4 siblings, 1 reply; 19+ messages in thread From: Nix @ 2015-04-06 21:46 UTC (permalink / raw) To: Francois Romieu; +Cc: netdev, David S. Miller, rl, Bjarke Istrup Pedersen On 6 Apr 2015, Francois Romieu outgrape: > 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. -- NULL && (void) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RFT net-next 0/4] via-rhine receive buffers rework 2015-04-06 21:46 ` [PATCH RFT net-next 0/4] via-rhine receive buffers rework Nix @ 2015-04-07 10:46 ` Nix 2015-04-07 19:31 ` Francois Romieu 0 siblings, 1 reply; 19+ messages in thread From: Nix @ 2015-04-07 10:46 UTC (permalink / raw) To: Francois Romieu; +Cc: netdev, David S. Miller, rl, Bjarke Istrup Pedersen On 6 Apr 2015, nix@esperi.org.uk verbalised: > On 6 Apr 2015, Francois Romieu outgrape: > >> 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! -- NULL && (void) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RFT net-next 0/4] via-rhine receive buffers rework 2015-04-07 10:46 ` Nix @ 2015-04-07 19:31 ` Francois Romieu 2015-04-07 19:41 ` Nix 0 siblings, 1 reply; 19+ messages in thread From: Francois Romieu @ 2015-04-07 19:31 UTC (permalink / raw) To: Nix; +Cc: netdev, David S. Miller, rl, Bjarke Istrup Pedersen Nix <nix@esperi.org.uk> : [...] > 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) diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c index a54a9f4..7f31c3a 100644 --- a/drivers/net/ethernet/via/via-rhine.c +++ b/drivers/net/ethernet/via/via-rhine.c @@ -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 */ ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH RFT net-next 0/4] via-rhine receive buffers rework 2015-04-07 19:31 ` Francois Romieu @ 2015-04-07 19:41 ` Nix 0 siblings, 0 replies; 19+ messages in thread From: Nix @ 2015-04-07 19:41 UTC (permalink / raw) To: Francois Romieu; +Cc: netdev, David S. Miller, rl, Bjarke Istrup Pedersen On 7 Apr 2015, Francois Romieu stated: > Nix <nix@esperi.org.uk> : > [...] >> 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 :) > (rx_head_desc should go away) I'll test this out on top of the other stuff tomorrow... -- NULL && (void) ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2015-04-08 11:41 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-06 18:01 [PATCH RFT net-next 0/4] via-rhine receive buffers rework Francois Romieu 2015-04-06 18:01 ` [PATCH net-next 1/4] via-rhine: commit receive buffer address before descriptor status update Francois Romieu 2015-04-07 19:52 ` David Miller 2015-04-07 21:02 ` Francois Romieu 2015-04-07 21:19 ` Alexander Duyck 2015-04-07 21:27 ` David Miller 2015-04-07 21:54 ` Eric Dumazet 2015-04-07 22:12 ` Alexander Duyck 2015-04-07 22:22 ` Eric Dumazet 2015-04-07 23:20 ` Alexander Duyck 2015-04-08 11:40 ` David Laight 2015-04-07 19:54 ` Alexander Duyck 2015-04-06 18:01 ` [PATCH net-next 2/4] via-rhine: add allocation helpers Francois Romieu 2015-04-06 18:01 ` [PATCH net-next 3/4] via-rhine: gotoize rhine_open error path Francois Romieu 2015-04-06 18:01 ` [PATCH net-next 4/4] via-rhine: forbid holes in the receive descriptor ring Francois Romieu 2015-04-06 21:46 ` [PATCH RFT net-next 0/4] via-rhine receive buffers rework Nix 2015-04-07 10:46 ` Nix 2015-04-07 19:31 ` Francois Romieu 2015-04-07 19:41 ` Nix
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).