statistics infrastructure
Subsystems (2)
PATCH 5/6 statistics infrastructure
... posting was too big and bounced, next try
[patch 5/6] statistics infrastructure
This patch adds statistics infrastructure as common code.
In accordance to pending changes for other archs I introduced an "Instrumentation Support" menu, where I added an entry for the statistics infrastructure. I didn't bother to add this entry for other architectures. Please do so if you are interested in exploiting statistics on non-s390 architectures.
---
MAINTAINERS | 7 arch/s390/Kconfig | 6 arch/s390/oprofile/Kconfig | 5 include/linux/statistic.h | 449 +++++++++ lib/Kconfig.statistic | 12 lib/Makefile | 1 lib/statistic.c | 2183 +++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 2658 insertions(+), 5 deletions(-)
arch/s390/Kconfig +6 -0
@@ -481,8 +481,14 @@ source "drivers/net/Kconfig" source "fs/Kconfig" +menu "Instrumentation Support" + source "arch/s390/oprofile/Kconfig" +source "lib/Kconfig.statistic" + +endmenu + source "arch/s390/Kconfig.debug" source "security/Kconfig"
arch/s390/oprofile/Kconfig +0 -5
@@ -1,6 +1,3 @@ - -menu "Profiling support" - config PROFILING bool "Profiling support" help
@@ -18,5 +15,3 @@ config OPROFILE If unsure, say N. -endmenu -
include/linux/statistic.h +449 -0
449 lines not on this page · raw message
lib/Kconfig.statistic +12 -0
@@ -0,0 +1,12 @@ +config STATISTICS + bool "Statistics infrastructure" + depends on DEBUG_FS + select SGRB + help + The statistics infrastructure provides a debug-fs based user interface + for statistics of kernel components, that is, usually device drivers. + Statistics are available for components that have been instrumented to + feed data into the statistics infrastructure. + This feature is useful for performance measurements or performance + debugging. + If in doubt, say "N".
lib/Makefile +1 -0
@@ -47,6 +47,7 @@ obj-$(CONFIG_TEXTSEARCH_FSM) += ts_fsm.o obj-$(CONFIG_SWIOTLB) += swiotlb.o obj-$(CONFIG_SGRB) += sgrb.o +obj-$(CONFIG_STATISTICS) += statistic.o hostprogs-y := gen_crc32table clean-files := crc32table.h
lib/statistic.c +2183 -0
2,183 lines not on this page · raw message
MAINTAINERS +7 -0
@@ -2464,6 +2464,13 @@ STARMODE RADIO IP (STRIP) PROTOCOL DRIVE W: http://mosquitonet.Stanford.EDU/strip.html S: Unsupported ? +STATISTICS INFRASTRUCTURE +P: Martin Peschke +M: [email protected] +M: [email protected] +W: http://oss.software.ibm.com/developerworks/opensource/linux390 +S: Supported + STRADIS MPEG-2 DECODER DRIVER P: Nathan Laredo M: [email protected]
include/linux/statistic.h line 266
Quoted 12 lines · Martin Peschke
+ */
include/linux/statistic.h +11
-0+static inline u64 statistic_add (struct statistic *stat,s64 value,u64 incr) +{ +unsigned long flags; +int retval; + +if (stat->on !=STATISTIC_DEF_ON ) +return 0 ; + +statistic_lock (stat->interface, flags); + retval = stat->add (stat, value, incr); +statistic_unlock (stat->interface, flags);
Locks and indirect function calls?
It seems very wrong to me to make such heavy weight statistic functions. Most likely you will disturb the performance whatever is being counted badly.
Take a look at many other subsystems - they do per CPU counters etc. to make this all fast.
But it's still unclear why it would need such an heavyweight infrastructure. Normally it's not that bad to reimplemented on the fly. Maybe some common code can be refactored out of that, but probably not too much.
[... lots of other code snipped ... ]
Looks all very very overdesigned to me. How about you just start with a minimum specification and describe what you want to do? And then start with a *simple* base implementation. If it's really needed it will grow later anyways, but starting off with such a complex monstrosity is just wrong.
-Andi
Quoted 6 lines · Andi Kleen
Locks and indirect function calls?
It seems very wrong to me to make such heavy weight statistic functions. Most likely you will disturb the performance whatever is being counted badly.
Well, that's a tradeoff between flexibility/function and performance.
I don't have any reliable numbers ready at hand. At least, doing I/O to my SCSI devices with enabled statistics didn't feel bad.
Here is the rational for both the indirect function call and the lock:
If a statistic is disabled (that's the default) neither is locking done nor is a function called indirectly. So far so good, I would say.
If a statistic is enabled the lock for this entity is grabbed and one indirect function call is done. Anything else is inlined. I use granular per-interface (per-entity, for example per-LUN or per-HBA) locking.
The indirect function call allows customization of the ways data processing is done for particular statistics.
For example, one could deflate a histogram of latencies into a counter providing the total of latency measurements, that is the total of requests observed; or inflate a statistic the other way around if required. Another example: one can make a statistic gather data for recurring periods of time, like megabytes per seconds instead of just the total amount of bytes transferred, or like queue utilization per whatever-unit-of-time instead of just an overall utilization.
A statistic that feeds on request sizes can be setup to provide the following "views":
- number of requests observed (counter)
- number of requests per unit of time (history based on a counter)
- number of bytes transfered (counter)
- number of bytes transfered per unit of time = transfer rate (history based on a counter)
- traffic pattern (histogram for descrete request sizes or for ranges of request sizes)
- raw measurement data gathered
- etc.
As a device driver programmer I might pick a "view" a user is interested in. My pick might miss by a mile. I simply don't know for sure beforehand.
The indirect function call could be a replaced by a switch statement. Not sure whether this is less critical and more acceptable than indirect function calls. Might be architucture dependent.
We can get rid of the indirect function call (or an alternative switch statement) if the vote is against this level of flexibility.
Then it would be solely up to the exploiter to define once and for all whether a particular sort of data is shown as a simple counter, a histogram, a fill level indicator, this history-type statistic thing in a ringbuffer etc. This might be fine for a considerable number of cases.
The lock is there to avoid trouble with concurrent updates to a statistic. If per-CPU data was used, concurrent updates are fine as long as they are done on different CPUs. Precautions for concurrent updates to the same per-CPU is still needed, though.
The current interface allows to use the lock this way:
lock(stat_x->interface);statistics_inc_nolock(stat_x, y);statistics_inc_nolock(stat_m, n);statistics_add_nolock(stat_a, e, f);unlock(stat_x->interface);
Because we hold the same lock when creating output for users, coherency of several statistics of a single entity can be achieved if statistic updates are done within one critical section as shown above.
The lock is also used to make sure that updates to a statistic don't happen while the setup of a statistic is changed by users. If we get rid of the indirect function call, some of these setup changes go away, anyway. Other cases, like statistic resets or inflating a 5-counter histogram to a 25-counter histogram, don't go away. If I can figure out how to reallocate, say, an array of counters for a histogram without holding a lock while updates happen... Maybe I could temporarily turn of a statistic.
Quoted 4 lines
Take a look at many other subsystems - they do per CPU counters etc. to make this all fast.
I am looking into per CPU data.
But, is this really required for _all_ statistics? I see that it makes sense to have per CPU optimizations for very critical components, like parts of VM. But there are still a lot of do-it-yourself type statistics around that use an atomic_t, for example, without implementing it per CPU.
Then, I am not sure yet whether per CPU data is feasible for histograms and other more complex statistics. I have got to find out.
I tried to write the code in a way that allows to add other statistic type, like counters, histograms and so on, with moderate effort. Maybe I can use the internal interface to plug in some disclipline based on per CPU counters...
Quoted 10 lines
But it's still unclear why it would need such an heavyweight infrastructure. Normally it's not that bad to reimplemented on the fly. Maybe some common code can be refactored out of that, but probably not too much.
[... lots of other code snipped ... ]
Looks all very very overdesigned to me. How about you just start with a minimum specification and describe what you want to do?
As a device driver programmer I don't want to reinvent the wheel when coding statistics. I would prefer to use a few and easy to use library functions. I don't want to worry about getting my personal wheel being functional. I'd rather use my time to worry about which kind of data is really needed and which is not.
I'd like to provide a tool that can be customized at run time to a certain degree because it might not be acceptable for customers to install private kernels in order to get tuned statistics. As a device driver programmer I can make an educated guess, at best, about certain parameters that impact the processing of statistic data.
Users might know better whether they need to focus on latencies from 2 ms up to 64 ms or from 100 ms up to 500 ms, because this kind of decisions depends on the environment to be measured, e.g. devices attached.
In a device driver, I don't want to spent much thought about the statistic's user interface. Particularly not if the statistic is a little bit more complex than a simple counter. Would be really nice to have a user interface that looks the same for all exploiters, i.e. to have common output formats for counters, histograms, fill level indicators etc.
Martin