Re: [PATCH net-next] ehea: Remove sleep at .ndo_get_stats

[Eric Dumazet <eric.dumazet@gmail.com>]

Le jeudi 22 septembre 2011 à 11:10 -0300, brenohl@br.ibm.com a écrit :
> Currently ehea ndo_get_stats can sleep in two places, in a hcall
> and in a GFP_KERNEL alloc, which is not correct.
> This patch creates a workqueue that grabs the information from time
> to time from the hardware, and place it into the device structure,
> so that, .ndo_get_stats quickly returns the device structure statistics
> block.
> 
> Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>
> ---


Hmm... trigerring a work queue everytime NAPI is run ? That seems very
expensive to me, if for example no one wants device stats at all.

I read ehea correctly, this blocking suff is only used to compute
stats->multicast and stats->rx_errors. 

They probably could be updated once per second, nobody will complain.
(So use a workqueue, and trigger the job once per second, not once per
incoming packet)

Also, you dont provide a safe ndo_get_stats() implementation.

Think that several process might read your device stats in //

So the memset(stats, 0, sizeof(*stats)) is wrong :
You can clear stats that are currently read by another thread.

This makes SNMP readers read intermediate null values, and they dont
like this.

Fix is real easy : remove the memset(stats, 0, sizeof(*stats)) :

You only write on some fields, other fields are already 0