net: tx scalability works : trans_start

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)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html