[PATCH] net: tx scalability works : trans_start

[PATCH] net: tx scalability works : trans_start

[Eric Dumazet]

struct net_device trans_start field is a hot spot on SMP and high performance
devices, particularly multi queues ones, because every transmitter dirties
it. Is main use is tx watchdog and bonding alive checks.

But as most devices dont use NETIF_F_LLTX, we have to lock
a netdev_queue before calling their ndo_start_xmit(). So it makes
sense to move trans_start from net_device to netdev_queue. Its update
will occur on a already present (and in exclusive state) cache line, for
free.

We can do this transition smoothly. An old driver continue to
update dev->trans_start, while an updated one updates txq->trans_start.

Further patches could also put tx_bytes/tx_packets counters in 
netdev_queue to avoid dirtying dev->stats (vlan device comes to mind)

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
---
 drivers/net/bonding/bond_main.c |    8 +++---
 include/linux/netdevice.h       |   11 ++++++++
 net/sched/sch_generic.c         |   40 +++++++++++++++++++++++-------
 3 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 815191d..30b9ea6 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2795,7 +2795,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
 	 */
 	bond_for_each_slave(bond, slave, i) {
 		if (slave->link != BOND_LINK_UP) {
-			if (time_before_eq(jiffies, slave->dev->trans_start + delta_in_ticks) &&
+			if (time_before_eq(jiffies, dev_trans_start(slave->dev) + delta_in_ticks) &&
 			    time_before_eq(jiffies, slave->dev->last_rx + delta_in_ticks)) {
 
 				slave->link  = BOND_LINK_UP;
@@ -2827,7 +2827,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
 			 * when the source ip is 0, so don't take the link down
 			 * if we don't know our ip yet
 			 */
-			if (time_after_eq(jiffies, slave->dev->trans_start + 2*delta_in_ticks) ||
+			if (time_after_eq(jiffies, dev_trans_start(slave->dev) + 2*delta_in_ticks) ||
 			    (time_after_eq(jiffies, slave->dev->last_rx + 2*delta_in_ticks))) {
 
 				slave->link  = BOND_LINK_DOWN;
@@ -2938,7 +2938,7 @@ static int bond_ab_arp_inspect(struct bonding *bond, int delta_in_ticks)
 		 *    the bond has an IP address)
 		 */
 		if ((slave->state == BOND_STATE_ACTIVE) &&
-		    (time_after_eq(jiffies, slave->dev->trans_start +
+		    (time_after_eq(jiffies, dev_trans_start(slave->dev) +
 				    2 * delta_in_ticks) ||
 		      (time_after_eq(jiffies, slave_last_rx(bond, slave)
 				     + 2 * delta_in_ticks)))) {
@@ -2982,7 +2982,7 @@ static void bond_ab_arp_commit(struct bonding *bond, int delta_in_ticks)
 			write_lock_bh(&bond->curr_slave_lock);
 
 			if (!bond->curr_active_slave &&
-			    time_before_eq(jiffies, slave->dev->trans_start +
+			    time_before_eq(jiffies, dev_trans_start(slave->dev) +
 					   delta_in_ticks)) {
 				slave->link = BOND_LINK_UP;
 				bond_change_active_slave(bond, slave);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 2af89b6..cd547d0 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -470,6 +470,10 @@ struct netdev_queue {
  */
 	spinlock_t		_xmit_lock ____cacheline_aligned_in_smp;
 	int			xmit_lock_owner;
+	/*
+	 * please use this field instead of dev->trans_start
+	 */
+	unsigned long		trans_start;
 } ____cacheline_aligned_in_smp;
 
 
@@ -819,6 +823,11 @@ struct net_device
  * One part is mostly used on xmit path (device)
  */
 	/* These may be needed for future network-power-down code. */
+
+	/*
+	 * trans_start here is expensive for high speed devices on SMP,
+	 * please use netdev_queue->trans_start instead.
+	 */
 	unsigned long		trans_start;	/* Time (in jiffies) of last Tx	*/
 
 	int			watchdog_timeo; /* used by dev_watchdog() */
@@ -1541,6 +1550,8 @@ static inline int netif_carrier_ok(const struct net_device *dev)
 	return !test_bit(__LINK_STATE_NOCARRIER, &dev->state);
 }
 
+extern unsigned long dev_trans_start(struct net_device *dev);
+
 extern void __netdev_watchdog_up(struct net_device *dev);
 
 extern void netif_carrier_on(struct net_device *dev);
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 5f5efe4..abdad6c 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -196,6 +196,21 @@ void __qdisc_run(struct Qdisc *q)
 	clear_bit(__QDISC_STATE_RUNNING, &q->state);
 }
 
+unsigned long dev_trans_start(struct net_device *dev)
+{
+	unsigned long val, res = dev->trans_start;
+	unsigned int i;
+
+	for (i = 0; i < dev->num_tx_queues; i++) {
+		val = netdev_get_tx_queue(dev, i)->trans_start;
+		if (val && time_after(val, res))
+			res = val;
+	}
+	dev->trans_start = res;
+	return res;
+}
+EXPORT_SYMBOL(dev_trans_start);
+
 static void dev_watchdog(unsigned long arg)
 {
 	struct net_device *dev = (struct net_device *)arg;
@@ -205,25 +220,30 @@ static void dev_watchdog(unsigned long arg)
 		if (netif_device_present(dev) &&
 		    netif_running(dev) &&
 		    netif_carrier_ok(dev)) {
-			int some_queue_stopped = 0;
+			int some_queue_timedout = 0;
 			unsigned int i;
+			unsigned long trans_start;
 
 			for (i = 0; i < dev->num_tx_queues; i++) {
 				struct netdev_queue *txq;
 
 				txq = netdev_get_tx_queue(dev, i);
-				if (netif_tx_queue_stopped(txq)) {
-					some_queue_stopped = 1;
+				/*
+				 * old device drivers set dev->trans_start
+				 */
+				trans_start = txq->trans_start ? : dev->trans_start;
+				if (netif_tx_queue_stopped(txq) && 
+				    time_after(jiffies, (trans_start +
+							 dev->watchdog_timeo))) {
+					some_queue_timedout = 1;
 					break;
 				}
 			}
 
-			if (some_queue_stopped &&
-			    time_after(jiffies, (dev->trans_start +
-						 dev->watchdog_timeo))) {
+			if (some_queue_timedout) {
 				char drivername[64];
-				WARN_ONCE(1, KERN_INFO "NETDEV WATCHDOG: %s (%s): transmit timed out\n",
-				       dev->name, netdev_drivername(dev, drivername, 64));
+				WARN_ONCE(1, KERN_INFO "NETDEV WATCHDOG: %s (%s): transmit queue %u timed out\n",
+				       dev->name, netdev_drivername(dev, drivername, 64), i);
 				dev->netdev_ops->ndo_tx_timeout(dev);
 			}
 			if (!mod_timer(&dev->watchdog_timer,
@@ -602,8 +622,10 @@ static void transition_one_qdisc(struct net_device *dev,
 		clear_bit(__QDISC_STATE_DEACTIVATED, &new_qdisc->state);
 
 	rcu_assign_pointer(dev_queue->qdisc, new_qdisc);
-	if (need_watchdog_p && new_qdisc != &noqueue_qdisc)
+	if (need_watchdog_p && new_qdisc != &noqueue_qdisc) {
+		dev_queue->trans_start = 0;
 		*need_watchdog_p = 1;
+	}
 }
 
 void dev_activate(struct net_device *dev)

Re: [PATCH] net: tx scalability works : trans_start

[David Miller]

From: Eric Dumazet <dada1@cosmosbay.com>
Date: Tue, 12 May 2009 21:47:53 +0200

> struct net_device trans_start field is a hot spot on SMP and high performance
> devices, particularly multi queues ones, because every transmitter dirties
> it. Is main use is tx watchdog and bonding alive checks.
> 
> But as most devices dont use NETIF_F_LLTX, we have to lock
> a netdev_queue before calling their ndo_start_xmit(). So it makes
> sense to move trans_start from net_device to netdev_queue. Its update
> will occur on a already present (and in exclusive state) cache line, for
> free.
> 
> We can do this transition smoothly. An old driver continue to
> update dev->trans_start, while an updated one updates txq->trans_start.
> 
> Further patches could also put tx_bytes/tx_packets counters in 
> netdev_queue to avoid dirtying dev->stats (vlan device comes to mind)
> 
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>

I like this patch so I'm adding it to net-next-2.6

But you can go one step further and untangle what is arguably
a huge mess.  There is no reason for the driver to set
->trans_start.

The driver gives us a success indication from ->hard_start_xmit()
and we can use that to set txq->trans_start.

Then you can kill the driver code that sets dev->trans_start and
then kill the netdev struct member as well.

Could you please do this work, thanks?

Re: [PATCH] net: tx scalability works : trans_start

[Eric Dumazet]

David Miller a écrit :
> From: Eric Dumazet <dada1@cosmosbay.com>
> Date: Tue, 12 May 2009 21:47:53 +0200
> 
>> struct net_device trans_start field is a hot spot on SMP and high performance
>> devices, particularly multi queues ones, because every transmitter dirties
>> it. Is main use is tx watchdog and bonding alive checks.
>>
>> But as most devices dont use NETIF_F_LLTX, we have to lock
>> a netdev_queue before calling their ndo_start_xmit(). So it makes
>> sense to move trans_start from net_device to netdev_queue. Its update
>> will occur on a already present (and in exclusive state) cache line, for
>> free.
>>
>> We can do this transition smoothly. An old driver continue to
>> update dev->trans_start, while an updated one updates txq->trans_start.
>>
>> Further patches could also put tx_bytes/tx_packets counters in 
>> netdev_queue to avoid dirtying dev->stats (vlan device comes to mind)
>>
>> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
> 
> I like this patch so I'm adding it to net-next-2.6
> 
> But you can go one step further and untangle what is arguably
> a huge mess.  There is no reason for the driver to set
> ->trans_start.
> 
> The driver gives us a success indication from ->hard_start_xmit()
> and we can use that to set txq->trans_start.
> 
> Then you can kill the driver code that sets dev->trans_start and
> then kill the netdev struct member as well.
> 
> Could you please do this work, thanks?

Indeed we can do this factorization, just have to take care of not adding
a new cache line miss on loopback device, as this one doesnt touch dev->trans_start.

Adding the update in dev_hard_start_xmit() would cost two additional tests...

if (rc == NETDEV_TX_OK && dev->netdev_ops->ndo_tx_timeout)
	txq->trans_start = jiffies;

Relying on NETIF_F_LLTX is a litle bit odd, since some drivers would
have to set txq->trans_start themselves, but unfortunatly ndo_start_xmit(skb, dev)
doesnt have txq as a third parameter. (we should add it, as high performance drivers 
have to recompute txq themselves). Ah, yes old drivers have one tx queue so it is
cheap to get txq, never mind :)



What do you think ? (following patch as RFC only, not even booted)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cd547d0..fe301b0 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1773,8 +1773,10 @@ static inline void netif_tx_unlock_bh(struct net_device *dev)
 	}						\
 }
 
-#define HARD_TX_UNLOCK(dev, txq) {			\
+#define HARD_TX_UNLOCK(dev, txq, update_trans) {	\
 	if ((dev->features & NETIF_F_LLTX) == 0) {	\
+		if (update_trans)			\
+			txq->trans_start = jiffies;	\
 		__netif_tx_unlock(txq);			\
 	}						\
 }
diff --git a/net/core/dev.c b/net/core/dev.c
index 14dd725..f71a69d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1879,11 +1879,11 @@ gso:
 			if (!netif_tx_queue_stopped(txq)) {
 				rc = 0;
 				if (!dev_hard_start_xmit(skb, dev, txq)) {
-					HARD_TX_UNLOCK(dev, txq);
+					HARD_TX_UNLOCK(dev, txq, 1);
 					goto out;
 				}
 			}
-			HARD_TX_UNLOCK(dev, txq);
+			HARD_TX_UNLOCK(dev, txq, 0);
 			if (net_ratelimit())
 				printk(KERN_CRIT "Virtual device %s asks to "
 				       "queue packet!\n", dev->name);
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 27d0381..2f69420 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -145,7 +145,7 @@ static inline int qdisc_restart(struct Qdisc *q)
 	if (!netif_tx_queue_stopped(txq) &&
 	    !netif_tx_queue_frozen(txq))
 		ret = dev_hard_start_xmit(skb, dev, txq);
-	HARD_TX_UNLOCK(dev, txq);
+	HARD_TX_UNLOCK(dev, txq, ret == NETDEV_TX_OK);
 
 	spin_lock(root_lock);
 

Re: [PATCH] net: tx scalability works : trans_start

[David Miller]

From: Eric Dumazet <dada1@cosmosbay.com>
Date: Mon, 18 May 2009 09:21:31 +0200

> Indeed we can do this factorization, just have to take care of not
> adding a new cache line miss on loopback device, as this one doesnt
> touch dev->trans_start.

If you want to test IFF_LOOPBACK or similar, I can accept that.
Or it can be some NETIF_F_* flag.

> What do you think ? (following patch as RFC only, not even booted)

This patch looks fine to me.

[PATCH] net: txq_trans_update() helper

[Eric Dumazet]

David Miller a écrit :
> From: Eric Dumazet <dada1@cosmosbay.com>
> Date: Mon, 18 May 2009 09:21:31 +0200
> 
>> Indeed we can do this factorization, just have to take care of not
>> adding a new cache line miss on loopback device, as this one doesnt
>> touch dev->trans_start.
> 
> If you want to test IFF_LOOPBACK or similar, I can accept that.
> Or it can be some NETIF_F_* flag.
 

Or use txq->xmit_lock_owner != -1 test ?

Please note I had to change my email.

Thank you

[PATCH] net: txq_trans_update() helper

We would like to get rid of netdev->trans_start = jiffies; that about all net
drivers have to use in their start_xmit() function, and use txq->trans_start
instead.

This can be done generically in core network, as suggested by David.

Some devices, (particularly loopback) dont need trans_start update, because
they dont have transmit watchdog. We could add a new device flag, or rely
on fact that txq->tran_start can be updated is txq->xmit_lock_owner is
different than -1. Use a helper function to hide our choice.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/linux/netdevice.h |    6 ++++++
 net/core/dev.c            |    3 +++
 net/core/netpoll.c        |    5 ++++-
 net/core/pktgen.c         |    1 +
 net/sched/sch_teql.c      |    1 +
 5 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f8574e7..a6aa468 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1712,6 +1712,12 @@ static inline void __netif_tx_unlock_bh(struct netdev_queue *txq)
 	spin_unlock_bh(&txq->_xmit_lock);
 }
 
+static inline void txq_trans_update(struct netdev_queue *txq)
+{
+	if (txq->xmit_lock_owner != -1)
+		txq->trans_start = jiffies;
+}
+
 /**
  *	netif_tx_lock - grab network device transmit lock
  *	@dev: network device
diff --git a/net/core/dev.c b/net/core/dev.c
index 3942266..29fa4a1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1698,6 +1698,8 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 			skb->dst = NULL;
 		}
 		rc = ops->ndo_start_xmit(skb, dev);
+		if (rc == 0)
+			txq_trans_update(txq);
 		/*
 		 * TODO: if skb_orphan() was called by
 		 * dev->hard_start_xmit() (for example, the unmodified
@@ -1727,6 +1729,7 @@ gso:
 			skb->next = nskb;
 			return rc;
 		}
+		txq_trans_update(txq);
 		if (unlikely(netif_tx_queue_stopped(txq) && skb->next))
 			return NETDEV_TX_BUSY;
 	} while (skb->next);
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 67b4f3e..35850e6 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -302,8 +302,11 @@ static void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
 		for (tries = jiffies_to_usecs(1)/USEC_PER_POLL;
 		     tries > 0; --tries) {
 			if (__netif_tx_trylock(txq)) {
-				if (!netif_tx_queue_stopped(txq))
+				if (!netif_tx_queue_stopped(txq)) {
 					status = ops->ndo_start_xmit(skb, dev);
+					if (status == NETDEV_TX_OK)
+						txq_trans_update(txq);
+					}
 				__netif_tx_unlock(txq);
 
 				if (status == NETDEV_TX_OK)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 0666a82..b8ccd3c 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -3438,6 +3438,7 @@ static __inline__ void pktgen_xmit(struct pktgen_dev *pkt_dev)
 	      retry_now:
 		ret = (*xmit)(pkt_dev->skb, odev);
 		if (likely(ret == NETDEV_TX_OK)) {
+			txq_trans_update(txq);
 			pkt_dev->last_ok = 1;
 			pkt_dev->sofar++;
 			pkt_dev->seq_num++;
diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index 428a5ef..a886496 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -308,6 +308,7 @@ restart:
 				if (!netif_tx_queue_stopped(slave_txq) &&
 				    !netif_tx_queue_frozen(slave_txq) &&
 				    slave_ops->ndo_start_xmit(skb, slave) == 0) {
+					txq_trans_update(slave_txq);
 					__netif_tx_unlock(slave_txq);
 					master->slaves = NEXT_SLAVE(q);
 					netif_wake_queue(dev);

Re: [PATCH] net: txq_trans_update() helper

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 26 May 2009 07:37:11 +0200

> @@ -302,8 +302,11 @@ static void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
>  		for (tries = jiffies_to_usecs(1)/USEC_PER_POLL;
>  		     tries > 0; --tries) {
>  			if (__netif_tx_trylock(txq)) {
> -				if (!netif_tx_queue_stopped(txq))
> +				if (!netif_tx_queue_stopped(txq)) {
>  					status = ops->ndo_start_xmit(skb, dev);
> +					if (status == NETDEV_TX_OK)
> +						txq_trans_update(txq);
> +					}

Closing brace here has strange indentation, which doesn't match
the statement of the openning brace.

Please fix this, thanks.

Re: [PATCH] net: txq_trans_update() helper

[Eric Dumazet]

David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 26 May 2009 07:37:11 +0200
> 
>> @@ -302,8 +302,11 @@ static void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
>>  		for (tries = jiffies_to_usecs(1)/USEC_PER_POLL;
>>  		     tries > 0; --tries) {
>>  			if (__netif_tx_trylock(txq)) {
>> -				if (!netif_tx_queue_stopped(txq))
>> +				if (!netif_tx_queue_stopped(txq)) {
>>  					status = ops->ndo_start_xmit(skb, dev);
>> +					if (status == NETDEV_TX_OK)
>> +						txq_trans_update(txq);
>> +					}
> 
> Closing brace here has strange indentation, which doesn't match
> the statement of the openning brace.
> 
> Please fix this, thanks.

Oops you are right, this is a leftover.

[PATCH] net: txq_trans_update() helper

We would like to get rid of netdev->trans_start = jiffies; that about all net
drivers have to use in their start_xmit() function, and use txq->trans_start
instead.

This can be done generically in core network, as suggested by David.

Some devices, (particularly loopback) dont need trans_start update, because
they dont have transmit watchdog. We could add a new device flag, or rely
on fact that txq->tran_start can be updated is txq->xmit_lock_owner is
different than -1. Use a helper function to hide our choice.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/linux/netdevice.h |    6 ++++++
 net/core/dev.c            |    3 +++
 net/core/netpoll.c        |    5 ++++-
 net/core/pktgen.c         |    1 +
 net/sched/sch_teql.c      |    1 +
 5 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f8574e7..a6aa468 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1712,6 +1712,12 @@ static inline void __netif_tx_unlock_bh(struct netdev_queue *txq)
 	spin_unlock_bh(&txq->_xmit_lock);
 }
 
+static inline void txq_trans_update(struct netdev_queue *txq)
+{
+	if (txq->xmit_lock_owner != -1)
+		txq->trans_start = jiffies;
+}
+
 /**
  *	netif_tx_lock - grab network device transmit lock
  *	@dev: network device
diff --git a/net/core/dev.c b/net/core/dev.c
index 3942266..29fa4a1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1698,6 +1698,8 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 			skb->dst = NULL;
 		}
 		rc = ops->ndo_start_xmit(skb, dev);
+		if (rc == 0)
+			txq_trans_update(txq);
 		/*
 		 * TODO: if skb_orphan() was called by
 		 * dev->hard_start_xmit() (for example, the unmodified
@@ -1727,6 +1729,7 @@ gso:
 			skb->next = nskb;
 			return rc;
 		}
+		txq_trans_update(txq);
 		if (unlikely(netif_tx_queue_stopped(txq) && skb->next))
 			return NETDEV_TX_BUSY;
 	} while (skb->next);
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 67b4f3e..35850e6 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -302,8 +302,11 @@ static void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
 		for (tries = jiffies_to_usecs(1)/USEC_PER_POLL;
 		     tries > 0; --tries) {
 			if (__netif_tx_trylock(txq)) {
-				if (!netif_tx_queue_stopped(txq))
+				if (!netif_tx_queue_stopped(txq)) {
 					status = ops->ndo_start_xmit(skb, dev);
+					if (status == NETDEV_TX_OK)
+						txq_trans_update(txq);
+				}
 				__netif_tx_unlock(txq);
 
 				if (status == NETDEV_TX_OK)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 0666a82..b8ccd3c 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -3438,6 +3438,7 @@ static __inline__ void pktgen_xmit(struct pktgen_dev *pkt_dev)
 	      retry_now:
 		ret = (*xmit)(pkt_dev->skb, odev);
 		if (likely(ret == NETDEV_TX_OK)) {
+			txq_trans_update(txq);
 			pkt_dev->last_ok = 1;
 			pkt_dev->sofar++;
 			pkt_dev->seq_num++;
diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index 428a5ef..a886496 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -308,6 +308,7 @@ restart:
 				if (!netif_tx_queue_stopped(slave_txq) &&
 				    !netif_tx_queue_frozen(slave_txq) &&
 				    slave_ops->ndo_start_xmit(skb, slave) == 0) {
+					txq_trans_update(slave_txq);
 					__netif_tx_unlock(slave_txq);
 					master->slaves = NEXT_SLAVE(q);
 					netif_wake_queue(dev);

Re: [PATCH] net: txq_trans_update() helper

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 26 May 2009 07:57:01 +0200

> David Miller a écrit :
>> Closing brace here has strange indentation, which doesn't match
>> the statement of the openning brace.
>> 
>> Please fix this, thanks.
> 
> Oops you are right, this is a leftover.
> 
> [PATCH] net: txq_trans_update() helper
> 
> We would like to get rid of netdev->trans_start = jiffies; that about all net
> drivers have to use in their start_xmit() function, and use txq->trans_start
> instead.
> 
> This can be done generically in core network, as suggested by David.
> 
> Some devices, (particularly loopback) dont need trans_start update, because
> they dont have transmit watchdog. We could add a new device flag, or rely
> on fact that txq->tran_start can be updated is txq->xmit_lock_owner is
> different than -1. Use a helper function to hide our choice.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, thanks a lot Eric.