On Wed, Nov 30, 2016 at 11:00 PM, Eric Dumazet <eric.duma...@gmail.com> wrote:
> On Wed, 2016-11-30 at 22:42 +0200, Saeed Mahameed wrote:
>> On Wed, Nov 30, 2016 at 7:35 PM, Eric Dumazet <eric.duma...@gmail.com> wrote:
>> > On Wed, 2016-11-30 at 18:46 +0200, Saeed Mahameed wrote:
>> >> we had/still have the proper stats they are the ones that
>> >> mlx4_en_fold_software_stats is trying to cache into (they always
>> >> exist),
>> >> but the ones that you are trying to read from (the mlx4 rings) are gone !
>> >> This bug is totally new and as i warned, this is another symptom of
>> >> the real root cause (can't sleep while reading stats).
>> >> Eric what do you suggest ? Keep pre-allocated MAX_RINGS stats and
>> >> always iterate over all of them to query stats ?
>> >> what if you have one ring/none/1K ? how would you know how many to query ?
>> > I am suggesting I will fix the bug I introduced.
>> > Do not panic.
>> Not at all, I trust you are the only one who is capable of providing
>> the best solution.
>> I am just trying to read your mind :-).
>> As i said i like the solution and i want to adapt it to mlx5, so I am
>> a little bit enthusiastic :)
> What about the following fix guys ?
> As a bonus we update the stats right before they are sent to monitors
> via rtnetlink ;)
Hi Eric, Thanks for the patch, I already acked it.
I have one educational question (not related to this patch, but
related to stats reading in general).
I was wondering why do we need to disable bh every time we read stats
"spin_lock_bh" ? is it essential ?
I checked and in mlx4 we don't hold stats_lock in softirq
(en_rx.c/en_tx.c), so I don't see any deadlock risk in here..