diff mbox

[2/3] r8169: Use dev_kfree_skb in Tx cleanup path

Message ID 20150430205824.1512.35158.stgit@ahduyck-vm-fedora22
State Not Applicable
Headers show

Commit Message

Alexander Duyck April 30, 2015, 8:58 p.m. UTC
This change replaces the use of either dev_kfree_skb_any or
dev_consume_skb_any in the Tx cleanup path of this driver with
dev_kfree_skb.  There isn't any need for the "_any" version of these
functions since the NAPI cleanup context is not a hard irq context.

This change allows us to drop one function call from the clean-up path
since dev_kfree_skb_any was just calling into dev_kfree_skb in these paths
anyway.

Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
 drivers/net/ethernet/realtek/r8169.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Francois Romieu May 1, 2015, 9:34 a.m. UTC | #1
Alexander Duyck <alexander.h.duyck@redhat.com> :
> This change replaces the use of either dev_kfree_skb_any or
> dev_consume_skb_any in the Tx cleanup path of this driver with
> dev_kfree_skb.  There isn't any need for the "_any" version of these
> functions since the NAPI cleanup context is not a hard irq context.

netconsole ?

__dev_kfree_skb_any contains a big "if (... || irqs_disabled())" test
and the NAPI poll handler is called with irq disabled from
netpoll_send_skb_on_dev.
Lino Sanfilippo May 1, 2015, 12:06 p.m. UTC | #2
On 01.05.2015 11:34, Francois Romieu wrote:
> Alexander Duyck <alexander.h.duyck@redhat.com> :
>> This change replaces the use of either dev_kfree_skb_any or
>> dev_consume_skb_any in the Tx cleanup path of this driver with
>> dev_kfree_skb.  There isn't any need for the "_any" version of these
>> functions since the NAPI cleanup context is not a hard irq context.
> 
> netconsole ?
> 
> __dev_kfree_skb_any contains a big "if (... || irqs_disabled())" test
> and the NAPI poll handler is called with irq disabled from
> netpoll_send_skb_on_dev.
> 

Indeed. Also there have been changes from Eric Biederman not long ago,
which did just the opposite: replace dev_kfree() with dev_kfree_any()
for that reason. See

http://marc.info/?l=linux-stable-commits&m=142965098111361&w=2

[CCing Eric]

Regards,
Lino
Lino Sanfilippo May 1, 2015, 12:15 p.m. UTC | #3
On 01.05.2015 14:06, Lino Sanfilippo wrote:

> which did just the opposite: replace dev_kfree() with dev_kfree_any()
> for that reason. See
> 

Um, this should of course be dev_kfree_skb() and dev_kfree_skb_any().

Lino
diff mbox

Patch

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index c70ab40d8698..ca94040b0ec2 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -7215,7 +7215,7 @@  static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
 			tp->tx_stats.packets++;
 			tp->tx_stats.bytes += tx_skb->skb->len;
 			u64_stats_update_end(&tp->tx_stats.syncp);
-			dev_kfree_skb_any(tx_skb->skb);
+			dev_kfree_skb(tx_skb->skb);
 			tx_skb->skb = NULL;
 		}
 		dirty_tx++;