[dm-devel] [PATCH 3/4] dm stats: report histogram of latencies

[Coly Li]

在 15/6/15 下午9:06, Mikulas Patocka 写道:
> These micro optimizations would save one or two instructions, so they are 
> not really needed. Also, these micro optimizations are at a code path that 
> prints the statistics, not the code path that processes i/o requests.
It is not one or two instructions, it depends on how big
s->n_histogram_entries is.
Yes I agree, it is not on critical I/O path :-)

Coly
> On Sun, 14 Jun 2015, Coly Li wrote:
>
>> ? 15/6/10 ??5:22, Mikulas Patocka ??:
>>> This patch adds an option to dm statistics to collect and report the
>>> histogram of latencies.
>>>
>>> Signed-off-by: Mikulas Patocka <mpatocka at redhat.com>
>>>
>>> ---
>>>  Documentation/device-mapper/statistics.txt |   21 ++-
>>>  drivers/md/dm-stats.c                      |  201 +++++++++++++++++++++++++----
>>>  2 files changed, 196 insertions(+), 26 deletions(-)
>>>
>>> Index: linux-4.1-rc7/drivers/md/dm-stats.c
>>> ===================================================================
>>> --- linux-4.1-rc7.orig/drivers/md/dm-stats.c	2015-06-08 16:38:43.000000000 +0200
>>> +++ linux-4.1-rc7/drivers/md/dm-stats.c	2015-06-08 17:02:01.000000000 +0200
>>> @@ -29,6 +29,7 @@ struct dm_stat_percpu {
>>>  	unsigned long long io_ticks[2];
>>>  	unsigned long long io_ticks_total;
>>>  	unsigned long long time_in_queue;
>>> +	unsigned long long *histogram;
>>>  };
>>>  
>>>  struct dm_stat_shared {
>> [snip]
>>> @@ -619,6 +700,11 @@ static void __dm_stat_init_temporary_per
>>>  		shared->tmp.io_ticks[WRITE] += ACCESS_ONCE(p->io_ticks[WRITE]);
>>>  		shared->tmp.io_ticks_total += ACCESS_ONCE(p->io_ticks_total);
>>>  		shared->tmp.time_in_queue += ACCESS_ONCE(p->time_in_queue);
>>> +		if (s->n_histogram_entries) {
>>> +			unsigned i;
>>> +			for (i = 0; i < s->n_histogram_entries + 1; i++)
>>> +				shared->tmp.histogram[i] += ACCESS_ONCE(p->histogram[i]);
>> s->n_histogram_entries + 1 in for() looping will be calculated many times, maybe this way could be better,
>>
>> +		if (s->n_histogram_entries) {
>> +			unsigned i, j;
>> +			j = s->n_histogram_entries + 1;
>> +			for (i = 0; i < j; i++)
>> +				shared->tmp.histogram[i] += ACCESS_ONCE(p->histogram[i]);
>>
>>
[snip]