All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH 0/3] tracing: Add Hardware Latency detector tracer
@ 2016-08-04 14:57 Steven Rostedt
  2016-08-04 14:57 ` [RFC][PATCH 1/3] tracing: Added hardware latency tracer Steven Rostedt
                   ` (5 more replies)
  0 siblings, 6 replies; 24+ messages in thread
From: Steven Rostedt @ 2016-08-04 14:57 UTC (permalink / raw)
  To: linux-kernel, linux-rt-users
  Cc: Ingo Molnar, Andrew Morton, Clark Williams, Thomas Gleixner,
	Jon Masters, Daniel Wagner, Carsten Emde,
	Sebastian Andrzej Siewior


This adds the PREEMPT_RT hwlat detector as a Linux tracer in mainline.
In the PREEMPT_RT patch set, it is a separate entity that is controlled
by the debugfs file system. I found that it is better suited as a
latency tracer in the tracing directory, as it follows pretty much the
same paradigm as the other latency tracers that already exist. All
I had to add was a hwlat_detector directory that contained a window
and width for the period and duration respectively of the test. But
the samples would just write to the tracing ring buffer and the max
latency would be stored in tracing_max_latency, and the threshold can
be set by the existing tracing_threshold. The last patch also adds a
new feature that would have the kthread migrate after each period to
another CPU specified by tracing_cpumask.


Here's the documention that is added to hwlat_detector.txt in the
Documentation directory:

Introduction:
-------------

The tracer hwlat_detector is a special purpose tracer that is used to
detect large system latencies induced by the behavior of certain underlying
hardware or firmware, independent of Linux itself. The code was developed
originally to detect SMIs (System Management Interrupts) on x86 systems,
however there is nothing x86 specific about this patchset. It was
originally written for use by the "RT" patch since the Real Time
kernel is highly latency sensitive.

SMIs are not serviced by the Linux kernel, which means that it does not
even know that they are occuring. SMIs are instead set up by BIOS code
and are serviced by BIOS code, usually for "critical" events such as
management of thermal sensors and fans. Sometimes though, SMIs are used for
other tasks and those tasks can spend an inordinate amount of time in the
handler (sometimes measured in milliseconds). Obviously this is a problem if
you are trying to keep event service latencies down in the microsecond range.

The hardware latency detector works by hogging one of the cpus for configurable
amounts of time (with interrupts disabled), polling the CPU Time Stamp Counter
for some period, then looking for gaps in the TSC data. Any gap indicates a
time when the polling was interrupted and since the interrupts are disabled,
the only thing that could do that would be an SMI or other hardware hiccup
(or an NMI, but those can be tracked).

Note that the hwlat detector should *NEVER* be used in a production environment.
It is intended to be run manually to determine if the hardware platform has a
problem with long system firmware service routines.

Usage:
------

Write the ASCII text "hwlat" into the current_tracer file of the tracing system
(mounted at /sys/kernel/tracing or /sys/kernel/tracing). It is possible to
redefine the threshold in microseconds (us) above which latency spikes will
be taken into account.

Example:

	# echo hwlat > /sys/kernel/tracing/current_tracer
	# echo 100 > /sys/kernel/tracing/tracing_thresh

The /sys/kernel/tracing/hwlat_detector interface contains the following files:

width			- time period to sample with CPUs held (usecs)
			  must be less than the total window size (enforced)
window			- total period of sampling, width being inside (usecs)

By default the width is set to 500,000 and window to 1,000,000, meaning that
for every 1,000,000 usecs (1s) the hwlat detector will spin for 500,000 usecs
(0.5s). If tracing_thresh contains zero when hwlat tracer is enabled, it will
change to a default of 10 usecs. If any latencies that exceed the threshold is
observed then the data will be written to the tracing ring buffer.

The minimum sleep time between periods is 1 millisecond. Even if width
is less than 1 millisecond apart from window, to allow the system to not
be totally starved.

If tracing_thresh was zero when hwlat detector was started, it will be set
back to zero if another tracer is loaded. Note, the last value in
tracing_thresh that hwlat detector had will be saved and this value will
be restored in tracing_thresh if it is still zero when hwlat detector is
started again.

The following tracing directory files are used by the hwlat_detector:

in /sys/kernel/tracing:

 tracing_threshold	- minimum latency value to be considered (usecs)
 tracing_max_latency	- maximum hardware latency actually observed (usecs)
 hwlat_detector/width	- specified amount of time to spin within window (usecs)
 hwlat_detector/window	- amount of time between (width) runs (usecs)

-- Steve


Jon Masters (1):
      tracing: Add documentation for hwlat_detector tracer

Steven Rostedt (Red Hat) (2):
      tracing: Added hardware latency tracer
      tracing: Have hwlat trace migrate across tracing_cpumask CPUs

----
 Documentation/trace/hwlat_detector.txt |  79 +++++
 kernel/trace/Kconfig                   |  35 ++
 kernel/trace/Makefile                  |   1 +
 kernel/trace/trace.c                   |   2 +-
 kernel/trace/trace.h                   |   3 +
 kernel/trace/trace_entries.h           |  23 ++
 kernel/trace/trace_hwlat.c             | 582 +++++++++++++++++++++++++++++++++
 kernel/trace/trace_output.c            |  52 +++
 8 files changed, 776 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/trace/hwlat_detector.txt
 create mode 100644 kernel/trace/trace_hwlat.c

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

* [RFC][PATCH 1/3] tracing: Added hardware latency tracer
  2016-08-04 14:57 [RFC][PATCH 0/3] tracing: Add Hardware Latency detector tracer Steven Rostedt
@ 2016-08-04 14:57 ` Steven Rostedt
  2016-08-05 14:25   ` Sebastian Andrzej Siewior
  2016-08-04 14:57 ` [RFC][PATCH 2/3] tracing: Add documentation for hwlat_detector tracer Steven Rostedt
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 24+ messages in thread
From: Steven Rostedt @ 2016-08-04 14:57 UTC (permalink / raw)
  To: linux-kernel, linux-rt-users
  Cc: Ingo Molnar, Andrew Morton, Clark Williams, Thomas Gleixner,
	Jon Masters, Daniel Wagner, Carsten Emde,
	Sebastian Andrzej Siewior

[-- Attachment #1: 0001-tracing-Added-hardware-latency-tracer.patch --]
[-- Type: text/plain, Size: 23662 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

The hardware latency tracer has been in the PREEMPT_RT patch for some time.
It is used to detect possible SMIs or any other hardware interruptions that
the kernel is unaware of. Note, NMIs may also be detected, but that may be
good to note as well.

The logic is pretty simple. It simply creates a thread that spins on a
single CPU for a specified amount of time (width) within a periodic window
(window). These numbers may be adjusted by their cooresponding names in

   /sys/kernel/tracing/hwlat_detector/

The defaults are window = 1000000 us (1 second)
                 width  =  500000 us (1/2 second)

The loop consists of:

	t1 = trace_clock_local();
	t2 = trace_clock_local();

Where trace_clock_local() is a variant of sched_clock().

The difference of t2 - t1 is recorded as the "inner" timestamp and also the
timestamp  t1 - prev_t2 is recorded as the "outer" timestamp. If either of
these differences are greater than the time denoted in
/sys/kernel/tracing/tracing_thresh then it records the event.

When this tracer is started, and tracing_thresh is zero, it changes to the
default threshold of 10 us.

The hwlat tracer in the PREEMPT_RT patch was originally written by
Jon Masters. I have modified it quite a bit and turned it into a
tracer.

Based-on-code-by: Jon Masters <jcm@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/Kconfig         |  35 +++
 kernel/trace/Makefile        |   1 +
 kernel/trace/trace.c         |   2 +-
 kernel/trace/trace.h         |   3 +
 kernel/trace/trace_entries.h |  23 ++
 kernel/trace/trace_hwlat.c   | 527 +++++++++++++++++++++++++++++++++++++++++++
 kernel/trace/trace_output.c  |  52 +++++
 7 files changed, 642 insertions(+), 1 deletion(-)
 create mode 100644 kernel/trace/trace_hwlat.c

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index f4b86e8ca1e7..72c07c2ffd79 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -221,6 +221,41 @@ config SCHED_TRACER
 	  This tracer tracks the latency of the highest priority task
 	  to be scheduled in, starting from the point it has woken up.
 
+config HWLAT_TRACER
+	bool "Tracer to detect hardware latencies (like SMIs)"
+	select GENERIC_TRACER
+	help
+	 This tracer, when enabled will create one or more kernel threads,
+	 depening on what the cpumask file is set to, which each thread
+	 spinning in a loop looking for interruptions caused by
+	 something other than the kernel. For example, if a
+	 System Management Interrupt (SMI) takes a noticeable amount of
+	 time, this tracer will detect it. This is useful for testing
+	 if a system is reliable for Real Time tasks.
+
+	 Some files are created in the tracing directory when this
+	 is enabled:
+
+	   hwlat_detector/width   - time in usecs for how long to spin for
+	   hwlat_detector/window  - time in usecs between the start of each
+				     iteration
+
+	 A kernel thread is created that will spin with interrupts disabled
+	 for "width" microseconds in every "widow" cycle. It will not spin
+	 for "window - width" microseconds, where the system can
+	 continue to operate.
+
+	 The output will appear in the trace and trace_pipe files.
+
+	 When the tracer is not running, it has no affect on the system,
+	 but when it is running, it can cause the system to be
+	 periodically non responsive. Do not run this tracer on a
+	 production system.
+
+	 To enable this tracer, echo in "hwlat" into the current_tracer
+	 file. Every time a latency is greater than tracing_thresh, it will
+	 be recorded into the ring buffer.
+
 config ENABLE_DEFAULT_TRACERS
 	bool "Trace process context switches and events"
 	depends on !GENERIC_TRACER
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 979e7bfbde7a..e57980845549 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -37,6 +37,7 @@ obj-$(CONFIG_FUNCTION_TRACER) += trace_functions.o
 obj-$(CONFIG_IRQSOFF_TRACER) += trace_irqsoff.o
 obj-$(CONFIG_PREEMPT_TRACER) += trace_irqsoff.o
 obj-$(CONFIG_SCHED_TRACER) += trace_sched_wakeup.o
+obj-$(CONFIG_HWLAT_TRACER) += trace_hwlat.o
 obj-$(CONFIG_NOP_TRACER) += trace_nop.o
 obj-$(CONFIG_STACK_TRACER) += trace_stack.o
 obj-$(CONFIG_MMIOTRACE) += trace_mmiotrace.o
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index dade4c9559cc..474cc814e16d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1047,7 +1047,7 @@ void disable_trace_on_warning(void)
  *
  * Shows real state of the ring buffer if it is enabled or not.
  */
-static int tracer_tracing_is_on(struct trace_array *tr)
+int tracer_tracing_is_on(struct trace_array *tr)
 {
 	if (tr->trace_buffer.buffer)
 		return ring_buffer_record_is_on(tr->trace_buffer.buffer);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index f783df416726..1d866b0c1567 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -38,6 +38,7 @@ enum trace_type {
 	TRACE_USER_STACK,
 	TRACE_BLK,
 	TRACE_BPUTS,
+	TRACE_HWLAT,
 
 	__TRACE_LAST_TYPE,
 };
@@ -326,6 +327,7 @@ extern void __ftrace_bad_type(void);
 		IF_ASSIGN(var, ent, struct print_entry, TRACE_PRINT);	\
 		IF_ASSIGN(var, ent, struct bprint_entry, TRACE_BPRINT);	\
 		IF_ASSIGN(var, ent, struct bputs_entry, TRACE_BPUTS);	\
+		IF_ASSIGN(var, ent, struct hwlat_entry, TRACE_HWLAT);	\
 		IF_ASSIGN(var, ent, struct trace_mmiotrace_rw,		\
 			  TRACE_MMIO_RW);				\
 		IF_ASSIGN(var, ent, struct trace_mmiotrace_map,		\
@@ -571,6 +573,7 @@ void tracing_reset_current(int cpu);
 void tracing_reset_all_online_cpus(void);
 int tracing_open_generic(struct inode *inode, struct file *filp);
 bool tracing_is_disabled(void);
+int tracer_tracing_is_on(struct trace_array *tr);
 struct dentry *trace_create_file(const char *name,
 				 umode_t mode,
 				 struct dentry *parent,
diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
index 5c30efcda5e6..70d47dd99359 100644
--- a/kernel/trace/trace_entries.h
+++ b/kernel/trace/trace_entries.h
@@ -322,3 +322,26 @@ FTRACE_ENTRY(branch, trace_branch,
 	FILTER_OTHER
 );
 
+
+FTRACE_ENTRY(hwlat, hwlat_entry,
+
+	TRACE_HWLAT,
+
+	F_STRUCT(
+		__field(	u64,			duration	)
+		__field(	u64,			outer_duration	)
+		__field_struct( struct timespec,	timestamp	)
+		__field_desc(	long,	timestamp,	tv_sec		)
+		__field_desc(	long,	timestamp,	tv_nsec		)
+		__field(	unsigned int,		seqnum		)
+	),
+
+	F_printk("cnt:%u\tts:%010lu.%010lu\tinner:%llu\touter:%llu\n",
+		 __entry->seqnum,
+		 __entry->tv_sec,
+		 __entry->tv_nsec,
+		 __entry->duration,
+		 __entry->outer_duration),
+
+	FILTER_OTHER
+);
diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
new file mode 100644
index 000000000000..08dfabe4e862
--- /dev/null
+++ b/kernel/trace/trace_hwlat.c
@@ -0,0 +1,527 @@
+/*
+ * trace_hwlatdetect.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.
+ * The code was developed originally to detect the presence of SMIs on Intel
+ * and AMD systems, although there is no dependency upon x86 herein.
+ *
+ * The classical example usage of this tracer is in detecting the presence of
+ * SMIs or System Management Interrupts on Intel and AMD systems. An SMI is a
+ * somewhat special form of hardware interrupt spawned from earlier CPU debug
+ * modes in which the (BIOS/EFI/etc.) firmware arranges for the South Bridge
+ * LPC (or other device) to generate a special interrupt under certain
+ * circumstances, for example, upon expiration of a special SMI timer device,
+ * due to certain external thermal readings, on certain I/O address accesses,
+ * and other situations. An SMI hits a special CPU pin, triggers a special
+ * SMI mode (complete with special memory map), and the OS is unaware.
+ *
+ * Although certain hardware-inducing latencies are necessary (for example,
+ * a modern system often requires an SMI handler for correct thermal control
+ * and remote management) they can wreak havoc upon any OS-level performance
+ * guarantees toward low-latency, especially when the OS is not even made
+ * aware of the presence of these interrupts. For this reason, we need a
+ * somewhat brute force mechanism to detect these interrupts. In this case,
+ * we do it by hogging all of the CPU(s) for configurable timer intervals,
+ * sampling the built-in CPU timer, looking for discontiguous readings.
+ *
+ * WARNING: This implementation necessarily introduces latencies. Therefore,
+ *          you should NEVER use this tracer while running in a production
+ *          environment requiring any kind of low-latency performance
+ *          guarantee(s).
+ *
+ * Copyright (C) 2008-2009 Jon Masters, Red Hat, Inc. <jcm@redhat.com>
+ * Copyright (C) 2013-2016 Steven Rostedt, Red Hat, Inc. <srostedt@redhat.com>
+ *
+ * Includes useful feedback from Clark Williams <clark@redhat.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+#include <linux/kthread.h>
+#include <linux/tracefs.h>
+#include <linux/uaccess.h>
+#include <linux/delay.h>
+#include "trace.h"
+
+static struct trace_array	*hwlat_trace;
+
+#define U64STR_SIZE		22			/* 20 digits max */
+
+#define BANNER			"hwlat_detector: "
+#define DEFAULT_SAMPLE_WINDOW	1000000			/* 1s */
+#define DEFAULT_SAMPLE_WIDTH	500000			/* 0.5s */
+#define DEFAULT_LAT_THRESHOLD	10			/* 10us */
+
+/* sampling thread*/
+static struct task_struct *hwlat_kthread;
+
+static struct dentry *hwlat_sample_width;	/* sample width us */
+static struct dentry *hwlat_sample_window;	/* sample window us */
+
+/* Save the previous tracing_thresh value */
+static unsigned long save_tracing_thresh;
+
+/* If the user changed threshold, remember it */
+static u64 last_tracing_thresh = DEFAULT_LAT_THRESHOLD * NSEC_PER_USEC;
+
+/* Individual latency samples are stored here when detected. */
+struct hwlat_sample {
+	u64		seqnum;		/* unique sequence */
+	u64		duration;	/* delta */
+	u64		outer_duration;	/* delta (outer loop) */
+	struct timespec	timestamp;	/* wall time */
+};
+
+/* keep the global state somewhere. */
+static struct hwlat_data {
+
+	struct mutex lock;		/* protect changes */
+
+	u64	count;			/* total since reset */
+
+	u64	sample_window;		/* total sampling window (on+off) */
+	u64	sample_width;		/* active sampling portion of window */
+
+} hwlat_data = {
+	.sample_window		= DEFAULT_SAMPLE_WINDOW,
+	.sample_width		= DEFAULT_SAMPLE_WIDTH,
+};
+
+static void trace_hwlat_sample(struct hwlat_sample *sample)
+{
+	struct trace_array *tr = hwlat_trace;
+	struct trace_event_call *call = &event_hwlat;
+	struct ring_buffer *buffer = tr->trace_buffer.buffer;
+	struct ring_buffer_event *event;
+	struct hwlat_entry *entry;
+	unsigned long flags;
+	int pc;
+
+	pc = preempt_count();
+	local_save_flags(flags);
+
+	event = trace_buffer_lock_reserve(buffer, TRACE_HWLAT, sizeof(*entry),
+					  flags, pc);
+	if (!event)
+		return;
+	entry	= ring_buffer_event_data(event);
+	entry->seqnum			= sample->seqnum;
+	entry->duration			= sample->duration;
+	entry->outer_duration		= sample->outer_duration;
+	entry->timestamp		= sample->timestamp;
+
+	if (!call_filter_check_discard(call, entry, buffer, event))
+		__buffer_unlock_commit(buffer, event);
+}
+
+/* Macros to encapsulate the time capturing infrastructure */
+#define time_type	u64
+#define time_get()	trace_clock_local()
+#define time_to_us(x)	div_u64(x, 1000)
+#define time_sub(a, b)	((a) - (b))
+#define init_time(a, b)	(a = b)
+#define time_u64(a)	a
+
+/**
+ * get_sample - sample the CPU TSC and look for likely hardware latencies
+ *
+ * Used to repeatedly capture the CPU TSC (or similar), looking for potential
+ * hardware-induced latency. Called with interrupts disabled and with
+ * hwlat_data.lock held.
+ */
+static int get_sample(void)
+{
+	struct trace_array *tr = hwlat_trace;
+	time_type start, t1, t2, last_t2;
+	s64 diff, total, last_total = 0;
+	u64 sample = 0;
+	u64 thresh = tracing_thresh;
+	u64 outer_sample = 0;
+	int ret = -1;
+
+	do_div(thresh, NSEC_PER_USEC); /* modifies interval value */
+
+	init_time(last_t2, 0);
+	start = time_get(); /* start timestamp */
+
+	do {
+
+		t1 = time_get();	/* we'll look for a discontinuity */
+		t2 = time_get();
+
+		if (time_u64(last_t2)) {
+			/* Check the delta from outer loop (t2 to next t1) */
+			diff = time_to_us(time_sub(t1, last_t2));
+			/* This shouldn't happen */
+			if (diff < 0) {
+				pr_err(BANNER "time running backwards\n");
+				goto out;
+			}
+			if (diff > outer_sample)
+				outer_sample = diff;
+		}
+		last_t2 = t2;
+
+		total = time_to_us(time_sub(t2, start)); /* sample width */
+
+		/* Check for possible overflows */
+		if (total < last_total) {
+			pr_err("Time total overflowed\n");
+			break;
+		}
+		last_total = total;
+
+		/* This checks the inner loop (t1 to t2) */
+		diff = time_to_us(time_sub(t2, t1));     /* current diff */
+
+		/* This shouldn't happen */
+		if (diff < 0) {
+			pr_err(BANNER "time running backwards\n");
+			goto out;
+		}
+
+		if (diff > sample)
+			sample = diff; /* only want highest value */
+
+	} while (total <= hwlat_data.sample_width);
+
+	ret = 0;
+
+	/* If we exceed the threshold value, we have found a hardware latency */
+	if (sample > thresh || outer_sample > thresh) {
+		struct hwlat_sample s;
+
+		ret = 1;
+
+		hwlat_data.count++;
+		s.seqnum = hwlat_data.count;
+		s.duration = sample;
+		s.outer_duration = outer_sample;
+		s.timestamp = CURRENT_TIME;
+		trace_hwlat_sample(&s);
+
+		/* Keep a running maximum ever recorded hardware latency */
+		if (sample > tr->max_latency)
+			tr->max_latency = sample;
+	}
+
+out:
+	return ret;
+}
+
+/*
+ * kthread_fn - The CPU time sampling/hardware latency detection kernel thread
+ *
+ * Used to periodically sample the CPU TSC via a call to get_sample. We
+ * disable interrupts, which does (intentionally) introduce latency since we
+ * need to ensure nothing else might be running (and thus preempting).
+ * Obviously this should never be used in production environments.
+ *
+ * Currently this runs on which ever CPU it was scheduled on, but most
+ * real-world hardware latency situations occur across several CPUs,
+ * but we might later generalize this if we find there are any actualy
+ * systems with alternate SMI delivery or other hardware latencies.
+ */
+static int kthread_fn(void *data)
+{
+	u64 interval;
+
+	while (!kthread_should_stop()) {
+
+		local_irq_disable();
+		get_sample();
+		local_irq_enable();
+
+		mutex_lock(&hwlat_data.lock);
+		interval = hwlat_data.sample_window - hwlat_data.sample_width;
+		mutex_unlock(&hwlat_data.lock);
+
+		do_div(interval, USEC_PER_MSEC); /* modifies interval value */
+
+		/* Always sleep for at least 1ms */
+		if (interval < 1)
+			interval = 1;
+
+		if (msleep_interruptible(interval))
+			break;
+	}
+
+	return 0;
+}
+
+/**
+ * start_kthread - Kick off the hardware latency sampling/detector kthread
+ *
+ * This starts the kernel thread that will sit and sample the CPU timestamp
+ * counter (TSC or similar) and look for potential hardware latencies.
+ */
+static int start_kthread(struct trace_array *tr)
+{
+	struct task_struct *kthread;
+
+	kthread = kthread_create(kthread_fn, NULL, "hwlatd");
+	if (IS_ERR(kthread)) {
+		pr_err(BANNER "could not start sampling thread\n");
+		return -ENOMEM;
+	}
+	hwlat_kthread = kthread;
+	wake_up_process(kthread);
+
+	return 0;
+}
+
+/**
+ * stop_kthread - Inform the hardware latency samping/detector kthread to stop
+ *
+ * This kicks the running hardware latency sampling/detector kernel thread and
+ * tells it to stop sampling now. Use this on unload and at system shutdown.
+ */
+static void stop_kthread(void)
+{
+	if (!hwlat_kthread)
+		return;
+	kthread_stop(hwlat_kthread);
+	hwlat_kthread = NULL;
+}
+
+/*
+ * hwlat_read - Wrapper read function for reading both window and width
+ * @filp: The active open file structure
+ * @ubuf: The userspace provided buffer to read value into
+ * @cnt: The maximum number of bytes to read
+ * @ppos: The current "file" position
+ *
+ * This function provides a generic read implementation for the global state
+ * "hwlat_data" structure filesystem entries.
+ */
+static ssize_t hwlat_read(struct file *filp, char __user *ubuf,
+			  size_t cnt, loff_t *ppos)
+{
+	char buf[U64STR_SIZE];
+	u64 *entry = filp->private_data;
+	u64 val;
+	int len;
+
+	if (!entry)
+		return -EFAULT;
+
+	if (cnt > sizeof(buf))
+		cnt = sizeof(buf);
+
+	val = *entry;
+
+	len = snprintf(buf, sizeof(buf), "%llu\n", val);
+
+	return simple_read_from_buffer(ubuf, cnt, ppos, buf, len);
+}
+
+/**
+ * hwlat_width_write - Write function for "width" entry
+ * @filp: The active open file structure
+ * @ubuf: The user buffer that contains the value to write
+ * @cnt: The maximum number of bytes to write to "file"
+ * @ppos: The current position in @file
+ *
+ * This function provides a write implementation for the "width" interface
+ * to the hardware latency detector. It can be used to configure
+ * for how many us of the total window us we will actively sample for any
+ * hardware-induced latency periods. Obviously, it is not possible to
+ * sample constantly and have the system respond to a sample reader, or,
+ * worse, without having the system appear to have gone out to lunch. It
+ * is enforced that width is less that the total window size.
+ */
+static ssize_t
+hwlat_width_write(struct file *filp, const char __user *ubuf,
+		  size_t cnt, loff_t *ppos)
+{
+	u64 val;
+	int err;
+
+	err = kstrtoull_from_user(ubuf, cnt, 10, &val);
+	if (err)
+		return err;
+
+	mutex_lock(&hwlat_data.lock);
+	if (val < hwlat_data.sample_window)
+		hwlat_data.sample_width = val;
+	else
+		err = -EINVAL;
+	mutex_unlock(&hwlat_data.lock);
+
+	if (err)
+		return err;
+
+	return cnt;
+}
+
+/**
+ * hwlat_window_write - Write function for "window" entry
+ * @filp: The active open file structure
+ * @ubuf: The user buffer that contains the value to write
+ * @cnt: The maximum number of bytes to write to "file"
+ * @ppos: The current position in @file
+ *
+ * This function provides a write implementation for the "window" interface
+ * to the hardware latency detetector. The window is the total time
+ * in us that will be considered one sample period. Conceptually, windows
+ * occur back-to-back and contain a sample width period during which
+ * actual sampling occurs. Can be used to write a new total window size. It
+ * is enfoced that any value written must be greater than the sample width
+ * size, or an error results.
+ */
+static ssize_t
+hwlat_window_write(struct file *filp, const char __user *ubuf,
+		   size_t cnt, loff_t *ppos)
+{
+	u64 val;
+	int err;
+
+	err = kstrtoull_from_user(ubuf, cnt, 10, &val);
+	if (err)
+		return err;
+
+	mutex_lock(&hwlat_data.lock);
+	if (hwlat_data.sample_width < val)
+		hwlat_data.sample_window = val;
+	else
+		err = -EINVAL;
+	mutex_unlock(&hwlat_data.lock);
+
+	if (err)
+		return err;
+
+	return cnt;
+}
+
+static const struct file_operations width_fops = {
+	.open		= tracing_open_generic,
+	.read		= hwlat_read,
+	.write		= hwlat_width_write,
+};
+
+static const struct file_operations window_fops = {
+	.open		= tracing_open_generic,
+	.read		= hwlat_read,
+	.write		= hwlat_window_write,
+};
+
+/**
+ * init_tracefs - A function to initialize the tracefs interface files
+ *
+ * This function creates entries in tracefs for "hwlat_detector".
+ * It creates the hwlat_detector directory in the tracing directory,
+ * and within that directory is the count, width and window files to
+ * change and view those values.
+ */
+static int init_tracefs(void)
+{
+	struct dentry *d_tracer;
+	struct dentry *top_dir;
+
+	d_tracer = tracing_init_dentry();
+	if (IS_ERR(d_tracer))
+		return -ENOMEM;
+
+	top_dir = tracefs_create_dir("hwlat_detector", d_tracer);
+	if (!top_dir)
+		return -ENOMEM;
+
+	hwlat_sample_window = tracefs_create_file("window", 0640,
+						  top_dir,
+						  &hwlat_data.sample_window,
+						  &window_fops);
+	if (!hwlat_sample_window)
+		goto err;
+
+	hwlat_sample_width = tracefs_create_file("width", 0644,
+						 top_dir,
+						 &hwlat_data.sample_width,
+						 &width_fops);
+	if (!hwlat_sample_width)
+		goto err;
+
+	return 0;
+
+ err:
+	tracefs_remove_recursive(top_dir);
+	return -ENOMEM;
+}
+
+static void hwlat_tracer_start(struct trace_array *tr)
+{
+	int err;
+
+	err = start_kthread(tr);
+	if (err)
+		pr_err(BANNER "Cannot start hwlat kthread\n");
+}
+
+static void hwlat_tracer_stop(struct trace_array *tr)
+{
+	stop_kthread();
+}
+
+static bool hwlat_busy;
+
+static int hwlat_tracer_init(struct trace_array *tr)
+{
+	/* Only allow one instance to enable this */
+	if (hwlat_busy)
+		return -EBUSY;
+
+	hwlat_trace = tr;
+
+	hwlat_data.count = 0;
+	tr->max_latency = 0;
+	save_tracing_thresh = tracing_thresh;
+
+	/* tracing_thresh is in nsecs, we speak in usecs */
+	if (!tracing_thresh)
+		tracing_thresh = last_tracing_thresh;
+
+	if (tracer_tracing_is_on(tr))
+		hwlat_tracer_start(tr);
+
+	hwlat_busy = true;
+
+	return 0;
+}
+
+static void hwlat_tracer_reset(struct trace_array *tr)
+{
+	stop_kthread();
+
+	/* the tracing threshold is static between runs */
+	last_tracing_thresh = tracing_thresh;
+
+	tracing_thresh = save_tracing_thresh;
+	hwlat_busy = false;
+}
+
+static struct tracer hwlat_tracer __read_mostly =
+{
+	.name		= "hwlat",
+	.init		= hwlat_tracer_init,
+	.reset		= hwlat_tracer_reset,
+	.start		= hwlat_tracer_start,
+	.stop		= hwlat_tracer_stop,
+	.allow_instances = true,
+};
+
+__init static int init_hwlat_tracer(void)
+{
+	int ret;
+
+	mutex_init(&hwlat_data.lock);
+
+	ret = register_tracer(&hwlat_tracer);
+	if (ret)
+		return ret;
+
+	init_tracefs();
+
+	return 0;
+}
+late_initcall(init_hwlat_tracer);
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 0bb9cf2d53e6..cc5db62a3929 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -1098,6 +1098,57 @@ static struct trace_event trace_user_stack_event = {
 	.funcs		= &trace_user_stack_funcs,
 };
 
+/* TRACE_HWLAT */
+static enum print_line_t
+trace_hwlat_print(struct trace_iterator *iter, int flags,
+		  struct trace_event *event)
+{
+	struct trace_entry *entry = iter->ent;
+	struct trace_seq *s = &iter->seq;
+	struct hwlat_entry *field;
+
+	trace_assign_type(field, entry);
+
+	trace_seq_printf(s, "[%u] inner:%llu outer:%lld ts:%ld.%09ld\n",
+			 field->seqnum,
+			 field->duration,
+			 field->outer_duration,
+			 field->timestamp.tv_sec,
+			 field->timestamp.tv_nsec);
+
+	return trace_handle_return(s);
+}
+
+
+static enum print_line_t
+trace_hwlat_raw(struct trace_iterator *iter, int flags,
+		struct trace_event *event)
+{
+	struct hwlat_entry *field;
+	struct trace_seq *s = &iter->seq;
+
+	trace_assign_type(field, iter->ent);
+
+	trace_seq_printf(s, "%llu %lld %ld %09ld %u\n",
+			 field->duration,
+			 field->outer_duration,
+			 field->timestamp.tv_sec,
+			 field->timestamp.tv_nsec,
+			 field->seqnum);
+
+	return trace_handle_return(s);
+}
+
+static struct trace_event_functions trace_hwlat_funcs = {
+	.trace		= trace_hwlat_print,
+	.raw		= trace_hwlat_raw,
+};
+
+static struct trace_event trace_hwlat_event = {
+	.type		= TRACE_HWLAT,
+	.funcs		= &trace_hwlat_funcs,
+};
+
 /* TRACE_BPUTS */
 static enum print_line_t
 trace_bputs_print(struct trace_iterator *iter, int flags,
@@ -1233,6 +1284,7 @@ static struct trace_event *events[] __initdata = {
 	&trace_bputs_event,
 	&trace_bprint_event,
 	&trace_print_event,
+	&trace_hwlat_event,
 	NULL
 };
 
-- 
2.8.1

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

* [RFC][PATCH 2/3] tracing: Add documentation for hwlat_detector tracer
  2016-08-04 14:57 [RFC][PATCH 0/3] tracing: Add Hardware Latency detector tracer Steven Rostedt
  2016-08-04 14:57 ` [RFC][PATCH 1/3] tracing: Added hardware latency tracer Steven Rostedt
@ 2016-08-04 14:57 ` Steven Rostedt
  2016-08-10 14:12   ` Jon Masters
  2016-08-04 14:57 ` [RFC][PATCH 3/3] tracing: Have hwlat trace migrate across tracing_cpumask CPUs Steven Rostedt
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 24+ messages in thread
From: Steven Rostedt @ 2016-08-04 14:57 UTC (permalink / raw)
  To: linux-kernel, linux-rt-users
  Cc: Ingo Molnar, Andrew Morton, Clark Williams, Thomas Gleixner,
	Jon Masters, Daniel Wagner, Carsten Emde,
	Sebastian Andrzej Siewior

[-- Attachment #1: 0002-tracing-Add-documentation-for-hwlat_detector-tracer.patch --]
[-- Type: text/plain, Size: 4251 bytes --]

From: Jon Masters <jcm@redhat.com>

Added the documentation on how to use th hwlat_detector.

Signed-off-by: Jon Masters <jcm@redhat.com>
[ Various updates and modified to show hwlat as a tracer ]
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 Documentation/trace/hwlat_detector.txt | 73 ++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/trace/hwlat_detector.txt

diff --git a/Documentation/trace/hwlat_detector.txt b/Documentation/trace/hwlat_detector.txt
new file mode 100644
index 000000000000..c02e8ef800cf
--- /dev/null
+++ b/Documentation/trace/hwlat_detector.txt
@@ -0,0 +1,73 @@
+Introduction:
+-------------
+
+The tracer hwlat_detector is a special purpose tracer that is used to
+detect large system latencies induced by the behavior of certain underlying
+hardware or firmware, independent of Linux itself. The code was developed
+originally to detect SMIs (System Management Interrupts) on x86 systems,
+however there is nothing x86 specific about this patchset. It was
+originally written for use by the "RT" patch since the Real Time
+kernel is highly latency sensitive.
+
+SMIs are not serviced by the Linux kernel, which means that it does not
+even know that they are occuring. SMIs are instead set up by BIOS code
+and are serviced by BIOS code, usually for "critical" events such as
+management of thermal sensors and fans. Sometimes though, SMIs are used for
+other tasks and those tasks can spend an inordinate amount of time in the
+handler (sometimes measured in milliseconds). Obviously this is a problem if
+you are trying to keep event service latencies down in the microsecond range.
+
+The hardware latency detector works by hogging one of the cpus for configurable
+amounts of time (with interrupts disabled), polling the CPU Time Stamp Counter
+for some period, then looking for gaps in the TSC data. Any gap indicates a
+time when the polling was interrupted and since the interrupts are disabled,
+the only thing that could do that would be an SMI or other hardware hiccup
+(or an NMI, but those can be tracked).
+
+Note that the hwlat detector should *NEVER* be used in a production environment.
+It is intended to be run manually to determine if the hardware platform has a
+problem with long system firmware service routines.
+
+Usage:
+------
+
+Write the ASCII text "hwlat" into the current_tracer file of the tracing system
+(mounted at /sys/kernel/tracing or /sys/kernel/tracing). It is possible to
+redefine the threshold in microseconds (us) above which latency spikes will
+be taken into account.
+
+Example:
+
+	# echo hwlat > /sys/kernel/tracing/current_tracer
+	# echo 100 > /sys/kernel/tracing/tracing_thresh
+
+The /sys/kernel/tracing/hwlat_detector interface contains the following files:
+
+width			- time period to sample with CPUs held (usecs)
+			  must be less than the total window size (enforced)
+window			- total period of sampling, width being inside (usecs)
+
+By default the width is set to 500,000 and window to 1,000,000, meaning that
+for every 1,000,000 usecs (1s) the hwlat detector will spin for 500,000 usecs
+(0.5s). If tracing_thresh contains zero when hwlat tracer is enabled, it will
+change to a default of 10 usecs. If any latencies that exceed the threshold is
+observed then the data will be written to the tracing ring buffer.
+
+The minimum sleep time between periods is 1 millisecond. Even if width
+is less than 1 millisecond apart from window, to allow the system to not
+be totally starved.
+
+If tracing_thresh was zero when hwlat detector was started, it will be set
+back to zero if another tracer is loaded. Note, the last value in
+tracing_thresh that hwlat detector had will be saved and this value will
+be restored in tracing_thresh if it is still zero when hwlat detector is
+started again.
+
+The following tracing directory files are used by the hwlat_detector:
+
+in /sys/kernel/tracing:
+
+ tracing_threshold	- minimum latency value to be considered (usecs)
+ tracing_max_latency	- maximum hardware latency actually observed (usecs)
+ hwlat_detector/width	- specified amount of time to spin within window (usecs)
+ hwlat_detector/window	- amount of time between (width) runs (usecs)
-- 
2.8.1

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

* [RFC][PATCH 3/3] tracing: Have hwlat trace migrate across tracing_cpumask CPUs
  2016-08-04 14:57 [RFC][PATCH 0/3] tracing: Add Hardware Latency detector tracer Steven Rostedt
  2016-08-04 14:57 ` [RFC][PATCH 1/3] tracing: Added hardware latency tracer Steven Rostedt
  2016-08-04 14:57 ` [RFC][PATCH 2/3] tracing: Add documentation for hwlat_detector tracer Steven Rostedt
@ 2016-08-04 14:57 ` Steven Rostedt
  2016-08-04 15:30 ` [RFC][PATCH 0/3] tracing: Add Hardware Latency detector tracer Steven Rostedt
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 24+ messages in thread
From: Steven Rostedt @ 2016-08-04 14:57 UTC (permalink / raw)
  To: linux-kernel, linux-rt-users
  Cc: Ingo Molnar, Andrew Morton, Clark Williams, Thomas Gleixner,
	Jon Masters, Daniel Wagner, Carsten Emde,
	Sebastian Andrzej Siewior

[-- Attachment #1: 0003-tracing-Have-hwlat-trace-migrate-across-tracing_cpum.patch --]
[-- Type: text/plain, Size: 3613 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

Instead of having the hwlat detector thread stay on one CPU, have it migrate
across all the CPUs specified by tracing_cpumask. If the user modifies the
thread's CPU affinity, the migration will stop until the next instance that
the tracer is instantiated. The migration happens at the end of each window
(period).

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 Documentation/trace/hwlat_detector.txt |  6 ++++
 kernel/trace/trace_hwlat.c             | 55 ++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/Documentation/trace/hwlat_detector.txt b/Documentation/trace/hwlat_detector.txt
index c02e8ef800cf..3207717a0d1a 100644
--- a/Documentation/trace/hwlat_detector.txt
+++ b/Documentation/trace/hwlat_detector.txt
@@ -69,5 +69,11 @@ in /sys/kernel/tracing:
 
  tracing_threshold	- minimum latency value to be considered (usecs)
  tracing_max_latency	- maximum hardware latency actually observed (usecs)
+ tracing_cpumask	- the CPUs to move the hwlat thread across
  hwlat_detector/width	- specified amount of time to spin within window (usecs)
  hwlat_detector/window	- amount of time between (width) runs (usecs)
+
+The hwlat detector's kernel thread will migrate across each CPU specified in
+tracing_cpumask between each window. To limit the migration, either modify
+tracing_cpumask, or modify the hwlat kernel thread (named [hwlatd]) CPU
+affinity directly, and the migration will stop.
diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
index 08dfabe4e862..f0b5b2944ee7 100644
--- a/kernel/trace/trace_hwlat.c
+++ b/kernel/trace/trace_hwlat.c
@@ -42,6 +42,7 @@
 #include <linux/kthread.h>
 #include <linux/tracefs.h>
 #include <linux/uaccess.h>
+#include <linux/cpumask.h>
 #include <linux/delay.h>
 #include "trace.h"
 
@@ -211,6 +212,57 @@ out:
 	return ret;
 }
 
+static struct cpumask save_cpumask;
+static bool disable_migrate;
+
+static void move_to_next_cpu(void)
+{
+	static struct cpumask *current_mask;
+	int next_cpu;
+
+	if (disable_migrate)
+		return;
+
+	/* Just pick the first CPU on first iteration */
+	if (!current_mask) {
+		current_mask = &save_cpumask;
+		get_online_cpus();
+		cpumask_and(current_mask, cpu_online_mask, tracing_buffer_mask);
+		put_online_cpus();
+		next_cpu = cpumask_first(current_mask);
+		goto set_affinity;
+	}
+		
+	/*
+	 * If for some reason the user modifies the CPU affinity
+	 * of this thread, than stop migrating for the duration
+	 * of the current test.
+	 */
+	if (!cpumask_equal(current_mask, &current->cpus_allowed))
+		goto disable;
+
+	get_online_cpus();
+	cpumask_and(current_mask, cpu_online_mask, tracing_buffer_mask);
+	next_cpu = cpumask_next(smp_processor_id(), current_mask);
+	put_online_cpus();
+
+	if (next_cpu >= nr_cpu_ids)
+		next_cpu = cpumask_first(current_mask);
+
+ set_affinity:
+	if (next_cpu >= nr_cpu_ids) /* Shouldn't happen! */
+		goto disable;
+
+	cpumask_clear(current_mask);
+	cpumask_set_cpu(next_cpu, current_mask);
+
+	sched_setaffinity(0, current_mask);
+	return;
+
+ disable:
+	disable_migrate = true;
+}
+
 /*
  * kthread_fn - The CPU time sampling/hardware latency detection kernel thread
  *
@@ -230,6 +282,8 @@ static int kthread_fn(void *data)
 
 	while (!kthread_should_stop()) {
 
+		move_to_next_cpu();
+
 		local_irq_disable();
 		get_sample();
 		local_irq_enable();
@@ -473,6 +527,7 @@ static int hwlat_tracer_init(struct trace_array *tr)
 
 	hwlat_trace = tr;
 
+	disable_migrate = false;
 	hwlat_data.count = 0;
 	tr->max_latency = 0;
 	save_tracing_thresh = tracing_thresh;
-- 
2.8.1

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

* Re: [RFC][PATCH 0/3] tracing: Add Hardware Latency detector tracer
  2016-08-04 14:57 [RFC][PATCH 0/3] tracing: Add Hardware Latency detector tracer Steven Rostedt
                   ` (2 preceding siblings ...)
  2016-08-04 14:57 ` [RFC][PATCH 3/3] tracing: Have hwlat trace migrate across tracing_cpumask CPUs Steven Rostedt
@ 2016-08-04 15:30 ` Steven Rostedt
  2016-08-09 18:15   ` Clark Williams
  2016-08-04 16:56 ` [RFC][PATCH 4/3] tracing: Add NMI tracing in hwlat detector Steven Rostedt
  2016-08-09 18:05 ` [RFC][PATCH 5/3] tracing: Add smi counting to HWLAT Steven Rostedt
  5 siblings, 1 reply; 24+ messages in thread
From: Steven Rostedt @ 2016-08-04 15:30 UTC (permalink / raw)
  To: linux-kernel, linux-rt-users
  Cc: Ingo Molnar, Andrew Morton, Clark Williams, Thomas Gleixner,
	Jon Masters, Daniel Wagner, Carsten Emde,
	Sebastian Andrzej Siewior

Note, I'm currently working on adding code to detect NMIs to this as
well. And perhaps even tracing SMI counters. Just to show what caused
the latency, as latency isn't measured by the counters (that I know of).

-- Steve

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

* [RFC][PATCH 4/3] tracing: Add NMI tracing in hwlat detector
  2016-08-04 14:57 [RFC][PATCH 0/3] tracing: Add Hardware Latency detector tracer Steven Rostedt
                   ` (3 preceding siblings ...)
  2016-08-04 15:30 ` [RFC][PATCH 0/3] tracing: Add Hardware Latency detector tracer Steven Rostedt
@ 2016-08-04 16:56 ` Steven Rostedt
  2016-08-04 17:16   ` Steven Rostedt
  2016-08-09 18:05 ` [RFC][PATCH 5/3] tracing: Add smi counting to HWLAT Steven Rostedt
  5 siblings, 1 reply; 24+ messages in thread
From: Steven Rostedt @ 2016-08-04 16:56 UTC (permalink / raw)
  To: linux-kernel, linux-rt-users
  Cc: Ingo Molnar, Andrew Morton, Clark Williams, Thomas Gleixner,
	Jon Masters, Daniel Wagner, Carsten Emde,
	Sebastian Andrzej Siewior


As NMIs can also cause latency when interrupts are disabled, the hwlat
detectory has no way to know if the latency it detects is from an NMI or an
SMI or some other hardware glitch.

As ftrace_nmi_enter/exit() funtions are no longer used (except for sh, which
isn't supported anymore), I converted those to "arch_ftrace_nmi_enter/exit"
and use ftrace_nmi_enter/exit() to check if hwlat detector is tracing or
not, and if so, it calls into the hwlat utility.

Since the hwlat detector only has a single kthread that is spinning with
interrupts disabled, it marks what CPU it is on, and if the NMI callback
happens on that CPU, it records the time spent in that NMI. This is added to
the output that is generated by the hwlat detector as:

 [1] inner:0 outer:8 ts:1470329211.586481492 nmi-total:0 nmi-count:0
 [2] inner:0 outer:4 ts:1470329231.619726657 nmi-total:4 nmi-count:1
 [3] inner:0 outer:5 ts:1470329234.550761052 nmi-total:4 nmi-count:1
 [4] inner:8 outer:0 ts:1470329263.655911668 nmi-total:0 nmi-count:0

All time is still tracked in microseconds.

Is the nmi-count even required? I could also modify it to not show the
NMI information if there wasn't any NMIs.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/sh/kernel/ftrace.c      |  4 ++--
 include/linux/ftrace_irq.h   | 31 +++++++++++++++++++++++++++----
 kernel/trace/trace_entries.h |  8 ++++++--
 kernel/trace/trace_hwlat.c   | 44 ++++++++++++++++++++++++++++++++++++++++++++
 kernel/trace/trace_output.c  |  6 ++++--
 5 files changed, 83 insertions(+), 10 deletions(-)

diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c
index 38993e09ef03..deb9eba3a43d 100644
--- a/arch/sh/kernel/ftrace.c
+++ b/arch/sh/kernel/ftrace.c
@@ -139,7 +139,7 @@ static void ftrace_mod_code(void)
 		clear_mod_flag();
 }
 
-void ftrace_nmi_enter(void)
+void arch_ftrace_nmi_enter(void)
 {
 	if (atomic_inc_return(&nmi_running) & MOD_CODE_WRITE_FLAG) {
 		smp_rmb();
@@ -150,7 +150,7 @@ void ftrace_nmi_enter(void)
 	smp_mb();
 }
 
-void ftrace_nmi_exit(void)
+void arch_ftrace_nmi_exit(void)
 {
 	/* Finish all executions before clearing nmi_running */
 	smp_mb();
diff --git a/include/linux/ftrace_irq.h b/include/linux/ftrace_irq.h
index dca7bf8cffe2..4ec2c9b205f2 100644
--- a/include/linux/ftrace_irq.h
+++ b/include/linux/ftrace_irq.h
@@ -3,11 +3,34 @@
 
 
 #ifdef CONFIG_FTRACE_NMI_ENTER
-extern void ftrace_nmi_enter(void);
-extern void ftrace_nmi_exit(void);
+extern void arch_ftrace_nmi_enter(void);
+extern void arch_ftrace_nmi_exit(void);
 #else
-static inline void ftrace_nmi_enter(void) { }
-static inline void ftrace_nmi_exit(void) { }
+static inline void arch_ftrace_nmi_enter(void) { }
+static inline void arch_ftrace_nmi_exit(void) { }
 #endif
 
+#ifdef CONFIG_HWLAT_TRACER
+extern bool trace_hwlat_callback_enabled;
+extern void trace_hwlat_callback(bool enter);
+#endif
+
+static inline void ftrace_nmi_enter(void)
+{
+#ifdef CONFIG_HWLAT_TRACER
+	if (trace_hwlat_callback_enabled)
+		trace_hwlat_callback(true);
+#endif
+	arch_ftrace_nmi_enter();
+}
+
+static inline void ftrace_nmi_exit(void)
+{
+	arch_ftrace_nmi_exit();
+#ifdef CONFIG_HWLAT_TRACER
+	if (trace_hwlat_callback_enabled)
+		trace_hwlat_callback(false);
+#endif
+}
+
 #endif /* _LINUX_FTRACE_IRQ_H */
diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
index 70d47dd99359..d1cc37e78f99 100644
--- a/kernel/trace/trace_entries.h
+++ b/kernel/trace/trace_entries.h
@@ -330,18 +330,22 @@ FTRACE_ENTRY(hwlat, hwlat_entry,
 	F_STRUCT(
 		__field(	u64,			duration	)
 		__field(	u64,			outer_duration	)
+		__field(	u64,			nmi_total_ts	)
 		__field_struct( struct timespec,	timestamp	)
 		__field_desc(	long,	timestamp,	tv_sec		)
 		__field_desc(	long,	timestamp,	tv_nsec		)
+		__field(	unsigned int,		nmi_count	)
 		__field(	unsigned int,		seqnum		)
 	),
 
-	F_printk("cnt:%u\tts:%010lu.%010lu\tinner:%llu\touter:%llu\n",
+	F_printk("cnt:%u\tts:%010lu.%010lu\tinner:%llu\touter:%llunmi-ts:%llu\tnmi-count:%u\n",
 		 __entry->seqnum,
 		 __entry->tv_sec,
 		 __entry->tv_nsec,
 		 __entry->duration,
-		 __entry->outer_duration),
+		 __entry->outer_duration,
+		 __entry->nmi_total_ts,
+		 __entry->nmi_count),
 
 	FILTER_OTHER
 );
diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
index f0b5b2944ee7..0c7ca6082721 100644
--- a/kernel/trace/trace_hwlat.c
+++ b/kernel/trace/trace_hwlat.c
@@ -64,6 +64,15 @@ static struct dentry *hwlat_sample_window;	/* sample window us */
 /* Save the previous tracing_thresh value */
 static unsigned long save_tracing_thresh;
 
+/* NMI timestamp counters */
+static u64 nmi_ts_start;
+static u64 nmi_total_ts;
+static int nmi_count;
+static int nmi_cpu;
+
+/* Tells NMIs to call back to the hwlat tracer to record timestamps */
+bool trace_hwlat_callback_enabled;
+
 /* If the user changed threshold, remember it */
 static u64 last_tracing_thresh = DEFAULT_LAT_THRESHOLD * NSEC_PER_USEC;
 
@@ -72,7 +81,9 @@ struct hwlat_sample {
 	u64		seqnum;		/* unique sequence */
 	u64		duration;	/* delta */
 	u64		outer_duration;	/* delta (outer loop) */
+	u64		nmi_total_ts;	/* Total time spent in NMIs */
 	struct timespec	timestamp;	/* wall time */
+	int		nmi_count;	/* # NMIs during this sample */
 };
 
 /* keep the global state somewhere. */
@@ -112,6 +123,8 @@ static void trace_hwlat_sample(struct hwlat_sample *sample)
 	entry->duration			= sample->duration;
 	entry->outer_duration		= sample->outer_duration;
 	entry->timestamp		= sample->timestamp;
+	entry->nmi_total_ts		= sample->nmi_total_ts;
+	entry->nmi_count		= sample->nmi_count;
 
 	if (!call_filter_check_discard(call, entry, buffer, event))
 		__buffer_unlock_commit(buffer, event);
@@ -125,6 +138,19 @@ static void trace_hwlat_sample(struct hwlat_sample *sample)
 #define init_time(a, b)	(a = b)
 #define time_u64(a)	a
 
+void trace_hwlat_callback(bool enter)
+{
+	if (smp_processor_id() != nmi_cpu)
+		return;
+
+	if (enter)
+		nmi_ts_start = time_get();
+	else {
+		nmi_total_ts = time_get() - nmi_ts_start;
+		nmi_count++;
+	}
+}
+
 /**
  * get_sample - sample the CPU TSC and look for likely hardware latencies
  *
@@ -144,6 +170,14 @@ static int get_sample(void)
 
 	do_div(thresh, NSEC_PER_USEC); /* modifies interval value */
 
+	nmi_cpu = smp_processor_id();
+	nmi_total_ts = 0;
+	nmi_count = 0;
+	/* Make sure NMIs see this first */
+	barrier();
+
+	trace_hwlat_callback_enabled = true;
+
 	init_time(last_t2, 0);
 	start = time_get(); /* start timestamp */
 
@@ -188,6 +222,10 @@ static int get_sample(void)
 
 	} while (total <= hwlat_data.sample_width);
 
+	barrier(); /* finish the above in the view for NMIs */
+	trace_hwlat_callback_enabled = false;
+	barrier(); /* Make sure nmi_total_ts is no longer updated */
+
 	ret = 0;
 
 	/* If we exceed the threshold value, we have found a hardware latency */
@@ -196,11 +234,17 @@ static int get_sample(void)
 
 		ret = 1;
 
+		/* We read in microseconds */
+		if (nmi_total_ts)
+			do_div(nmi_total_ts, NSEC_PER_USEC);
+
 		hwlat_data.count++;
 		s.seqnum = hwlat_data.count;
 		s.duration = sample;
 		s.outer_duration = outer_sample;
 		s.timestamp = CURRENT_TIME;
+		s.nmi_total_ts = nmi_total_ts;
+		s.nmi_count = nmi_count;
 		trace_hwlat_sample(&s);
 
 		/* Keep a running maximum ever recorded hardware latency */
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index cc5db62a3929..892359b75b61 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -1109,12 +1109,14 @@ trace_hwlat_print(struct trace_iterator *iter, int flags,
 
 	trace_assign_type(field, entry);
 
-	trace_seq_printf(s, "[%u] inner:%llu outer:%lld ts:%ld.%09ld\n",
+	trace_seq_printf(s, "[%u] inner:%llu outer:%lld ts:%ld.%09ld nmi-total:%llu nmi-count:%u\n",
 			 field->seqnum,
 			 field->duration,
 			 field->outer_duration,
 			 field->timestamp.tv_sec,
-			 field->timestamp.tv_nsec);
+			 field->timestamp.tv_nsec,
+			 field->nmi_total_ts,
+			 field->nmi_count);
 
 	return trace_handle_return(s);
 }
-- 
1.9.3

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

* Re: [RFC][PATCH 4/3] tracing: Add NMI tracing in hwlat detector
  2016-08-04 16:56 ` [RFC][PATCH 4/3] tracing: Add NMI tracing in hwlat detector Steven Rostedt
@ 2016-08-04 17:16   ` Steven Rostedt
  2016-08-05 14:35     ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 24+ messages in thread
From: Steven Rostedt @ 2016-08-04 17:16 UTC (permalink / raw)
  To: linux-kernel, linux-rt-users
  Cc: Ingo Molnar, Andrew Morton, Clark Williams, Thomas Gleixner,
	Jon Masters, Daniel Wagner, Carsten Emde,
	Sebastian Andrzej Siewior

On Thu, 4 Aug 2016 12:56:18 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> Is the nmi-count even required? I could also modify it to not show the
> NMI information if there wasn't any NMIs.

Yeah, only showing NMI info when nmi_count is > 0 is much nicer.
Updated patch:

---
As NMIs can also cause latency when interrupts are disabled, the hwlat
detectory has no way to know if the latency it detects is from an NMI or an
SMI or some other hardware glitch.

As ftrace_nmi_enter/exit() funtions are no longer used (except for sh, which
isn't supported anymore), I converted those to "arch_ftrace_nmi_enter/exit"
and use ftrace_nmi_enter/exit() to check if hwlat detector is tracing or
not, and if so, it calls into the hwlat utility.

Since the hwlat detector only has a single kthread that is spinning with
interrupts disabled, it marks what CPU it is on, and if the NMI callback
happens on that CPU, it records the time spent in that NMI. This is added to
the output that is generated by the hwlat detector as:

 [1] inner:0 outer:8 ts:1470329211.586481492
 [2] inner:0 outer:4 ts:1470329231.619726657 nmi-total:4 nmi-count:1
 [3] inner:0 outer:5 ts:1470329234.550761052 nmi-total:4 nmi-count:1
 [4] inner:8 outer:0 ts:1470329263.655911668

All time is still tracked in microseconds.

The NMI information is only shown when an NMI occurred during the sample.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/sh/kernel/ftrace.c      |  4 ++--
 include/linux/ftrace_irq.h   | 31 +++++++++++++++++++++++++++----
 kernel/trace/trace_entries.h |  8 ++++++--
 kernel/trace/trace_hwlat.c   | 44 ++++++++++++++++++++++++++++++++++++++++++++
 kernel/trace/trace_output.c  |  8 +++++++-
 5 files changed, 86 insertions(+), 9 deletions(-)

diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c
index 38993e09ef03..deb9eba3a43d 100644
--- a/arch/sh/kernel/ftrace.c
+++ b/arch/sh/kernel/ftrace.c
@@ -139,7 +139,7 @@ static void ftrace_mod_code(void)
 		clear_mod_flag();
 }
 
-void ftrace_nmi_enter(void)
+void arch_ftrace_nmi_enter(void)
 {
 	if (atomic_inc_return(&nmi_running) & MOD_CODE_WRITE_FLAG) {
 		smp_rmb();
@@ -150,7 +150,7 @@ void ftrace_nmi_enter(void)
 	smp_mb();
 }
 
-void ftrace_nmi_exit(void)
+void arch_ftrace_nmi_exit(void)
 {
 	/* Finish all executions before clearing nmi_running */
 	smp_mb();
diff --git a/include/linux/ftrace_irq.h b/include/linux/ftrace_irq.h
index dca7bf8cffe2..4ec2c9b205f2 100644
--- a/include/linux/ftrace_irq.h
+++ b/include/linux/ftrace_irq.h
@@ -3,11 +3,34 @@
 
 
 #ifdef CONFIG_FTRACE_NMI_ENTER
-extern void ftrace_nmi_enter(void);
-extern void ftrace_nmi_exit(void);
+extern void arch_ftrace_nmi_enter(void);
+extern void arch_ftrace_nmi_exit(void);
 #else
-static inline void ftrace_nmi_enter(void) { }
-static inline void ftrace_nmi_exit(void) { }
+static inline void arch_ftrace_nmi_enter(void) { }
+static inline void arch_ftrace_nmi_exit(void) { }
 #endif
 
+#ifdef CONFIG_HWLAT_TRACER
+extern bool trace_hwlat_callback_enabled;
+extern void trace_hwlat_callback(bool enter);
+#endif
+
+static inline void ftrace_nmi_enter(void)
+{
+#ifdef CONFIG_HWLAT_TRACER
+	if (trace_hwlat_callback_enabled)
+		trace_hwlat_callback(true);
+#endif
+	arch_ftrace_nmi_enter();
+}
+
+static inline void ftrace_nmi_exit(void)
+{
+	arch_ftrace_nmi_exit();
+#ifdef CONFIG_HWLAT_TRACER
+	if (trace_hwlat_callback_enabled)
+		trace_hwlat_callback(false);
+#endif
+}
+
 #endif /* _LINUX_FTRACE_IRQ_H */
diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
index 70d47dd99359..d1cc37e78f99 100644
--- a/kernel/trace/trace_entries.h
+++ b/kernel/trace/trace_entries.h
@@ -330,18 +330,22 @@ FTRACE_ENTRY(hwlat, hwlat_entry,
 	F_STRUCT(
 		__field(	u64,			duration	)
 		__field(	u64,			outer_duration	)
+		__field(	u64,			nmi_total_ts	)
 		__field_struct( struct timespec,	timestamp	)
 		__field_desc(	long,	timestamp,	tv_sec		)
 		__field_desc(	long,	timestamp,	tv_nsec		)
+		__field(	unsigned int,		nmi_count	)
 		__field(	unsigned int,		seqnum		)
 	),
 
-	F_printk("cnt:%u\tts:%010lu.%010lu\tinner:%llu\touter:%llu\n",
+	F_printk("cnt:%u\tts:%010lu.%010lu\tinner:%llu\touter:%llunmi-ts:%llu\tnmi-count:%u\n",
 		 __entry->seqnum,
 		 __entry->tv_sec,
 		 __entry->tv_nsec,
 		 __entry->duration,
-		 __entry->outer_duration),
+		 __entry->outer_duration,
+		 __entry->nmi_total_ts,
+		 __entry->nmi_count),
 
 	FILTER_OTHER
 );
diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
index f0b5b2944ee7..0c7ca6082721 100644
--- a/kernel/trace/trace_hwlat.c
+++ b/kernel/trace/trace_hwlat.c
@@ -64,6 +64,15 @@ static struct dentry *hwlat_sample_window;	/* sample window us */
 /* Save the previous tracing_thresh value */
 static unsigned long save_tracing_thresh;
 
+/* NMI timestamp counters */
+static u64 nmi_ts_start;
+static u64 nmi_total_ts;
+static int nmi_count;
+static int nmi_cpu;
+
+/* Tells NMIs to call back to the hwlat tracer to record timestamps */
+bool trace_hwlat_callback_enabled;
+
 /* If the user changed threshold, remember it */
 static u64 last_tracing_thresh = DEFAULT_LAT_THRESHOLD * NSEC_PER_USEC;
 
@@ -72,7 +81,9 @@ struct hwlat_sample {
 	u64		seqnum;		/* unique sequence */
 	u64		duration;	/* delta */
 	u64		outer_duration;	/* delta (outer loop) */
+	u64		nmi_total_ts;	/* Total time spent in NMIs */
 	struct timespec	timestamp;	/* wall time */
+	int		nmi_count;	/* # NMIs during this sample */
 };
 
 /* keep the global state somewhere. */
@@ -112,6 +123,8 @@ static void trace_hwlat_sample(struct hwlat_sample *sample)
 	entry->duration			= sample->duration;
 	entry->outer_duration		= sample->outer_duration;
 	entry->timestamp		= sample->timestamp;
+	entry->nmi_total_ts		= sample->nmi_total_ts;
+	entry->nmi_count		= sample->nmi_count;
 
 	if (!call_filter_check_discard(call, entry, buffer, event))
 		__buffer_unlock_commit(buffer, event);
@@ -125,6 +138,19 @@ static void trace_hwlat_sample(struct hwlat_sample *sample)
 #define init_time(a, b)	(a = b)
 #define time_u64(a)	a
 
+void trace_hwlat_callback(bool enter)
+{
+	if (smp_processor_id() != nmi_cpu)
+		return;
+
+	if (enter)
+		nmi_ts_start = time_get();
+	else {
+		nmi_total_ts = time_get() - nmi_ts_start;
+		nmi_count++;
+	}
+}
+
 /**
  * get_sample - sample the CPU TSC and look for likely hardware latencies
  *
@@ -144,6 +170,14 @@ static int get_sample(void)
 
 	do_div(thresh, NSEC_PER_USEC); /* modifies interval value */
 
+	nmi_cpu = smp_processor_id();
+	nmi_total_ts = 0;
+	nmi_count = 0;
+	/* Make sure NMIs see this first */
+	barrier();
+
+	trace_hwlat_callback_enabled = true;
+
 	init_time(last_t2, 0);
 	start = time_get(); /* start timestamp */
 
@@ -188,6 +222,10 @@ static int get_sample(void)
 
 	} while (total <= hwlat_data.sample_width);
 
+	barrier(); /* finish the above in the view for NMIs */
+	trace_hwlat_callback_enabled = false;
+	barrier(); /* Make sure nmi_total_ts is no longer updated */
+
 	ret = 0;
 
 	/* If we exceed the threshold value, we have found a hardware latency */
@@ -196,11 +234,17 @@ static int get_sample(void)
 
 		ret = 1;
 
+		/* We read in microseconds */
+		if (nmi_total_ts)
+			do_div(nmi_total_ts, NSEC_PER_USEC);
+
 		hwlat_data.count++;
 		s.seqnum = hwlat_data.count;
 		s.duration = sample;
 		s.outer_duration = outer_sample;
 		s.timestamp = CURRENT_TIME;
+		s.nmi_total_ts = nmi_total_ts;
+		s.nmi_count = nmi_count;
 		trace_hwlat_sample(&s);
 
 		/* Keep a running maximum ever recorded hardware latency */
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index cc5db62a3929..c737d4d304f4 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -1109,12 +1109,18 @@ trace_hwlat_print(struct trace_iterator *iter, int flags,
 
 	trace_assign_type(field, entry);
 
-	trace_seq_printf(s, "[%u] inner:%llu outer:%lld ts:%ld.%09ld\n",
+	trace_seq_printf(s, "[%u] inner:%llu outer:%lld ts:%ld.%09ld",
 			 field->seqnum,
 			 field->duration,
 			 field->outer_duration,
 			 field->timestamp.tv_sec,
 			 field->timestamp.tv_nsec);
+	if (field->nmi_count)
+		trace_seq_printf(s, " nmi-total:%llu nmi-count:%u",
+			 field->nmi_total_ts,
+			 field->nmi_count);
+
+	trace_seq_putc(s, '\n');
 
 	return trace_handle_return(s);
 }
-- 
1.9.3

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

* Re: [RFC][PATCH 1/3] tracing: Added hardware latency tracer
  2016-08-04 14:57 ` [RFC][PATCH 1/3] tracing: Added hardware latency tracer Steven Rostedt
@ 2016-08-05 14:25   ` Sebastian Andrzej Siewior
  2016-08-05 14:44     ` Steven Rostedt
  0 siblings, 1 reply; 24+ messages in thread
From: Sebastian Andrzej Siewior @ 2016-08-05 14:25 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, linux-rt-users, Ingo Molnar, Andrew Morton,
	Clark Williams, Thomas Gleixner, Jon Masters, Daniel Wagner,
	Carsten Emde

* Steven Rostedt | 2016-08-04 10:57:09 [-0400]:

>diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
>new file mode 100644
>index 000000000000..08dfabe4e862
>--- /dev/null
>+++ b/kernel/trace/trace_hwlat.c
>+/* Macros to encapsulate the time capturing infrastructure */
>+#define time_type	u64
>+#define time_get()	trace_clock_local()
>+#define time_to_us(x)	div_u64(x, 1000)
>+#define time_sub(a, b)	((a) - (b))
>+#define init_time(a, b)	(a = b)
>+#define time_u64(a)	a

Do we need a macro for this? In the old code we could choose between
CONFIG_TRACING but now we don't.

Sebastian

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

* Re: [RFC][PATCH 4/3] tracing: Add NMI tracing in hwlat detector
  2016-08-04 17:16   ` Steven Rostedt
@ 2016-08-05 14:35     ` Sebastian Andrzej Siewior
  2016-08-05 14:52       ` Steven Rostedt
  0 siblings, 1 reply; 24+ messages in thread
From: Sebastian Andrzej Siewior @ 2016-08-05 14:35 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, linux-rt-users, Ingo Molnar, Andrew Morton,
	Clark Williams, Thomas Gleixner, Jon Masters, Daniel Wagner,
	Carsten Emde

* Steven Rostedt | 2016-08-04 13:16:45 [-0400]:

>diff --git a/include/linux/ftrace_irq.h b/include/linux/ftrace_irq.h
>index dca7bf8cffe2..4ec2c9b205f2 100644
>--- a/include/linux/ftrace_irq.h
>+++ b/include/linux/ftrace_irq.h
>@@ -3,11 +3,34 @@
>+static inline void ftrace_nmi_enter(void)
>+{
>+#ifdef CONFIG_HWLAT_TRACER
>+	if (trace_hwlat_callback_enabled)
>+		trace_hwlat_callback(true);

so we take a tracepoint while we enter an nmi

>--- a/kernel/trace/trace_hwlat.c
>+++ b/kernel/trace/trace_hwlat.c
>@@ -64,6 +64,15 @@ static struct dentry *hwlat_sample_window;	/* sample window us */
> /* Save the previous tracing_thresh value */
> static unsigned long save_tracing_thresh;
> 
>+/* NMI timestamp counters */
>+static u64 nmi_ts_start;
>+static u64 nmi_total_ts;
>+static int nmi_count;
>+static int nmi_cpu;

and this is always limited to one CPU at a time?

…
>@@ -125,6 +138,19 @@ static void trace_hwlat_sample(struct hwlat_sample *sample)
> #define init_time(a, b)	(a = b)
> #define time_u64(a)	a
> 
>+void trace_hwlat_callback(bool enter)
>+{
>+	if (smp_processor_id() != nmi_cpu)
>+		return;
>+
>+	if (enter)
>+		nmi_ts_start = time_get();

but more interestingly: trace_clock_local() -> sched_clock()
and of kernel/time/sched_clock.c we do raw_read_seqcount(&cd.seq) which
means we are busted if the NMI triggers during update_clock_read_data().

>+	else {
>+		nmi_total_ts = time_get() - nmi_ts_start;
>+		nmi_count++;
>+	}
>+}

Sebastian

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

* Re: [RFC][PATCH 1/3] tracing: Added hardware latency tracer
  2016-08-05 14:25   ` Sebastian Andrzej Siewior
@ 2016-08-05 14:44     ` Steven Rostedt
  2016-08-05 15:28       ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 24+ messages in thread
From: Steven Rostedt @ 2016-08-05 14:44 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: linux-kernel, linux-rt-users, Ingo Molnar, Andrew Morton,
	Clark Williams, Thomas Gleixner, Jon Masters, Daniel Wagner,
	Carsten Emde

On Fri, 5 Aug 2016 16:25:21 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> * Steven Rostedt | 2016-08-04 10:57:09 [-0400]:
> 
> >diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
> >new file mode 100644
> >index 000000000000..08dfabe4e862
> >--- /dev/null
> >+++ b/kernel/trace/trace_hwlat.c  
> …
> >+/* Macros to encapsulate the time capturing infrastructure */
> >+#define time_type	u64
> >+#define time_get()	trace_clock_local()
> >+#define time_to_us(x)	div_u64(x, 1000)
> >+#define time_sub(a, b)	((a) - (b))
> >+#define init_time(a, b)	(a = b)
> >+#define time_u64(a)	a  
> 
> Do we need a macro for this? In the old code we could choose between
> CONFIG_TRACING but now we don't.
> 

Probably not, I kept it for two reasons. 1) to keep the same logic as
what was in PREEMPT_RT, and 2) in case we can come up with a better
clock.

But it's not that important. Should it be nuked? They do somewhat make
the code easier to read.

-- Steve

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

* Re: [RFC][PATCH 4/3] tracing: Add NMI tracing in hwlat detector
  2016-08-05 14:35     ` Sebastian Andrzej Siewior
@ 2016-08-05 14:52       ` Steven Rostedt
  2016-08-05 15:40         ` Sebastian Andrzej Siewior
  2016-08-10 14:10         ` Jon Masters
  0 siblings, 2 replies; 24+ messages in thread
From: Steven Rostedt @ 2016-08-05 14:52 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: linux-kernel, linux-rt-users, Ingo Molnar, Andrew Morton,
	Clark Williams, Thomas Gleixner, Jon Masters, Daniel Wagner,
	Carsten Emde

On Fri, 5 Aug 2016 16:35:55 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> * Steven Rostedt | 2016-08-04 13:16:45 [-0400]:
> 
> >diff --git a/include/linux/ftrace_irq.h b/include/linux/ftrace_irq.h
> >index dca7bf8cffe2..4ec2c9b205f2 100644
> >--- a/include/linux/ftrace_irq.h
> >+++ b/include/linux/ftrace_irq.h
> >@@ -3,11 +3,34 @@  
> …
> >+static inline void ftrace_nmi_enter(void)
> >+{
> >+#ifdef CONFIG_HWLAT_TRACER
> >+	if (trace_hwlat_callback_enabled)
> >+		trace_hwlat_callback(true);  
> 
> so we take a tracepoint while we enter an nmi

It's not technically a tracepoint. I'm not sure tracepoints
(jumplabels) may be located this early in the NMI handler. This is
before some of the magic of having NMIs dealing with page faults and
break points.

> 
> >--- a/kernel/trace/trace_hwlat.c
> >+++ b/kernel/trace/trace_hwlat.c
> >@@ -64,6 +64,15 @@ static struct dentry *hwlat_sample_window;	/* sample window us */
> > /* Save the previous tracing_thresh value */
> > static unsigned long save_tracing_thresh;
> > 
> >+/* NMI timestamp counters */
> >+static u64 nmi_ts_start;
> >+static u64 nmi_total_ts;
> >+static int nmi_count;
> >+static int nmi_cpu;  
> 
> and this is always limited to one CPU at a time?

Yes. Hence the "nmi_cpu".

> 
> …
> >@@ -125,6 +138,19 @@ static void trace_hwlat_sample(struct hwlat_sample *sample)
> > #define init_time(a, b)	(a = b)
> > #define time_u64(a)	a
> > 
> >+void trace_hwlat_callback(bool enter)
> >+{
> >+	if (smp_processor_id() != nmi_cpu)
> >+		return;
> >+
> >+	if (enter)
> >+		nmi_ts_start = time_get();  
> 
> but more interestingly: trace_clock_local() -> sched_clock()
> and of kernel/time/sched_clock.c we do raw_read_seqcount(&cd.seq) which
> means we are busted if the NMI triggers during update_clock_read_data().

Hmm, interesting. Because this is true for general tracing from an NMI.

/me looks at code.

Ah, this is when we have GENERIC_SCHED_CLOCK, which would break tracing
if any arch that has this also has NMIs. Probably need to look at arm64.

For x86, it has its own NMI safe sched_clock. I could make this "NMI"
code depend on:

#ifndef CONFIG_GENERIC_SCHED_CLOCK


-- Steve


> 
> >+	else {
> >+		nmi_total_ts = time_get() - nmi_ts_start;
> >+		nmi_count++;
> >+	}
> >+}  
> 
> Sebastian

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

* Re: [RFC][PATCH 1/3] tracing: Added hardware latency tracer
  2016-08-05 14:44     ` Steven Rostedt
@ 2016-08-05 15:28       ` Sebastian Andrzej Siewior
  2016-08-05 15:36         ` Steven Rostedt
  0 siblings, 1 reply; 24+ messages in thread
From: Sebastian Andrzej Siewior @ 2016-08-05 15:28 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, linux-rt-users, Ingo Molnar, Andrew Morton,
	Clark Williams, Thomas Gleixner, Jon Masters, Daniel Wagner,
	Carsten Emde

On 08/05/2016 04:44 PM, Steven Rostedt wrote:
> On Fri, 5 Aug 2016 16:25:21 +0200
> Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> 
>> * Steven Rostedt | 2016-08-04 10:57:09 [-0400]:
>>
>>> diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
>>> new file mode 100644
>>> index 000000000000..08dfabe4e862
>>> --- /dev/null
>>> +++ b/kernel/trace/trace_hwlat.c  
>> …
>>> +/* Macros to encapsulate the time capturing infrastructure */
>>> +#define time_type	u64
>>> +#define time_get()	trace_clock_local()
>>> +#define time_to_us(x)	div_u64(x, 1000)
>>> +#define time_sub(a, b)	((a) - (b))
>>> +#define init_time(a, b)	(a = b)
>>> +#define time_u64(a)	a  
>>
>> Do we need a macro for this? In the old code we could choose between
>> CONFIG_TRACING but now we don't.
>>
> 
> Probably not, I kept it for two reasons. 1) to keep the same logic as
> what was in PREEMPT_RT, and 2) in case we can come up with a better
> clock.

I assumed it was a leftover.

> But it's not that important. Should it be nuked? They do somewhat make
> the code easier to read.

that time_get() is close to ktime_get() which is almost u64 nowadays.
So it might not be that cool for upstream. A hwlat prefix makes the
whole thing not prettier.

1. PREEMPT_RT. Do I need any changes? I assumed I could keep this 1:1
   (once it is merged) and throw the current hwlat out.
2. a better clock is an argument. But why would you have a better clock
   for hwlat and not for the whole tracing infrastructure?

If you want to keep it, keep it. I just assumed it was a leftover.
> 
> -- Steve

Sebastian

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

* Re: [RFC][PATCH 1/3] tracing: Added hardware latency tracer
  2016-08-05 15:28       ` Sebastian Andrzej Siewior
@ 2016-08-05 15:36         ` Steven Rostedt
  0 siblings, 0 replies; 24+ messages in thread
From: Steven Rostedt @ 2016-08-05 15:36 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: linux-kernel, linux-rt-users, Ingo Molnar, Andrew Morton,
	Clark Williams, Thomas Gleixner, Jon Masters, Daniel Wagner,
	Carsten Emde

On Fri, 5 Aug 2016 17:28:33 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:


> If you want to keep it, keep it. I just assumed it was a leftover.

I have mixed feelings about it. I may just nuke it anyway. I forgot to
add #3 laziness, it worked and I didn't want to add bugs by removing it.

--Steve

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

* Re: [RFC][PATCH 4/3] tracing: Add NMI tracing in hwlat detector
  2016-08-05 14:52       ` Steven Rostedt
@ 2016-08-05 15:40         ` Sebastian Andrzej Siewior
  2016-08-05 16:17           ` Steven Rostedt
  2016-08-09 17:22           ` Steven Rostedt
  2016-08-10 14:10         ` Jon Masters
  1 sibling, 2 replies; 24+ messages in thread
From: Sebastian Andrzej Siewior @ 2016-08-05 15:40 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, linux-rt-users, Ingo Molnar, Andrew Morton,
	Clark Williams, Thomas Gleixner, Jon Masters, Daniel Wagner,
	Carsten Emde

On 08/05/2016 04:52 PM, Steven Rostedt wrote:
>>> --- a/kernel/trace/trace_hwlat.c
>>> +++ b/kernel/trace/trace_hwlat.c
>>> @@ -64,6 +64,15 @@ static struct dentry *hwlat_sample_window;	/* sample window us */
>>> /* Save the previous tracing_thresh value */
>>> static unsigned long save_tracing_thresh;
>>>
>>> +/* NMI timestamp counters */
>>> +static u64 nmi_ts_start;
>>> +static u64 nmi_total_ts;
>>> +static int nmi_count;
>>> +static int nmi_cpu;  
>>
>> and this is always limited to one CPU at a time?
> 
> Yes. Hence the "nmi_cpu".

I was just confused. So we check one CPU at a time. Okay.

>>> @@ -125,6 +138,19 @@ static void trace_hwlat_sample(struct hwlat_sample *sample)
>>> #define init_time(a, b)	(a = b)
>>> #define time_u64(a)	a
>>>
>>> +void trace_hwlat_callback(bool enter)
>>> +{
>>> +	if (smp_processor_id() != nmi_cpu)
>>> +		return;
>>> +
>>> +	if (enter)
>>> +		nmi_ts_start = time_get();  
>>
>> but more interestingly: trace_clock_local() -> sched_clock()
>> and of kernel/time/sched_clock.c we do raw_read_seqcount(&cd.seq) which
>> means we are busted if the NMI triggers during update_clock_read_data().
> 
> Hmm, interesting. Because this is true for general tracing from an NMI.
> 
> /me looks at code.
> 
> Ah, this is when we have GENERIC_SCHED_CLOCK, which would break tracing
> if any arch that has this also has NMIs. Probably need to look at arm64.

arm64 should use the generic code as they don't provide sched_clock()
(and I doubt they go for the weak jiffy version).

> For x86, it has its own NMI safe sched_clock. I could make this "NMI"
> code depend on:
> 
> #ifndef CONFIG_GENERIC_SCHED_CLOCK

that would be nice. That would be disable approx $(git grep
sched_clock_register | wc -l) users but better than a lock up I guess.

> 
> -- Steve

Sebastian

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

* Re: [RFC][PATCH 4/3] tracing: Add NMI tracing in hwlat detector
  2016-08-05 15:40         ` Sebastian Andrzej Siewior
@ 2016-08-05 16:17           ` Steven Rostedt
  2016-08-09 17:22           ` Steven Rostedt
  1 sibling, 0 replies; 24+ messages in thread
From: Steven Rostedt @ 2016-08-05 16:17 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: linux-kernel, linux-rt-users, Ingo Molnar, Andrew Morton,
	Clark Williams, Thomas Gleixner, Jon Masters, Daniel Wagner,
	Carsten Emde

On Fri, 5 Aug 2016 17:40:43 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:


> > Ah, this is when we have GENERIC_SCHED_CLOCK, which would break tracing
> > if any arch that has this also has NMIs. Probably need to look at arm64.  
> 
> arm64 should use the generic code as they don't provide sched_clock()
> (and I doubt they go for the weak jiffy version).

And I'm guessing that arm64 supports NMIs. Is there no NMI safe clock?
I probably need to look at the tracing code. Because you can have the
same issue while tracing in an NMI as well.

-- Steve

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

* Re: [RFC][PATCH 4/3] tracing: Add NMI tracing in hwlat detector
  2016-08-05 15:40         ` Sebastian Andrzej Siewior
  2016-08-05 16:17           ` Steven Rostedt
@ 2016-08-09 17:22           ` Steven Rostedt
  1 sibling, 0 replies; 24+ messages in thread
From: Steven Rostedt @ 2016-08-09 17:22 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: linux-kernel, linux-rt-users, Ingo Molnar, Andrew Morton,
	Clark Williams, Thomas Gleixner, Jon Masters, Daniel Wagner,
	Carsten Emde

On Fri, 5 Aug 2016 17:40:43 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> > For x86, it has its own NMI safe sched_clock. I could make this "NMI"
> > code depend on:
> > 
> > #ifndef CONFIG_GENERIC_SCHED_CLOCK  
> 
> that would be nice. That would be disable approx $(git grep
> sched_clock_register | wc -l) users but better than a lock up I guess.

How about this patch. It still records nmi_count, just not the time
spent in the NMI.

-- Steve


As NMIs can also cause latency when interrupts are disabled, the hwlat
detectory has no way to know if the latency it detects is from an NMI or an
SMI or some other hardware glitch.

As ftrace_nmi_enter/exit() funtions are no longer used (except for sh, which
isn't supported anymore), I converted those to "arch_ftrace_nmi_enter/exit"
and use ftrace_nmi_enter/exit() to check if hwlat detector is tracing or
not, and if so, it calls into the hwlat utility.

Since the hwlat detector only has a single kthread that is spinning with
interrupts disabled, it marks what CPU it is on, and if the NMI callback
happens on that CPU, it records the time spent in that NMI. This is added to
the output that is generated by the hwlat detector as:

 [1] inner:0 outer:8 ts:1470329211.586481492
 [2] inner:0 outer:4 ts:1470329231.619726657 nmi-total:4 nmi-count:1
 [3] inner:0 outer:5 ts:1470329234.550761052 nmi-total:4 nmi-count:1
 [4] inner:8 outer:0 ts:1470329263.655911668

All time is still tracked in microseconds.

The NMI information is only shown when an NMI occurred during the sample.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/sh/kernel/ftrace.c      |  4 ++--
 include/linux/ftrace_irq.h   | 31 +++++++++++++++++++++++----
 kernel/trace/trace_entries.h |  8 +++++--
 kernel/trace/trace_hwlat.c   | 51 ++++++++++++++++++++++++++++++++++++++++++++
 kernel/trace/trace_output.c  | 16 +++++++++++++-
 5 files changed, 101 insertions(+), 9 deletions(-)

diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c
index 38993e09ef03..deb9eba3a43d 100644
--- a/arch/sh/kernel/ftrace.c
+++ b/arch/sh/kernel/ftrace.c
@@ -139,7 +139,7 @@ static void ftrace_mod_code(void)
 		clear_mod_flag();
 }
 
-void ftrace_nmi_enter(void)
+void arch_ftrace_nmi_enter(void)
 {
 	if (atomic_inc_return(&nmi_running) & MOD_CODE_WRITE_FLAG) {
 		smp_rmb();
@@ -150,7 +150,7 @@ void ftrace_nmi_enter(void)
 	smp_mb();
 }
 
-void ftrace_nmi_exit(void)
+void arch_ftrace_nmi_exit(void)
 {
 	/* Finish all executions before clearing nmi_running */
 	smp_mb();
diff --git a/include/linux/ftrace_irq.h b/include/linux/ftrace_irq.h
index dca7bf8cffe2..4ec2c9b205f2 100644
--- a/include/linux/ftrace_irq.h
+++ b/include/linux/ftrace_irq.h
@@ -3,11 +3,34 @@
 
 
 #ifdef CONFIG_FTRACE_NMI_ENTER
-extern void ftrace_nmi_enter(void);
-extern void ftrace_nmi_exit(void);
+extern void arch_ftrace_nmi_enter(void);
+extern void arch_ftrace_nmi_exit(void);
 #else
-static inline void ftrace_nmi_enter(void) { }
-static inline void ftrace_nmi_exit(void) { }
+static inline void arch_ftrace_nmi_enter(void) { }
+static inline void arch_ftrace_nmi_exit(void) { }
 #endif
 
+#ifdef CONFIG_HWLAT_TRACER
+extern bool trace_hwlat_callback_enabled;
+extern void trace_hwlat_callback(bool enter);
+#endif
+
+static inline void ftrace_nmi_enter(void)
+{
+#ifdef CONFIG_HWLAT_TRACER
+	if (trace_hwlat_callback_enabled)
+		trace_hwlat_callback(true);
+#endif
+	arch_ftrace_nmi_enter();
+}
+
+static inline void ftrace_nmi_exit(void)
+{
+	arch_ftrace_nmi_exit();
+#ifdef CONFIG_HWLAT_TRACER
+	if (trace_hwlat_callback_enabled)
+		trace_hwlat_callback(false);
+#endif
+}
+
 #endif /* _LINUX_FTRACE_IRQ_H */
diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
index 70d47dd99359..d1cc37e78f99 100644
--- a/kernel/trace/trace_entries.h
+++ b/kernel/trace/trace_entries.h
@@ -330,18 +330,22 @@ FTRACE_ENTRY(hwlat, hwlat_entry,
 	F_STRUCT(
 		__field(	u64,			duration	)
 		__field(	u64,			outer_duration	)
+		__field(	u64,			nmi_total_ts	)
 		__field_struct( struct timespec,	timestamp	)
 		__field_desc(	long,	timestamp,	tv_sec		)
 		__field_desc(	long,	timestamp,	tv_nsec		)
+		__field(	unsigned int,		nmi_count	)
 		__field(	unsigned int,		seqnum		)
 	),
 
-	F_printk("cnt:%u\tts:%010lu.%010lu\tinner:%llu\touter:%llu\n",
+	F_printk("cnt:%u\tts:%010lu.%010lu\tinner:%llu\touter:%llunmi-ts:%llu\tnmi-count:%u\n",
 		 __entry->seqnum,
 		 __entry->tv_sec,
 		 __entry->tv_nsec,
 		 __entry->duration,
-		 __entry->outer_duration),
+		 __entry->outer_duration,
+		 __entry->nmi_total_ts,
+		 __entry->nmi_count),
 
 	FILTER_OTHER
 );
diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
index f0b5b2944ee7..2a668e55dcc6 100644
--- a/kernel/trace/trace_hwlat.c
+++ b/kernel/trace/trace_hwlat.c
@@ -64,6 +64,15 @@ static struct dentry *hwlat_sample_window;	/* sample window us */
 /* Save the previous tracing_thresh value */
 static unsigned long save_tracing_thresh;
 
+/* NMI timestamp counters */
+static u64 nmi_ts_start;
+static u64 nmi_total_ts;
+static int nmi_count;
+static int nmi_cpu;
+
+/* Tells NMIs to call back to the hwlat tracer to record timestamps */
+bool trace_hwlat_callback_enabled;
+
 /* If the user changed threshold, remember it */
 static u64 last_tracing_thresh = DEFAULT_LAT_THRESHOLD * NSEC_PER_USEC;
 
@@ -72,7 +81,9 @@ struct hwlat_sample {
 	u64		seqnum;		/* unique sequence */
 	u64		duration;	/* delta */
 	u64		outer_duration;	/* delta (outer loop) */
+	u64		nmi_total_ts;	/* Total time spent in NMIs */
 	struct timespec	timestamp;	/* wall time */
+	int		nmi_count;	/* # NMIs during this sample */
 };
 
 /* keep the global state somewhere. */
@@ -112,6 +123,8 @@ static void trace_hwlat_sample(struct hwlat_sample *sample)
 	entry->duration			= sample->duration;
 	entry->outer_duration		= sample->outer_duration;
 	entry->timestamp		= sample->timestamp;
+	entry->nmi_total_ts		= sample->nmi_total_ts;
+	entry->nmi_count		= sample->nmi_count;
 
 	if (!call_filter_check_discard(call, entry, buffer, event))
 		__buffer_unlock_commit(buffer, event);
@@ -125,6 +138,26 @@ static void trace_hwlat_sample(struct hwlat_sample *sample)
 #define init_time(a, b)	(a = b)
 #define time_u64(a)	a
 
+void trace_hwlat_callback(bool enter)
+{
+	if (smp_processor_id() != nmi_cpu)
+		return;
+
+	/*
+	 * Currently trace_clock_local() calls sched_clock() and the
+	 * generic version is not NMI safe.
+	 */
+	if (!IS_ENABLED(CONFIG_GENERIC_SCHED_CLOCK)) {
+		if (enter)
+			nmi_ts_start = time_get();
+		else
+			nmi_total_ts = time_get() - nmi_ts_start;
+	}
+
+	if (enter)
+		nmi_count++;
+}
+
 /**
  * get_sample - sample the CPU TSC and look for likely hardware latencies
  *
@@ -144,6 +177,14 @@ static int get_sample(void)
 
 	do_div(thresh, NSEC_PER_USEC); /* modifies interval value */
 
+	nmi_cpu = smp_processor_id();
+	nmi_total_ts = 0;
+	nmi_count = 0;
+	/* Make sure NMIs see this first */
+	barrier();
+
+	trace_hwlat_callback_enabled = true;
+
 	init_time(last_t2, 0);
 	start = time_get(); /* start timestamp */
 
@@ -188,6 +229,10 @@ static int get_sample(void)
 
 	} while (total <= hwlat_data.sample_width);
 
+	barrier(); /* finish the above in the view for NMIs */
+	trace_hwlat_callback_enabled = false;
+	barrier(); /* Make sure nmi_total_ts is no longer updated */
+
 	ret = 0;
 
 	/* If we exceed the threshold value, we have found a hardware latency */
@@ -196,11 +241,17 @@ static int get_sample(void)
 
 		ret = 1;
 
+		/* We read in microseconds */
+		if (nmi_total_ts)
+			do_div(nmi_total_ts, NSEC_PER_USEC);
+
 		hwlat_data.count++;
 		s.seqnum = hwlat_data.count;
 		s.duration = sample;
 		s.outer_duration = outer_sample;
 		s.timestamp = CURRENT_TIME;
+		s.nmi_total_ts = nmi_total_ts;
+		s.nmi_count = nmi_count;
 		trace_hwlat_sample(&s);
 
 		/* Keep a running maximum ever recorded hardware latency */
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index cc5db62a3929..5478a97e8db3 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -1109,13 +1109,27 @@ trace_hwlat_print(struct trace_iterator *iter, int flags,
 
 	trace_assign_type(field, entry);
 
-	trace_seq_printf(s, "[%u] inner:%llu outer:%lld ts:%ld.%09ld\n",
+	trace_seq_printf(s, "[%u] inner:%llu outer:%lld ts:%ld.%09ld",
 			 field->seqnum,
 			 field->duration,
 			 field->outer_duration,
 			 field->timestamp.tv_sec,
 			 field->timestamp.tv_nsec);
 
+	if (field->nmi_count) {
+		/*
+		 * The generic sched_clock() is not NMI safe, thus
+		 * we only record the count and not the time.
+		 */
+		if (!IS_ENABLED(CONFIG_GENERIC_SCHED_CLOCK))
+			trace_seq_printf(s, " nmi-total:%llu",
+					 field->nmi_total_ts);
+		trace_seq_printf(s, " nmi-count:%u",
+				 field->nmi_count);
+	}
+
+	trace_seq_putc(s, '\n');
+
 	return trace_handle_return(s);
 }
 
-- 
1.9.3

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

* [RFC][PATCH 5/3] tracing: Add smi counting to HWLAT
  2016-08-04 14:57 [RFC][PATCH 0/3] tracing: Add Hardware Latency detector tracer Steven Rostedt
                   ` (4 preceding siblings ...)
  2016-08-04 16:56 ` [RFC][PATCH 4/3] tracing: Add NMI tracing in hwlat detector Steven Rostedt
@ 2016-08-09 18:05 ` Steven Rostedt
  2016-08-09 18:28   ` Daniel Bristot de Oliveira
  2016-08-09 21:25   ` Peter Zijlstra
  5 siblings, 2 replies; 24+ messages in thread
From: Steven Rostedt @ 2016-08-09 18:05 UTC (permalink / raw)
  To: linux-kernel, linux-rt-users
  Cc: Ingo Molnar, Andrew Morton, Clark Williams, Thomas Gleixner,
	Jon Masters, Daniel Wagner, Carsten Emde,
	Sebastian Andrzej Siewior, Peter Zijlstra


If an arch supports counting of SMIs (like newer intel chips do), then it
can implement arch_smi_count() to return the number of SMIs that were
triggered. The hwlat detector will call this function to get the current
number of SMIs, and then after a period, it will read that function again,
and if there's a difference, it will record that into the sample.

For example:

 [99] inner:13 outer:16 ts:1470352534.886878855
 [100] inner:14 outer:18747 ts:1470352538.917966818 smi-count:2
 [101] inner:0 outer:19162 ts:1470352539.920988709 smi-count:6
 [102] inner:19376 outer:19276 ts:1470352540.923010578 smi-count:6
 [103] inner:19650 outer:20665 ts:1470352541.926032469 smi-count:6
 [104] inner:20526 outer:20680 ts:1470352542.973055312 smi-count:6
 [105] inner:17 outer:17 ts:1470352543.990077507

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/events/msr.c        | 12 ++++++++++++
 kernel/trace/trace_entries.h |  1 +
 kernel/trace/trace_hwlat.c   | 11 +++++++++++
 kernel/trace/trace_output.c  |  4 ++++
 4 files changed, 28 insertions(+)

diff --git a/arch/x86/events/msr.c b/arch/x86/events/msr.c
index 85ef3c2e80e0..ff0c6e6351b0 100644
--- a/arch/x86/events/msr.c
+++ b/arch/x86/events/msr.c
@@ -27,6 +27,18 @@ static bool test_irperf(int idx)
 	return boot_cpu_has(X86_FEATURE_IRPERF);
 }
 
+int arch_smi_count(void)
+{
+	unsigned long long count;
+	int err;
+
+	err = rdmsrl_safe(MSR_SMI_COUNT, &count);
+	if (err)
+		return 0;
+
+	return count;
+}
+
 static bool test_intel(int idx)
 {
 	if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL ||
diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
index d1cc37e78f99..207faa837d3d 100644
--- a/kernel/trace/trace_entries.h
+++ b/kernel/trace/trace_entries.h
@@ -335,6 +335,7 @@ FTRACE_ENTRY(hwlat, hwlat_entry,
 		__field_desc(	long,	timestamp,	tv_sec		)
 		__field_desc(	long,	timestamp,	tv_nsec		)
 		__field(	unsigned int,		nmi_count	)
+		__field(	unsigned int,		smi_count	)
 		__field(	unsigned int,		seqnum		)
 	),
 
diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
index 2a668e55dcc6..1d60ef5c404f 100644
--- a/kernel/trace/trace_hwlat.c
+++ b/kernel/trace/trace_hwlat.c
@@ -69,6 +69,7 @@ static u64 nmi_ts_start;
 static u64 nmi_total_ts;
 static int nmi_count;
 static int nmi_cpu;
+static int smi_count;
 
 /* Tells NMIs to call back to the hwlat tracer to record timestamps */
 bool trace_hwlat_callback_enabled;
@@ -84,6 +85,7 @@ struct hwlat_sample {
 	u64		nmi_total_ts;	/* Total time spent in NMIs */
 	struct timespec	timestamp;	/* wall time */
 	int		nmi_count;	/* # NMIs during this sample */
+	int		smi_count;	/* # SMIs during sampling (if arch supported) */
 };
 
 /* keep the global state somewhere. */
@@ -125,6 +127,7 @@ static void trace_hwlat_sample(struct hwlat_sample *sample)
 	entry->timestamp		= sample->timestamp;
 	entry->nmi_total_ts		= sample->nmi_total_ts;
 	entry->nmi_count		= sample->nmi_count;
+	entry->smi_count		= sample->smi_count;
 
 	if (!call_filter_check_discard(call, entry, buffer, event))
 		__buffer_unlock_commit(buffer, event);
@@ -138,6 +141,11 @@ static void trace_hwlat_sample(struct hwlat_sample *sample)
 #define init_time(a, b)	(a = b)
 #define time_u64(a)	a
 
+__weak unsigned long long arch_smi_count(void)
+{
+	return 0;
+}
+
 void trace_hwlat_callback(bool enter)
 {
 	if (smp_processor_id() != nmi_cpu)
@@ -180,6 +188,7 @@ static int get_sample(void)
 	nmi_cpu = smp_processor_id();
 	nmi_total_ts = 0;
 	nmi_count = 0;
+	smi_count = arch_smi_count();
 	/* Make sure NMIs see this first */
 	barrier();
 
@@ -231,6 +240,7 @@ static int get_sample(void)
 
 	barrier(); /* finish the above in the view for NMIs */
 	trace_hwlat_callback_enabled = false;
+	smi_count = arch_smi_count() - smi_count;
 	barrier(); /* Make sure nmi_total_ts is no longer updated */
 
 	ret = 0;
@@ -252,6 +262,7 @@ static int get_sample(void)
 		s.timestamp = CURRENT_TIME;
 		s.nmi_total_ts = nmi_total_ts;
 		s.nmi_count = nmi_count;
+		s.smi_count = smi_count;
 		trace_hwlat_sample(&s);
 
 		/* Keep a running maximum ever recorded hardware latency */
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 5478a97e8db3..498eb7363e05 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -1128,6 +1128,10 @@ trace_hwlat_print(struct trace_iterator *iter, int flags,
 				 field->nmi_count);
 	}
 
+	if (field->smi_count)
+		trace_seq_printf(s, " smi-count:%u",
+			 field->smi_count);
+
 	trace_seq_putc(s, '\n');
 
 	return trace_handle_return(s);
-- 
1.9.3

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

* Re: [RFC][PATCH 0/3] tracing: Add Hardware Latency detector tracer
  2016-08-04 15:30 ` [RFC][PATCH 0/3] tracing: Add Hardware Latency detector tracer Steven Rostedt
@ 2016-08-09 18:15   ` Clark Williams
  2016-08-09 18:30     ` Steven Rostedt
  0 siblings, 1 reply; 24+ messages in thread
From: Clark Williams @ 2016-08-09 18:15 UTC (permalink / raw)
  To: Steven Rostedt, Carsten Emde
  Cc: linux-kernel, linux-rt-users, Ingo Molnar, Andrew Morton,
	Thomas Gleixner, Jon Masters, Daniel Wagner,
	Sebastian Andrzej Siewior

[-- Attachment #1: Type: text/plain, Size: 1240 bytes --]

On Thu, 4 Aug 2016 11:30:33 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> Note, I'm currently working on adding code to detect NMIs to this as
> well. And perhaps even tracing SMI counters. Just to show what caused
> the latency, as latency isn't measured by the counters (that I know of).
> 

I like the trace report, but for one small thing; I think it's worth either adding units to the reported latency values (us suffix) or putting in a header line that hwlatdetect values are reported in microseconds.

>    <...>-2644  [001] d...   202.735708: [99] inner:13 outer:16 ts:1470352534.886878855
>    <...>-2644  [001] d...   206.767274: [100] inner:14 outer:18747 ts:1470352538.917966818 smi-total:2
>    <...>-2644  [002] d...   207.775164: [101] inner:0 outer:19162 ts:1470352539.920988709 smi-total:6
>    <...>-2644  [003] d...   208.783067: [102] inner:19376 outer:19276 ts:1470352540.923010578 smi-total:6
>    <...>-2644  [000] d...   209.827416: [103] inner:19650 outer:20665 ts:1470352541.926032469 smi-total:6
>    <...>-2644  [001] d...   210.831062: [104] inner:20526 outer:20680 ts:1470352542.973055312 smi-total:6

Clark

-- 
The United States Coast Guard:
Ruining natural selection since 1790

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [RFC][PATCH 5/3] tracing: Add smi counting to HWLAT
  2016-08-09 18:05 ` [RFC][PATCH 5/3] tracing: Add smi counting to HWLAT Steven Rostedt
@ 2016-08-09 18:28   ` Daniel Bristot de Oliveira
  2016-08-09 18:35     ` Steven Rostedt
  2016-08-09 21:25   ` Peter Zijlstra
  1 sibling, 1 reply; 24+ messages in thread
From: Daniel Bristot de Oliveira @ 2016-08-09 18:28 UTC (permalink / raw)
  To: Steven Rostedt, linux-kernel, linux-rt-users
  Cc: Ingo Molnar, Andrew Morton, Clark Williams, Thomas Gleixner,
	Jon Masters, Daniel Wagner, Carsten Emde,
	Sebastian Andrzej Siewior, Peter Zijlstra

On 08/09/2016 03:05 PM, Steven Rostedt wrote:
> If an arch supports counting of SMIs (like newer intel chips do), then it
> can implement arch_smi_count() to return the number of SMIs that were
> triggered. The hwlat detector will call this function to get the current
> number of SMIs, and then after a period, it will read that function again,
> and if there's a difference, it will record that into the sample.
> 
> For example:
> 
>  [99] inner:13 outer:16 ts:1470352534.886878855
>  [100] inner:14 outer:18747 ts:1470352538.917966818 smi-count:2
>  [101] inner:0 outer:19162 ts:1470352539.920988709 smi-count:6
>  [102] inner:19376 outer:19276 ts:1470352540.923010578 smi-count:6
>  [103] inner:19650 outer:20665 ts:1470352541.926032469 smi-count:6
>  [104] inner:20526 outer:20680 ts:1470352542.973055312 smi-count:6
>  [105] inner:17 outer:17 ts:1470352543.990077507
> 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

It worked fine in a system that I can manually cause SMIs (by turning
keyboard's backlight on and off).

Tested-by: Daniel Bristot de Oliveira <bristot@redhat.com>

-- Daniel

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

* Re: [RFC][PATCH 0/3] tracing: Add Hardware Latency detector tracer
  2016-08-09 18:15   ` Clark Williams
@ 2016-08-09 18:30     ` Steven Rostedt
  0 siblings, 0 replies; 24+ messages in thread
From: Steven Rostedt @ 2016-08-09 18:30 UTC (permalink / raw)
  To: Clark Williams
  Cc: Carsten Emde, linux-kernel, linux-rt-users, Ingo Molnar,
	Andrew Morton, Thomas Gleixner, Jon Masters, Daniel Wagner,
	Sebastian Andrzej Siewior

On Tue, 9 Aug 2016 13:15:16 -0500
Clark Williams <williams@redhat.com> wrote:

> On Thu, 4 Aug 2016 11:30:33 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > Note, I'm currently working on adding code to detect NMIs to this as
> > well. And perhaps even tracing SMI counters. Just to show what caused
> > the latency, as latency isn't measured by the counters (that I know of).
> >   
> 
> I like the trace report, but for one small thing; I think it's worth
> either adding units to the reported latency values (us suffix) or
> putting in a header line that hwlatdetect values are reported in
> microseconds.

Yeah, adding "us" to the time stamps would be useful. I'll update.

-- Steve

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

* Re: [RFC][PATCH 5/3] tracing: Add smi counting to HWLAT
  2016-08-09 18:28   ` Daniel Bristot de Oliveira
@ 2016-08-09 18:35     ` Steven Rostedt
  0 siblings, 0 replies; 24+ messages in thread
From: Steven Rostedt @ 2016-08-09 18:35 UTC (permalink / raw)
  To: Daniel Bristot de Oliveira
  Cc: linux-kernel, linux-rt-users, Ingo Molnar, Andrew Morton,
	Clark Williams, Thomas Gleixner, Jon Masters, Daniel Wagner,
	Carsten Emde, Sebastian Andrzej Siewior, Peter Zijlstra

On Tue, 9 Aug 2016 15:28:57 -0300
Daniel Bristot de Oliveira <bristot@redhat.com> wrote:

> On 08/09/2016 03:05 PM, Steven Rostedt wrote:
> > If an arch supports counting of SMIs (like newer intel chips do), then it
> > can implement arch_smi_count() to return the number of SMIs that were
> > triggered. The hwlat detector will call this function to get the current
> > number of SMIs, and then after a period, it will read that function again,
> > and if there's a difference, it will record that into the sample.
> > 
> > For example:
> > 
> >  [99] inner:13 outer:16 ts:1470352534.886878855
> >  [100] inner:14 outer:18747 ts:1470352538.917966818 smi-count:2
> >  [101] inner:0 outer:19162 ts:1470352539.920988709 smi-count:6
> >  [102] inner:19376 outer:19276 ts:1470352540.923010578 smi-count:6
> >  [103] inner:19650 outer:20665 ts:1470352541.926032469 smi-count:6
> >  [104] inner:20526 outer:20680 ts:1470352542.973055312 smi-count:6
> >  [105] inner:17 outer:17 ts:1470352543.990077507
> > 
> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>  
> 
> It worked fine in a system that I can manually cause SMIs (by turning
> keyboard's backlight on and off).
> 
> Tested-by: Daniel Bristot de Oliveira <bristot@redhat.com>

Thanks! I should also add:

Suggested-by: Peter Zijlstra <peterz@infradead.org>

As Peter was the one that recommended adding the arch_smi_count()
function.

But I can't push this yet as this would require acks from the x86
maintainers.

But if there's no complaints about the rest of the patches, I could
work to get that ready.

-- Steve

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

* Re: [RFC][PATCH 5/3] tracing: Add smi counting to HWLAT
  2016-08-09 18:05 ` [RFC][PATCH 5/3] tracing: Add smi counting to HWLAT Steven Rostedt
  2016-08-09 18:28   ` Daniel Bristot de Oliveira
@ 2016-08-09 21:25   ` Peter Zijlstra
  1 sibling, 0 replies; 24+ messages in thread
From: Peter Zijlstra @ 2016-08-09 21:25 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, linux-rt-users, Ingo Molnar, Andrew Morton,
	Clark Williams, Thomas Gleixner, Jon Masters, Daniel Wagner,
	Carsten Emde, Sebastian Andrzej Siewior

On Tue, Aug 09, 2016 at 02:05:43PM -0400, Steven Rostedt wrote:
> +int arch_smi_count(void)
> +{
> +	unsigned long long count;
> +	int err;
> +
> +	err = rdmsrl_safe(MSR_SMI_COUNT, &count);
> +	if (err)
> +		return 0;

That's really yucky, relying on _safe() to detect availability.

Also, I just found AMD Fam15h has this counter through PMCs (event
0x2b).

> +
> +	return count;
> +}

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

* Re: [RFC][PATCH 4/3] tracing: Add NMI tracing in hwlat detector
  2016-08-05 14:52       ` Steven Rostedt
  2016-08-05 15:40         ` Sebastian Andrzej Siewior
@ 2016-08-10 14:10         ` Jon Masters
  1 sibling, 0 replies; 24+ messages in thread
From: Jon Masters @ 2016-08-10 14:10 UTC (permalink / raw)
  To: Steven Rostedt, Sebastian Andrzej Siewior
  Cc: linux-kernel, linux-rt-users, Ingo Molnar, Andrew Morton,
	Clark Williams, Thomas Gleixner, Daniel Wagner, Carsten Emde

On 08/05/2016 10:52 AM, Steven Rostedt wrote:

> Ah, this is when we have GENERIC_SCHED_CLOCK, which would break tracing
> if any arch that has this also has NMIs. Probably need to look at arm64.

Today, we don't have NMIs on arm64 that are supported in Linux.

Jon.

-- 
Computer Architect | Sent from my Fedora powered laptop

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

* Re: [RFC][PATCH 2/3] tracing: Add documentation for hwlat_detector tracer
  2016-08-04 14:57 ` [RFC][PATCH 2/3] tracing: Add documentation for hwlat_detector tracer Steven Rostedt
@ 2016-08-10 14:12   ` Jon Masters
  0 siblings, 0 replies; 24+ messages in thread
From: Jon Masters @ 2016-08-10 14:12 UTC (permalink / raw)
  To: Steven Rostedt, linux-kernel, linux-rt-users
  Cc: Ingo Molnar, Andrew Morton, Clark Williams, Thomas Gleixner,
	Daniel Wagner, Carsten Emde, Sebastian Andrzej Siewior

On 08/04/2016 10:57 AM, Steven Rostedt wrote:
> From: Jon Masters <jcm@redhat.com>
> 
> Added the documentation on how to use th hwlat_detector.
> 
> Signed-off-by: Jon Masters <jcm@redhat.com>
> [ Various updates and modified to show hwlat as a tracer ]
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Thanks for cleaning up the original hwlat mess, making it actually
useful and not an embarrassing hack, and shepherding this upstream
Steven. I owe you many beers :)

I wish someone could get as motivated about cleaning my apartment, or
doing my laundry...volunteers are always welcome...

Jon.

-- 
Computer Architect | Sent from my Fedora powered laptop

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

end of thread, other threads:[~2016-08-10 19:21 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-04 14:57 [RFC][PATCH 0/3] tracing: Add Hardware Latency detector tracer Steven Rostedt
2016-08-04 14:57 ` [RFC][PATCH 1/3] tracing: Added hardware latency tracer Steven Rostedt
2016-08-05 14:25   ` Sebastian Andrzej Siewior
2016-08-05 14:44     ` Steven Rostedt
2016-08-05 15:28       ` Sebastian Andrzej Siewior
2016-08-05 15:36         ` Steven Rostedt
2016-08-04 14:57 ` [RFC][PATCH 2/3] tracing: Add documentation for hwlat_detector tracer Steven Rostedt
2016-08-10 14:12   ` Jon Masters
2016-08-04 14:57 ` [RFC][PATCH 3/3] tracing: Have hwlat trace migrate across tracing_cpumask CPUs Steven Rostedt
2016-08-04 15:30 ` [RFC][PATCH 0/3] tracing: Add Hardware Latency detector tracer Steven Rostedt
2016-08-09 18:15   ` Clark Williams
2016-08-09 18:30     ` Steven Rostedt
2016-08-04 16:56 ` [RFC][PATCH 4/3] tracing: Add NMI tracing in hwlat detector Steven Rostedt
2016-08-04 17:16   ` Steven Rostedt
2016-08-05 14:35     ` Sebastian Andrzej Siewior
2016-08-05 14:52       ` Steven Rostedt
2016-08-05 15:40         ` Sebastian Andrzej Siewior
2016-08-05 16:17           ` Steven Rostedt
2016-08-09 17:22           ` Steven Rostedt
2016-08-10 14:10         ` Jon Masters
2016-08-09 18:05 ` [RFC][PATCH 5/3] tracing: Add smi counting to HWLAT Steven Rostedt
2016-08-09 18:28   ` Daniel Bristot de Oliveira
2016-08-09 18:35     ` Steven Rostedt
2016-08-09 21:25   ` Peter Zijlstra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.