From: Srivatsa S. Bhat (VMware) <[email protected]>
nmi_total_ts is supposed to record the total time spent in *all* NMIs
that occur on the given CPU during the (active portion of the)
sampling window. However, the code seems to be overwriting this
variable for each NMI, thereby only recording the time spent in the
most recent NMI. Fix it by accumulating the duration instead.
Fixes: 7b2c86250122 ("tracing: Add NMI tracing in hwlat detector")
Cc: [email protected]
Signed-off-by: Srivatsa S. Bhat (VMware) <[email protected]>
---
kernel/trace/trace_hwlat.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
index fa95139..a0251a7 100644
--- a/kernel/trace/trace_hwlat.c
+++ b/kernel/trace/trace_hwlat.c
@@ -150,7 +150,7 @@ void trace_hwlat_callback(bool enter)
if (enter)
nmi_ts_start = time_get();
else
- nmi_total_ts = time_get() - nmi_ts_start;
+ nmi_total_ts += time_get() - nmi_ts_start;
}
if (enter)
From: Srivatsa S. Bhat (VMware) <[email protected]>
max_latency is intended to record the maximum ever observed hardware
latency, which may occur in either part of the loop (inner/outer). So
we need to also consider the outer-loop sample when updating
max_latency.
Fixes: e7c15cd8a113 ("tracing: Added hardware latency tracer")
Cc: [email protected]
Signed-off-by: Srivatsa S. Bhat (VMware) <[email protected]>
---
kernel/trace/trace_hwlat.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
index a0251a7..862f4b0 100644
--- a/kernel/trace/trace_hwlat.c
+++ b/kernel/trace/trace_hwlat.c
@@ -256,6 +256,8 @@ static int get_sample(void)
/* Keep a running maximum ever recorded hardware latency */
if (sample > tr->max_latency)
tr->max_latency = sample;
+ if (outer_sample > tr->max_latency)
+ tr->max_latency = outer_sample;
}
out:
From: Srivatsa S. Bhat (VMware) <[email protected]>
Update the source file name in the comments, and fix a grammatical
error.
Signed-off-by: Srivatsa S. Bhat (VMware) <[email protected]>
---
kernel/trace/trace_hwlat.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
index 862f4b0..941cb82 100644
--- a/kernel/trace/trace_hwlat.c
+++ b/kernel/trace/trace_hwlat.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * trace_hwlatdetect.c - A simple Hardware Latency detector.
+ * trace_hwlat.c - A simple Hardware Latency detector.
*
* Use this tracer to detect large system latencies induced by the behavior of
* certain underlying system hardware or firmware, independent of Linux itself.
@@ -276,7 +276,7 @@ static void move_to_next_cpu(void)
return;
/*
* If for some reason the user modifies the CPU affinity
- * of this thread, than stop migrating for the duration
+ * of this thread, then stop migrating for the duration
* of the current test.
*/
if (!cpumask_equal(current_mask, current->cpus_ptr))
On Thu, 2019-10-10 at 11:51 -0700, Srivatsa S. Bhat wrote:
> Update the source file name in the comments, and fix a grammatical
> error.
[]
> diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
[]
> @@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> - * trace_hwlatdetect.c - A simple Hardware Latency detector.
> + * trace_hwlat.c - A simple Hardware Latency detector.
trivia:
Generally it's not useful to have the filename in a comment
so I think maybe delete the "trace_hwlatdetect.c - ".
btw:
$ git ls-files -- '*.[ch]' | \
while read file ; do git grep $file -- $file; done | wc -l
About 5% (2500 of the 50000 or so) *.[ch] files in the kernel
source tree contain their filename in a comment, so it's
certainly not that unusual.
On Thu, 10 Oct 2019 14:34:55 -0700
Joe Perches <[email protected]> wrote:
> On Thu, 2019-10-10 at 11:51 -0700, Srivatsa S. Bhat wrote:
> > Update the source file name in the comments, and fix a grammatical
> > error.
> []
> > diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
> []
> > @@ -1,6 +1,6 @@
> > // SPDX-License-Identifier: GPL-2.0
> > /*
> > - * trace_hwlatdetect.c - A simple Hardware Latency detector.
> > + * trace_hwlat.c - A simple Hardware Latency detector.
>
> trivia:
>
> Generally it's not useful to have the filename in a comment
> so I think maybe delete the "trace_hwlatdetect.c - ".
Not a big deal to keep it. The original proposed name for the tracer was
hwlatdetect, but people said it was too long and hwlat was good enough.
Thus we changed the name to that, but didn't change the comment here.
We could remove it, but I think it's fine to keep it.
-- Steve
>
> btw:
>
> $ git ls-files -- '*.[ch]' | \
> while read file ; do git grep $file -- $file; done | wc -l
>
> About 5% (2500 of the 50000 or so) *.[ch] files in the kernel
> source tree contain their filename in a comment, so it's
> certainly not that unusual.
>
On Thu, 10 Oct 2019 11:51:17 -0700
"Srivatsa S. Bhat" <[email protected]> wrote:
> From: Srivatsa S. Bhat (VMware) <[email protected]>
>
> Update the source file name in the comments, and fix a grammatical
> error.
Patch 1 and 2 have already been applied to Linus's tree.
I've queued this one up for the next merge window.
Thanks!
-- Steve
On 10/14/19 12:24 PM, Steven Rostedt wrote:
> On Thu, 10 Oct 2019 11:51:17 -0700
> "Srivatsa S. Bhat" <[email protected]> wrote:
>
>> From: Srivatsa S. Bhat (VMware) <[email protected]>
>>
>> Update the source file name in the comments, and fix a grammatical
>> error.
>
> Patch 1 and 2 have already been applied to Linus's tree.
>
> I've queued this one up for the next merge window.
>
> Thanks!
>
Thanks a lot Steve!
Regards,
Srivatsa