Skip to content

Commit

Permalink
Merge branch 'netdev_tx_locked-removal'
Browse files Browse the repository at this point in the history
Florian Westphal says:

====================
net: core: remove TX_LOCKED support

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.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Apr 26, 2016
2 parents c971c0e + f0cdf76 commit c578e9a
Show file tree
Hide file tree
Showing 21 changed files with 43 additions and 121 deletions.
10 changes: 4 additions & 6 deletions Documentation/networking/netdev-features.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 3 additions & 6 deletions Documentation/networking/netdevices.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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.
Expand Down
13 changes: 5 additions & 8 deletions drivers/infiniband/hw/nes/nes_nic.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}


Expand Down Expand Up @@ -673,13 +672,11 @@ static int nes_netdev_start_xmit(struct sk_buff *skb, struct net_device *netdev)
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;
}

Expand Down
8 changes: 5 additions & 3 deletions drivers/net/ethernet/amd/7990.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions drivers/net/ethernet/amd/a2065.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions drivers/net/ethernet/atheros/atl1c/atl1c.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down Expand Up @@ -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;
Expand Down
11 changes: 0 additions & 11 deletions drivers/net/ethernet/atheros/atl1c/atl1c_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -2217,24 +2215,17 @@ 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;
}

tpd = atl1c_get_tpd(adapter, type);

/* 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;
}
Expand All @@ -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;
Expand Down
1 change: 0 additions & 1 deletion drivers/net/ethernet/atheros/atl1e/atl1e.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 1 addition & 11 deletions drivers/net/ethernet/atheros/atl1e/atl1e_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;

Expand All @@ -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;
}

Expand All @@ -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;
}
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions drivers/net/ethernet/chelsio/cxgb/sge.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
7 changes: 5 additions & 2 deletions drivers/net/ethernet/dec/tulip/de4x5.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}

/*
Expand Down
9 changes: 1 addition & 8 deletions drivers/net/ethernet/neterion/s2io.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
}
Expand All @@ -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)) {
Expand Down
6 changes: 2 additions & 4 deletions drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 1 addition & 7 deletions drivers/net/ethernet/tehuti/tehuti.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 */
Expand Down
6 changes: 4 additions & 2 deletions drivers/net/hamradio/baycom_epp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions drivers/net/hamradio/hdlcdrv.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading

0 comments on commit c578e9a

Please sign in to comment.