* [PATCH net-next #3 1/8] via-rhine: commit receive buffer address before descriptor status update.
2015-05-01 20:14 [PATCH net-next #3 0/8] via-rhine rework Francois Romieu
@ 2015-05-01 20:14 ` Francois Romieu
2015-05-01 20:14 ` [PATCH net-next #3 2/8] via-rhine: allocate and map receive buffer in a single transaction Francois Romieu
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Francois Romieu @ 2015-05-01 20:14 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Nix, Roger.Luethi.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 de28504..88b45c7 100644
--- a/drivers/net/ethernet/via/via-rhine.c
+++ b/drivers/net/ethernet/via/via-rhine.c
@@ -2075,6 +2075,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]);
+ dma_wmb();
}
rp->rx_ring[entry].rx_status = cpu_to_le32(DescOwn);
}
--
Ueimor
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next #3 2/8] via-rhine: allocate and map receive buffer in a single transaction
2015-05-01 20:14 [PATCH net-next #3 0/8] via-rhine rework Francois Romieu
2015-05-01 20:14 ` [PATCH net-next #3 1/8] via-rhine: commit receive buffer address before descriptor status update Francois Romieu
@ 2015-05-01 20:14 ` Francois Romieu
2015-05-01 20:14 ` [PATCH net-next #3 3/8] via-rhine: gotoize rhine_open error path Francois Romieu
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Francois Romieu @ 2015-05-01 20:14 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Nix, Roger.Luethi.rl, Bjarke Istrup Pedersen
It's used to initialize the receive ring but it will actually shine when
the receive poll code is reworked.
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 88b45c7..aa398ea 100644
--- a/drivers/net/ethernet/via/via-rhine.c
+++ b/drivers/net/ethernet/via/via-rhine.c
@@ -1213,12 +1213,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);
+ 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;
@@ -1239,20 +1274,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);
--
Ueimor
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next #3 3/8] via-rhine: gotoize rhine_open error path.
2015-05-01 20:14 [PATCH net-next #3 0/8] via-rhine rework Francois Romieu
2015-05-01 20:14 ` [PATCH net-next #3 1/8] via-rhine: commit receive buffer address before descriptor status update Francois Romieu
2015-05-01 20:14 ` [PATCH net-next #3 2/8] via-rhine: allocate and map receive buffer in a single transaction Francois Romieu
@ 2015-05-01 20:14 ` Francois Romieu
2015-05-01 20:14 ` [PATCH net-next #3 4/8] via-rhine: forbid holes in the receive descriptor ring Francois Romieu
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Francois Romieu @ 2015-05-01 20:14 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Nix, Roger.Luethi.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 aa398ea..91661e0 100644
--- a/drivers/net/ethernet/via/via-rhine.c
+++ b/drivers/net/ethernet/via/via-rhine.c
@@ -1688,15 +1688,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);
@@ -1709,7 +1708,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)
--
Ueimor
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next #3 4/8] via-rhine: forbid holes in the receive descriptor ring.
2015-05-01 20:14 [PATCH net-next #3 0/8] via-rhine rework Francois Romieu
` (2 preceding siblings ...)
2015-05-01 20:14 ` [PATCH net-next #3 3/8] via-rhine: gotoize rhine_open error path Francois Romieu
@ 2015-05-01 20:14 ` Francois Romieu
2015-05-01 20:14 ` [PATCH net-next #3 5/8] via-rhine: kiss rx_head_desc goodbye Francois Romieu
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Francois Romieu @ 2015-05-01 20:14 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Nix, Roger.Luethi.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:
- it fails early if the receive ring can't be completely initialized
at dev->open() time
- it drops packets on the floor in the napi receive handler so as to
keep the received ring full
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
drivers/net/ethernet/via/via-rhine.c | 103 ++++++++++++++++++-----------------
1 file changed, 53 insertions(+), 50 deletions(-)
diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
index 91661e0..8d322bb 100644
--- a/drivers/net/ethernet/via/via-rhine.c
+++ b/drivers/net/ethernet/via/via-rhine.c
@@ -473,7 +473,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;
@@ -1239,6 +1239,17 @@ 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;
+ 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);
+}
+
static inline void rhine_skb_dma_nic_store(struct rhine_private *rp,
struct rhine_skb_dma *sd, int entry)
{
@@ -1249,16 +1260,15 @@ static inline void rhine_skb_dma_nic_store(struct rhine_private *rp,
dma_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;
/* Init the ring entries */
@@ -1277,14 +1287,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)
@@ -1696,7 +1709,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);
@@ -1711,6 +1727,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;
@@ -1733,9 +1751,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);
@@ -2033,16 +2051,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,
@@ -2051,25 +2071,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);
@@ -2084,36 +2107,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:
+ desc->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]);
- dma_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) {
@@ -2518,9 +2522,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);
--
Ueimor
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next #3 5/8] via-rhine: kiss rx_head_desc goodbye.
2015-05-01 20:14 [PATCH net-next #3 0/8] via-rhine rework Francois Romieu
` (3 preceding siblings ...)
2015-05-01 20:14 ` [PATCH net-next #3 4/8] via-rhine: forbid holes in the receive descriptor ring Francois Romieu
@ 2015-05-01 20:14 ` Francois Romieu
2015-05-01 20:14 ` [PATCH net-next #3 6/8] via-rhine: add consistent memory barrier in vlan receive code Francois Romieu
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Francois Romieu @ 2015-05-01 20:14 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Nix, Roger.Luethi.rl, Bjarke Istrup Pedersen
The driver no longer produces holes in its receive ring so rx_head_desc
only duplicates cur_rx.
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
drivers/net/ethernet/via/via-rhine.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
index 8d322bb..fede9fc 100644
--- a/drivers/net/ethernet/via/via-rhine.c
+++ b/drivers/net/ethernet/via/via-rhine.c
@@ -472,7 +472,6 @@ struct rhine_private {
/* Frequently used values: keep some adjacent for cache effect. */
u32 quirks;
- struct rx_desc *rx_head_desc;
unsigned int cur_rx;
unsigned int cur_tx, dirty_tx;
unsigned int rx_buf_sz; /* Based on MTU+slack. */
@@ -1244,7 +1243,6 @@ 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);
@@ -2000,15 +1998,15 @@ static int rhine_rx(struct net_device *dev, int limit)
{
struct rhine_private *rp = netdev_priv(dev);
struct device *hwdev = dev->dev.parent;
- int count;
int entry = rp->cur_rx % RX_RING_SIZE;
+ int count;
netif_dbg(rp, rx_status, dev, "%s(), entry %d status %08x\n", __func__,
- entry, le32_to_cpu(rp->rx_head_desc->rx_status));
+ entry, le32_to_cpu(rp->rx_ring[entry].rx_status));
/* If EOP is set on the next entry, it's a new packet. Send it up. */
for (count = 0; count < limit; ++count) {
- struct rx_desc *desc = rp->rx_head_desc;
+ struct rx_desc *desc = rp->rx_ring + entry;
u32 desc_status = le32_to_cpu(desc->rx_status);
u32 desc_length = le32_to_cpu(desc->desc_length);
int data_size = desc_status >> 16;
@@ -2026,10 +2024,6 @@ static int rhine_rx(struct net_device *dev, int limit)
"entry %#x length %d status %08x!\n",
entry, data_size,
desc_status);
- netdev_warn(dev,
- "Oversized Ethernet frame %p vs %p\n",
- rp->rx_head_desc,
- &rp->rx_ring[entry]);
dev->stats.rx_length_errors++;
} else if (desc_status & RxErr) {
/* There was a error. */
@@ -2110,7 +2104,6 @@ static int rhine_rx(struct net_device *dev, int limit)
give_descriptor_to_nic:
desc->rx_status = cpu_to_le32(DescOwn);
entry = (++rp->cur_rx) % RX_RING_SIZE;
- rp->rx_head_desc = &rp->rx_ring[entry];
}
return count;
--
Ueimor
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next #3 6/8] via-rhine: add consistent memory barrier in vlan receive code.
2015-05-01 20:14 [PATCH net-next #3 0/8] via-rhine rework Francois Romieu
` (4 preceding siblings ...)
2015-05-01 20:14 ` [PATCH net-next #3 5/8] via-rhine: kiss rx_head_desc goodbye Francois Romieu
@ 2015-05-01 20:14 ` Francois Romieu
2015-05-01 20:14 ` [PATCH net-next #3 7/8] via-rhine: dma_wmb transmit barrier Francois Romieu
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Francois Romieu @ 2015-05-01 20:14 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Nix, Roger.Luethi.rl, Bjarke Istrup Pedersen
The NAPI receive path depends on desc->rx_status but it does not
enforce any explicit receive barrier.
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
drivers/net/ethernet/via/via-rhine.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
index fede9fc..8195fcd 100644
--- a/drivers/net/ethernet/via/via-rhine.c
+++ b/drivers/net/ethernet/via/via-rhine.c
@@ -1993,6 +1993,18 @@ static inline u16 rhine_get_vlan_tci(struct sk_buff *skb, int data_size)
return be16_to_cpup((__be16 *)trailer);
}
+static inline void rhine_rx_vlan_tag(struct sk_buff *skb, struct rx_desc *desc,
+ int data_size)
+{
+ dma_rmb();
+ if (unlikely(desc->desc_length & cpu_to_le32(DescTag))) {
+ u16 vlan_tci;
+
+ vlan_tci = rhine_get_vlan_tci(skb, data_size);
+ __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlan_tci);
+ }
+}
+
/* Process up to limit frames from receive ring */
static int rhine_rx(struct net_device *dev, int limit)
{
@@ -2008,7 +2020,6 @@ static int rhine_rx(struct net_device *dev, int limit)
for (count = 0; count < limit; ++count) {
struct rx_desc *desc = rp->rx_ring + entry;
u32 desc_status = le32_to_cpu(desc->rx_status);
- u32 desc_length = le32_to_cpu(desc->desc_length);
int data_size = desc_status >> 16;
if (desc_status & DescOwn)
@@ -2048,7 +2059,6 @@ static int rhine_rx(struct net_device *dev, int limit)
/* 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. */
@@ -2086,14 +2096,10 @@ static int rhine_rx(struct net_device *dev, int limit)
}
skb_put(skb, pkt_len);
-
- if (unlikely(desc_length & DescTag))
- vlan_tci = rhine_get_vlan_tci(skb, data_size);
-
skb->protocol = eth_type_trans(skb, dev);
- if (unlikely(desc_length & DescTag))
- __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlan_tci);
+ rhine_rx_vlan_tag(skb, desc, data_size);
+
netif_receive_skb(skb);
u64_stats_update_begin(&rp->rx_stats.syncp);
--
Ueimor
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next #3 7/8] via-rhine: dma_wmb transmit barrier.
2015-05-01 20:14 [PATCH net-next #3 0/8] via-rhine rework Francois Romieu
` (5 preceding siblings ...)
2015-05-01 20:14 ` [PATCH net-next #3 6/8] via-rhine: add consistent memory barrier in vlan receive code Francois Romieu
@ 2015-05-01 20:14 ` Francois Romieu
2015-05-01 20:14 ` [PATCH net-next #3 8/8] via-rhine: close SMP transmit races Francois Romieu
2015-05-04 4:18 ` [PATCH net-next #3 0/8] via-rhine rework David Miller
8 siblings, 0 replies; 10+ messages in thread
From: Francois Romieu @ 2015-05-01 20:14 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Nix, Roger.Luethi.rl, Bjarke Istrup Pedersen
Follow the now usual transmit descriptor update path:
1. content change
2. dma_wmb
3. ownership change
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
drivers/net/ethernet/via/via-rhine.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
index 8195fcd..86ca8b2 100644
--- a/drivers/net/ethernet/via/via-rhine.c
+++ b/drivers/net/ethernet/via/via-rhine.c
@@ -1849,7 +1849,7 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
netdev_sent_queue(dev, skb->len);
/* lock eth irq */
- wmb();
+ dma_wmb();
rp->tx_ring[entry].tx_status |= cpu_to_le32(DescOwn);
wmb();
--
Ueimor
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net-next #3 8/8] via-rhine: close SMP transmit races.
2015-05-01 20:14 [PATCH net-next #3 0/8] via-rhine rework Francois Romieu
` (6 preceding siblings ...)
2015-05-01 20:14 ` [PATCH net-next #3 7/8] via-rhine: dma_wmb transmit barrier Francois Romieu
@ 2015-05-01 20:14 ` Francois Romieu
2015-05-04 4:18 ` [PATCH net-next #3 0/8] via-rhine rework David Miller
8 siblings, 0 replies; 10+ messages in thread
From: Francois Romieu @ 2015-05-01 20:14 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Nix, Roger.Luethi.rl, Bjarke Istrup Pedersen
7ab87ff4c770eed71e3777936299292739fcd0fe ("via-rhine: move work from
irq handler to softirq and beyond") forgot to explicitely control the
lifespan of the tx_dirty and tx_cur pointers.
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
drivers/net/ethernet/via/via-rhine.c | 51 +++++++++++++++++++++++++++++++-----
1 file changed, 45 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/via/via-rhine.c b/drivers/net/ethernet/via/via-rhine.c
index 86ca8b2..725106f 100644
--- a/drivers/net/ethernet/via/via-rhine.c
+++ b/drivers/net/ethernet/via/via-rhine.c
@@ -1779,6 +1779,11 @@ static void rhine_tx_timeout(struct net_device *dev)
schedule_work(&rp->reset_task);
}
+static inline bool rhine_tx_queue_full(struct rhine_private *rp)
+{
+ return (rp->cur_tx - rp->dirty_tx) >= TX_QUEUE_LEN;
+}
+
static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
struct net_device *dev)
{
@@ -1854,6 +1859,12 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
wmb();
rp->cur_tx++;
+ /*
+ * Nobody wants cur_tx write to rot for ages after the NIC will have
+ * seen the transmit request, especially as the transmit completion
+ * handler could miss it.
+ */
+ smp_wmb();
/* Non-x86 Todo: explicitly flush cache lines here. */
@@ -1866,8 +1877,14 @@ static netdev_tx_t rhine_start_tx(struct sk_buff *skb,
ioaddr + ChipCmd1);
IOSYNC;
- if (rp->cur_tx == rp->dirty_tx + TX_QUEUE_LEN)
+ /* dirty_tx may be pessimistically out-of-sync. See rhine_tx. */
+ if (rhine_tx_queue_full(rp)) {
netif_stop_queue(dev);
+ smp_rmb();
+ /* Rejuvenate. */
+ if (!rhine_tx_queue_full(rp))
+ netif_wake_queue(dev);
+ }
netif_dbg(rp, tx_queued, dev, "Transmit frame #%d queued in slot %d\n",
rp->cur_tx - 1, entry);
@@ -1915,13 +1932,24 @@ static void rhine_tx(struct net_device *dev)
{
struct rhine_private *rp = netdev_priv(dev);
struct device *hwdev = dev->dev.parent;
- int txstatus = 0, entry = rp->dirty_tx % TX_RING_SIZE;
unsigned int pkts_compl = 0, bytes_compl = 0;
+ unsigned int dirty_tx = rp->dirty_tx;
+ unsigned int cur_tx;
struct sk_buff *skb;
+ /*
+ * The race with rhine_start_tx does not matter here as long as the
+ * driver enforces a value of cur_tx that was relevant when the
+ * packet was scheduled to the network chipset.
+ * Executive summary: smp_rmb() balances smp_wmb() in rhine_start_tx.
+ */
+ smp_rmb();
+ cur_tx = rp->cur_tx;
/* find and cleanup dirty tx descriptors */
- while (rp->dirty_tx != rp->cur_tx) {
- txstatus = le32_to_cpu(rp->tx_ring[entry].tx_status);
+ while (dirty_tx != cur_tx) {
+ unsigned int entry = dirty_tx % TX_RING_SIZE;
+ u32 txstatus = le32_to_cpu(rp->tx_ring[entry].tx_status);
+
netif_dbg(rp, tx_done, dev, "Tx scavenge %d status %08x\n",
entry, txstatus);
if (txstatus & DescOwn)
@@ -1970,12 +1998,23 @@ static void rhine_tx(struct net_device *dev)
pkts_compl++;
dev_consume_skb_any(skb);
rp->tx_skbuff[entry] = NULL;
- entry = (++rp->dirty_tx) % TX_RING_SIZE;
+ dirty_tx++;
}
+ rp->dirty_tx = dirty_tx;
+ /* Pity we can't rely on the nearby BQL completion implicit barrier. */
+ smp_wmb();
+
netdev_completed_queue(dev, pkts_compl, bytes_compl);
- if ((rp->cur_tx - rp->dirty_tx) < TX_QUEUE_LEN - 4)
+
+ /* cur_tx may be optimistically out-of-sync. See rhine_start_tx. */
+ if (!rhine_tx_queue_full(rp) && netif_queue_stopped(dev)) {
netif_wake_queue(dev);
+ smp_rmb();
+ /* Rejuvenate. */
+ if (rhine_tx_queue_full(rp))
+ netif_stop_queue(dev);
+ }
}
/**
--
Ueimor
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net-next #3 0/8] via-rhine rework
2015-05-01 20:14 [PATCH net-next #3 0/8] via-rhine rework Francois Romieu
` (7 preceding siblings ...)
2015-05-01 20:14 ` [PATCH net-next #3 8/8] via-rhine: close SMP transmit races Francois Romieu
@ 2015-05-04 4:18 ` David Miller
8 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2015-05-04 4:18 UTC (permalink / raw)
To: romieu; +Cc: netdev, nix, Roger.Luethi.rl, gurligebis
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Fri, 1 May 2015 22:14:37 +0200
> The series applies against davem-next as of
> 9dd3c797496affd699805c8a9d8429ad318c892f ("drivers: net: xgene: fix kbuild
> warnings").
>
> Patches #1..#4 avoid holes in the receive ring.
>
> Patch #5 is a small leftover cleanup for #1..#4.
>
> Patches #6 and #7 are fairly simple barrier stuff.
>
> Patch #8 closes some SMP transmit races - not that anyone really
> complained about these but it's a bit hard to handwave that they
> can be safely ignored. Some testing, especially SMP testing of
> course, would be welcome.
Looks great, series applied, thanks a lot!
^ permalink raw reply [flat|nested] 10+ messages in thread