LKML Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/3] tracing/hwlat: Report total time spent in all NMIs during the sample
@ 2019-10-10 18:50 Srivatsa S. Bhat
  2019-10-10 18:51 ` [PATCH 2/3] tracing/hwlat: Don't ignore outer-loop duration when calculating max_latency Srivatsa S. Bhat
  2019-10-10 18:51 ` [PATCH 3/3] tracing/hwlat: Fix a few trivial nits Srivatsa S. Bhat
  0 siblings, 2 replies; 7+ messages in thread
From: Srivatsa S. Bhat @ 2019-10-10 18:50 UTC (permalink / raw)
  To: linux-kernel, rostedt, mingo
  Cc: amakhalov, akaher, anishs, bordoloih, srivatsab, srivatsa

From: Srivatsa S. Bhat (VMware) <srivatsa@csail.mit.edu>

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: stable@vger.kernel.org
Signed-off-by: Srivatsa S. Bhat (VMware) <srivatsa@csail.mit.edu>
---

 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)


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/3] tracing/hwlat: Don't ignore outer-loop duration when calculating max_latency
  2019-10-10 18:50 [PATCH 1/3] tracing/hwlat: Report total time spent in all NMIs during the sample Srivatsa S. Bhat
@ 2019-10-10 18:51 ` Srivatsa S. Bhat
  2019-10-10 18:51 ` [PATCH 3/3] tracing/hwlat: Fix a few trivial nits Srivatsa S. Bhat
  1 sibling, 0 replies; 7+ messages in thread
From: Srivatsa S. Bhat @ 2019-10-10 18:51 UTC (permalink / raw)
  To: linux-kernel, rostedt, mingo
  Cc: amakhalov, akaher, anishs, bordoloih, srivatsab, srivatsa

From: Srivatsa S. Bhat (VMware) <srivatsa@csail.mit.edu>

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: stable@vger.kernel.org
Signed-off-by: Srivatsa S. Bhat (VMware) <srivatsa@csail.mit.edu>
---

 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:


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/3] tracing/hwlat: Fix a few trivial nits
  2019-10-10 18:50 [PATCH 1/3] tracing/hwlat: Report total time spent in all NMIs during the sample Srivatsa S. Bhat
  2019-10-10 18:51 ` [PATCH 2/3] tracing/hwlat: Don't ignore outer-loop duration when calculating max_latency Srivatsa S. Bhat
@ 2019-10-10 18:51 ` Srivatsa S. Bhat
  2019-10-10 21:34   ` Joe Perches
  2019-10-14 19:24   ` Steven Rostedt
  1 sibling, 2 replies; 7+ messages in thread
From: Srivatsa S. Bhat @ 2019-10-10 18:51 UTC (permalink / raw)
  To: linux-kernel, rostedt, mingo
  Cc: amakhalov, akaher, anishs, bordoloih, srivatsab, srivatsa

From: Srivatsa S. Bhat (VMware) <srivatsa@csail.mit.edu>

Update the source file name in the comments, and fix a grammatical
error.

Signed-off-by: Srivatsa S. Bhat (VMware) <srivatsa@csail.mit.edu>
---

 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))


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] tracing/hwlat: Fix a few trivial nits
  2019-10-10 18:51 ` [PATCH 3/3] tracing/hwlat: Fix a few trivial nits Srivatsa S. Bhat
@ 2019-10-10 21:34   ` Joe Perches
  2019-10-10 21:44     ` Steven Rostedt
  2019-10-14 19:24   ` Steven Rostedt
  1 sibling, 1 reply; 7+ messages in thread
From: Joe Perches @ 2019-10-10 21:34 UTC (permalink / raw)
  To: Srivatsa S. Bhat, linux-kernel, Steven Rostedt
  Cc: amakhalov, akaher, anishs, bordoloih, srivatsab

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.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] tracing/hwlat: Fix a few trivial nits
  2019-10-10 21:34   ` Joe Perches
@ 2019-10-10 21:44     ` Steven Rostedt
  0 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2019-10-10 21:44 UTC (permalink / raw)
  To: Joe Perches
  Cc: Srivatsa S. Bhat, linux-kernel, amakhalov, akaher, anishs,
	bordoloih, srivatsab

On Thu, 10 Oct 2019 14:34:55 -0700
Joe Perches <joe@perches.com> 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.
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] tracing/hwlat: Fix a few trivial nits
  2019-10-10 18:51 ` [PATCH 3/3] tracing/hwlat: Fix a few trivial nits Srivatsa S. Bhat
  2019-10-10 21:34   ` Joe Perches
@ 2019-10-14 19:24   ` Steven Rostedt
  2019-10-14 19:26     ` Srivatsa S. Bhat
  1 sibling, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2019-10-14 19:24 UTC (permalink / raw)
  To: Srivatsa S. Bhat
  Cc: linux-kernel, mingo, amakhalov, akaher, anishs, bordoloih, srivatsab

On Thu, 10 Oct 2019 11:51:17 -0700
"Srivatsa S. Bhat" <srivatsa@csail.mit.edu> wrote:

> From: Srivatsa S. Bhat (VMware) <srivatsa@csail.mit.edu>
> 
> 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] tracing/hwlat: Fix a few trivial nits
  2019-10-14 19:24   ` Steven Rostedt
@ 2019-10-14 19:26     ` Srivatsa S. Bhat
  0 siblings, 0 replies; 7+ messages in thread
From: Srivatsa S. Bhat @ 2019-10-14 19:26 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, mingo, amakhalov, akaher, anishs, bordoloih, srivatsab

On 10/14/19 12:24 PM, Steven Rostedt wrote:
> On Thu, 10 Oct 2019 11:51:17 -0700
> "Srivatsa S. Bhat" <srivatsa@csail.mit.edu> wrote:
> 
>> From: Srivatsa S. Bhat (VMware) <srivatsa@csail.mit.edu>
>>
>> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-10 18:50 [PATCH 1/3] tracing/hwlat: Report total time spent in all NMIs during the sample Srivatsa S. Bhat
2019-10-10 18:51 ` [PATCH 2/3] tracing/hwlat: Don't ignore outer-loop duration when calculating max_latency Srivatsa S. Bhat
2019-10-10 18:51 ` [PATCH 3/3] tracing/hwlat: Fix a few trivial nits Srivatsa S. Bhat
2019-10-10 21:34   ` Joe Perches
2019-10-10 21:44     ` Steven Rostedt
2019-10-14 19:24   ` Steven Rostedt
2019-10-14 19:26     ` Srivatsa S. Bhat

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox