On Mon, Nov 28, 2016 at 10:40 PM, David Miller <da...@davemloft.net> wrote: > From: Eric Dumazet <eric.duma...@gmail.com> > Date: Sat, 26 Nov 2016 18:16:00 -0800 > >> On Sun, 2016-11-27 at 00:47 +0200, Saeed Mahameed wrote: >>> On Fri, Nov 25, 2016 at 5:46 PM, Eric Dumazet <eric.duma...@gmail.com> >>> wrote: >> >>> As you see here in SRIOV mode (PF only) reads sw stats from FW. >>> Tariq, I think we need to fix this. >> >> Sure, my patch does not change this at all. >> >> If mlx4_is_master() is false, then we aggregate the software states and >> only the software stats. >> >> My patch makes this aggregation possible at the time ethtool or >> ndo_get_stat64() are called, since this absolutely not depend on the 250 >> ms timer fetching hardware stats. > > Saeed please provide counter arguments or ACK this patch, thank you.
I have nothing against this patch, I just wanted to point out that this patch is just fixing the symptom. We keep ignoring the root cause that dev_get_stats is called under a spin_lock which really ties our hands "us device drivers developers" and push us towards those fragile solutions like deferred work for caching statistics. I am not saying that Eric should fix this in his patch, i just wanted to raise the awareness of the root cause. Regarding the SRIOV PF stats, i will take it with Tariq internally. Bottom line, I ACK this patch, I might even do the same myself for mlx5 :), but there are some follow ups that need to be done.