netpoll: Don't call driver methods from interrupt context.
Subsystems (60)
Re: [PATCH 50/54] xen-netfront: Call dev_kfree_skb_any instead of dev_kfree_skb.
In reply to Eric W. BiedermanQuoted 8 lines · Eric W. Biederman
From: "Eric W. Biederman" <[email protected]>
Replace
dev_kfree_skbwithdev_kfree_skb_anyinxennet_start_xmitwhich can be called in hard irq and other contexts.xennet_start_xmitonly fress skbs which it drops.Signed-off-by "Eric W. Biederman" <[email protected]>---
PATCH 51/54 wlags49_h2: Call dev_kfree/consume_skb_any instead of dev_kfree_skb.
In reply to Eric W. BiedermanFrom: "Eric W. Biederman" <[email protected]>
Replace dev_kfree_skb with dev_consume_skb_any in wl_send and wl_send_dma which can be called in hard irq and other contexts, on the code paths where the skb was transmitted successfully.
Replace dev_kfree_skb with dev_kfree_skb_any in wl_send_dmay which can be called in hard irq and other contexts, on the code path where a skb is dropped.
--- drivers/staging/wlags49_h2/wl_netdev.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/staging/wlags49_h2/wl_netdev.c +3 -3
@@ -715,7 +715,7 @@ int wl_send( struct wl_private *lp )/* Free the skb and perform queue cleanup, as the buffer was transmitted successfully */ -dev_kfree_skb ( lp->txF.skb ); +dev_consume_skb_any ( lp->txF.skb ); lp->txF.skb =NULL ; lp->txF.port =0 ;
@@ -1730,7 +1730,7 @@ int wl_send_dma( struct wl_private *lp, struct sk_buff *skb, int port )WL_WDS_NETIF_STOP_QUEUE ( lp ); lp->netif_queue_on = FALSE; -dev_kfree_skb ( skb ); +dev_kfree_skb_any ( skb );return 0 ; } }
@@ -1755,7 +1755,7 @@ int wl_send_dma( struct wl_private *lp, struct sk_buff *skb, int port )/* Free the skb and perform queue cleanup, as the buffer was transmitted successfully */ -dev_kfree_skb ( skb ); +dev_consume_skb_any ( skb );return TRUE; }// wl_send_dma
Signature · 1 line
1.7.10.4
Re: [PATCH 51/54] wlags49_h2: Call dev_kfree/consume_skb_any instead of dev_kfree_skb.
Quoted 6 lines · Eric W. Biederman
From: "Eric W. Biederman" <[email protected]>
Replace
dev_kfree_skbwithdev_consume_skb_anyinwl_sendandwl_send_dmawhich can be called in hard irq and other contexts, on the code paths where the skb was transmitted successfully.
Quoted 6 lines
Replace
dev_kfree_skbwithdev_kfree_skb_anyinwl_send_dmaywhich can be called in hard irq and other contexts, on the code path where a skb is dropped.Signed-off-by "Eric W. Biederman" <[email protected]>---
PATCH 52/54 staging/octeon-ethernet: Call dev_kfree/consume_skb_any instead of dev_kfree_skb.
In reply to Eric W. BiedermanFrom: "Eric W. Biederman" <[email protected]>
Replace dev_kfree_skb with dev_kfree_skb_any in cvm_oct_xmit_pow which can be called in hard irq and other contexts, on the code paths that drop packets.
Replace dev_kfree_skb with dev_consume_skb_any in cvm_oct_xmit_pow which can be called in hard irq and other contexts, on the code path where the packet is transmitted successfully.
--- drivers/staging/octeon/ethernet-tx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/staging/octeon/ethernet-tx.c +3 -3
@@ -554,7 +554,7 @@ int cvm_oct_xmit_pow(struct sk_buff *skb, struct net_device *dev)printk_ratelimited ("%s: Failed to allocate a work queue entry\n" , dev->name); priv->stats.tx_dropped++; -dev_kfree_skb (skb); +dev_kfree_skb_any (skb);return 0 ; }
@@ -565,7 +565,7 @@ int cvm_oct_xmit_pow(struct sk_buff *skb, struct net_device *dev) dev->name);cvmx_fpa_free (work,CVMX_FPA_WQE_POOL ,DONT_WRITEBACK (1 )); priv->stats.tx_dropped++; -dev_kfree_skb (skb); +dev_kfree_skb_any (skb);return 0 ; }
@@ -682,7 +682,7 @@ int cvm_oct_xmit_pow(struct sk_buff *skb, struct net_device *dev) work->grp); priv->stats.tx_packets++; priv->stats.tx_bytes += skb->len; -dev_kfree_skb (skb); +dev_consume_skb_any (skb);return 0 ; }
Signature · 1 line
1.7.10.4
Re: [PATCH 52/54] staging/octeon-ethernet: Call dev_kfree/consume_skb_any instead of dev_kfree_skb.
Quoted 6 lines · Eric W. Biederman
From: "Eric W. Biederman" <[email protected]>
Replace
dev_kfree_skbwithdev_kfree_skb_anyincvm_oct_xmit_powwhich can be called in hard irq and other contexts, on the code paths that drop packets.
Quoted 6 lines
Replace
dev_kfree_skbwithdev_consume_skb_anyincvm_oct_xmit_powwhich can be called in hard irq and other contexts, on the code path where the packet is transmitted successfully.Signed-off-by "Eric W. Biederman" <[email protected]>---
PATCH 53/54 virtio_net: Call dev_kfree_skb_any instead of dev_kfree_skb.
In reply to Eric W. BiedermanFrom: "Eric W. Biederman" <[email protected]>
Replace dev_kfree_skb with dev_kfree_skb_any in start_xmit which can be called in hard irq and other contexts.
start_xmit only frees skbs that it is dropping.
--- drivers/net/virtio_net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/virtio_net.c +1 -1
@@ -883,7 +883,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)dev_warn (&dev->dev,"Unexpected TXQ (%d) queue failure: %d\n" , qnum, err); dev->stats.tx_dropped++; -kfree_skb (skb); +dev_kfree_skb_any (skb);return NETDEV_TX_OK ; }
Signature · 1 line
1.7.10.4
Re: [PATCH 53/54] virtio_net: Call dev_kfree_skb_any instead of dev_kfree_skb.
Quoted 8 lines · Eric W. Biederman
From: "Eric W. Biederman" <[email protected]>
Replace
dev_kfree_skbwithdev_kfree_skb_anyinstart_xmitwhich can be called in hard irq and other contexts.
start_xmitonly frees skbs that it is dropping.Signed-off-by "Eric W. Biederman" <[email protected]>
PATCH 54/54 if_vlan: Call dev_kfree_skb_any instead of kfree_skb.
In reply to Eric W. BiedermanFrom: "Eric W. Biederman" <[email protected]>
Replace kfree_skb with dev_kfree_skb_any in vlan_insert_tag as vlan_insert_tag can be called from hard irq context (netpoll) and from other contexts.
dev_kfree_skb_any is used as vlan_insert_tag only frees the skb if the skb can not be modified to insert a tag, in which case vlan_insert_tag drops the skb.
--- include/linux/if_vlan.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/if_vlan.h +1 -1
@@ -288,7 +288,7 @@ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb,struct vlan_ethhdr *veth;if (skb_cow_head (skb,VLAN_HLEN ) <0 ) { -kfree_skb (skb); +dev_kfree_skb_any (skb);return NULL ; } veth = (struct vlan_ethhdr *)skb_push (skb,VLAN_HLEN );
Signature · 1 line
1.7.10.4
Re: [PATCH 54/54] if_vlan: Call dev_kfree_skb_any instead of kfree_skb.
Quoted 11 lines · Eric W. Biederman
From: "Eric W. Biederman" <[email protected]>
Replace
kfree_skbwithdev_kfree_skb_anyinvlan_insert_tagasvlan_insert_tagcan be called from hard irq context (netpoll) and from other contexts.
dev_kfree_skb_anyis used asvlan_insert_tagonly frees the skb if the skb can not be modified to insert a tag, in which casevlan_insert_tagdrops the skb.Signed-off-by "Eric W. Biederman" <[email protected]>
Re: [PATCH 01/54] uml/net_kern: Call dev_consume_skb_any instead of dev_kfree_skb.
In reply to Eric W. BiedermanQuoted 7 lines · Eric W. Biederman
From: "Eric W. Biederman" <[email protected]>
Replace
dev_kfree_skbwithdev_consume_skb_anyinuml_net_start_xmitas it can be called in hard irq and other contexts.
dev_consume_skb_anyis used asuml_net_start_xmittypically consumes (not drops) packets.
Well, this is not exactly true. This driver certainly can drop packets.
Here is an untested/not even compiled patch.
arch/um/drivers/net_kern.c +8 -8
@@ -212,6 +212,7 @@ static int uml_net_start_xmit(struct sk_buff *skb, struct net_device *dev)struct uml_net_private *lp =netdev_priv (dev);unsigned long flags;int len; +enum skb_free_reason reason =SKB_REASON_CONSUMED ;netif_stop_queue (dev);
@@ -228,19 +229,18 @@ static int uml_net_start_xmit(struct sk_buff *skb, struct net_device *dev)/* this is normally done in the interrupt when tx finishes */ netif_wake_queue (dev); - } -else if (len ==0 ) { -netif_start_queue (dev); - dev->stats.tx_dropped++; - } -else { + }else { + reason =SKB_REASON_DROPPED ;netif_start_queue (dev); -printk (KERN_ERR "uml_net_start_xmit: failed(%d)\n" , len); +if (len ==0 ) + dev->stats.tx_dropped++; +else +pr_err ("uml_net_start_xmit: failed(%d)\n" , len); }spin_unlock_irqrestore (&lp->lock, flags); -dev_kfree_skb (skb); +__dev_kfree_skb_any (skb, reason);return NETDEV_TX_OK ; }
Re: [PATCH 01/54] uml/net_kern: Call dev_consume_skb_any instead of dev_kfree_skb.
Quoted 6 lines · Eric Dumazet
Quoted 5 lines · Eric W. Biederman
From: "Eric W. Biederman" <[email protected]>
Replace
dev_kfree_skbwithdev_consume_skb_anyinuml_net_start_xmitas it can be called in hard irq and other contexts.
Quoted 6 lines
Quoted 2 lines
dev_consume_skb_anyis used asuml_net_start_xmittypically consumes (not drops) packets.Well, this is not exactly true. This driver certainly can drop packets.
Here is an untested/not even compiled patch.
I said typically which does make it true :P.
I care because I trying to keep us from calling kfree_skb or consume_skb aka dev_kfree_skb in hard irq context, as that can result in nasty issues.
Since I am touching those places I am doing my best to pick the correct consume or kfree variant that matches what the code does, and there isn't always one. But that is all at a best effort so I can preserve the code logic.
These patches are deliberately very conservative so I can successfully make and test them with simply code inspection.
I really don't think using enum skb_free_reason makes any sense whatsoever. Not in the implementation of dev_kfree_skb_any and dev_kfree_skb_irq and certainly not in a driver. What net/core/drop_monitor.c wants is the address of the function where drops occur (so we can track down and debug why the kernel is dropping packets) and the existing implementation of dev_kfree_skb_any and dev_kfree_skb_irq loose that information. The use of enum skb_free_reason is a big part of the reason why we loose that information. (We should be using a (void *) so that we can capture __builtin_return_address(0) instead. Your expanded use of enum skb_free_reason below seems to encourage the bad implemenation and make it even harder to fix dev_kfree_skb_any and dev_kfree_skb_irq.
In other locations with the same logic I justified the change as not changing semantics when going from consume_skb (aka dev_kfree_skb) to dev_consume_skb. My apologies for not mentioning that in uml/net_kern. I think not causing a regression in the kfree/consume distinction is more important than getting the code exactly right.
If you are really interested in seeing us get the consume_skb vs kfree_skb difference correct in drivers I recommend an audit of drivers yourself. A few weeks ago when I started looking at this there was exactly one instance of dev_consume_skb_any or was it just consume_skb in the entire driver tree.
So I really think getting the drop vs consume distinction perfect right now is silly when it is hard to test and we have so much low hanging fruit where the distinction was not even recognized.
So please ack my patches (especially this one) on the basis of execution context correctness and drop/kfree distinction no regression or best effort correctness.
Eric
Quoted 39 lines
arch/um/drivers/net_kern.c +8
-8@@ -212,6 +212,7 @@ static int uml_net_start_xmit(struct sk_buff *skb, struct net_device *dev)struct uml_net_private *lp =netdev_priv (dev);unsigned long flags;int len; +enum skb_free_reason reason =SKB_REASON_CONSUMED ;netif_stop_queue (dev);@@ -228,19 +229,18 @@ static int uml_net_start_xmit(struct sk_buff *skb, struct net_device *dev)/* this is normally done in the interrupt when tx finishes */ netif_wake_queue (dev); - } -else if (len ==0 ) { -netif_start_queue (dev); - dev->stats.tx_dropped++; - } -else { + }else { + reason =SKB_REASON_DROPPED ;netif_start_queue (dev); -printk (KERN_ERR "uml_net_start_xmit: failed(%d)\n" , len); +if (len ==0 ) + dev->stats.tx_dropped++; +else +pr_err ("uml_net_start_xmit: failed(%d)\n" , len); }spin_unlock_irqrestore (&lp->lock, flags); -dev_kfree_skb (skb); +__dev_kfree_skb_any (skb, reason);return NETDEV_TX_OK ; }
RE: [PATCH 01/54] uml/net_kern: Call dev_consume_skb_any instead of dev_kfree_skb.
From: Of Eric W. Biederman
...
Quoted 10 lines
I really don't think using enum
skb_free_reasonmakes any sense whatsoever. Not in the implementation ofdev_kfree_skb_anyanddev_kfree_skb_irqand certainly not in a driver. Whatnet/core/drop_monitor.cwants is the address of the function where drops occur (so we can track down and debug why the kernel is dropping packets) and the existing implementation ofdev_kfree_skb_anyanddev_kfree_skb_irqloose that information. The use of enumskb_free_reasonis a big part of the reason why we loose that information. (We should be using a (void *) so that we can capture__builtin_return_address(0)instead...
Maybe more useful to allow a literal string be given. Easier to find in the source tree than the return address.
Or (OTT) create a linkset data item containing info about the call site and a counter....
David
Eric Dumazet
Acked-byRe: [net-next 00/54][pull request] Using dev_kfree/consume_skb_any for functions called in multiple contexts
In reply to Eric W. BiedermanQuoted 24 lines · Eric W. Biederman
These changes are a result of walking through the network drivers supporting netpoll and verifying the code paths that netpoll can cause to be called in hard irq context use an appropriate flavor of
kfree_skb. Eitherdev_kfree_skb_anyordev_consume_skb_any.Since my last pass at this I have become aware of the small differences between
dev_kfree_skb_anyanddev_consume_skb_any.net/core/drop_monitor.creports thedev_kfree_skb_anyas a drop and while being quite about the second. With the weird twist thatdev_kfree_skbis unintuitivelyconsume_skb.As netpoll now calls the napi poll function with budget == 0, pieces of a drivers the napi poll function that don't run when budget == 0 have been ignored.
The most interesting change is to the atl1c which tried unsuccesfully to tell one of it's functions which context it is called in so that it could call
dev_kfree_skb_irqordev_kfree_skbas appropriate. I have just removed the extra parameter and calleddev_consume_skb_any.At 54 separate changes I will post each change as a separate patch (so they can be reviewed) but for general sanity sake I have gathered them all into a git branch for easy acces.
Quoted 6 lines
David when you are satisified with these changes please pull:
git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/net-next.gitmasterHopefully this will allow me to forget this class of error when dealing with netpoll.
Perfect, thanks Eric.
(the minor issues can be addressed later in a single followup)
David Miller
maintainerRe: [net-next 00/54][pull request] Using dev_kfree/consume_skb_any for functions called in multiple contexts
From: Eric Dumazet <[email protected]>
Date: Tue, 25 Mar 2014 13:49:51 -0700
Quoted 33 lines · Eric W. Biederman
Quoted 24 lines
These changes are a result of walking through the network drivers supporting netpoll and verifying the code paths that netpoll can cause to be called in hard irq context use an appropriate flavor of
kfree_skb. Eitherdev_kfree_skb_anyordev_consume_skb_any.Since my last pass at this I have become aware of the small differences between
dev_kfree_skb_anyanddev_consume_skb_any.net/core/drop_monitor.creports thedev_kfree_skb_anyas a drop and while being quite about the second. With the weird twist thatdev_kfree_skbis unintuitivelyconsume_skb.As netpoll now calls the napi poll function with budget == 0, pieces of a drivers the napi poll function that don't run when budget == 0 have been ignored.
The most interesting change is to the atl1c which tried unsuccesfully to tell one of it's functions which context it is called in so that it could call
dev_kfree_skb_irqordev_kfree_skbas appropriate. I have just removed the extra parameter and calleddev_consume_skb_any.At 54 separate changes I will post each change as a separate patch (so they can be reviewed) but for general sanity sake I have gathered them all into a git branch for easy acces.
Quoted 6 lines
David when you are satisified with these changes please pull:
git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/net-next.gitmasterHopefully this will allow me to forget this class of error when dealing with netpoll.
Quoted 5 lines
Perfect, thanks Eric.
(the minor issues can be addressed later in a single followup)
Acked-by Eric Dumazet <[email protected]>
Pulled, thanks everyone.
cover Don't receive packets when the napi budget == 0
In reply to David MillerTo the best of understanding processing any received packets when the napi budget == 0 is broken driver behavior. At the same time I don't think we have ever cared before so there are a handful of drivers that need fixes.
I care now as I will shortly be using htis in netpoll to get the tx queue processing without the rx queue processing.
Drivers that need fixes are few and far between, and so far I have only found two of them. More similar patches later if I find more drivers that need fixes.
Eric W. Biederman (2):
bnx2: Don't receive packets when the napi budget == 0
8139cp: Don't receive packets when the napi budget == 0drivers/net/ethernet/broadcom/bnx2.c | 3 +++ drivers/net/ethernet/realtek/8139cp.c | 5 +---- 2 files changed, 4 insertions(+), 4 deletions(-)
Eric
PATCH 1/2 bnx2: Don't receive packets when the napi budget == 0
Processing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/broadcom/bnx2.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/net/ethernet/broadcom/bnx2.c +3 -0
@@ -3132,6 +3132,9 @@ bnx2_rx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget)struct l2_fhdr *rx_hdr;int rx_pkt =0 , pg_ring_used =0 ; +if (budget <=0 ) +return rx_pkt; + hw_cons =bnx2_get_hw_rx_cons (bnapi); sw_cons = rxr->rx_cons; sw_prod = rxr->rx_prod;
Signature · 1 line
1.7.5.4
Re: [PATCH net-next 1/2] bnx2: Don't receive packets when the napi budget == 0
Quoted 10 lines · Eric W. Biederman
Processing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
Signed-off-by "Eric W. Biederman" <[email protected]>--- drivers/net/ethernet/broadcom/bnx2.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
PATCH 2/2 8139cp: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/realtek/8139cp.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-)
drivers/net/ethernet/realtek/8139cp.c +1 -4
@@ -476,7 +476,7 @@ rx_status_loop: rx =0 ;cpw16 (IntrStatus, cp_rx_intr_mask); -while (1 ) { +while (rx < budget) {u32 status, len;dma_addr_t mapping, new_mapping;struct sk_buff *skb, *new_skb;
@@ -554,9 +554,6 @@ rx_next:else desc->opts1 =cpu_to_le32 (DescOwn | cp->rx_buf_sz); rx_tail =NEXT_RX (rx_tail); - -if (rx >= budget) -break ; } cp->rx_tail = rx_tail;
Signature · 1 line
1.7.5.4
Re: [PATCH net-next 2/2] 8139cp: Don't receive packets when the napi budget == 0
Quoted 10 lines · Eric W. Biederman
Processing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
Signed-off-by "Eric W. Biederman" <[email protected]--- drivers/net/ethernet/realtek/8139cp.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-)
David Miller
maintainerRe: [PATCH net-next 0/2] Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanFrom: [email protected] (Eric W. Biederman)
Date: Tue, 11 Mar 2014 14:30:11 -0700
Quoted 11 lines
To the best of understanding processing any received packets when the napi budget == 0 is broken driver behavior. At the same time I don't think we have ever cared before so there are a handful of drivers that need fixes.
I care now as I will shortly be using htis in netpoll to get the tx queue processing without the rx queue processing.
Drivers that need fixes are few and far between, and so far I have only found two of them. More similar patches later if I find more drivers that need fixes.
Series applied, thanks Eric.
cover Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanAfter reading through all 120 drivers supporting netpoll I have found 16 more that process at least received packet when the napi budget == 0.
Processing more packets than your budget has always been a bug but we haven't cared before so it looks like these drivers slipped through, and need fixes.
As netpoll will shortly be using a budget of 0 to get the tx queue processing with the rx queue processing we now care.
Eric W. Biederman (16):
bnx2x: Don't receive packets when the napi budget == 0
i40e: Don't receive packets when the napi budget == 0
igb: Don't receive packets when the napi budget == 0
ixgbe: Don't receive packets when the napi budget == 0
amd8111e: Don't receive packets when the napi budget == 0
enic: Don't receive packets when the napi budget == 0
fs_enet: Don't receive packets when the napi budget == 0
ibmveth: Don't receive packets when the napi budget == 0
sky2: Don't receive packets when the napi budget == 0
mlx4: Don't receive packets when the napi budget == 0
s2io: Don't receive packets when the napi budget == 0
tilegx: Don't receive packets when the napi budget == 0
tilepro: Don't receive packets when the napi budget == 0
tc35815: Don't receive packets when the napi budget == 0
vxge: Don't receive packets when the napi budget == 0
sfc: Don't receive packets when the napi budget == 0drivers/net/ethernet/amd/amd8111e.c | 3 +++ drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 2 ++ drivers/net/ethernet/cisco/enic/enic_main.c | 14 ++++++++------ .../net/ethernet/freescale/fs_enet/fs_enet-main.c | 3 +++ drivers/net/ethernet/ibm/ibmveth.c | 4 ++-- drivers/net/ethernet/intel/i40e/i40e_txrx.c | 3 +++ drivers/net/ethernet/intel/igb/igb_main.c | 4 ++-- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++-- drivers/net/ethernet/marvell/sky2.c | 3 +++ drivers/net/ethernet/mellanox/mlx4/en_rx.c | 3 +++ drivers/net/ethernet/neterion/s2io.c | 3 +++ drivers/net/ethernet/neterion/vxge/vxge-main.c | 4 ++++ drivers/net/ethernet/sfc/ef10.c | 3 +++ drivers/net/ethernet/sfc/farch.c | 3 +++ drivers/net/ethernet/tile/tilegx.c | 3 +++ drivers/net/ethernet/tile/tilepro.c | 3 +++ drivers/net/ethernet/toshiba/tc35815.c | 3 +++ 17 files changed, 53 insertions(+), 12 deletions(-)
Eric
PATCH 1/16 bnx2x: Don't receive packets when the napi budget == 0
Processing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c +2 -0
@@ -872,6 +872,8 @@ static int bnx2x_rx_int(struct bnx2x_fastpath *fp, int budget)if (unlikely (bp->panic))return 0 ;#endif +if (budget <=0 ) +return rx_pkt; bd_cons = fp->rx_bd_cons; bd_prod = fp->rx_bd_prod;
Signature · 1 line
1.7.5.4
PATCH 2/16 i40e: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/intel/i40e/i40e_txrx.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/net/ethernet/intel/i40e/i40e_txrx.c +3 -0
@@ -1264,6 +1264,9 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget)u8 rx_ptype;u64 qword; +if (budget <=0 ) +return 0 ; + rx_desc =I40E_RX_DESC (rx_ring, i); qword =le64_to_cpu (rx_desc->wb.qword1.status_error_len); rx_status = (qword & I40E_RXD_QW1_STATUS_MASK) >>
Signature · 1 line
1.7.5.4
PATCH 3/16 igb: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/intel/igb/igb_main.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/intel/igb/igb_main.c +2 -2
@@ -6946,7 +6946,7 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget)unsigned int total_bytes =0 , total_packets =0 ;u16 cleaned_count =igb_desc_unused (rx_ring); - do { +while (likely (total_packets < budget)) {union e1000_adv_rx_desc *rx_desc;/* return some buffers to hardware, one at a time is too slow */
@@ -6998,7 +6998,7 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget)/* update budget accounting */ total_packets++; - }while (likely (total_packets < budget)); + }/* place incomplete frames back on ring for completion */ rx_ring->skb = skb;
Signature · 1 line
1.7.5.4
PATCH 4/16 ixgbe: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +2 -2
@@ -2076,7 +2076,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,#endif /* IXGBE_FCOE */ u16 cleaned_count =ixgbe_desc_unused (rx_ring); - do { +while (likely (total_rx_packets < budget)) {union ixgbe_adv_rx_desc *rx_desc;struct sk_buff *skb;
@@ -2151,7 +2151,7 @@ static int ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,/* update budget accounting */ total_rx_packets++; - }while (likely (total_rx_packets < budget)); + }u64_stats_update_begin (&rx_ring->syncp); rx_ring->stats.packets += total_rx_packets;
Signature · 1 line
1.7.5.4
PATCH 5/16 amd8111e: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/amd/amd8111e.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/net/ethernet/amd/amd8111e.c +3 -0
@@ -720,6 +720,9 @@ static int amd8111e_rx_poll(struct napi_struct *napi, int budget)int rx_pkt_limit = budget;unsigned long flags; +if (rx_pkt_limit <=0 ) + goto rx_not_empty; + do{ /* process receive packets until we use the quota*/ /* If we own the next entry, it's a new packet. Send it up. */
Signature · 1 line
1.7.5.4
PATCH 6/16 enic: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/cisco/enic/enic_main.c | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-)
drivers/net/ethernet/cisco/enic/enic_main.c +8 -6
@@ -1086,14 +1086,15 @@ static int enic_poll(struct napi_struct *napi, int budget)unsigned int intr =enic_legacy_io_intr ();unsigned int rq_work_to_do = budget;unsigned int wq_work_to_do =-1 ;/* no limit */ -unsigned int work_done, rq_work_done, wq_work_done; +unsigned int work_done, rq_work_done =0 , wq_work_done;int err;/* Service RQ (first) and WQ */ - rq_work_done =vnic_cq_service (&enic->cq[cq_rq], - rq_work_to_do, enic_rq_service,NULL ); +if (budget >0 ) + rq_work_done =vnic_cq_service (&enic->cq[cq_rq], + rq_work_to_do, enic_rq_service,NULL ); wq_work_done =vnic_cq_service (&enic->cq[cq_wq], wq_work_to_do, enic_wq_service,NULL );
@@ -1141,14 +1142,15 @@ static int enic_poll_msix(struct napi_struct *napi, int budget)unsigned int cq =enic_cq_rq (enic, rq);unsigned int intr =enic_msix_rq_intr (enic, rq);unsigned int work_to_do = budget; -unsigned int work_done; +unsigned int work_done =0 ;int err;/* Service RQ */ - work_done =vnic_cq_service (&enic->cq[cq], - work_to_do, enic_rq_service,NULL ); +if (budget >0 ) + work_done =vnic_cq_service (&enic->cq[cq], + work_to_do, enic_rq_service,NULL );/* Return intr event credits for this polling * cycle. An intr event is the completion of a
Signature · 1 line
1.7.5.4
PATCH 7/16 fs_enet: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- .../net/ethernet/freescale/fs_enet/fs_enet-main.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c +3 -0
@@ -91,6 +91,9 @@ static int fs_enet_rx_napi(struct napi_struct *napi, int budget)u16 pkt_len, sc;int curidx; +if (budget <=0 ) +return received; +/* * First, grab all of the stats for the incoming packet. * These get messed up if we get called due to a busy condition.
Signature · 1 line
1.7.5.4
PATCH 8/16 ibmveth: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/ibm/ibmveth.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/ibm/ibmveth.c +2 -2
@@ -1062,7 +1062,7 @@ static int ibmveth_poll(struct napi_struct *napi, int budget)unsigned long lpar_rc; restart_poll: - do { +while (frames_processed < budget) {if (!ibmveth_rxq_pending_buffer (adapter))break ;
@@ -1111,7 +1111,7 @@ restart_poll: netdev->stats.rx_bytes += length; frames_processed++; } - }while (frames_processed < budget); + }ibmveth_replenish_task (adapter);
Signature · 1 line
1.7.5.4
PATCH 9/16 sky2: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/marvell/sky2.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/net/ethernet/marvell/sky2.c +3 -0
@@ -2735,6 +2735,9 @@ static int sky2_status_intr(struct sky2_hw *hw, int to_do, u16 idx)unsigned int total_bytes[2 ] = {0 };unsigned int total_packets[2 ] = {0 }; +if (to_do <=0 ) +return work_done; +rmb (); do { struct sky2_port *sky2;
Signature · 1 line
1.7.5.4
Re: [PATCH net-next 09/16] sky2: Don't receive packets when the napi budget == 0
Quoted 17 lines · On Fri, 14 Mar 2014 18:05:26 -0700 [email protected] (Eric W. Biederman)…
Processing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
Signed-off-by "Eric W. Biederman" <[email protected]>--- drivers/net/ethernet/marvell/sky2.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)drivers/net/ethernet/marvell/sky2.c +0
-0
drivers/net/ethernet/marvell/sky2.c static int sky2_status_intr(struct sky2_hw *hw, int to_do, u16 idx) · line 2743
Quoted 9 lines
drivers/net/ethernet/marvell/sky2.c +3
-0@@ -2735,6 +2735,9 @@ static int sky2_status_intr(struct sky2_hw *hw, int to_do, u16 idx)unsigned int total_bytes[2 ] = {0 };unsigned int total_packets[2 ] = {0 }; +if (to_do <=0 ) +return work_done; +rmb (); do { struct sky2_port *sky2;
I am fine with this.
Really should change to_do to an unsigned but that is another battle.
PATCH 10/16 mlx4: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/mellanox/mlx4/en_rx.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/net/ethernet/mellanox/mlx4/en_rx.c +3 -0
@@ -661,6 +661,9 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int budif (!priv->port_up)return 0 ; +if (budget <=0 ) +return polled; +/* We assume a 1:1 mapping between CQEs and Rx descriptors, so Rx * descriptor offset can be deduced from the CQE index instead of * reading 'cqe->index' */
Signature · 1 line
1.7.5.4
PATCH 11/16 s2io: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/neterion/s2io.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/net/ethernet/neterion/s2io.c +3 -0
@@ -2914,6 +2914,9 @@ static int rx_intr_handler(struct ring_info *ring_data, int budget)struct RxD1 *rxdp1;struct RxD3 *rxdp3; +if (budget <=0 ) +return napi_pkts; + get_info = ring_data->rx_curr_get_info; get_block = get_info.block_index;memcpy (&put_info, &ring_data->rx_curr_put_info,sizeof (put_info));
Signature · 1 line
1.7.5.4
PATCH 12/16 tilegx: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/tile/tilegx.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/net/ethernet/tile/tilegx.c +3 -0
@@ -659,6 +659,9 @@ static int tile_net_poll(struct napi_struct *napi, int budget)struct info_mpipe *info_mpipe = container_of(napi, struct info_mpipe, napi); +if (budget <=0 ) + goto done; + instance = info_mpipe->instance; while ((n = gxio_mpipe_iqueue_try_peek( &info_mpipe->iqueue,
Signature · 1 line
1.7.5.4
PATCH 13/16 tilepro: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/tile/tilepro.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/net/ethernet/tile/tilepro.c +3 -0
@@ -831,6 +831,9 @@ static int tile_net_poll(struct napi_struct *napi, int budget)unsigned int work =0 ; +if (budget <=0 ) + goto done; +while (priv->active) {int index = qup->__packet_receive_read;if (index == qsp->__packet_receive_queue.__packet_write)
Signature · 1 line
1.7.5.4
PATCH 14/16 tc35815: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/toshiba/tc35815.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/net/ethernet/toshiba/tc35815.c +3 -0
@@ -1645,6 +1645,9 @@ static int tc35815_poll(struct napi_struct *napi, int budget)int received =0 , handled;u32 status; +if (budget <=0 ) +return received; +spin_lock (&lp->rx_lock); status =tc_readl (&tr->Int_Src); do {
Signature · 1 line
1.7.5.4
PATCH 15/16 vxge: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/neterion/vxge/vxge-main.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
drivers/net/ethernet/neterion/vxge/vxge-main.c +4 -0
@@ -368,6 +368,9 @@ vxge_rx_1b_compl(struct __vxge_hw_ring *ringh, void *dtr,vxge_debug_entryexit (VXGE_TRACE ,"%s: %s:%d" , ring->ndev->name, __func__, __LINE__); +if (ring->budget <=0 ) + goto out; + do { prefetch((char *)dtr + L1_CACHE_BYTES); rx_priv = vxge_hw_ring_rxd_private_get(dtr);
@@ -525,6 +528,7 @@ vxge_rx_1b_compl(struct __vxge_hw_ring *ringh, void *dtr,if (first_dtr)vxge_hw_ring_rxd_post_post_wmb (ringh, first_dtr); +out:vxge_debug_entryexit (VXGE_TRACE ,"%s:%d Exiting..." , __func__, __LINE__);
Signature · 1 line
1.7.5.4
PATCH 16/16 sfc: Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanProcessing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
--- drivers/net/ethernet/sfc/ef10.c | 3 +++ drivers/net/ethernet/sfc/farch.c | 3 +++ 2 files changed, 6 insertions(+), 0 deletions(-)
drivers/net/ethernet/sfc/ef10.c +3 -0
@@ -1955,6 +1955,9 @@ static int efx_ef10_ev_process(struct efx_channel *channel, int quota)int tx_descs =0 ;int spent =0 ; +if (quota <=0 ) +return spent; + read_ptr = channel->eventq_read_ptr;for (;;) {
drivers/net/ethernet/sfc/farch.c +3 -0
@@ -1248,6 +1248,9 @@ int efx_farch_ev_process(struct efx_channel *channel, int budget)int tx_packets =0 ;int spent =0 ; +if (budget <=0 ) +return spent; + read_ptr = channel->eventq_read_ptr;for (;;) {
Signature · 1 line
1.7.5.4
Re: [PATCH net-next 16/16] sfc: Don't receive packets when the napi budget == 0
Quoted 5 lines · Eric W. Biederman
Processing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
But this also prevents handling TX completions, at which point you may as well change efx_netpoll() to a no-op. And then, does it make sense to implement ndo_poll_controller at all?
Note that sfc does have a module parameter to enable separate RX and TX completions so they could be polled separately, but it is disabled by default.
Ben.
Quoted 34 lines
Signed-off-by "Eric W. Biederman" <[email protected]>--- drivers/net/ethernet/sfc/ef10.c | 3 +++ drivers/net/ethernet/sfc/farch.c | 3 +++ 2 files changed, 6 insertions(+), 0 deletions(-)drivers/net/ethernet/sfc/ef10.c +3
-0@@ -1955,6 +1955,9 @@ static int efx_ef10_ev_process(struct efx_channel *channel, int quota)int tx_descs =0 ;int spent =0 ; +if (quota <=0 ) +return spent; + read_ptr = channel->eventq_read_ptr;for (;;) {drivers/net/ethernet/sfc/farch.c +3
-0@@ -1248,6 +1248,9 @@ int efx_farch_ev_process(struct efx_channel *channel, int budget)int tx_packets =0 ;int spent =0 ; +if (budget <=0 ) +return spent; + read_ptr = channel->eventq_read_ptr;for (;;) {
Signature · 3 lines
Ben Hutchings When you say `I wrote a program that crashed Windows', people just stare ... and say `Hey, I got those with the system, *for free*'. - Linus Torvalds
David Miller
maintainerRe: [PATCH net-next 16/16] sfc: Don't receive packets when the napi budget == 0
From: Ben Hutchings <[email protected]>
Date: Sat, 15 Mar 2014 15:23:34 +0000
Quoted 7 lines · Eric W. Biederman
Quoted 5 lines
Processing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
Quoted 7 lines
But this also prevents handling TX completions, at which point you may as well change
efx_netpoll()to a no-op. And then, does it make sense to implementndo_poll_controllerat all?Note that sfc does have a module parameter to enable separate RX and TX completions so they could be polled separately, but it is disabled by default.
TX completions should run unconditionally, irregardless of the given budget.
This is how I have coded all of my drivers, and I how I tell others to do so.
Re: [PATCH net-next 16/16] sfc: Don't receive packets when the napi budget == 0
Quoted 18 lines · David Miller
From: Ben Hutchings <[email protected]>
Date: Sat, 15 Mar 2014 15:23:34 +0000Quoted 7 lines · Eric W. Biederman
Quoted 5 lines
Processing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
Quoted 7 lines
But this also prevents handling TX completions, at which point you may as well change
efx_netpoll()to a no-op. And then, does it make sense to implementndo_poll_controllerat all?Note that sfc does have a module parameter to enable separate RX and TX completions so they could be polled separately, but it is disabled by default.
Quoted 5 lines
TX completions should run unconditionally, irregardless of the given budget.
This is how I have coded all of my drivers, and I how I tell others to do so.
The Solarflare hardware provides generic event queues for RX and TX completions, link changes, errors, etc. The driver can't process TX completions without going through all the other events mixed in with them.
It is possible to allocate separate event queues for TX completions but that isn't done by default because it also requires extra IRQs. The driver could be restructured to allocate some event queues without IRQs of their own, but it probably requires a lot of work.
Ben.
Signature · 3 lines
Ben Hutchings When you say `I wrote a program that crashed Windows', people just stare ... and say `Hey, I got those with the system, *for free*'. - Linus Torvalds
Re: [PATCH net-next 16/16] sfc: Don't receive packets when the napi budget == 0
Quoted 5 lines · Ben Hutchings
The Solarflare hardware provides generic event queues for RX and TX completions, link changes, errors, etc. The driver can't process TX completions without going through all the other events mixed in with them.
ndo_poll_controller() could process all events, and queue incoming packets through netif_rx().
Nobody cares about performance in this path.
Re: [PATCH net-next 16/16] sfc: Don't receive packets when the napi budget == 0
Quoted 8 lines · Eric Dumazet
Quoted 5 lines · Ben Hutchings
The Solarflare hardware provides generic event queues for RX and TX completions, link changes, errors, etc. The driver can't process TX completions without going through all the other events mixed in with them.
Quoted 4 lines
ndo_poll_controller()could process all events, and queue incoming packets throughnetif_rx().Nobody cares about performance in this path.
I made this change so that we don't process rx packets in hard irq context and cause things to break, and I made the most minimal most obvious change I could.
queueing incoming packets with trough netif_rx sounds elegant.
The practical challenge is that there might be napi bottom halves already scheduled to run before ndo_poll_controller is called so even if ndo_poll_controller processes all of the events and calls netif_rx to deal with the incoming packets the napi bottom halves will have to still not process the queues when they are called.
Eric
mlx4 netpoll and rx/tx weirdness
In reply to David MillerQuoted 18 lines · David Miller
From: Ben Hutchings <[email protected]>
Date: Sat, 15 Mar 2014 15:23:34 +0000Quoted 7 lines · Eric W. Biederman
Quoted 5 lines
Processing any incoming packets with a with a napi budget of 0 is incorrect driver behavior.
This matters as netpoll will shortly call drivers with a budget of 0 to avoid receive packet processing happening in hard irq context.
Quoted 7 lines
But this also prevents handling TX completions, at which point you may as well change
efx_netpoll()to a no-op. And then, does it make sense to implementndo_poll_controllerat all?Note that sfc does have a module parameter to enable separate RX and TX completions so they could be polled separately, but it is disabled by default.
Quoted 5 lines
TX completions should run unconditionally, irregardless of the given budget.
This is how I have coded all of my drivers, and I how I tell others to do so.
Given this comment I have to ask: How insane is the mellanox mlx4 driver that has separate rx and tx queues, separate rx and tx interrupts and uses separate napi bottom halves to process each, and honors the budget passed into it's rx napi handler.
Right now the mlx4 code disables interrupts and calls napi_synchronize in it's netpoll routine instead of just scheduling the napi bottom halves as all of the sane drivers do. napi_synchronize trying to sleep in hard irq context is pretty terrible, but I can see roughly how that should be fixed.
drivers/net/ethernet/mellanox/mlx4/en_netdev.c +1 -7
@@ -1302,16 +1302,10 @@ out:static void mlx4_en_netpoll (struct net_device *dev) {struct mlx4_en_priv *priv =netdev_priv (dev); -struct mlx4_en_cq *cq; -unsigned long flags;int i;for (i =0 ; i < priv->rx_ring_num; i++) { - cq = priv->rx_cq[i]; -spin_lock_irqsave (&cq->lock, flags); -napi_synchronize (&cq->napi); -mlx4_en_process_rx_cq (dev, cq,0 ); -spin_unlock_irqrestore (&cq->lock, flags); +napi_schedule (&priv->tx_cq[i]->napi); } }
What I can't see is what is a clean thing to do with the mlx4 tx bottom napi bottom half. As it won't processes the tx cq when I pass in a budget of 0.
What I can't see is how to prevent a netpoll stalling after enough packets are transmitted from hard irq context say with sysrq-l on a machine with enough cpus to fill the tx queue.
If possible I would very much like to get this to work.
Eric
Re: mlx4 netpoll and rx/tx weirdness
Quoted 4 lines · Eric W. Biederman
Given this comment I have to ask: How insane is the mellanox mlx4 driver that has separate rx and tx queues, separate rx and tx interrupts and uses separate napi bottom halves to process each, and honors the budget passed into it's rx napi handler.
I believe this driver had TX completion from hard irq, and NAPI polling for RX.
This was changed quite recently to also do TX completion from softirq handler.
Quoted 18 lines
Right now the mlx4 code disables interrupts and calls
napi_synchronizein it's netpoll routine instead of just scheduling the napi bottom halves as all of the sane drivers do.napi_synchronizetrying to sleep in hard irq context is pretty terrible, but I can see roughly how that should be fixed.drivers/net/ethernet/mellanox/mlx4/en_netdev.c +0
-2@@ -1302,16 +1302,10 @@ out:static void mlx4_en_netpoll (struct net_device *dev) {struct mlx4_en_priv *priv =netdev_priv (dev); -struct mlx4_en_cq *cq; -unsigned long flags;int i;
drivers/net/ethernet/mellanox/mlx4/en_netdev.c out: · line 1310
Quoted 10 lines
drivers/net/ethernet/mellanox/mlx4/en_netdev.c +1
-5for (i =0 ; i < priv->rx_ring_num; i++) { - cq = priv->rx_cq[i]; -spin_lock_irqsave (&cq->lock, flags); -napi_synchronize (&cq->napi); -mlx4_en_process_rx_cq (dev, cq,0 ); -spin_unlock_irqrestore (&cq->lock, flags); +napi_schedule (&priv->tx_cq[i]->napi); } }
Wait a minute, I thought ndo_poll_controller() had to be synchronous, not schedule a napi ?
Quoted 4 lines
What I can't see is what is a clean thing to do with the mlx4 tx bottom napi bottom half. As it won't processes the tx cq when I pass in a budget of 0.
Just ignore the budget to drain tx queues, as David said.
Quoted 4 lines
What I can't see is how to prevent a netpoll stalling after enough packets are transmitted from hard irq context say with sysrq-l on a machine with enough cpus to fill the tx queue.
Is this specific to mlx4 ?
David Miller
maintainerRe: mlx4 netpoll and rx/tx weirdness
From: Eric Dumazet <[email protected]>
Date: Sun, 16 Mar 2014 09:17:01 -0700
Quoted 2 lines
Wait a minute, I thought
ndo_poll_controller()had to be synchronous, not schedule a napi ?
ndo_poll_controller() basically runs the hardware interrupt handler, which might schedule NAPI processing. Then netpoll invokes the NAPI poll in whatever state ndo_poll_controller() left it in.
Re: mlx4 netpoll and rx/tx weirdness
Quoted 9 lines · David Miller
From: Eric Dumazet <[email protected]>
Date: Sun, 16 Mar 2014 09:17:01 -0700Quoted 2 lines
Wait a minute, I thought
ndo_poll_controller()had to be synchronous, not schedule a napi ?
ndo_poll_controller()basically runs the hardware interrupt handler, which might schedule NAPI processing. Then netpoll invokes the NAPI poll in whatever statendo_poll_controller()left it in.
Ah right, this makes sense, poll_napi() uses a spin_trylock()...
David Miller
maintainerRe: [PATCH net-next 0/16] Don't receive packets when the napi budget == 0
In reply to Eric W. BiedermanFrom: [email protected] (Eric W. Biederman)
Date: Fri, 14 Mar 2014 17:56:47 -0700
Quoted 9 lines
After reading through all 120 drivers supporting netpoll I have found 16 more that process at least received packet when the napi budget == 0.
Processing more packets than your budget has always been a bug but we haven't cared before so it looks like these drivers slipped through, and need fixes.
As netpoll will shortly be using a budget of 0 to get the tx queue processing with the rx queue processing we now care.
Series applied, thanks Eric.
PATCH 1/1 bcm63xx_enet: Stop pretending to support netpoll
In reply to David Millerbcm_enet_netpoll does not exist, and causing bcm63xx_net to fail to build when NET_POLL_CONTROLLER is defined.
Remove the bogus .ndo_poll_controller = bcm_enet_netpoll
--- drivers/net/ethernet/broadcom/bcm63xx_enet.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-)
drivers/net/ethernet/broadcom/bcm63xx_enet.c +0 -3
@@ -1722,9 +1722,6 @@ static const struct net_device_ops bcm_enet_ops = { .ndo_set_rx_mode = bcm_enet_set_multicast_list, .ndo_do_ioctl = bcm_enet_ioctl, .ndo_change_mtu = bcm_enet_change_mtu, -#ifdef CONFIG_NET_POLL_CONTROLLER - .ndo_poll_controller = bcm_enet_netpoll, -#endif };/*
Signature · 1 line
1.7.5.4
Re: [PATCH net-next] bcm63xx_enet: Stop pretending to support netpoll
From: [email protected] (Eric W. Biederman)
Date: Tue, 11 Mar 2014 14:33:35 -0700
Quoted 7 lines
bcm_enet_netpolldoes not exist, and causingbcm63xx_netto fail to build whenNET_POLL_CONTROLLERis defined.Remove the bogus .ndo_poll_controller =
bcm_enet_netpollSigned-off-by "Eric W. Biederman" <[email protected]>
Applied.
This driver doesn't get a lot of build testing, and that's due to the platform BCM63XX kconfig option it depends upon. Probably it should be exposed more widely to perhaps a more broad dependency such as CONFIG_OF.
Re: [PATCH net-next] bcm63xx_enet: Stop pretending to support netpoll
2014-03-13 12:26 GMT-07:00 David Miller <[email protected]>:
Quoted 9 lines
From: [email protected] (Eric W. Biederman)
Date: Tue, 11 Mar 2014 14:33:35 -0700Quoted 6 lines
bcm_enet_netpolldoes not exist, and causingbcm63xx_netto fail to build whenNET_POLL_CONTROLLERis defined.Remove the bogus .ndo_poll_controller =
bcm_enet_netpoll
Quoted 8 lines
Quoted 1 line
Signed-off-by "Eric W. Biederman" <[email protected]>Applied.
This driver doesn't get a lot of build testing, and that's due to the platform BCM63XX kconfig option it depends upon. Probably it should be exposed more widely to perhaps a more broad dependency such as
CONFIG_OF.
There is no OF-aware platform using that driver. Part of the reason why it does not get much build coverage is the large header dependencies provided by arch/mips/include/asm/mach-bcm63xx. I will work on improving that.
Signature · 1 line
Florian
David Miller
maintainerRe: [PATCH net-next] bcm63xx_enet: Stop pretending to support netpoll
From: Florian Fainelli <[email protected]>
Date: Thu, 13 Mar 2014 12:42:22 -0700
Quoted 4 lines
There is no OF-aware platform using that driver. Part of the reason why it does not get much build coverage is the large header dependencies provided by
arch/mips/include/asm/mach-bcm63xx. I will work on improving that.
Thanks in advance.
David Miller
maintainerRe: [PATCH 01/11] bonding: Call dev_kfree_skby_any instead of kfree_skb.
In reply to Eric DumazetFrom: Eric Dumazet <[email protected]>
Date: Mon, 10 Mar 2014 22:02:38 -0700
Quoted 1 line
It seems netpoll should not drain rx queues.
There are netpoll users (admittedly, out of tree) that make use of packet receive.
kdump is one.
That's what all of the __netpoll_rx() et al. stuff is for.
Therefore we'll probably just have to enforce that if a netpoll user wants RX packets, he has to invoke netpoll from a reasonable context.
But if we're called from hardware interrupts, yes we block RX processing.
Re: [PATCH 01/11] bonding: Call dev_kfree_skby_any instead of kfree_skb.
In reply to David MillerQuoted 25 lines · David Miller
From: Eric Dumazet <[email protected]>
Date: Mon, 10 Mar 2014 20:44:27 -0700Quoted 11 lines · Eric W. Biederman
drivers/net/bonding/bond_main.c static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb) · line 1118
Quoted 2 lines
drivers/net/bonding/bond_main.c +1
-1-consume_skb (skb); +dev_consume_skb_any (skb);Why is this needed ? AFAIK we run in softirq here.
Quoted 5 lines
drivers/net/bonding/bond_main.c +0
-0return ret; } }@@ -1132,7 +1132,7 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb)Quoted 6 lines
drivers/net/bonding/bond_main.c static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb) · line 1135
Quoted 4 lines
drivers/net/bonding/bond_main.c +1
-1if (unlikely (skb_cow_head (skb, skb->data -skb_mac_header (skb)))) { -kfree_skb (skb); +dev_kfree_skb_any (skb);same here.
These changes eminate from a recent discussion about netpoll, which can call into the driver from hardware interrupts, particularly when netconsole services a printk from hardware interrupt context.
Quoted 5 lines
bnx2x already makes similar ammends.
I hope that Eric B. here audited to make sure he's only doing this transformation in situations that actually need this treatment for the above mentioned issue.
It looks like our replies crossed. Yes every location that I changed is a location that I can currently find a code path to from a printk in a hard irq context.
Furthermore the difference between dev_kfree_skb/kfree_skb and dev_kfree_skb_any is simply a test to see if you are in interrupt context so there should be no functional differences except when we make it to these code paths in interrupt context.
Eric
PATCH 2/11 bnx2: Call dev_kfree_skby_any instead of dev_kfree_skb.
In reply to Eric W. BiedermanReplace dev_kfree_skb with dev_kfree_skb_any in functions that can be called in hard irq and other contexts.
--- drivers/net/ethernet/broadcom/bnx2.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
drivers/net/ethernet/broadcom/bnx2.c +5 -5
@@ -2885,7 +2885,7 @@ bnx2_tx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget) sw_cons =BNX2_NEXT_TX_BD (sw_cons); tx_bytes += skb->len; -dev_kfree_skb (skb); +dev_kfree_skb_any (skb); tx_pkt++;if (tx_pkt == budget)break ;
@@ -2943,7 +2943,7 @@ bnx2_reuse_rx_skb_pages(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr,__skb_frag_set_page (&shinfo->frags[shinfo->nr_frags],NULL ); cons_rx_pg->page = page; -dev_kfree_skb (skb); +dev_kfree_skb_any (skb); } hw_prod = rxr->rx_pg_prod;
@@ -3234,7 +3234,7 @@ bnx2_rx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget)if ((len > (bp->dev->mtu +ETH_HLEN )) && (ntohs (skb->protocol) !=0x8100 )) { -dev_kfree_skb (skb); +dev_kfree_skb_any (skb); goto next_rx; }
@@ -6604,7 +6604,7 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev) mapping =dma_map_single (&bp->pdev->dev, skb->data, len,PCI_DMA_TODEVICE );if (dma_mapping_error (&bp->pdev->dev, mapping)) { -dev_kfree_skb (skb); +dev_kfree_skb_any (skb);return NETDEV_TX_OK ; }
@@ -6697,7 +6697,7 @@ dma_error: PCI_DMA_TODEVICE); } -dev_kfree_skb (skb); +dev_kfree_skb_any (skb);return NETDEV_TX_OK ; }
Signature · 1 line
1.7.5.4
Re: [PATCH 02/11] bnx2: Call dev_kfree_skby_any instead of dev_kfree_skb.
Quoted 13 lines · Eric W. Biederman
Replace
dev_kfree_skbwithdev_kfree_skb_anyin functions that can be called in hard irq and other contexts.Signed-off-by "Eric W. Biederman" <[email protected]>--- drivers/net/ethernet/broadcom/bnx2.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)drivers/net/ethernet/broadcom/bnx2.c +0
-0
drivers/net/ethernet/broadcom/bnx2.c bnx2_tx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget) · line 2888
Quoted 5 lines
drivers/net/ethernet/broadcom/bnx2.c +1
-1@@ -2885,7 +2885,7 @@ bnx2_tx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget) sw_cons =BNX2_NEXT_TX_BD (sw_cons); tx_bytes += skb->len; -dev_kfree_skb (skb); +dev_kfree_skb_any (skb);
This looks like a dev_consume_skb_any() candidate ?
Anyway, why can this be called from hard irq ?
I'll stop my review here, it seems either me or you are confused/tired.
Re: [PATCH 02/11] bnx2: Call dev_kfree_skby_any instead of dev_kfree_skb.
Quoted 16 lines · Eric Dumazet
Quoted 15 lines · Eric W. Biederman
Replace
dev_kfree_skbwithdev_kfree_skb_anyin functions that can be called in hard irq and other contexts.Signed-off-by "Eric W. Biederman" <[email protected]>--- drivers/net/ethernet/broadcom/bnx2.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)drivers/net/ethernet/broadcom/bnx2.c +0
-0@@ -2885,7 +2885,7 @@ bnx2_tx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget) sw_cons =BNX2_NEXT_TX_BD (sw_cons);
Quoted 5 lines
drivers/net/ethernet/broadcom/bnx2.c bnx2_tx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget) · line 2888
Quoted 3 lines
drivers/net/ethernet/broadcom/bnx2.c +1
-1tx_bytes += skb->len; -dev_kfree_skb (skb); +dev_kfree_skb_any (skb);This looks like a
dev_consume_skb_any()candidate ?
That seems reasonable. I am focusing on one dimension at a time.
Quoted 1 line
Anyway, why can this be called from hard irq ?
netpoll_poll_dev
bnx2_poll
bnx2_poll_work
bnx2_tx_intQuoted 1 line
I'll stop my review here, it seems either me or you are confused/tired.
I did my best to verify the code paths I am changing actually exist. I think I even have a stack backtrace from skb_release_head_state around somewhere. Transmitted packets frequently have dst cache entries, conntrack entries, and destructores which make them actually problematic to free in hard irq context.
Eric
David Miller
maintainerRe: [PATCH 02/11] bnx2: Call dev_kfree_skby_any instead of dev_kfree_skb.
In reply to Eric DumazetFrom: Eric Dumazet <[email protected]>
Date: Mon, 10 Mar 2014 20:47:57 -0700
Quoted 1 line
Anyway, why can this be called from hard irq ?
Eric B. explains this in his 0/11 posting, the culprit is netpoll.
I had a discussion about this with him on this list last week.
PATCH 3/11 bnx2x: Call dev_kfree_skby_any instead of dev_kfree_skb.
In reply to Eric W. BiedermanReplace dev_kfree_skb with dev_kfree_skb_any in functions that can be called in hard irq and other contexts.
--- drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c +1 -1
@@ -3719,7 +3719,7 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev)struct bnx2x_eth_q_stats *q_stats =bnx2x_fp_qstats (bp, txdata->parent_fp); q_stats->driver_filtered_tx_pkt++; -dev_kfree_skb (skb); +dev_kfree_skb_any (skb);return NETDEV_TX_OK ; }bnx2x_fp_qstats (bp, txdata->parent_fp)->driver_xoff++;
Signature · 1 line
1.7.5.4
PATCH 4/11 tg3: Call dev_kfree_skby_any instead of dev_kfree_skb.
In reply to Eric W. BiedermanReplace dev_kfree_skb with dev_kfree_skb_any in functions that can be called in hard irq and other contexts.
--- drivers/net/ethernet/broadcom/tg3.c | 14 +++++++------- 1 files changed, 7 insertions(+), 7 deletions(-)
drivers/net/ethernet/broadcom/tg3.c +7 -7
@@ -6593,7 +6593,7 @@ static void tg3_tx(struct tg3_napi *tnapi) pkts_compl++; bytes_compl += skb->len; -dev_kfree_skb (skb); +dev_kfree_skb_any (skb);if (unlikely (tx_bug)) {tg3_tx_recover (tp);
@@ -6924,7 +6924,7 @@ static int tg3_rx(struct tg3_napi *tnapi, int budget)if (len > (tp->dev->mtu +ETH_HLEN ) && skb->protocol !=htons (ETH_P_8021Q )) { -dev_kfree_skb (skb); +dev_kfree_skb_any (skb); goto drop_it_no_recycle; }
@@ -7807,7 +7807,7 @@ static int tigon3_dma_hwbug_workaround(struct tg3_napi *tnapi, PCI_DMA_TODEVICE);/* Make sure the mapping succeeded */ if (pci_dma_mapping_error (tp->pdev, new_addr)) { -dev_kfree_skb (new_skb); +dev_kfree_skb_any (new_skb); ret =-1 ; }else {u32 save_entry = *entry;
@@ -7822,13 +7822,13 @@ static int tigon3_dma_hwbug_workaround(struct tg3_napi *tnapi, new_skb->len, base_flags, mss, vlan)) {tg3_tx_skb_unmap (tnapi, save_entry,-1 ); -dev_kfree_skb (new_skb); +dev_kfree_skb_any (new_skb); ret =-1 ; } } } -dev_kfree_skb (skb); +dev_kfree_skb_any (skb); *pskb = new_skb;return ret; }
@@ -7871,7 +7871,7 @@ static int tg3_tso_bug(struct tg3 *tp, struct sk_buff *skb) }while (segs); tg3_tso_bug_end: -dev_kfree_skb (skb); +dev_kfree_skb_any (skb);return NETDEV_TX_OK ; }
@@ -8093,7 +8093,7 @@ dma_error:tg3_tx_skb_unmap (tnapi, tnapi->tx_prod, --i); tnapi->tx_buffers[tnapi->tx_prod].skb =NULL ; drop: -dev_kfree_skb (skb); +dev_kfree_skb_any (skb); drop_nofree: tp->tx_dropped++;return NETDEV_TX_OK ;
Signature · 1 line
1.7.5.4
PATCH 5/11 bcm63xx_enet: Call dev_kfree_skby_any instead of dev_kfree_skb.
In reply to Eric W. BiedermanReplace dev_kfree_skb with dev_kfree_skb_any in functions that can be called in hard irq and other contexts.
--- drivers/net/ethernet/broadcom/bcm63xx_enet.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/broadcom/bcm63xx_enet.c +2 -2
@@ -469,7 +469,7 @@ static int bcm_enet_tx_reclaim(struct net_device *dev, int force)if (desc->len_stat &DMADESC_UNDER_MASK ) dev->stats.tx_errors++; -dev_kfree_skb (skb); +dev_kfree_skb_any (skb); released++; }
@@ -606,7 +606,7 @@ static int bcm_enet_start_xmit(struct sk_buff *skb, struct net_device *dev) ret =NETDEV_TX_BUSY ; goto out_unlock; } -dev_kfree_skb (skb); +dev_kfree_skb_any (skb); skb = nskb; } data =skb_put (skb, needed);
Signature · 1 line
1.7.5.4
PATCH 6/11 e1000: Call dev_kfree_skby_any instead of dev_kfree_skb.
In reply to Eric W. BiedermanReplace dev_kfree_skb with dev_kfree_skb_any in functions that can be called in hard irq and other contexts.
--- drivers/net/ethernet/intel/e1000/e1000_main.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-)
drivers/net/ethernet/intel/e1000/e1000_main.c +9 -9
@@ -4066,7 +4066,7 @@ static bool e1000_clean_jumbo_rx_irq(struct e1000_adapter *adapter,* too */ if (rx_ring->rx_skb_top) -dev_kfree_skb (rx_ring->rx_skb_top); +dev_kfree_skb_any (rx_ring->rx_skb_top); rx_ring->rx_skb_top =NULL ; goto next_desc; }
@@ -4143,7 +4143,7 @@ process_skb:/* eth type trans needs skb->data to point to something */ if (!pskb_may_pull (skb,ETH_HLEN )) {e_err (drv,"pskb_may_pull failed.\n" ); -dev_kfree_skb (skb); +dev_kfree_skb_any (skb); goto next_desc; }
@@ -4394,7 +4394,7 @@ check_page: DMA_FROM_DEVICE);if (dma_mapping_error (&pdev->dev, buffer_info->dma)) {put_page (buffer_info->page); -dev_kfree_skb (skb); +dev_kfree_skb_any (skb); buffer_info->page =NULL ; buffer_info->skb =NULL ; buffer_info->dma =0 ;
@@ -4469,21 +4469,21 @@ static void e1000_alloc_rx_buffers(struct e1000_adapter *adapter, skb =netdev_alloc_skb_ip_align (netdev, bufsz);/* Failed allocation, critical failure */ if (!skb) { -dev_kfree_skb (oldskb); +dev_kfree_skb_any (oldskb); adapter->alloc_rx_buff_failed++;break ; }if (!e1000_check_64k_bound (adapter, skb->data, bufsz)) {/* give up */ -dev_kfree_skb (skb); -dev_kfree_skb (oldskb); +dev_kfree_skb_any (skb); +dev_kfree_skb_any (oldskb); adapter->alloc_rx_buff_failed++;break ;/* while !buffer_info->skb */ }/* Use new allocation */ -dev_kfree_skb (oldskb); +dev_kfree_skb_any (oldskb); } buffer_info->skb = skb; buffer_info->length = adapter->rx_buffer_len;
@@ -4493,7 +4493,7 @@ map_skb: buffer_info->length, DMA_FROM_DEVICE);if (dma_mapping_error (&pdev->dev, buffer_info->dma)) { -dev_kfree_skb (skb); +dev_kfree_skb_any (skb); buffer_info->skb =NULL ; buffer_info->dma =0 ; adapter->alloc_rx_buff_failed++;
@@ -4511,7 +4511,7 @@ map_skb:e_err (rx_err,"dma align check failed: %u bytes at " "%p\n" , adapter->rx_buffer_len, (void *)(unsigned long )buffer_info->dma); -dev_kfree_skb (skb); +dev_kfree_skb_any (skb); buffer_info->skb =NULL ; dma_unmap_single(&pdev->dev, buffer_info->dma,
Signature · 1 line
1.7.5.4
PATCH 7/11 igbvf: Call dev_kfree_skby_any instead of dev_kfree_skb.
In reply to Eric W. BiedermanReplace dev_kfree_skb with dev_kfree_skb_any in functions that can be called in hard irq and other contexts.
--- drivers/net/ethernet/intel/igbvf/netdev.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/net/ethernet/intel/igbvf/netdev.c +1 -1
@@ -212,7 +212,7 @@ static void igbvf_alloc_rx_buffers(struct igbvf_ring *rx_ring, bufsz, DMA_FROM_DEVICE);if (dma_mapping_error (&pdev->dev, buffer_info->dma)) { -dev_kfree_skb (buffer_info->skb); +dev_kfree_skb_any (buffer_info->skb); buffer_info->skb =NULL ;dev_err (&pdev->dev,"RX DMA map failed\n" ); goto no_buffers;
Signature · 1 line
1.7.5.4
PATCH 8/11 ixgb: Call dev_kfree_skby_any instead of dev_kfree_skb.
In reply to Eric W. BiedermanReplace dev_kfree_skb with dev_kfree_skb_any in functions that can be called in hard irq and other contexts.
--- drivers/net/ethernet/intel/ixgb/ixgb_main.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
drivers/net/ethernet/intel/ixgb/ixgb_main.c +3 -3
@@ -1521,12 +1521,12 @@ ixgb_xmit_frame(struct sk_buff *skb, struct net_device *netdev)int tso;if (test_bit (__IXGB_DOWN, &adapter->flags)) { -dev_kfree_skb (skb); +dev_kfree_skb_any (skb);return NETDEV_TX_OK ; }if (skb->len <=0 ) { -dev_kfree_skb (skb); +dev_kfree_skb_any (skb);return NETDEV_TX_OK ; }
@@ -1543,7 +1543,7 @@ ixgb_xmit_frame(struct sk_buff *skb, struct net_device *netdev) tso =ixgb_tso (adapter, skb);if (tso <0 ) { -dev_kfree_skb (skb); +dev_kfree_skb_any (skb);return NETDEV_TX_OK ; }
Signature · 1 line
1.7.5.4
PATCH 9/11 mlx4: Call dev_kfree_skby_any instead of dev_kfree_skb.
In reply to Eric W. BiedermanReplace dev_kfree_skb with dev_kfree_skb_any in functions that can be called in hard irq and other contexts.
--- drivers/net/ethernet/mellanox/mlx4/en_tx.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/net/ethernet/mellanox/mlx4/en_tx.c +1 -1
@@ -314,7 +314,7 @@ static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv, } } } -dev_kfree_skb (skb); +dev_kfree_skb_any (skb);return tx_info->nr_txbb; }
Signature · 1 line
1.7.5.4
PATCH 10/11 benet: Call dev_kfree_skby_any instead of kfree_skb.
In reply to Eric W. BiedermanReplace free_skb with dev_kfree_skb_any in be_tx_compl_process as which can be called in hard irq by netpoll, softirq context by normal napi polling, and in normal sleepable context by the network device close method.
--- drivers/net/ethernet/emulex/benet/be_main.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/net/ethernet/emulex/benet/be_main.c +1 -1
@@ -1897,7 +1897,7 @@ static u16 be_tx_compl_process(struct be_adapter *adapter,queue_tail_inc (txq); }while (cur_index != last_index); -kfree_skb (sent_skb); +dev_kfree_skb_any (sent_skb);return num_wrbs; }
Signature · 1 line
1.7.5.4
PATCH 11/11 gianfar: Carefully free skbs in functions called by netpoll.
In reply to Eric W. Biedermannetpoll can call functions in hard irq context that are ordinarily called in lesser contexts. For those functions use dev_kfree_skb_any and dev_consume_skb_any so skbs are freed safely from hard irq context.
--- drivers/net/ethernet/freescale/gianfar.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
drivers/net/ethernet/freescale/gianfar.c +3 -3
@@ -2146,13 +2146,13 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev) skb_new =skb_realloc_headroom (skb, fcb_len);if (!skb_new) { dev->stats.tx_errors++; -kfree_skb (skb); +dev_kfree_skb_any (skb);return NETDEV_TX_OK ; }if (skb->sk)skb_set_owner_w (skb_new, skb->sk); -consume_skb (skb); +dev_consume_skb_any (skb); skb = skb_new; }
@@ -2744,7 +2744,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)if (unlikely (!newskb)) newskb = skb;else if (skb) -dev_kfree_skb (skb); +dev_kfree_skb_any (skb); }else {/* Increment the number of packets */ rx_queue->stats.rx_packets++;
Signature · 1 line
1.7.5.4