gianfar: multi queue stuff not complete
Subsystems (2)
RFC 1/1 gianfar: multi queue stuff not complete
I noticed gianfar got multiqueue support recently, but it still updates dev->stats.{tx_bytes|tx_packets|tx_dropped} without proper locking (several cpus could update stats at same time while xmitting on different tx queues)
It should use txq->{tx_bytes|tx_packets|tx_dropped} to avoid wrong tx stats
I cooked following preliminar patch but its only for discussion, because I dont know yet how to transform dev->stats.tx_dropped++; in gfar_error(), and also because I cannot compile this driver on my dev machine.
(if a driver uses txq->tx... counters, it must not use any dev->stats.{tx_bytes|tx_packets|tx_dropped} that are overwritten by dev_txq_stats_fold(). Or we could change dev_txq_stats_fold() logic to get a mask of what fields a drivers updates in txq-> or dev->stats
drivers/net/gianfar.c +2 -3
@@ -1937,7 +1937,8 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev) }/* Update transmit stats */ - dev->stats.tx_bytes += skb->len; + txq->tx_bytes += skb->len; + txq->tx_packets++; txbdp = txbdp_start = tx_queue->cur_tx;
@@ -2295,8 +2296,6 @@ static int gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue) tx_queue->skb_dirtytx = skb_dirtytx; tx_queue->dirty_tx = bdp; - dev->stats.tx_packets += howmany; -return howmany; }
Quoted 15 lines
I noticed gianfar got multiqueue support recently, but it still updates dev->stats.{tx_bytes|tx_packets|tx_dropped} without proper locking (several cpus could update stats at same time while xmitting on different tx queues)
It should use txq->{tx_bytes|tx_packets|tx_dropped} to avoid wrong tx stats
I cooked following preliminar patch but its only for discussion, because I dont know yet how to transform dev->stats.tx_dropped++; in
gfar_error(), and also because I cannot compile this driver on my dev machine.
Quoted 7 lines
(if a driver uses
txq->tx... counters, it must not use any dev->stats.{tx_bytes|tx_packets|tx_dropped}
that are overwritten bydev_txq_stats_fold(). Or we could changedev_txq_stats_fold()logic to get a mask of what fields a drivers updates in txq-> ordev->stats
Eric, Yes I agree that still there are a couple of changes left and I am working on it.
One of them being the stats. Currently caught up in something else, I will update them ASAP.
Also, suggest me on what kind of stats do I need to maintain ? Both per device and per queue
or only per queue would be sufficient ?
--
Thanks
Sandeep
Quoted 10 lines · Kumar Gopalpet-B05799 a écrit :
Eric, Yes I agree that still there are a couple of changes left and I am working on it.
One of them being the stats. Currently caught up in something else, I will update them ASAP.Also, suggest me on what kind of stats do I need to maintain ? Both per device and per queue
or only per queue would be sufficient ?
Optimum would be per queue only, to avoid locking and let each queue be really independant.
Quoted 5 lines
Quoted 2 lines
Eric, Yes I agree that still there are a couple of changes
left and I
Quoted 2 lines
am working on it.
One of them being the stats. Currently caught up in
Quoted 9 lines
something else, I
Quoted 5 lines
will update them ASAP.
Also, suggest me on what kind of stats do I need to maintain ? Both per device and per queue or only per queue would be sufficient ?
Optimum would be per queue only, to avoid locking and let each queue be really independant.
Thanks Eric, I will proceed with per queue stats then.