Re: [PATCH 2/3] percpu_stats: Simple per-cpu statistics count helper functions
From: Waiman Long
Date: Thu Apr 07 2016 - 16:37:39 EST
On 04/07/2016 02:58 PM, Tejun Heo wrote:
Hello, Waiman.
On Thu, Apr 07, 2016 at 02:52:33PM -0400, Waiman Long wrote:
As long as atomic reset is an optional feature that caller can choose at
init time, I am OK to provide this functionality. I just don't want it to be
the default because of the performance overhead.
Please take a look at how percpu-ref coordinates global
synchronization. The hot path overhead is one branch which is
extremely easy to predict and shouldn't show up anywhere. If you're
gonna provide reset at all (which btw always kinda baffles me, what's
wrong with taking a snapshot value and taking delta from there?), you
need to make it actually work reliably.
Thanks.
I would say that because I am lazy, I don't want compute the deltas
every time I want to see the effect of running a certain type of
workload on the statistics counts. I have use case that I need to track
10 or so statistics counts and monitor their changes after running a
job. It is much more convenient to do a reset and see what you get than
doing manual subtractions to find out.
I had taken a look at percpu-refcount.[ch]. I think the synchronization
code is a bit overkill for this purpose as no one really need a very
precise statistics counts nor precise atomic reset. I would prefer
providing an optional atomic reset feature with slower statistics count
update path for the time being. If we come across a use case where we
need atomic reset with negligible slowdown, we could then refactor the
code to use something similar to what the percpu-refcount code is doing.
Cheers,
Longman