[RFC] gianfar: multi queue stuff not complete

[RFC] gianfar: multi queue stuff not complete

[Eric Dumazet]

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


diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 16def13..8fe38aa 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -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;
 }
 

RE: [RFC] gianfar: multi queue stuff not complete

[Kumar Gopalpet-B05799]

 
>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
>
>

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

Re: [RFC] gianfar: multi queue stuff not complete

[Eric Dumazet]

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.

RE: [RFC] gianfar: multi queue stuff not complete

[Kumar Gopalpet-B05799]

 
>> 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.
>

Thanks Eric, I will proceed with per queue stats then.