* [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
@ 2012-10-16 17:02 Bhavesh Davda
2012-10-23 1:16 ` Frank Rowand
2012-10-25 21:36 ` Frank Rowand
0 siblings, 2 replies; 19+ messages in thread
From: Bhavesh Davda @ 2012-10-16 17:02 UTC (permalink / raw)
To: linux-rt-users@vger.kernel.org; +Cc: John Kacur, Frank Rowand
From: Bhavesh Davda <bhavesh@vmware.com>
Add feature to cyclictest histogram mode to track cycle counts every time a
sample overflows the histogram limit. This should help identify if there is a
timing pattern to jitters in cyclictest runs.
Example output (with -h 10):
...
Histogram Overflows: 00001 00007 00000 00009 00004 00007 00000 00001
Histogram Overflow at cycle number:
Thread 0: 09964
Thread 1: 00000 00004 00006 00008 00010 09962 11594
Thread 2:
Thread 3: 01169 04698 06782 09033 10299 11561 21517 28734 29532
Thread 4: 11574 11580 11583 11586
Thread 5: 00020 09448 13954 14954 18954 20587 24973
Thread 6:
Thread 7: 18950
...
Signed-off-by: Bhavesh Davda <bhavesh@vmware.com>
---
src/cyclictest/cyclictest.c | 26 +++++++++++++++++++++++---
1 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/src/cyclictest/cyclictest.c b/src/cyclictest/cyclictest.c
index 418538f..824544f 100644
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -150,6 +150,7 @@ struct thread_stat {
double avg;
long *values;
long *hist_array;
+ long *outliers;
pthread_t thread;
int threadstarted;
int tid;
@@ -157,6 +158,7 @@ struct thread_stat {
long redmax;
long cycleofmax;
long hist_overflow;
+ long num_outliers;
};
static int shutdown;
@@ -850,8 +852,11 @@ void *timerthread(void *param)
/* Update the histogram */
if (histogram) {
- if (diff >= histogram)
+ if (diff >= histogram) {
stat->hist_overflow++;
+ if (stat->num_outliers < histogram)
+ stat->outliers[stat->num_outliers++] = stat->cycles;
+ }
else
stat->hist_array[diff]++;
}
@@ -1401,6 +1406,17 @@ static void print_hist(struct thread_param *par[], int nthreads)
if (histofall && nthreads > 1)
printf(" %05lu", alloverflows);
printf("\n");
+
+ printf("# Histogram Overflow at cycle number:\n");
+ for (i = 0; i < nthreads; i++) {
+ printf("# Thread %d:", i);
+ for (j = 0; j < par[i]->stats->num_outliers; j++)
+ printf(" %05lu", par[i]->stats->outliers[j]);
+ if (par[i]->stats->num_outliers < par[i]->stats->hist_overflow)
+ printf(" # %05lu others", par[i]->stats->hist_overflow - par[i]->stats->num_outliers);
+ printf("\n");
+ }
+ printf("\n");
}
static void print_stat(struct thread_param *par, int index, int verbose)
@@ -1549,10 +1565,12 @@ int main(int argc, char **argv)
int bufsize = histogram * sizeof(long);
stat->hist_array = threadalloc(bufsize, node);
- if (stat->hist_array == NULL)
+ stat->outliers = threadalloc(bufsize, node);
+ if (stat->hist_array == NULL || stat->outliers == NULL)
fatal("failed to allocate histogram of size %d on node %d\n",
histogram, i);
memset(stat->hist_array, 0, bufsize);
+ memset(stat->outliers, 0, bufsize);
}
if (verbose) {
@@ -1668,8 +1686,10 @@ int main(int argc, char **argv)
if (histogram) {
print_hist(parameters, num_threads);
- for (i = 0; i < num_threads; i++)
+ for (i = 0; i < num_threads; i++) {
threadfree(statistics[i]->hist_array, histogram*sizeof(long), parameters[i]->node);
+ threadfree(statistics[i]->outliers, histogram*sizeof(long), parameters[i]->node);
+ }
}
if (tracelimit) {
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-10-16 17:02 [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking Bhavesh Davda
@ 2012-10-23 1:16 ` Frank Rowand
2012-10-23 1:41 ` Frank Rowand
2012-10-25 21:36 ` Frank Rowand
1 sibling, 1 reply; 19+ messages in thread
From: Frank Rowand @ 2012-10-23 1:16 UTC (permalink / raw)
To: Bhavesh Davda; +Cc: linux-rt-users@vger.kernel.org, John Kacur, Rowand, Frank
On 10/16/12 10:02, Bhavesh Davda wrote:
> From: Bhavesh Davda <bhavesh@vmware.com>
>
> Add feature to cyclictest histogram mode to track cycle counts every time a
> sample overflows the histogram limit. This should help identify if there is a
> timing pattern to jitters in cyclictest runs.
>
> Example output (with -h 10):
> ...
> Histogram Overflows: 00001 00007 00000 00009 00004 00007 00000 00001
> Histogram Overflow at cycle number:
> Thread 0: 09964
> Thread 1: 00000 00004 00006 00008 00010 09962 11594
> Thread 2:
> Thread 3: 01169 04698 06782 09033 10299 11561 21517 28734 29532
> Thread 4: 11574 11580 11583 11586
> Thread 5: 00020 09448 13954 14954 18954 20587 24973
> Thread 6:
> Thread 7: 18950
> ...
>
> Signed-off-by: Bhavesh Davda <bhavesh@vmware.com>
After the patches survive checkpatch, I'll review and test them.
As far as the line length warnings are concerned, cyclictest has
a _lot_ of lines longer than 80 characters, so just use your
best judgement as to whether there is a good way to change the
patch to fit within 80 characters, or if the change is consistent
with cyclictest style.
>From checkpatch:
If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
ERROR: code indent should use tabs where possible
#38: FILE: src/cyclictest/cyclictest.c:153:
+ long *outliers;$
WARNING: please, no spaces at the start of a line
#38: FILE: src/cyclictest/cyclictest.c:153:
+ long *outliers;$
ERROR: code indent should use tabs where possible
#46: FILE: src/cyclictest/cyclictest.c:161:
+ long num_outliers;$
WARNING: please, no spaces at the start of a line
#46: FILE: src/cyclictest/cyclictest.c:161:
+ long num_outliers;$
ERROR: code indent should use tabs where possible
#57: FILE: src/cyclictest/cyclictest.c:857:
+ if (stat->num_outliers < histogram)$
WARNING: please, no spaces at the start of a line
#57: FILE: src/cyclictest/cyclictest.c:857:
+ if (stat->num_outliers < histogram)$
WARNING: line over 80 characters
#58: FILE: src/cyclictest/cyclictest.c:858:
+ stat->outliers[stat->num_outliers++] = stat->cycles;
WARNING: line over 80 characters
#74: FILE: src/cyclictest/cyclictest.c:1416:
+ printf(" # %05lu others", par[i]->stats->hist_overflow - par[i]->stats->num_outliers);
WARNING: line over 80 characters
#102: FILE: src/cyclictest/cyclictest.c:1691:
+ threadfree(statistics[i]->outliers, histogram*sizeof(long), parameters[i]->node);
total: 3 errors, 6 warnings, 67 lines checked
NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
scripts/cleanfile
/home/frowand/tmp/hist_overflow_xxx_1 has style problems, please review.
If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
ERROR: Does not appear to be a unified-diff format patch
total: 1 errors, 0 warnings, 0 lines checked
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-10-23 1:16 ` Frank Rowand
@ 2012-10-23 1:41 ` Frank Rowand
2012-10-23 18:59 ` Bhavesh Davda
0 siblings, 1 reply; 19+ messages in thread
From: Frank Rowand @ 2012-10-23 1:41 UTC (permalink / raw)
To: Rowand, Frank; +Cc: Bhavesh Davda, linux-rt-users@vger.kernel.org, John Kacur
On 10/22/12 18:16, Frank Rowand wrote:
> On 10/16/12 10:02, Bhavesh Davda wrote:
>> From: Bhavesh Davda <bhavesh@vmware.com>
>>
>> Add feature to cyclictest histogram mode to track cycle counts every time a
>> sample overflows the histogram limit. This should help identify if there is a
>> timing pattern to jitters in cyclictest runs.
>>
>> Example output (with -h 10):
>> ...
>> Histogram Overflows: 00001 00007 00000 00009 00004 00007 00000 00001
>> Histogram Overflow at cycle number:
>> Thread 0: 09964
>> Thread 1: 00000 00004 00006 00008 00010 09962 11594
>> Thread 2:
>> Thread 3: 01169 04698 06782 09033 10299 11561 21517 28734 29532
>> Thread 4: 11574 11580 11583 11586
>> Thread 5: 00020 09448 13954 14954 18954 20587 24973
>> Thread 6:
>> Thread 7: 18950
>> ...
>>
>> Signed-off-by: Bhavesh Davda <bhavesh@vmware.com>
>
>
> After the patches survive checkpatch, I'll review and test them.
< snip >
> /home/frowand/tmp/hist_overflow_xxx_1 has style problems, please review.
>
> If any of these errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
Oooooops, the following message was about the series file I would
use to apply the patches with quilt. Please ignore it.
> ERROR: Does not appear to be a unified-diff format patch
>
> total: 1 errors, 0 warnings, 0 lines checked
-Frank
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-10-23 1:41 ` Frank Rowand
@ 2012-10-23 18:59 ` Bhavesh Davda
2012-10-23 19:25 ` John Kacur
0 siblings, 1 reply; 19+ messages in thread
From: Bhavesh Davda @ 2012-10-23 18:59 UTC (permalink / raw)
To: frank rowand; +Cc: linux-rt-users, John Kacur, Frank Rowand
Hi Frank,
Thanks for the review.
Unfortunately my git-fu is super weak, and I don't know how to re-spin the patches by simply editing the source file in my git tree and fixing these warnings and errors that checkpatch.pl is identifying.
Can you either (a) help me with step-by-step instructions on how to do so in git (perhaps RTFM with a pointer to one), or (b) help fix up my 2 patches yourself in preparation for a review and a commit?
Thanks
- Bhavesh
----- Original Message -----
From: "Frank Rowand" <frank.rowand@am.sony.com>
To: "Frank Rowand" <Frank_Rowand@sonyusa.com>
Cc: "Bhavesh Davda" <bhavesh@vmware.com>, linux-rt-users@vger.kernel.org, "John Kacur" <jkacur@redhat.com>
Sent: Monday, October 22, 2012 6:41:57 PM
Subject: Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
On 10/22/12 18:16, Frank Rowand wrote:
> On 10/16/12 10:02, Bhavesh Davda wrote:
>> From: Bhavesh Davda <bhavesh@vmware.com>
>>
>> Add feature to cyclictest histogram mode to track cycle counts every time a
>> sample overflows the histogram limit. This should help identify if there is a
>> timing pattern to jitters in cyclictest runs.
>>
>> Example output (with -h 10):
>> ...
>> Histogram Overflows: 00001 00007 00000 00009 00004 00007 00000 00001
>> Histogram Overflow at cycle number:
>> Thread 0: 09964
>> Thread 1: 00000 00004 00006 00008 00010 09962 11594
>> Thread 2:
>> Thread 3: 01169 04698 06782 09033 10299 11561 21517 28734 29532
>> Thread 4: 11574 11580 11583 11586
>> Thread 5: 00020 09448 13954 14954 18954 20587 24973
>> Thread 6:
>> Thread 7: 18950
>> ...
>>
>> Signed-off-by: Bhavesh Davda <bhavesh@vmware.com>
>
>
> After the patches survive checkpatch, I'll review and test them.
< snip >
> /home/frowand/tmp/hist_overflow_xxx_1 has style problems, please review.
>
> If any of these errors are false positives, please report
> them to the maintainer, see CHECKPATCH in MAINTAINERS.
Oooooops, the following message was about the series file I would
use to apply the patches with quilt. Please ignore it.
> ERROR: Does not appear to be a unified-diff format patch
>
> total: 1 errors, 0 warnings, 0 lines checked
-Frank
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-10-23 18:59 ` Bhavesh Davda
@ 2012-10-23 19:25 ` John Kacur
2012-10-23 19:55 ` Frank Rowand
0 siblings, 1 reply; 19+ messages in thread
From: John Kacur @ 2012-10-23 19:25 UTC (permalink / raw)
To: Bhavesh Davda; +Cc: linux-rt-users, Frank Rowand, frank rowand
----- Original Message -----
> Hi Frank,
>
> Thanks for the review.
>
> Unfortunately my git-fu is super weak, and I don't know how to
> re-spin the patches by simply editing the source file in my git tree
> and fixing these warnings and errors that checkpatch.pl is
> identifying.
>
> Can you either (a) help me with step-by-step instructions on how to
> do so in git (perhaps RTFM with a pointer to one), or (b) help fix
> up my 2 patches yourself in preparation for a review and a commit?
>
> Thanks
>
> - Bhavesh
>
> ----- Original Message -----
>
> From: "Frank Rowand" <frank.rowand@am.sony.com>
> To: "Frank Rowand" <Frank_Rowand@sonyusa.com>
> Cc: "Bhavesh Davda" <bhavesh@vmware.com>,
> linux-rt-users@vger.kernel.org, "John Kacur" <jkacur@redhat.com>
> Sent: Monday, October 22, 2012 6:41:57 PM
> Subject: Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance
> tracking
>
> Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
> On 10/22/12 18:16, Frank Rowand wrote:
> > On 10/16/12 10:02, Bhavesh Davda wrote:
> >> From: Bhavesh Davda <bhavesh@vmware.com>
> >>
> >> Add feature to cyclictest histogram mode to track cycle counts
> >> every time a
> >> sample overflows the histogram limit. This should help identify if
> >> there is a
> >> timing pattern to jitters in cyclictest runs.
> >>
> >> Example output (with -h 10):
> >> ...
> >> Histogram Overflows: 00001 00007 00000 00009 00004 00007 00000
> >> 00001
> >> Histogram Overflow at cycle number:
> >> Thread 0: 09964
> >> Thread 1: 00000 00004 00006 00008 00010 09962 11594
> >> Thread 2:
> >> Thread 3: 01169 04698 06782 09033 10299 11561 21517 28734 29532
> >> Thread 4: 11574 11580 11583 11586
> >> Thread 5: 00020 09448 13954 14954 18954 20587 24973
> >> Thread 6:
> >> Thread 7: 18950
> >> ...
> >>
> >> Signed-off-by: Bhavesh Davda <bhavesh@vmware.com>
> >
> >
> > After the patches survive checkpatch, I'll review and test them.
>
> < snip >
>
> > /home/frowand/tmp/hist_overflow_xxx_1 has style problems, please
> > review.
> >
> > If any of these errors are false positives, please report
> > them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
> Oooooops, the following message was about the series file I would
> use to apply the patches with quilt. Please ignore it.
>
> > ERROR: Does not appear to be a unified-diff format patch
> >
Let's not overdo it here, I've actually already added these patches in my private repo. I only want to hear if there are functional problems. I'm wondering if this is fine with histograms by default or if we want to protect it with an option? Or do we need to add something to make it easy to filter out?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-10-23 19:25 ` John Kacur
@ 2012-10-23 19:55 ` Frank Rowand
2012-10-23 19:59 ` Sven-Thorsten Dietrich
0 siblings, 1 reply; 19+ messages in thread
From: Frank Rowand @ 2012-10-23 19:55 UTC (permalink / raw)
To: John Kacur; +Cc: Bhavesh Davda, linux-rt-users@vger.kernel.org, Rowand, Frank
On 10/23/12 12:25, John Kacur wrote:
>
< snip >
> Let's not overdo it here, I've actually already added these patches
> in my private repo. I only want to hear if there are functional
> problems. I'm wondering if this is fine with histograms by default or
> if we want to protect it with an option? Or do we need to add
> something to make it easy to filter out?
OK. I'll apply the patches, test the result, and do a review.
-Frank
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-10-23 19:55 ` Frank Rowand
@ 2012-10-23 19:59 ` Sven-Thorsten Dietrich
0 siblings, 0 replies; 19+ messages in thread
From: Sven-Thorsten Dietrich @ 2012-10-23 19:59 UTC (permalink / raw)
To: frank.rowand
Cc: John Kacur, Bhavesh Davda, linux-rt-users@vger.kernel.org,
Rowand, Frank
On Oct 23, 2012, at 12:55 PM, Frank Rowand <frank.rowand@am.sony.com> wrote:
> On 10/23/12 12:25, John Kacur wrote:
>>
>
> < snip >
>
>> Let's not overdo it here, I've actually already added these patches
>> in my private repo. I only want to hear if there are functional
>> problems. I'm wondering if this is fine with histograms by default or
>> if we want to protect it with an option? Or do we need to add
>> something to make it easy to filter out?
>
> OK. I'll apply the patches, test the result, and do a review.\
At one point the overflows were already being tracked.
In any case it is helpful along with the max to adjust the histogram range and buckets as needed.
Sven
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-10-16 17:02 [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking Bhavesh Davda
2012-10-23 1:16 ` Frank Rowand
@ 2012-10-25 21:36 ` Frank Rowand
2012-10-25 22:05 ` Frank Rowand
2012-11-13 23:41 ` Frank Rowand
1 sibling, 2 replies; 19+ messages in thread
From: Frank Rowand @ 2012-10-25 21:36 UTC (permalink / raw)
To: Bhavesh Davda; +Cc: linux-rt-users@vger.kernel.org, John Kacur, Rowand, Frank
On 10/16/12 10:02, Bhavesh Davda wrote:
> From: Bhavesh Davda <bhavesh@vmware.com>
>
> Add feature to cyclictest histogram mode to track cycle counts every time a
> sample overflows the histogram limit. This should help identify if there is a
> timing pattern to jitters in cyclictest runs.
>
> Example output (with -h 10):
> ...
> Histogram Overflows: 00001 00007 00000 00009 00004 00007 00000 00001
> Histogram Overflow at cycle number:
> Thread 0: 09964
> Thread 1: 00000 00004 00006 00008 00010 09962 11594
> Thread 2:
> Thread 3: 01169 04698 06782 09033 10299 11561 21517 28734 29532
> Thread 4: 11574 11580 11583 11586
> Thread 5: 00020 09448 13954 14954 18954 20587 24973
> Thread 6:
> Thread 7: 18950
> ...
>
> Signed-off-by: Bhavesh Davda <bhavesh@vmware.com>
My comments up to "--" are not meant to be part of the header of the
following patch.
I am including the following patch as my review of the patch I am
replying to. If the following patch is incorporated into the
reviewed patch, then you can add my:
Reviewed-by: Frank Rowand <frank.rowand@am.sony.com>
Tested-by: Frank Rowand <frank.rowand@am.sony.com>
to the combined patch.
The following patch fails checkpatch:
total: 2 errors, 11 warnings, 148 lines checked
but the warnings and errors seem consistent with existing cyclictest
practice.
I did not fix any of the white space issues in the reviewed patch.
There are already some existing white space issues in cyclictest,
so after all these patches are applied I can submit a small patch
to finish cleaning up leading and trailing white space issues.
The single letter command line flags that seemed most intuitive
for the overflow cycle report are already taken. I chose "g"
simply because it was adjacent to "h" in the -help report and
it is related to the histogram feature. I chose "of_cycle"
as the long option name because a longer, more descriptive
name would change the formatting of the -help report. Feel
free to bike shed and change to another name.
I picked an arbitrary value for OVERFLOW_CYCLE_MAX. Feel free
to pick another value if needed.
--
Changed size of stats->outliers[] from histogram to hist_overflow.
histogram is the number of histogram buckets, which is not related
to the time dimension, which is the purpose of the histogram
overflow cycle report.
Added command line option to set the value of hist_overflow. This
has the additional side effect of controlling whether the histogram
overflow cycle report is printed.
Add a line to the histogram overflow cycle report that merges all
threads into a single time line. This is controlled by the -H
option which already serves a similar purpose for histograms.
Removed leading zeros from cycle values reported in the histogram
overflow cycle report to make the values more readable.
Removed extra blank line printed at end of Histogram output.
$ cyclictest -h 10 -a -t 8 -l 5000 -g 15
# /dev/cpu_dma_latency set to 0us
policy: other/other: loadavg: 0.03 0.02 0.00 1/374 29840
T: 0 (29833) P: 0 I:1000 C: 5000 Min: 5 Act: 7 Avg: 6 Max: 45
T: 1 (29834) P: 0 I:1000 C: 5000 Min: 5 Act: 6 Avg: 6 Max: 121
T: 2 (29835) P: 0 I:1000 C: 5000 Min: 6 Act: 7 Avg: 8 Max: 13
T: 3 (29836) P: 0 I:1000 C: 5000 Min: 5 Act: 7 Avg: 6 Max: 89
T: 4 (29837) P: 0 I:1000 C: 5000 Min: 5 Act: 9 Avg: 6 Max: 9
T: 5 (29838) P: 0 I:1000 C: 5000 Min: 6 Act: 9 Avg: 6 Max: 23
T: 6 (29839) P: 0 I:1000 C: 5000 Min: 5 Act: 7 Avg: 6 Max: 81
T: 7 (29840) P: 0 I:1000 C: 5000 Min: 5 Act: 9 Avg: 6 Max: 9
# Histogram
000000 000000 000000 000000 000000 000000 000000 000000 000000
000001 000000 000000 000000 000000 000000 000000 000000 000000
000002 000000 000000 000000 000000 000000 000000 000000 000000
000003 000000 000000 000000 000000 000000 000000 000000 000000
000004 000000 000000 000000 000000 000000 000000 000000 000000
000005 001741 001034 000000 000012 000005 000000 000014 000002
000006 000587 001455 000053 000928 003540 000286 003018 002194
000007 002658 002410 000444 004045 001423 004705 001413 002799
000008 000006 000093 003353 000001 000030 000002 000527 000003
000009 000001 000005 000499 000011 000002 000006 000018 000002
# Total: 000004993 000004997 000004349 000004997 000005000 000004999 000004990 000005000
# Min Latencies: 00005 00005 00006 00005 00005 00006 00005 00005
# Avg Latencies: 00006 00006 00008 00006 00006 00006 00006 00006
# Max Latencies: 00045 00121 00013 00089 00009 00023 00081 00009
# Histogram Overflows: 00007 00003 00651 00003 00000 00001 00010 00000
# Histogram Overflow at cycle number:
# Thread 0: 161 2163 3014 3159 4015 4162 4924
# Thread 1: 3649 3940 4940
# Thread 2: 0 1 10 20 31 41 51 61 71 81 91 92 101 102 111 # 636 others
# Thread 3: 0 940 1161
# Thread 4:
# Thread 5: 824
# Thread 6: 70 212 283 415 486 1724 2201 2690 4170 4640
# Thread 7:
$ cyclictest -H 10 -a -t 8 -l 5000 -g 15
# /dev/cpu_dma_latency set to 0us
policy: other/other: loadavg: 0.05 0.02 0.00 1/374 29831
T: 0 (29824) P: 0 I:1000 C: 5000 Min: 2 Act: 6 Avg: 5 Max: 51
T: 1 (29825) P: 0 I:1000 C: 5000 Min: 3 Act: 7 Avg: 7 Max: 74
T: 2 (29826) P: 0 I:1000 C: 5000 Min: 3 Act: 7 Avg: 7 Max: 29
T: 3 (29827) P: 0 I:1000 C: 5000 Min: 3 Act: 7 Avg: 7 Max: 91
T: 4 (29828) P: 0 I:1000 C: 5000 Min: 2 Act: 9 Avg: 6 Max: 25
T: 5 (29829) P: 0 I:1000 C: 5000 Min: 3 Act: 10 Avg: 6 Max: 44
T: 6 (29830) P: 0 I:1000 C: 5000 Min: 3 Act: 9 Avg: 7 Max: 22
T: 7 (29831) P: 0 I:1000 C: 5000 Min: 2 Act: 8 Avg: 6 Max: 156
# Histogram
000000 000000 000000 000000 000000 000000 000000 000000 000000 000000
000001 000000 000000 000000 000000 000000 000000 000000 000000 000000
000002 000002 000000 000000 000000 000003 000000 000000 000003 000008
000003 000001 000003 000002 000003 000001 000003 000003 000001 000017
000004 000001 000000 000001 000000 000000 000001 000001 000000 000004
000005 003537 000003 000034 000001 000004 000000 000000 001141 004720
000006 000834 000282 000540 000029 002797 000347 000201 002889 007919
000007 000610 004184 003629 004423 002160 004630 004019 000952 024607
000008 000005 000482 000228 000128 000031 000012 000721 000010 001617
000009 000001 000041 000068 000409 000003 000001 000051 000002 000576
# Total: 000004991 000004995 000004502 000004993 000004999 000004994 000004996 000004998 000039468
# Min Latencies: 00002 00003 00003 00003 00002 00003 00003 00002
# Avg Latencies: 00005 00007 00007 00007 00006 00006 00007 00006
# Max Latencies: 00051 00074 00029 00091 00025 00044 00022 00156 00156
# Histogram Overflows: 00009 00005 00498 00007 00001 00006 00004 00002 00532
# Histogram Overflow at cycle number:
# Thread all: 4 10 20 31 41 50 61 71 81 91 102 112 122 132 142 354 502 536 1172 1315 1355 1535 2029 2114 2318 2356 2537 2971 3276 3357 3541 3542 3997 4264 4358 4535 4537 4544 4551 4782 4999
# Thread 0: 354 536 1355 2356 2537 3357 4358 4537 4551
# Thread 1: 4 3276 3542 3997 4535
# Thread 2: 4 10 20 31 41 50 61 71 81 91 102 112 122 132 142 # 483 others
# Thread 3: 4 1315 1535 2114 4264 4535 4782
# Thread 4: 502
# Thread 5: 4 1172 3541 3997 4544 4999
# Thread 6: 4 2029 2971 4544
# Thread 7: 1535 2318
Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
---
src/cyclictest/cyclictest.c | 75 61 + 14 - 0 !
1 file changed, 61 insertions(+), 14 deletions(-)
Index: b/src/cyclictest/cyclictest.c
===================================================================
--- a/src/cyclictest/cyclictest.c
+++ b/src/cyclictest/cyclictest.c
@@ -91,6 +91,7 @@ extern int clock_nanosleep(clockid_t __c
#define NSEC_PER_SEC 1000000000
#define HIST_MAX 1000000
+#define OVERFLOW_CYCLE_MAX 1000000
#define MODE_CYCLIC 0
#define MODE_CLOCK_NANOSLEEP 1
@@ -171,6 +172,7 @@ static int lockall = 0;
static int tracetype = NOTRACE;
static int histogram = 0;
static int histofall = 0;
+static int overflow_cycle = 0;
static int duration = 0;
static int use_nsecs = 0;
static int refresh_on_max;
@@ -854,7 +856,7 @@ void *timerthread(void *param)
if (histogram) {
if (diff >= histogram) {
stat->hist_overflow++;
- if (stat->num_outliers < histogram)
+ if (stat->num_outliers < overflow_cycle)
stat->outliers[stat->num_outliers++] = stat->cycles;
}
else
@@ -930,6 +932,7 @@ static void display_help(int error)
" to modify value to minutes, hours or days\n"
"-E --event event tracing (used with -b)\n"
"-f --ftrace function trace (when -b is active)\n"
+ "-g MAX --of_cycle=MAX Report the cycle of up to MAX histogram overflows\n"
"-h --histogram=US dump a latency histogram to stdout after the run\n"
" (with same priority about many threads)\n"
" US is the max time to be be tracked in microseconds\n"
@@ -1059,6 +1062,7 @@ static void process_options (int argc, c
{"distance", required_argument, NULL, 'd'},
{"event", no_argument, NULL, 'E'},
{"ftrace", no_argument, NULL, 'f'},
+ {"overflow_cycle", required_argument, NULL, 'g'},
{"histogram", required_argument, NULL, 'h'},
{"histofall", required_argument, NULL, 'H'},
{"interval", required_argument, NULL, 'i'},
@@ -1090,7 +1094,7 @@ static void process_options (int argc, c
{"priospread", no_argument, NULL, 'Q'},
{NULL, 0, NULL, 0}
};
- int c = getopt_long(argc, argv, "a::b:Bc:Cd:Efh:H:i:Il:MnNo:O:p:PmqQrsSt::uUvD:wWT:y:e:",
+ int c = getopt_long(argc, argv, "a::b:Bc:Cd:Efg:h:H:i:Il:MnNo:O:p:PmqQrsSt::uUvD:wWT:y:e:",
long_options, &option_index);
if (c == -1)
break;
@@ -1116,6 +1120,7 @@ static void process_options (int argc, c
case 'd': distance = atoi(optarg); break;
case 'E': enable_events = 1; break;
case 'f': tracetype = FUNCTION; ftrace = 1; break;
+ case 'g': overflow_cycle = atoi(optarg); break;
case 'H': histofall = 1; /* fall through */
case 'h': histogram = atoi(optarg); break;
case 'i': interval = atoi(optarg); break;
@@ -1240,6 +1245,12 @@ static void process_options (int argc, c
error = 1;
}
+ if (overflow_cycle < 0)
+ error = 1;
+
+ if (overflow_cycle > OVERFLOW_CYCLE_MAX)
+ overflow_cycle = OVERFLOW_CYCLE_MAX;
+
if (histogram < 0)
error = 1;
@@ -1407,16 +1418,51 @@ static void print_hist(struct thread_par
printf(" %05lu", alloverflows);
printf("\n");
- printf("# Histogram Overflow at cycle number:\n");
- for (i = 0; i < nthreads; i++) {
- printf("# Thread %d:", i);
- for (j = 0; j < par[i]->stats->num_outliers; j++)
- printf(" %05lu", par[i]->stats->outliers[j]);
- if (par[i]->stats->num_outliers < par[i]->stats->hist_overflow)
- printf(" # %05lu others", par[i]->stats->hist_overflow - par[i]->stats->num_outliers);
- printf("\n");
+ if (overflow_cycle) {
+ printf("# Histogram Overflow at cycle number:\n");
+ if (histofall && nthreads > 1) {
+ long prev_time = -1;
+ int t_next[nthreads];
+ int thread;
+ long time;
+
+ memset(t_next, 0, sizeof(t_next));
+ printf("# Thread all:");
+ do {
+ thread = -1;
+ time = -1;
+ for (i = 0; i < nthreads; i++) {
+ /* do not report the same cycle multiple times */
+ if ((t_next[i] < par[i]->stats->num_outliers) &&
+ (par[i]->stats->outliers[t_next[i]] <= prev_time)) {
+ t_next[i]++;
+ }
+ if ((t_next[i] < par[i]->stats->num_outliers) &&
+ ((time == -1) ||
+ (par[i]->stats->outliers[t_next[i]] < time))) {
+ thread = i;
+ time = par[i]->stats->outliers[t_next[i]];
+ }
+ }
+ if (time != -1) {
+ prev_time = par[thread]->stats->outliers[t_next[thread]];
+ printf(" %5lu", prev_time);
+ t_next[i]++;
+ }
+ } while (time != -1);
+
+ printf("\n");
+ }
+ for (i = 0; i < nthreads; i++) {
+ printf("# Thread %d:", i);
+ for (j = 0; j < par[i]->stats->num_outliers; j++)
+ printf(" %5lu", par[i]->stats->outliers[j]);
+ if (par[i]->stats->num_outliers < par[i]->stats->hist_overflow)
+ printf(" # %5lu others",
+ par[i]->stats->hist_overflow - par[i]->stats->num_outliers);
+ printf("\n");
+ }
}
- printf("\n");
}
static void print_stat(struct thread_param *par, int index, int verbose)
@@ -1563,14 +1609,15 @@ int main(int argc, char **argv)
/* allocate the histogram if requested */
if (histogram) {
int bufsize = histogram * sizeof(long);
+ int of_size = overflow_cycle * sizeof(long);
stat->hist_array = threadalloc(bufsize, node);
- stat->outliers = threadalloc(bufsize, node);
+ stat->outliers = threadalloc(of_size, node);
if (stat->hist_array == NULL || stat->outliers == NULL)
fatal("failed to allocate histogram of size %d on node %d\n",
histogram, i);
memset(stat->hist_array, 0, bufsize);
- memset(stat->outliers, 0, bufsize);
+ memset(stat->outliers, 0, of_size);
}
if (verbose) {
@@ -1688,7 +1735,7 @@ int main(int argc, char **argv)
print_hist(parameters, num_threads);
for (i = 0; i < num_threads; i++) {
threadfree(statistics[i]->hist_array, histogram*sizeof(long), parameters[i]->node);
- threadfree(statistics[i]->outliers, histogram*sizeof(long), parameters[i]->node);
+ threadfree(statistics[i]->outliers, overflow_cycle*sizeof(long), parameters[i]->node);
}
}
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-10-25 21:36 ` Frank Rowand
@ 2012-10-25 22:05 ` Frank Rowand
2012-11-13 23:41 ` Frank Rowand
2012-11-13 23:41 ` Frank Rowand
1 sibling, 1 reply; 19+ messages in thread
From: Frank Rowand @ 2012-10-25 22:05 UTC (permalink / raw)
To: Rowand, Frank; +Cc: Bhavesh Davda, linux-rt-users@vger.kernel.org, John Kacur
On 10/25/12 14:36, Frank Rowand wrote:
> On 10/16/12 10:02, Bhavesh Davda wrote:
>> From: Bhavesh Davda <bhavesh@vmware.com>
>>
>> Add feature to cyclictest histogram mode to track cycle counts every time a
>> sample overflows the histogram limit. This should help identify if there is a
>> timing pattern to jitters in cyclictest runs.
>>
>> Example output (with -h 10):
>> ...
>> Histogram Overflows: 00001 00007 00000 00009 00004 00007 00000 00001
>> Histogram Overflow at cycle number:
>> Thread 0: 09964
>> Thread 1: 00000 00004 00006 00008 00010 09962 11594
>> Thread 2:
>> Thread 3: 01169 04698 06782 09033 10299 11561 21517 28734 29532
>> Thread 4: 11574 11580 11583 11586
>> Thread 5: 00020 09448 13954 14954 18954 20587 24973
>> Thread 6:
>> Thread 7: 18950
>> ...
>>
>> Signed-off-by: Bhavesh Davda <bhavesh@vmware.com>
>
> My comments up to "--" are not meant to be part of the header of the
> following patch.
>
> I am including the following patch as my review of the patch I am
> replying to. If the following patch is incorporated into the
One further thought... The histogram overflow cycle report shows
what cycle the overflow occurred in, not the actual time. Adding
the merged for all threads cycle times works because the histogram
turns off the "different intervals for different threads" option:
if (!histogram) /* same interval on CPUs */
interval += distance;
but if that ever changes then cycle is not a useful value to be
reporting.
So it seems like it would be useful to convert cycle to a time
in the report. This is something that would have to be done
anyway in post processing when trying to make use of the report.
-Frank
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-10-25 21:36 ` Frank Rowand
2012-10-25 22:05 ` Frank Rowand
@ 2012-11-13 23:41 ` Frank Rowand
2012-11-13 23:48 ` Bhavesh Davda
1 sibling, 1 reply; 19+ messages in thread
From: Frank Rowand @ 2012-11-13 23:41 UTC (permalink / raw)
To: Bhavesh Davda, John Kacur; +Cc: linux-rt-users@vger.kernel.org
On 10/25/12 14:36, Frank Rowand wrote:
> On 10/16/12 10:02, Bhavesh Davda wrote:
>> From: Bhavesh Davda <bhavesh@vmware.com>
>>
>> Add feature to cyclictest histogram mode to track cycle counts every time a
>> sample overflows the histogram limit. This should help identify if there is a
>> timing pattern to jitters in cyclictest runs.
>>
>> Example output (with -h 10):
>> ...
>> Histogram Overflows: 00001 00007 00000 00009 00004 00007 00000 00001
>> Histogram Overflow at cycle number:
>> Thread 0: 09964
>> Thread 1: 00000 00004 00006 00008 00010 09962 11594
>> Thread 2:
>> Thread 3: 01169 04698 06782 09033 10299 11561 21517 28734 29532
>> Thread 4: 11574 11580 11583 11586
>> Thread 5: 00020 09448 13954 14954 18954 20587 24973
>> Thread 6:
>> Thread 7: 18950
>> ...
>>
>> Signed-off-by: Bhavesh Davda <bhavesh@vmware.com>
>
> My comments up to "--" are not meant to be part of the header of the
> following patch.
>
> I am including the following patch as my review of the patch I am
> replying to. If the following patch is incorporated into the
> reviewed patch, then you can add my:
>
> Reviewed-by: Frank Rowand <frank.rowand@am.sony.com>
> Tested-by: Frank Rowand <frank.rowand@am.sony.com>
>
> to the combined patch.
< snip >
Bhavesh,
I never saw I reply to my suggestions. Unfortunately John took my
reviewed-by for your white space patch and applied it to your
"cyclictest: histogram overflow instance tracking" patch, without
my suggestions, and released it in v0.85 rt-tests.
So...., any thoughts about the suggestions in the email that
this one is responding to? And also any thoughts about my
second email in the series (I'll forward that one again to
you).
-Frank
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-10-25 22:05 ` Frank Rowand
@ 2012-11-13 23:41 ` Frank Rowand
0 siblings, 0 replies; 19+ messages in thread
From: Frank Rowand @ 2012-11-13 23:41 UTC (permalink / raw)
To: Bhavesh Davda, John Kacur; +Cc: linux-rt-users@vger.kernel.org
On 10/25/12 15:05, Frank Rowand wrote:
> On 10/25/12 14:36, Frank Rowand wrote:
>> On 10/16/12 10:02, Bhavesh Davda wrote:
>>> From: Bhavesh Davda <bhavesh@vmware.com>
>>>
>>> Add feature to cyclictest histogram mode to track cycle counts every time a
>>> sample overflows the histogram limit. This should help identify if there is a
>>> timing pattern to jitters in cyclictest runs.
>>>
>>> Example output (with -h 10):
>>> ...
>>> Histogram Overflows: 00001 00007 00000 00009 00004 00007 00000 00001
>>> Histogram Overflow at cycle number:
>>> Thread 0: 09964
>>> Thread 1: 00000 00004 00006 00008 00010 09962 11594
>>> Thread 2:
>>> Thread 3: 01169 04698 06782 09033 10299 11561 21517 28734 29532
>>> Thread 4: 11574 11580 11583 11586
>>> Thread 5: 00020 09448 13954 14954 18954 20587 24973
>>> Thread 6:
>>> Thread 7: 18950
>>> ...
>>>
>>> Signed-off-by: Bhavesh Davda <bhavesh@vmware.com>
>>
>> My comments up to "--" are not meant to be part of the header of the
>> following patch.
>>
>> I am including the following patch as my review of the patch I am
>> replying to. If the following patch is incorporated into the
>
> One further thought... The histogram overflow cycle report shows
> what cycle the overflow occurred in, not the actual time. Adding
> the merged for all threads cycle times works because the histogram
> turns off the "different intervals for different threads" option:
>
> if (!histogram) /* same interval on CPUs */
> interval += distance;
>
> but if that ever changes then cycle is not a useful value to be
> reporting.
>
> So it seems like it would be useful to convert cycle to a time
> in the report. This is something that would have to be done
> anyway in post processing when trying to make use of the report.
Bhavesh,
This is the second email that I mentioned in my last email to you
a minute ago. Again, looking for your thoughts on this.
-Frank
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-11-13 23:41 ` Frank Rowand
@ 2012-11-13 23:48 ` Bhavesh Davda
2012-11-13 23:53 ` John Kacur
0 siblings, 1 reply; 19+ messages in thread
From: Bhavesh Davda @ 2012-11-13 23:48 UTC (permalink / raw)
To: frank rowand; +Cc: linux-rt-users, John Kacur
Hello Frank,
I apologize I didn't have a chance to review your proposed modifications to my patch.
I had looked at your proposal to introduce a '-g' option (for lack of a better letter) instead of piggy-backing on the '-h' option, and I completely agree with that approach.
It would be very handy to specify, for example, '-h 50 -g 1000' to capture and print the 1000 outliers beyond 50 us to get a good sense of any patterns hidden in the outliers.
If John has already accepted my proposed patches, do you want to propose your changes on top of that as a separate patch now?
Thanks for your review, and your excellent suggestion to make this feature even more useful.
--
Bhavesh Davda
----- Original Message -----
> From: "Frank Rowand" <frank.rowand@am.sony.com>
> To: "Bhavesh Davda" <bhavesh@vmware.com>, "John Kacur" <jkacur@redhat.com>
> Cc: linux-rt-users@vger.kernel.org
> Sent: Tuesday, November 13, 2012 3:41:20 PM
> Subject: Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
>
> On 10/25/12 14:36, Frank Rowand wrote:
> > On 10/16/12 10:02, Bhavesh Davda wrote:
> >> From: Bhavesh Davda <bhavesh@vmware.com>
> >>
> >> Add feature to cyclictest histogram mode to track cycle counts
> >> every time a
> >> sample overflows the histogram limit. This should help identify if
> >> there is a
> >> timing pattern to jitters in cyclictest runs.
> >>
> >> Example output (with -h 10):
> >> ...
> >> Histogram Overflows: 00001 00007 00000 00009 00004 00007 00000
> >> 00001
> >> Histogram Overflow at cycle number:
> >> Thread 0: 09964
> >> Thread 1: 00000 00004 00006 00008 00010 09962 11594
> >> Thread 2:
> >> Thread 3: 01169 04698 06782 09033 10299 11561 21517 28734 29532
> >> Thread 4: 11574 11580 11583 11586
> >> Thread 5: 00020 09448 13954 14954 18954 20587 24973
> >> Thread 6:
> >> Thread 7: 18950
> >> ...
> >>
> >> Signed-off-by: Bhavesh Davda <bhavesh@vmware.com>
> >
> > My comments up to "--" are not meant to be part of the header of
> > the
> > following patch.
> >
> > I am including the following patch as my review of the patch I am
> > replying to. If the following patch is incorporated into the
> > reviewed patch, then you can add my:
> >
> > Reviewed-by: Frank Rowand <frank.rowand@am.sony.com>
> > Tested-by: Frank Rowand <frank.rowand@am.sony.com>
> >
> > to the combined patch.
>
> < snip >
>
> Bhavesh,
>
> I never saw I reply to my suggestions. Unfortunately John took my
> reviewed-by for your white space patch and applied it to your
> "cyclictest: histogram overflow instance tracking" patch, without
> my suggestions, and released it in v0.85 rt-tests.
>
> So...., any thoughts about the suggestions in the email that
> this one is responding to? And also any thoughts about my
> second email in the series (I'll forward that one again to
> you).
>
> -Frank
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-11-13 23:48 ` Bhavesh Davda
@ 2012-11-13 23:53 ` John Kacur
2012-11-14 0:02 ` Frank Rowand
2012-11-14 0:03 ` Bhavesh Davda
0 siblings, 2 replies; 19+ messages in thread
From: John Kacur @ 2012-11-13 23:53 UTC (permalink / raw)
To: Bhavesh Davda; +Cc: linux-rt-users, frank rowand
----- Original Message -----
> Hello Frank,
>
> I apologize I didn't have a chance to review your proposed
> modifications to my patch.
>
> I had looked at your proposal to introduce a '-g' option (for lack of
> a better letter) instead of piggy-backing on the '-h' option, and I
> completely agree with that approach.
>
> It would be very handy to specify, for example, '-h 50 -g 1000' to
> capture and print the 1000 outliers beyond 50 us to get a good sense
> of any patterns hidden in the outliers.
>
> If John has already accepted my proposed patches, do you want to
> propose your changes on top of that as a separate patch now?
Please pull the git tree, have a look at what I put in, and then think about
Frank's suggestions. Make sure you are "proactive" - (oh God, thinking about
a Simpson's episode now), and see what you can do with it, instead of just
waiting for Frank to tell you his opinions.
Thanks!
>
> Thanks for your review, and your excellent suggestion to make this
> feature even more useful.
>
> --
> Bhavesh Davda
>
> ----- Original Message -----
> > From: "Frank Rowand" <frank.rowand@am.sony.com>
> > To: "Bhavesh Davda" <bhavesh@vmware.com>, "John Kacur"
> > <jkacur@redhat.com>
> > Cc: linux-rt-users@vger.kernel.org
> > Sent: Tuesday, November 13, 2012 3:41:20 PM
> > Subject: Re: [PATCH RT-TESTS] cyclictest: histogram overflow
> > instance tracking
> >
> > On 10/25/12 14:36, Frank Rowand wrote:
> > > On 10/16/12 10:02, Bhavesh Davda wrote:
> > >> From: Bhavesh Davda <bhavesh@vmware.com>
> > >>
> > >> Add feature to cyclictest histogram mode to track cycle counts
> > >> every time a
> > >> sample overflows the histogram limit. This should help identify
> > >> if
> > >> there is a
> > >> timing pattern to jitters in cyclictest runs.
> > >>
> > >> Example output (with -h 10):
> > >> ...
> > >> Histogram Overflows: 00001 00007 00000 00009 00004 00007 00000
> > >> 00001
> > >> Histogram Overflow at cycle number:
> > >> Thread 0: 09964
> > >> Thread 1: 00000 00004 00006 00008 00010 09962 11594
> > >> Thread 2:
> > >> Thread 3: 01169 04698 06782 09033 10299 11561 21517 28734 29532
> > >> Thread 4: 11574 11580 11583 11586
> > >> Thread 5: 00020 09448 13954 14954 18954 20587 24973
> > >> Thread 6:
> > >> Thread 7: 18950
> > >> ...
> > >>
> > >> Signed-off-by: Bhavesh Davda <bhavesh@vmware.com>
> > >
> > > My comments up to "--" are not meant to be part of the header of
> > > the
> > > following patch.
> > >
> > > I am including the following patch as my review of the patch I am
> > > replying to. If the following patch is incorporated into the
> > > reviewed patch, then you can add my:
> > >
> > > Reviewed-by: Frank Rowand <frank.rowand@am.sony.com>
> > > Tested-by: Frank Rowand <frank.rowand@am.sony.com>
> > >
> > > to the combined patch.
> >
> > < snip >
> >
> > Bhavesh,
> >
> > I never saw I reply to my suggestions. Unfortunately John took my
> > reviewed-by for your white space patch and applied it to your
> > "cyclictest: histogram overflow instance tracking" patch, without
> > my suggestions, and released it in v0.85 rt-tests.
> >
> > So...., any thoughts about the suggestions in the email that
> > this one is responding to? And also any thoughts about my
> > second email in the series (I'll forward that one again to
> > you).
> >
> > -Frank
> >
> >
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-11-13 23:53 ` John Kacur
@ 2012-11-14 0:02 ` Frank Rowand
2012-11-14 0:03 ` Bhavesh Davda
1 sibling, 0 replies; 19+ messages in thread
From: Frank Rowand @ 2012-11-14 0:02 UTC (permalink / raw)
To: John Kacur, Bhavesh Davda; +Cc: linux-rt-users@vger.kernel.org
On 11/13/12 15:53, John Kacur wrote:
>
>
> ----- Original Message -----
>> Hello Frank,
>>
>> I apologize I didn't have a chance to review your proposed
>> modifications to my patch.
>>
>> I had looked at your proposal to introduce a '-g' option (for lack of
>> a better letter) instead of piggy-backing on the '-h' option, and I
>> completely agree with that approach.
>>
>> It would be very handy to specify, for example, '-h 50 -g 1000' to
>> capture and print the 1000 outliers beyond 50 us to get a good sense
>> of any patterns hidden in the outliers.
>>
>> If John has already accepted my proposed patches, do you want to
>> propose your changes on top of that as a separate patch now?
Bhavesh,
I would agree with John's suggestions (the following paragraph). I did not
want to hijack your feature, but instead just help make it better, so I was
phrasing my stuff as suggestions, not as "you must do this" type of
additions. So I would prefer you still own the feature. If you propose
a further patch based on my suggestions, I will review and test it.
>
> Please pull the git tree, have a look at what I put in, and then think about
> Frank's suggestions. Make sure you are "proactive" - (oh God, thinking about
> a Simpson's episode now), and see what you can do with it, instead of just
> waiting for Frank to tell you his opinions.
>
> Thanks!
>
>>
>> Thanks for your review, and your excellent suggestion to make this
>> feature even more useful.
>>
>> --
>> Bhavesh Davda
< snip >
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-11-13 23:53 ` John Kacur
2012-11-14 0:02 ` Frank Rowand
@ 2012-11-14 0:03 ` Bhavesh Davda
2012-11-14 0:04 ` Bhavesh Davda
1 sibling, 1 reply; 19+ messages in thread
From: Bhavesh Davda @ 2012-11-14 0:03 UTC (permalink / raw)
To: John Kacur; +Cc: linux-rt-users, frank rowand
To dispel any misunderstandings here, Frank had proposed a modified patch based on my original patch which introduces the new '-g' command line switch and the associated code. So all I'm saying is that since my original patch has been committed unmodified, Frank might want to submit a new patch off tip, if he feels strongly about his suggestions (which I agree are great suggestions).
I can promise to be "proactive" in reviewing the proposed patch :)
--
Bhavesh Davda
----- Original Message -----
> From: "John Kacur" <jkacur@redhat.com>
> To: "Bhavesh Davda" <bhavesh@vmware.com>
> Cc: linux-rt-users@vger.kernel.org, "frank rowand" <frank.rowand@am.sony.com>
> Sent: Tuesday, November 13, 2012 3:53:29 PM
> Subject: Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
>
>
>
> ----- Original Message -----
> > Hello Frank,
> >
> > I apologize I didn't have a chance to review your proposed
> > modifications to my patch.
> >
> > I had looked at your proposal to introduce a '-g' option (for lack
> > of
> > a better letter) instead of piggy-backing on the '-h' option, and I
> > completely agree with that approach.
> >
> > It would be very handy to specify, for example, '-h 50 -g 1000' to
> > capture and print the 1000 outliers beyond 50 us to get a good
> > sense
> > of any patterns hidden in the outliers.
> >
> > If John has already accepted my proposed patches, do you want to
> > propose your changes on top of that as a separate patch now?
>
> Please pull the git tree, have a look at what I put in, and then
> think about
> Frank's suggestions. Make sure you are "proactive" - (oh God,
> thinking about
> a Simpson's episode now), and see what you can do with it, instead of
> just
> waiting for Frank to tell you his opinions.
>
> Thanks!
>
> >
> > Thanks for your review, and your excellent suggestion to make this
> > feature even more useful.
> >
> > --
> > Bhavesh Davda
> >
> > ----- Original Message -----
> > > From: "Frank Rowand" <frank.rowand@am.sony.com>
> > > To: "Bhavesh Davda" <bhavesh@vmware.com>, "John Kacur"
> > > <jkacur@redhat.com>
> > > Cc: linux-rt-users@vger.kernel.org
> > > Sent: Tuesday, November 13, 2012 3:41:20 PM
> > > Subject: Re: [PATCH RT-TESTS] cyclictest: histogram overflow
> > > instance tracking
> > >
> > > On 10/25/12 14:36, Frank Rowand wrote:
> > > > On 10/16/12 10:02, Bhavesh Davda wrote:
> > > >> From: Bhavesh Davda <bhavesh@vmware.com>
> > > >>
> > > >> Add feature to cyclictest histogram mode to track cycle counts
> > > >> every time a
> > > >> sample overflows the histogram limit. This should help
> > > >> identify
> > > >> if
> > > >> there is a
> > > >> timing pattern to jitters in cyclictest runs.
> > > >>
> > > >> Example output (with -h 10):
> > > >> ...
> > > >> Histogram Overflows: 00001 00007 00000 00009 00004 00007
> > > >> 00000
> > > >> 00001
> > > >> Histogram Overflow at cycle number:
> > > >> Thread 0: 09964
> > > >> Thread 1: 00000 00004 00006 00008 00010 09962 11594
> > > >> Thread 2:
> > > >> Thread 3: 01169 04698 06782 09033 10299 11561 21517 28734
> > > >> 29532
> > > >> Thread 4: 11574 11580 11583 11586
> > > >> Thread 5: 00020 09448 13954 14954 18954 20587 24973
> > > >> Thread 6:
> > > >> Thread 7: 18950
> > > >> ...
> > > >>
> > > >> Signed-off-by: Bhavesh Davda <bhavesh@vmware.com>
> > > >
> > > > My comments up to "--" are not meant to be part of the header
> > > > of
> > > > the
> > > > following patch.
> > > >
> > > > I am including the following patch as my review of the patch I
> > > > am
> > > > replying to. If the following patch is incorporated into the
> > > > reviewed patch, then you can add my:
> > > >
> > > > Reviewed-by: Frank Rowand <frank.rowand@am.sony.com>
> > > > Tested-by: Frank Rowand <frank.rowand@am.sony.com>
> > > >
> > > > to the combined patch.
> > >
> > > < snip >
> > >
> > > Bhavesh,
> > >
> > > I never saw I reply to my suggestions. Unfortunately John took
> > > my
> > > reviewed-by for your white space patch and applied it to your
> > > "cyclictest: histogram overflow instance tracking" patch, without
> > > my suggestions, and released it in v0.85 rt-tests.
> > >
> > > So...., any thoughts about the suggestions in the email that
> > > this one is responding to? And also any thoughts about my
> > > second email in the series (I'll forward that one again to
> > > you).
> > >
> > > -Frank
> > >
> > >
> >
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-11-14 0:03 ` Bhavesh Davda
@ 2012-11-14 0:04 ` Bhavesh Davda
2012-11-14 0:09 ` John Kacur
0 siblings, 1 reply; 19+ messages in thread
From: Bhavesh Davda @ 2012-11-14 0:04 UTC (permalink / raw)
To: John Kacur; +Cc: linux-rt-users, frank rowand
Okay, Frank's response raced with mine. Since Frank is not super motivated to submit a patch with his proposed changes to my patch, I'll do so.
--
Bhavesh Davda
----- Original Message -----
> From: "Bhavesh Davda" <bhavesh@vmware.com>
> To: "John Kacur" <jkacur@redhat.com>
> Cc: linux-rt-users@vger.kernel.org, "frank rowand" <frank.rowand@am.sony.com>
> Sent: Tuesday, November 13, 2012 4:03:23 PM
> Subject: Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
>
> To dispel any misunderstandings here, Frank had proposed a modified
> patch based on my original patch which introduces the new '-g'
> command line switch and the associated code. So all I'm saying is
> that since my original patch has been committed unmodified, Frank
> might want to submit a new patch off tip, if he feels strongly about
> his suggestions (which I agree are great suggestions).
>
> I can promise to be "proactive" in reviewing the proposed patch :)
>
> --
> Bhavesh Davda
>
> ----- Original Message -----
> > From: "John Kacur" <jkacur@redhat.com>
> > To: "Bhavesh Davda" <bhavesh@vmware.com>
> > Cc: linux-rt-users@vger.kernel.org, "frank rowand"
> > <frank.rowand@am.sony.com>
> > Sent: Tuesday, November 13, 2012 3:53:29 PM
> > Subject: Re: [PATCH RT-TESTS] cyclictest: histogram overflow
> > instance tracking
> >
> >
> >
> > ----- Original Message -----
> > > Hello Frank,
> > >
> > > I apologize I didn't have a chance to review your proposed
> > > modifications to my patch.
> > >
> > > I had looked at your proposal to introduce a '-g' option (for
> > > lack
> > > of
> > > a better letter) instead of piggy-backing on the '-h' option, and
> > > I
> > > completely agree with that approach.
> > >
> > > It would be very handy to specify, for example, '-h 50 -g 1000'
> > > to
> > > capture and print the 1000 outliers beyond 50 us to get a good
> > > sense
> > > of any patterns hidden in the outliers.
> > >
> > > If John has already accepted my proposed patches, do you want to
> > > propose your changes on top of that as a separate patch now?
> >
> > Please pull the git tree, have a look at what I put in, and then
> > think about
> > Frank's suggestions. Make sure you are "proactive" - (oh God,
> > thinking about
> > a Simpson's episode now), and see what you can do with it, instead
> > of
> > just
> > waiting for Frank to tell you his opinions.
> >
> > Thanks!
> >
> > >
> > > Thanks for your review, and your excellent suggestion to make
> > > this
> > > feature even more useful.
> > >
> > > --
> > > Bhavesh Davda
> > >
> > > ----- Original Message -----
> > > > From: "Frank Rowand" <frank.rowand@am.sony.com>
> > > > To: "Bhavesh Davda" <bhavesh@vmware.com>, "John Kacur"
> > > > <jkacur@redhat.com>
> > > > Cc: linux-rt-users@vger.kernel.org
> > > > Sent: Tuesday, November 13, 2012 3:41:20 PM
> > > > Subject: Re: [PATCH RT-TESTS] cyclictest: histogram overflow
> > > > instance tracking
> > > >
> > > > On 10/25/12 14:36, Frank Rowand wrote:
> > > > > On 10/16/12 10:02, Bhavesh Davda wrote:
> > > > >> From: Bhavesh Davda <bhavesh@vmware.com>
> > > > >>
> > > > >> Add feature to cyclictest histogram mode to track cycle
> > > > >> counts
> > > > >> every time a
> > > > >> sample overflows the histogram limit. This should help
> > > > >> identify
> > > > >> if
> > > > >> there is a
> > > > >> timing pattern to jitters in cyclictest runs.
> > > > >>
> > > > >> Example output (with -h 10):
> > > > >> ...
> > > > >> Histogram Overflows: 00001 00007 00000 00009 00004 00007
> > > > >> 00000
> > > > >> 00001
> > > > >> Histogram Overflow at cycle number:
> > > > >> Thread 0: 09964
> > > > >> Thread 1: 00000 00004 00006 00008 00010 09962 11594
> > > > >> Thread 2:
> > > > >> Thread 3: 01169 04698 06782 09033 10299 11561 21517 28734
> > > > >> 29532
> > > > >> Thread 4: 11574 11580 11583 11586
> > > > >> Thread 5: 00020 09448 13954 14954 18954 20587 24973
> > > > >> Thread 6:
> > > > >> Thread 7: 18950
> > > > >> ...
> > > > >>
> > > > >> Signed-off-by: Bhavesh Davda <bhavesh@vmware.com>
> > > > >
> > > > > My comments up to "--" are not meant to be part of the header
> > > > > of
> > > > > the
> > > > > following patch.
> > > > >
> > > > > I am including the following patch as my review of the patch
> > > > > I
> > > > > am
> > > > > replying to. If the following patch is incorporated into the
> > > > > reviewed patch, then you can add my:
> > > > >
> > > > > Reviewed-by: Frank Rowand <frank.rowand@am.sony.com>
> > > > > Tested-by: Frank Rowand <frank.rowand@am.sony.com>
> > > > >
> > > > > to the combined patch.
> > > >
> > > > < snip >
> > > >
> > > > Bhavesh,
> > > >
> > > > I never saw I reply to my suggestions. Unfortunately John took
> > > > my
> > > > reviewed-by for your white space patch and applied it to your
> > > > "cyclictest: histogram overflow instance tracking" patch,
> > > > without
> > > > my suggestions, and released it in v0.85 rt-tests.
> > > >
> > > > So...., any thoughts about the suggestions in the email that
> > > > this one is responding to? And also any thoughts about my
> > > > second email in the series (I'll forward that one again to
> > > > you).
> > > >
> > > > -Frank
> > > >
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-11-14 0:04 ` Bhavesh Davda
@ 2012-11-14 0:09 ` John Kacur
2012-11-14 0:17 ` Bhavesh Davda
0 siblings, 1 reply; 19+ messages in thread
From: John Kacur @ 2012-11-14 0:09 UTC (permalink / raw)
To: Bhavesh Davda; +Cc: linux-rt-users, frank rowand
On Wed, Nov 14, 2012 at 1:04 AM, Bhavesh Davda <bhavesh@vmware.com> wrote:
> Okay, Frank's response raced with mine. Since Frank is not super motivated to submit a patch with his proposed changes to my patch, I'll do so.
>
Ummm, ok, this is sounding a little condescending. When people review
your patch, this is a gift they gave you which they did not need to.
It means they found your patch interesting enough to suggest how to
make it better. Adding an option to turn it on, is trivial, I believe,
the important point that Frank said is
"One further thought... The histogram overflow cycle report shows
what cycle the overflow occurred in, not the actual time. Adding
the merged for all threads cycle times works because the histogram
turns off the "different intervals for different threads" option:
if (!histogram) /* same interval on CPUs */
interval += distance;
but if that ever changes then cycle is not a useful value to be
reporting.
So it seems like it would be useful to convert cycle to a time
in the report. This is something that would have to be done
anyway in post processing when trying to make use of the report."
The motivation should belong to you. You should be proud that we
thought your ideas were important and good enough to incorporate into
the software and to review and comment on it. Careful there!
Thanks
John
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-11-14 0:09 ` John Kacur
@ 2012-11-14 0:17 ` Bhavesh Davda
2012-11-14 0:19 ` John Kacur
0 siblings, 1 reply; 19+ messages in thread
From: Bhavesh Davda @ 2012-11-14 0:17 UTC (permalink / raw)
To: John Kacur; +Cc: linux-rt-users, frank rowand
No disrespect intended, sorry if it came across as such.
And as I'm no cyclictest expert, I didn't quite catch the subtleties of Frank's important observation. I'll need to study the cyclictest code more carefully to really grok that, which I will.
Stay tuned for a patch...
--
Bhavesh Davda
----- Original Message -----
> From: "John Kacur" <jkacur@redhat.com>
> To: "Bhavesh Davda" <bhavesh@vmware.com>
> Cc: linux-rt-users@vger.kernel.org, "frank rowand" <frank.rowand@am.sony.com>
> Sent: Tuesday, November 13, 2012 4:09:22 PM
> Subject: Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
>
> On Wed, Nov 14, 2012 at 1:04 AM, Bhavesh Davda <bhavesh@vmware.com>
> wrote:
> > Okay, Frank's response raced with mine. Since Frank is not super
> > motivated to submit a patch with his proposed changes to my patch,
> > I'll do so.
> >
>
> Ummm, ok, this is sounding a little condescending. When people review
> your patch, this is a gift they gave you which they did not need to.
> It means they found your patch interesting enough to suggest how to
> make it better. Adding an option to turn it on, is trivial, I
> believe,
> the important point that Frank said is
>
> "One further thought... The histogram overflow cycle report shows
> what cycle the overflow occurred in, not the actual time. Adding
> the merged for all threads cycle times works because the histogram
> turns off the "different intervals for different threads" option:
>
> if (!histogram) /* same interval on CPUs */
> interval += distance;
>
> but if that ever changes then cycle is not a useful value to be
> reporting.
>
> So it seems like it would be useful to convert cycle to a time
> in the report. This is something that would have to be done
> anyway in post processing when trying to make use of the report."
>
> The motivation should belong to you. You should be proud that we
> thought your ideas were important and good enough to incorporate into
> the software and to review and comment on it. Careful there!
>
> Thanks
>
> John
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking
2012-11-14 0:17 ` Bhavesh Davda
@ 2012-11-14 0:19 ` John Kacur
0 siblings, 0 replies; 19+ messages in thread
From: John Kacur @ 2012-11-14 0:19 UTC (permalink / raw)
To: Bhavesh Davda; +Cc: linux-rt-users, frank rowand
On Wed, Nov 14, 2012 at 1:17 AM, Bhavesh Davda <bhavesh@vmware.com> wrote:
> No disrespect intended, sorry if it came across as such.
>
> And as I'm no cyclictest expert, I didn't quite catch the subtleties of Frank's important observation. I'll need to study the cyclictest code more carefully to really grok that, which I will.
>
> Stay tuned for a patch...
>
Glad to hear it - thank you. Btw, the norm for linux emails is to add
your comments either inline, or at the end. (not prepended)
Looking forward to your work, thanks.
John
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2012-11-14 0:19 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-16 17:02 [PATCH RT-TESTS] cyclictest: histogram overflow instance tracking Bhavesh Davda
2012-10-23 1:16 ` Frank Rowand
2012-10-23 1:41 ` Frank Rowand
2012-10-23 18:59 ` Bhavesh Davda
2012-10-23 19:25 ` John Kacur
2012-10-23 19:55 ` Frank Rowand
2012-10-23 19:59 ` Sven-Thorsten Dietrich
2012-10-25 21:36 ` Frank Rowand
2012-10-25 22:05 ` Frank Rowand
2012-11-13 23:41 ` Frank Rowand
2012-11-13 23:41 ` Frank Rowand
2012-11-13 23:48 ` Bhavesh Davda
2012-11-13 23:53 ` John Kacur
2012-11-14 0:02 ` Frank Rowand
2012-11-14 0:03 ` Bhavesh Davda
2012-11-14 0:04 ` Bhavesh Davda
2012-11-14 0:09 ` John Kacur
2012-11-14 0:17 ` Bhavesh Davda
2012-11-14 0:19 ` John Kacur
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).