Not that many users left, lets kill it.
TX_LOCKED was meant to be used by LLTX drivers when spin_trylock()
failed. Stack then re-queued if collisions happened on different
cpus or free'd the skb to prevent deadlocks.
Most of the driver removal patches fall into one of three categories:
1. remove the driver-private tx lock (and LLTX flag), or...
2. convert spin_trylock to plain spin_lock, or...
3. convert TX_LOCKED to free+TX_OK
Patches are grouped by these categories, last patch is the actual removal.
All driver changes were compile tested only with exception of atl1e.
Documentation/networking/netdev-features.txt | 10 ++---
Documentation/networking/netdevices.txt | 9 +----
drivers/infiniband/hw/nes/nes_nic.c | 13 ++-----
drivers/net/ethernet/amd/7990.c | 8 ++--
drivers/net/ethernet/amd/a2065.c | 7 +---
drivers/net/ethernet/atheros/atl1c/atl1c.h | 3 -
drivers/net/ethernet/atheros/atl1c/atl1c_main.c | 11 ------
drivers/net/ethernet/atheros/atl1e/atl1e.h | 1
drivers/net/ethernet/atheros/atl1e/atl1e_main.c | 12 -------
drivers/net/ethernet/chelsio/cxgb/sge.c | 3 -
drivers/net/ethernet/dec/tulip/de4x5.c | 7 ++--
drivers/net/ethernet/neterion/s2io.c | 9 -----
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 6 +--
drivers/net/ethernet/tehuti/tehuti.c | 8 ----
drivers/net/hamradio/baycom_epp.c | 6 ++-
drivers/net/hamradio/hdlcdrv.c | 6 ++-
drivers/net/rionet.c | 6 ---
include/linux/netdevice.h | 3 -
net/core/net-procfs.c | 3 +
net/core/pktgen.c | 1
net/sched/sch_generic.c | 32 -------------------
21 files changed, 43 insertions(+), 121 deletions(-)
ndo_start_xmit never returns it to stack, but nes_nic_send helper used it if
skb could not be queued to hardware. Switch to bool instead.
Cc: <[email protected]>
Signed-off-by: Florian Westphal <[email protected]>
---
drivers/infiniband/hw/nes/nes_nic.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/infiniband/hw/nes/nes_nic.c b/drivers/infiniband/hw/nes/nes_nic.c
index 3ea9e05..b09a6db 100644
--- a/drivers/infiniband/hw/nes/nes_nic.c
+++ b/drivers/infiniband/hw/nes/nes_nic.c
@@ -356,7 +356,7 @@ static int nes_netdev_stop(struct net_device *netdev)
/**
* nes_nic_send
*/
-static int nes_nic_send(struct sk_buff *skb, struct net_device *netdev)
+static bool nes_nic_send(struct sk_buff *skb, struct net_device *netdev)
{
struct nes_vnic *nesvnic = netdev_priv(netdev);
struct nes_device *nesdev = nesvnic->nesdev;
@@ -413,7 +413,7 @@ static int nes_nic_send(struct sk_buff *skb, struct net_device *netdev)
netdev->name, skb_shinfo(skb)->nr_frags + 2, skb_headlen(skb));
kfree_skb(skb);
nesvnic->tx_sw_dropped++;
- return NETDEV_TX_LOCKED;
+ return false;
}
set_bit(nesnic->sq_head, nesnic->first_frag_overflow);
bus_address = pci_map_single(nesdev->pcidev, skb->data + NES_FIRST_FRAG_SIZE,
@@ -454,8 +454,7 @@ static int nes_nic_send(struct sk_buff *skb, struct net_device *netdev)
set_wqe_32bit_value(nic_sqe->wqe_words, NES_NIC_SQ_WQE_MISC_IDX, wqe_misc);
nesnic->sq_head++;
nesnic->sq_head &= nesnic->sq_size - 1;
-
- return NETDEV_TX_OK;
+ return true;
}
@@ -673,13 +672,11 @@ tso_sq_no_longer_full:
skb_linearize(skb);
skb_set_transport_header(skb, hoffset);
skb_set_network_header(skb, nhoffset);
- send_rc = nes_nic_send(skb, netdev);
- if (send_rc != NETDEV_TX_OK)
+ if (!nes_nic_send(skb, netdev))
return NETDEV_TX_OK;
}
} else {
- send_rc = nes_nic_send(skb, netdev);
- if (send_rc != NETDEV_TX_OK)
+ if (!nes_nic_send(skb, netdev))
return NETDEV_TX_OK;
}
--
2.7.3
similar to atl1c: lock is only used in ndo_start_xmit, but we also
advertised LLTX, so remove that as well and let core stack handle
tx locking.
Allows to remove the TX_LOCKED return value from the driver.
Cc: Jay Cliburn <[email protected]>
Cc: Chris Snook <[email protected]>
Signed-off-by: Florian Westphal <[email protected]>
---
drivers/net/ethernet/atheros/atl1e/atl1e.h | 1 -
drivers/net/ethernet/atheros/atl1e/atl1e_main.c | 12 +-----------
2 files changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/atheros/atl1e/atl1e.h b/drivers/net/ethernet/atheros/atl1e/atl1e.h
index 0212dac..632bb84 100644
--- a/drivers/net/ethernet/atheros/atl1e/atl1e.h
+++ b/drivers/net/ethernet/atheros/atl1e/atl1e.h
@@ -442,7 +442,6 @@ struct atl1e_adapter {
u16 link_duplex;
spinlock_t mdio_lock;
- spinlock_t tx_lock;
atomic_t irq_sem;
struct work_struct reset_task;
diff --git a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
index 59a03a1..974713b 100644
--- a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
+++ b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
@@ -648,7 +648,6 @@ static int atl1e_sw_init(struct atl1e_adapter *adapter)
atomic_set(&adapter->irq_sem, 1);
spin_lock_init(&adapter->mdio_lock);
- spin_lock_init(&adapter->tx_lock);
set_bit(__AT_DOWN, &adapter->flags);
@@ -1866,7 +1865,6 @@ static netdev_tx_t atl1e_xmit_frame(struct sk_buff *skb,
struct net_device *netdev)
{
struct atl1e_adapter *adapter = netdev_priv(netdev);
- unsigned long flags;
u16 tpd_req = 1;
struct atl1e_tpd_desc *tpd;
@@ -1880,13 +1878,10 @@ static netdev_tx_t atl1e_xmit_frame(struct sk_buff *skb,
return NETDEV_TX_OK;
}
tpd_req = atl1e_cal_tdp_req(skb);
- if (!spin_trylock_irqsave(&adapter->tx_lock, flags))
- return NETDEV_TX_LOCKED;
if (atl1e_tpd_avail(adapter) < tpd_req) {
/* no enough descriptor, just stop queue */
netif_stop_queue(netdev);
- spin_unlock_irqrestore(&adapter->tx_lock, flags);
return NETDEV_TX_BUSY;
}
@@ -1910,7 +1905,6 @@ static netdev_tx_t atl1e_xmit_frame(struct sk_buff *skb,
/* do TSO and check sum */
if (atl1e_tso_csum(adapter, skb, tpd) != 0) {
- spin_unlock_irqrestore(&adapter->tx_lock, flags);
dev_kfree_skb_any(skb);
return NETDEV_TX_OK;
}
@@ -1921,10 +1915,7 @@ static netdev_tx_t atl1e_xmit_frame(struct sk_buff *skb,
}
atl1e_tx_queue(adapter, tpd_req, tpd);
-
- netdev->trans_start = jiffies; /* NETIF_F_LLTX driver :( */
out:
- spin_unlock_irqrestore(&adapter->tx_lock, flags);
return NETDEV_TX_OK;
}
@@ -2285,8 +2276,7 @@ static int atl1e_init_netdev(struct net_device *netdev, struct pci_dev *pdev)
netdev->hw_features = NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_TSO |
NETIF_F_HW_VLAN_CTAG_RX;
- netdev->features = netdev->hw_features | NETIF_F_LLTX |
- NETIF_F_HW_VLAN_CTAG_TX;
+ netdev->features = netdev->hw_features | NETIF_F_HW_VLAN_CTAG_TX;
/* not enabled by default */
netdev->hw_features |= NETIF_F_RXALL | NETIF_F_RXFCS;
return 0;
--
2.7.3
No more users in the tree, remove NETDEV_TX_LOCKED support.
Adds another hole in softnet_stats struct, but better than keeping
the unused collision counter around.
Signed-off-by: Florian Westphal <[email protected]>
---
Documentation/networking/netdev-features.txt | 10 ++++-----
Documentation/networking/netdevices.txt | 9 +++-----
include/linux/netdevice.h | 3 ---
net/core/net-procfs.c | 3 ++-
net/core/pktgen.c | 1 -
net/sched/sch_generic.c | 32 ----------------------------
6 files changed, 9 insertions(+), 49 deletions(-)
diff --git a/Documentation/networking/netdev-features.txt b/Documentation/networking/netdev-features.txt
index f310ede..7413eb0 100644
--- a/Documentation/networking/netdev-features.txt
+++ b/Documentation/networking/netdev-features.txt
@@ -131,13 +131,11 @@ stack. Driver should not change behaviour based on them.
* LLTX driver (deprecated for hardware drivers)
-NETIF_F_LLTX should be set in drivers that implement their own locking in
-transmit path or don't need locking at all (e.g. software tunnels).
-In ndo_start_xmit, it is recommended to use a try_lock and return
-NETDEV_TX_LOCKED when the spin lock fails. The locking should also properly
-protect against other callbacks (the rules you need to find out).
+NETIF_F_LLTX is meant to be used by drivers that don't need locking at all,
+e.g. software tunnels.
-Don't use it for new drivers.
+This is also used in a few legacy drivers that implement their
+own locking, don't use it for new (hardware) drivers.
* netns-local device
diff --git a/Documentation/networking/netdevices.txt b/Documentation/networking/netdevices.txt
index 0b1cf6b..7fec206 100644
--- a/Documentation/networking/netdevices.txt
+++ b/Documentation/networking/netdevices.txt
@@ -69,10 +69,9 @@ ndo_start_xmit:
When the driver sets NETIF_F_LLTX in dev->features this will be
called without holding netif_tx_lock. In this case the driver
- has to lock by itself when needed. It is recommended to use a try lock
- for this and return NETDEV_TX_LOCKED when the spin lock fails.
- The locking there should also properly protect against
- set_rx_mode. Note that the use of NETIF_F_LLTX is deprecated.
+ has to lock by itself when needed.
+ The locking there should also properly protect against
+ set_rx_mode. WARNING: use of NETIF_F_LLTX is deprecated.
Don't use it for new drivers.
Context: Process with BHs disabled or BH (timer),
@@ -83,8 +82,6 @@ ndo_start_xmit:
o NETDEV_TX_BUSY Cannot transmit packet, try later
Usually a bug, means queue start/stop flow control is broken in
the driver. Note: the driver must NOT put the skb in its DMA ring.
- o NETDEV_TX_LOCKED Locking failed, please retry quickly.
- Only valid when NETIF_F_LLTX is set.
ndo_tx_timeout:
Synchronization: netif_tx_lock spinlock; all TX queues frozen.
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 1f6d5db..18d8394 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -106,7 +106,6 @@ enum netdev_tx {
__NETDEV_TX_MIN = INT_MIN, /* make sure enum is signed */
NETDEV_TX_OK = 0x00, /* driver took care of packet */
NETDEV_TX_BUSY = 0x10, /* driver tx path was busy*/
- NETDEV_TX_LOCKED = 0x20, /* driver tx lock was already taken */
};
typedef enum netdev_tx netdev_tx_t;
@@ -831,7 +830,6 @@ struct tc_to_netdev {
* the queue before that can happen; it's for obsolete devices and weird
* corner cases, but the stack really does a non-trivial amount
* of useless work if you return NETDEV_TX_BUSY.
- * (can also return NETDEV_TX_LOCKED iff NETIF_F_LLTX)
* Required; cannot be NULL.
*
* netdev_features_t (*ndo_fix_features)(struct net_device *dev,
@@ -2737,7 +2735,6 @@ struct softnet_data {
/* stats */
unsigned int processed;
unsigned int time_squeeze;
- unsigned int cpu_collision;
unsigned int received_rps;
#ifdef CONFIG_RPS
struct softnet_data *rps_ipi_list;
diff --git a/net/core/net-procfs.c b/net/core/net-procfs.c
index 2bf8329..14d0934 100644
--- a/net/core/net-procfs.c
+++ b/net/core/net-procfs.c
@@ -162,7 +162,8 @@ static int softnet_seq_show(struct seq_file *seq, void *v)
"%08x %08x %08x %08x %08x %08x %08x %08x %08x %08x %08x\n",
sd->processed, sd->dropped, sd->time_squeeze, 0,
0, 0, 0, 0, /* was fastroute */
- sd->cpu_collision, sd->received_rps, flow_limit_count);
+ 0, /* was cpu_collision */
+ sd->received_rps, flow_limit_count);
return 0;
}
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 20999aa..8604ae2 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -3472,7 +3472,6 @@ xmit_more:
pkt_dev->odevname, ret);
pkt_dev->errors++;
/* fallthru */
- case NETDEV_TX_LOCKED:
case NETDEV_TX_BUSY:
/* Retry it next time */
atomic_dec(&(pkt_dev->skb->users));
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 80742ed..9c77562 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -108,35 +108,6 @@ static struct sk_buff *dequeue_skb(struct Qdisc *q, bool *validate,
return skb;
}
-static inline int handle_dev_cpu_collision(struct sk_buff *skb,
- struct netdev_queue *dev_queue,
- struct Qdisc *q)
-{
- int ret;
-
- if (unlikely(dev_queue->xmit_lock_owner == smp_processor_id())) {
- /*
- * Same CPU holding the lock. It may be a transient
- * configuration error, when hard_start_xmit() recurses. We
- * detect it by checking xmit owner and drop the packet when
- * deadloop is detected. Return OK to try the next skb.
- */
- kfree_skb_list(skb);
- net_warn_ratelimited("Dead loop on netdevice %s, fix it urgently!\n",
- dev_queue->dev->name);
- ret = qdisc_qlen(q);
- } else {
- /*
- * Another cpu is holding lock, requeue & delay xmits for
- * some time.
- */
- __this_cpu_inc(softnet_data.cpu_collision);
- ret = dev_requeue_skb(skb, q);
- }
-
- return ret;
-}
-
/*
* Transmit possibly several skbs, and handle the return status as
* required. Holding the __QDISC___STATE_RUNNING bit guarantees that
@@ -174,9 +145,6 @@ int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
if (dev_xmit_complete(ret)) {
/* Driver sent out skb successfully or skb was consumed */
ret = qdisc_qlen(q);
- } else if (ret == NETDEV_TX_LOCKED) {
- /* Driver try lock failed */
- ret = handle_dev_cpu_collision(skb, txq, q);
} else {
/* Driver returned NETDEV_TX_BUSY - requeue skb */
if (unlikely(ret != NETDEV_TX_BUSY))
--
2.7.3
replace the trylock by a full spin_lock and remove TX_LOCKED return value.
Followup patch will remove TX_LOCKED from the kernel.
Cc: Jon Mason <[email protected]>
Cc: Andy Gospodarek <[email protected]>
Signed-off-by: Florian Westphal <[email protected]>
---
drivers/net/ethernet/chelsio/cxgb/sge.c | 3 +--
drivers/net/ethernet/neterion/s2io.c | 9 +--------
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 6 ++----
drivers/net/ethernet/tehuti/tehuti.c | 8 +-------
drivers/net/rionet.c | 6 +-----
5 files changed, 6 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb/sge.c b/drivers/net/ethernet/chelsio/cxgb/sge.c
index 526ea74..86f467a 100644
--- a/drivers/net/ethernet/chelsio/cxgb/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb/sge.c
@@ -1664,8 +1664,7 @@ static int t1_sge_tx(struct sk_buff *skb, struct adapter *adapter,
struct cmdQ *q = &sge->cmdQ[qid];
unsigned int credits, pidx, genbit, count, use_sched_skb = 0;
- if (!spin_trylock(&q->lock))
- return NETDEV_TX_LOCKED;
+ spin_lock(&q->lock);
reclaim_completed_tx(sge, q);
diff --git a/drivers/net/ethernet/neterion/s2io.c b/drivers/net/ethernet/neterion/s2io.c
index 9ba9758..2874dff 100644
--- a/drivers/net/ethernet/neterion/s2io.c
+++ b/drivers/net/ethernet/neterion/s2io.c
@@ -4021,7 +4021,6 @@ static netdev_tx_t s2io_xmit(struct sk_buff *skb, struct net_device *dev)
unsigned long flags = 0;
u16 vlan_tag = 0;
struct fifo_info *fifo = NULL;
- int do_spin_lock = 1;
int offload_type;
int enable_per_list_interrupt = 0;
struct config_param *config = &sp->config;
@@ -4074,7 +4073,6 @@ static netdev_tx_t s2io_xmit(struct sk_buff *skb, struct net_device *dev)
queue += sp->udp_fifo_idx;
if (skb->len > 1024)
enable_per_list_interrupt = 1;
- do_spin_lock = 0;
}
}
}
@@ -4084,12 +4082,7 @@ static netdev_tx_t s2io_xmit(struct sk_buff *skb, struct net_device *dev)
[skb->priority & (MAX_TX_FIFOS - 1)];
fifo = &mac_control->fifos[queue];
- if (do_spin_lock)
- spin_lock_irqsave(&fifo->tx_lock, flags);
- else {
- if (unlikely(!spin_trylock_irqsave(&fifo->tx_lock, flags)))
- return NETDEV_TX_LOCKED;
- }
+ spin_lock_irqsave(&fifo->tx_lock, flags);
if (sp->config.multiq) {
if (__netif_subqueue_stopped(dev, fifo->fifo_no)) {
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 3b98b263b..4475dcc 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -2137,10 +2137,8 @@ static int pch_gbe_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
struct pch_gbe_tx_ring *tx_ring = adapter->tx_ring;
unsigned long flags;
- if (!spin_trylock_irqsave(&tx_ring->tx_lock, flags)) {
- /* Collision - tell upper layer to requeue */
- return NETDEV_TX_LOCKED;
- }
+ spin_trylock_irqsave(&tx_ring->tx_lock, flags);
+
if (unlikely(!PCH_GBE_DESC_UNUSED(tx_ring))) {
netif_stop_queue(netdev);
spin_unlock_irqrestore(&tx_ring->tx_lock, flags);
diff --git a/drivers/net/ethernet/tehuti/tehuti.c b/drivers/net/ethernet/tehuti/tehuti.c
index 14c9d1b..2524a69 100644
--- a/drivers/net/ethernet/tehuti/tehuti.c
+++ b/drivers/net/ethernet/tehuti/tehuti.c
@@ -1610,7 +1610,6 @@ static inline int bdx_tx_space(struct bdx_priv *priv)
* o NETDEV_TX_BUSY Cannot transmit packet, try later
* Usually a bug, means queue start/stop flow control is broken in
* the driver. Note: the driver must NOT put the skb in its DMA ring.
- * o NETDEV_TX_LOCKED Locking failed, please retry quickly.
*/
static netdev_tx_t bdx_tx_transmit(struct sk_buff *skb,
struct net_device *ndev)
@@ -1630,12 +1629,7 @@ static netdev_tx_t bdx_tx_transmit(struct sk_buff *skb,
ENTER;
local_irq_save(flags);
- if (!spin_trylock(&priv->tx_lock)) {
- local_irq_restore(flags);
- DBG("%s[%s]: TX locked, returning NETDEV_TX_LOCKED\n",
- BDX_DRV_NAME, ndev->name);
- return NETDEV_TX_LOCKED;
- }
+ spin_lock(&priv->tx_lock);
/* build tx descriptor */
BDX_ASSERT(f->m.wptr >= f->m.memsz); /* started with valid wptr */
diff --git a/drivers/net/rionet.c b/drivers/net/rionet.c
index 9cfe6ae..a31f461 100644
--- a/drivers/net/rionet.c
+++ b/drivers/net/rionet.c
@@ -179,11 +179,7 @@ static int rionet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
unsigned long flags;
int add_num = 1;
- local_irq_save(flags);
- if (!spin_trylock(&rnet->tx_lock)) {
- local_irq_restore(flags);
- return NETDEV_TX_LOCKED;
- }
+ spin_lock_irqsave(&rnet->tx_lock, flags);
if (is_multicast_ether_addr(eth->h_dest))
add_num = nets[rnet->mport->id].nact;
--
2.7.3
These drivers already call netif_stop_queue() so we should not be called
unless tx space is available. Just free the skb and return TX_OK.
Followup patch will remove NETDEV_TX_LOCKED from the kernel.
Cc: [email protected]
Cc: [email protected]
Cc: Thomas Sailer <[email protected]>
Signed-off-by: Florian Westphal <[email protected]>
---
checkpatch chokes on this one, tulip uses spaces, not tabs...
drivers/net/ethernet/amd/7990.c | 8 +++++---
drivers/net/ethernet/amd/a2065.c | 7 +++----
drivers/net/ethernet/dec/tulip/de4x5.c | 7 +++++--
drivers/net/hamradio/baycom_epp.c | 6 ++++--
drivers/net/hamradio/hdlcdrv.c | 6 ++++--
5 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/amd/7990.c b/drivers/net/ethernet/amd/7990.c
index 66d0b73c..8e75755 100644
--- a/drivers/net/ethernet/amd/7990.c
+++ b/drivers/net/ethernet/amd/7990.c
@@ -543,11 +543,13 @@ int lance_start_xmit(struct sk_buff *skb, struct net_device *dev)
static int outs;
unsigned long flags;
- if (!TX_BUFFS_AVAIL)
- return NETDEV_TX_LOCKED;
-
netif_stop_queue(dev);
+ if (!TX_BUFFS_AVAIL) {
+ dev_consume_skb_any(skb);
+ return NETDEV_TX_OK;
+ }
+
skblen = skb->len;
#ifdef DEBUG_DRIVER
diff --git a/drivers/net/ethernet/amd/a2065.c b/drivers/net/ethernet/amd/a2065.c
index 5613918..2a18d34 100644
--- a/drivers/net/ethernet/amd/a2065.c
+++ b/drivers/net/ethernet/amd/a2065.c
@@ -547,10 +547,8 @@ static netdev_tx_t lance_start_xmit(struct sk_buff *skb,
local_irq_save(flags);
- if (!lance_tx_buffs_avail(lp)) {
- local_irq_restore(flags);
- return NETDEV_TX_LOCKED;
- }
+ if (!lance_tx_buffs_avail(lp))
+ goto out_free;
#ifdef DEBUG
/* dump the packet */
@@ -573,6 +571,7 @@ static netdev_tx_t lance_start_xmit(struct sk_buff *skb,
/* Kick the lance: transmit now */
ll->rdp = LE_C0_INEA | LE_C0_TDMD;
+ out_free:
dev_kfree_skb(skb);
local_irq_restore(flags);
diff --git a/drivers/net/ethernet/dec/tulip/de4x5.c b/drivers/net/ethernet/dec/tulip/de4x5.c
index 3acde3b..d88fbab 100644
--- a/drivers/net/ethernet/dec/tulip/de4x5.c
+++ b/drivers/net/ethernet/dec/tulip/de4x5.c
@@ -1465,7 +1465,7 @@ de4x5_queue_pkt(struct sk_buff *skb, struct net_device *dev)
netif_stop_queue(dev);
if (!lp->tx_enable) /* Cannot send for now */
- return NETDEV_TX_LOCKED;
+ goto tx_err;
/*
** Clean out the TX ring asynchronously to interrupts - sometimes the
@@ -1478,7 +1478,7 @@ de4x5_queue_pkt(struct sk_buff *skb, struct net_device *dev)
/* Test if cache is already locked - requeue skb if so */
if (test_and_set_bit(0, (void *)&lp->cache.lock) && !lp->interrupt)
- return NETDEV_TX_LOCKED;
+ goto tx_err;
/* Transmit descriptor ring full or stale skb */
if (netif_queue_stopped(dev) || (u_long) lp->tx_skb[lp->tx_new] > 1) {
@@ -1519,6 +1519,9 @@ de4x5_queue_pkt(struct sk_buff *skb, struct net_device *dev)
lp->cache.lock = 0;
return NETDEV_TX_OK;
+tx_err:
+ dev_kfree_skb_any(skb);
+ return NETDEV_TX_OK;
}
/*
diff --git a/drivers/net/hamradio/baycom_epp.c b/drivers/net/hamradio/baycom_epp.c
index 72c9f1f..eb66638 100644
--- a/drivers/net/hamradio/baycom_epp.c
+++ b/drivers/net/hamradio/baycom_epp.c
@@ -780,8 +780,10 @@ static int baycom_send_packet(struct sk_buff *skb, struct net_device *dev)
dev_kfree_skb(skb);
return NETDEV_TX_OK;
}
- if (bc->skb)
- return NETDEV_TX_LOCKED;
+ if (bc->skb) {
+ dev_kfree_skb(skb);
+ return NETDEV_TX_OK;
+ }
/* strip KISS byte */
if (skb->len >= HDLCDRV_MAXFLEN+1 || skb->len < 3) {
dev_kfree_skb(skb);
diff --git a/drivers/net/hamradio/hdlcdrv.c b/drivers/net/hamradio/hdlcdrv.c
index 49fe59b..4bad0b8 100644
--- a/drivers/net/hamradio/hdlcdrv.c
+++ b/drivers/net/hamradio/hdlcdrv.c
@@ -412,8 +412,10 @@ static netdev_tx_t hdlcdrv_send_packet(struct sk_buff *skb,
dev_kfree_skb(skb);
return NETDEV_TX_OK;
}
- if (sm->skb)
- return NETDEV_TX_LOCKED;
+ if (sm->skb) {
+ dev_kfree_skb(skb);
+ return NETDEV_TX_OK;
+ }
netif_stop_queue(dev);
sm->skb = skb;
return NETDEV_TX_OK;
--
2.7.3
AFAICS this is safe: the lock is only used in the .ndo_start_xmit
function and this driver does not set LLTX.
Gets rid of TX_LOCKED return value, followup patches will remove it.
Cc: Jay Cliburn <[email protected]>
Cc: Chris Snook <[email protected]>
Signed-off-by: Florian Westphal <[email protected]>
---
drivers/net/ethernet/atheros/atl1c/atl1c.h | 3 +--
drivers/net/ethernet/atheros/atl1c/atl1c_main.c | 11 -----------
2 files changed, 1 insertion(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c.h b/drivers/net/ethernet/atheros/atl1c/atl1c.h
index b9203d9..c46b489 100644
--- a/drivers/net/ethernet/atheros/atl1c/atl1c.h
+++ b/drivers/net/ethernet/atheros/atl1c/atl1c.h
@@ -488,7 +488,7 @@ struct atl1c_tpd_ring {
dma_addr_t dma; /* descriptor ring physical address */
u16 size; /* descriptor ring length in bytes */
u16 count; /* number of descriptors in the ring */
- u16 next_to_use; /* this is protectd by adapter->tx_lock */
+ u16 next_to_use;
atomic_t next_to_clean;
struct atl1c_buffer *buffer_info;
};
@@ -542,7 +542,6 @@ struct atl1c_adapter {
u16 link_duplex;
spinlock_t mdio_lock;
- spinlock_t tx_lock;
atomic_t irq_sem;
struct work_struct common_task;
diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
index d0084d4..a3200ea 100644
--- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
+++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
@@ -821,7 +821,6 @@ static int atl1c_sw_init(struct atl1c_adapter *adapter)
atl1c_set_rxbufsize(adapter, adapter->netdev);
atomic_set(&adapter->irq_sem, 1);
spin_lock_init(&adapter->mdio_lock);
- spin_lock_init(&adapter->tx_lock);
set_bit(__AT_DOWN, &adapter->flags);
return 0;
@@ -2206,7 +2205,6 @@ static netdev_tx_t atl1c_xmit_frame(struct sk_buff *skb,
struct net_device *netdev)
{
struct atl1c_adapter *adapter = netdev_priv(netdev);
- unsigned long flags;
u16 tpd_req = 1;
struct atl1c_tpd_desc *tpd;
enum atl1c_trans_queue type = atl1c_trans_normal;
@@ -2217,16 +2215,10 @@ static netdev_tx_t atl1c_xmit_frame(struct sk_buff *skb,
}
tpd_req = atl1c_cal_tpd_req(skb);
- if (!spin_trylock_irqsave(&adapter->tx_lock, flags)) {
- if (netif_msg_pktdata(adapter))
- dev_info(&adapter->pdev->dev, "tx locked\n");
- return NETDEV_TX_LOCKED;
- }
if (atl1c_tpd_avail(adapter, type) < tpd_req) {
/* no enough descriptor, just stop queue */
netif_stop_queue(netdev);
- spin_unlock_irqrestore(&adapter->tx_lock, flags);
return NETDEV_TX_BUSY;
}
@@ -2234,7 +2226,6 @@ static netdev_tx_t atl1c_xmit_frame(struct sk_buff *skb,
/* do TSO and check sum */
if (atl1c_tso_csum(adapter, skb, &tpd, type) != 0) {
- spin_unlock_irqrestore(&adapter->tx_lock, flags);
dev_kfree_skb_any(skb);
return NETDEV_TX_OK;
}
@@ -2257,12 +2248,10 @@ static netdev_tx_t atl1c_xmit_frame(struct sk_buff *skb,
"tx-skb droppted due to dma error\n");
/* roll back tpd/buffer */
atl1c_tx_rollback(adapter, tpd, type);
- spin_unlock_irqrestore(&adapter->tx_lock, flags);
dev_kfree_skb_any(skb);
} else {
netdev_sent_queue(adapter->netdev, skb->len);
atl1c_tx_queue(adapter, skb, tpd, type);
- spin_unlock_irqrestore(&adapter->tx_lock, flags);
}
return NETDEV_TX_OK;
--
2.7.3
Francois Romieu <[email protected]> wrote:
> Florian Westphal <[email protected]> :
> [...]
> > diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> > index d0084d4..a3200ea 100644
> > --- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> > +++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> [...]
> > @@ -2217,16 +2215,10 @@ static netdev_tx_t atl1c_xmit_frame(struct sk_buff *skb,
> > }
> >
> > tpd_req = atl1c_cal_tpd_req(skb);
> > - if (!spin_trylock_irqsave(&adapter->tx_lock, flags)) {
> > - if (netif_msg_pktdata(adapter))
> > - dev_info(&adapter->pdev->dev, "tx locked\n");
> > - return NETDEV_TX_LOCKED;
> > - }
> >
> > if (atl1c_tpd_avail(adapter, type) < tpd_req) {
> > /* no enough descriptor, just stop queue */
> > netif_stop_queue(netdev);
> > - spin_unlock_irqrestore(&adapter->tx_lock, flags);
> > return NETDEV_TX_BUSY;
> > }
> >
>
> Play it safe and keep the implicit local_irq_{save / restore} call ?
>
> It may not be needed but it will help avoiding any unexpected regression
> report pointing at the NETDEV_TX_LOCKED removal change.
I thought about that but it doesn't prevent the irq handler from
running on another CPU, so leaving it around seemed like cargo culting
to me...
I don't have an atl1c, but the atl1e in my laptop seems to work fine
with the (similar) change.
If you disagree I can respin with local_irq_save of course, but, if
'playing it safe' is main goal then its simpler to convert
spin_trylock_irqsave to spin_lock_irqsave.
Florian Westphal <[email protected]> :
[...]
> diff --git a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> index d0084d4..a3200ea 100644
> --- a/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
> +++ b/drivers/net/ethernet/atheros/atl1c/atl1c_main.c
[...]
> @@ -2217,16 +2215,10 @@ static netdev_tx_t atl1c_xmit_frame(struct sk_buff *skb,
> }
>
> tpd_req = atl1c_cal_tpd_req(skb);
> - if (!spin_trylock_irqsave(&adapter->tx_lock, flags)) {
> - if (netif_msg_pktdata(adapter))
> - dev_info(&adapter->pdev->dev, "tx locked\n");
> - return NETDEV_TX_LOCKED;
> - }
>
> if (atl1c_tpd_avail(adapter, type) < tpd_req) {
> /* no enough descriptor, just stop queue */
> netif_stop_queue(netdev);
> - spin_unlock_irqrestore(&adapter->tx_lock, flags);
> return NETDEV_TX_BUSY;
> }
>
Play it safe and keep the implicit local_irq_{save / restore} call ?
It may not be needed but it will help avoiding any unexpected regression
report pointing at the NETDEV_TX_LOCKED removal change.
--
Ueimor
Florian Westphal <[email protected]> :
> Francois Romieu <[email protected]> wrote:
[...]
> > Play it safe and keep the implicit local_irq_{save / restore} call ?
> >
> > It may not be needed but it will help avoiding any unexpected regression
> > report pointing at the NETDEV_TX_LOCKED removal change.
>
> I thought about that but it doesn't prevent the irq handler from
> running on another CPU, so leaving it around seemed like cargo culting
> to me...
I don't mind removing it in a different patch at all. I'd rather see
the commit history underline that it's unrelated to whatever
NETDEV_TX_LOCKED / LLTX change.
> I don't have an atl1c, but the atl1e in my laptop seems to work fine
> with the (similar) change.
>
> If you disagree I can respin with local_irq_save of course, but, if
> 'playing it safe' is main goal then its simpler to convert
> spin_trylock_irqsave to spin_lock_irqsave.
Your call, really.
--
Ueimor
From: Florian Westphal <[email protected]>
Date: Sun, 24 Apr 2016 21:38:08 +0200
> Not that many users left, lets kill it.
>
> TX_LOCKED was meant to be used by LLTX drivers when spin_trylock()
> failed. Stack then re-queued if collisions happened on different
> cpus or free'd the skb to prevent deadlocks.
>
> Most of the driver removal patches fall into one of three categories:
> 1. remove the driver-private tx lock (and LLTX flag), or...
> 2. convert spin_trylock to plain spin_lock, or...
> 3. convert TX_LOCKED to free+TX_OK
>
> Patches are grouped by these categories, last patch is the actual removal.
> All driver changes were compile tested only with exception of atl1e.
This looks good to me, series applied, thanks Florian.