All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] ftrace patches for tip
@ 2008-11-13  5:09 Steven Rostedt
  2008-11-13  5:09 ` [PATCH 1/2] ftrace: do not update max buffer with no users Steven Rostedt
  2008-11-13  5:09 ` [PATCH 2/2] ftrace: show buffer size in kilobytes Steven Rostedt
  0 siblings, 2 replies; 4+ messages in thread
From: Steven Rostedt @ 2008-11-13  5:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Arjan van de Ven,
	Frederic Weisbecker

Ingo,

Note: first patch is the same as the one for 2.6.28. I just ported it for
tip but you can ignore it if you already pulled that one in.

The following patches are in:

  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git

    branch: tip/devel


Steven Rostedt (2):
      ftrace: do not update max buffer with no users
      ftrace: show buffer size in kilobytes

----
 Documentation/ftrace.txt |   22 ++-----
 kernel/trace/trace.c     |  176 ++++++++++++++++++++++++----------------------
 2 files changed, 97 insertions(+), 101 deletions(-)

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

* [PATCH 1/2] ftrace: do not update max buffer with no users
  2008-11-13  5:09 [PATCH 0/2] ftrace patches for tip Steven Rostedt
@ 2008-11-13  5:09 ` Steven Rostedt
  2008-11-13  5:09 ` [PATCH 2/2] ftrace: show buffer size in kilobytes Steven Rostedt
  1 sibling, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2008-11-13  5:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Arjan van de Ven,
	Frederic Weisbecker, Steven Rostedt

[-- Attachment #1: 0001-ftrace-do-not-update-max-buffer-with-no-users.patch --]
[-- Type: text/plain, Size: 6978 bytes --]

Impact: only use max latency buffer when a user is configured

Pekka reported a bug with the resizing of the buffers when only the
MMIO tracer was configured. The issue was, to save memory, the max
latency trace buffer was only initialized if a tracer that uses it
is configured in.

What happened was that the max latency buffer was never initialized
when only the MMIO tracer was configurued. The MMIO tracer does not
use the max tracer, which kept it from being initialized. But the
resize code still tried to resize the max latency buffers, but because
they were never allocated, the resize code was passed a NULL pointer.

This patch checks if the max tracer is NULL before resizing it.
It also hides the modification functions of the max tracer behind
the TRACER_MAX_TRACE config.

Reported-by: Pekka Paalanen <pq@iki.fi>
Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 kernel/trace/trace.c |  171 ++++++++++++++++++++++++++------------------------
 1 files changed, 88 insertions(+), 83 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 40c9cc1..20fe77d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -133,20 +133,6 @@ static struct trace_array	global_trace;
 
 static DEFINE_PER_CPU(struct trace_array_cpu, global_trace_cpu);
 
-/*
- * The max_tr is used to snapshot the global_trace when a maximum
- * latency is reached. Some tracers will use this to store a maximum
- * trace while it continues examining live traces.
- *
- * The buffers for the max_tr are set up the same as the global_trace.
- * When a snapshot is taken, the link list of the max_tr is swapped
- * with the link list of the global_trace and the buffers are reset for
- * the global_trace so the tracing can continue.
- */
-static struct trace_array	max_tr;
-
-static DEFINE_PER_CPU(struct trace_array_cpu, max_data);
-
 /* tracer_enabled is used to toggle activation of a tracer */
 static int			tracer_enabled = 1;
 
@@ -279,6 +265,21 @@ static raw_spinlock_t ftrace_max_lock =
 	(raw_spinlock_t)__RAW_SPIN_LOCK_UNLOCKED;
 
 /*
+ * The max_tr is used to snapshot the global_trace when a maximum
+ * latency is reached. Some tracers will use this to store a maximum
+ * trace while it continues examining live traces.
+ *
+ * The buffers for the max_tr are set up the same as the global_trace.
+ * When a snapshot is taken, the link list of the max_tr is swapped
+ * with the link list of the global_trace and the buffers are reset for
+ * the global_trace so the tracing can continue.
+ */
+static struct trace_array	max_tr;
+
+static DEFINE_PER_CPU(struct trace_array_cpu, max_data);
+
+#ifdef CONFIG_TRACER_MAX_TRACE
+/*
  * Copy the new maximum trace into the separate maximum-trace
  * structure. (this way the maximum trace is permanently saved,
  * for later retrieval via /debugfs/tracing/latency_trace)
@@ -306,6 +307,65 @@ __update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu)
 }
 
 /**
+ * update_max_tr - snapshot all trace buffers from global_trace to max_tr
+ * @tr: tracer
+ * @tsk: the task with the latency
+ * @cpu: The cpu that initiated the trace.
+ *
+ * Flip the buffers between the @tr and the max_tr and record information
+ * about which task was the cause of this latency.
+ */
+void
+update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu)
+{
+	struct ring_buffer *buf = tr->buffer;
+
+	WARN_ON_ONCE(!irqs_disabled());
+	__raw_spin_lock(&ftrace_max_lock);
+
+	tr->buffer = max_tr.buffer;
+	max_tr.buffer = buf;
+
+	ftrace_disable_cpu();
+	ring_buffer_reset(tr->buffer);
+	ftrace_enable_cpu();
+
+	__update_max_tr(tr, tsk, cpu);
+	__raw_spin_unlock(&ftrace_max_lock);
+}
+
+/**
+ * update_max_tr_single - only copy one trace over, and reset the rest
+ * @tr - tracer
+ * @tsk - task with the latency
+ * @cpu - the cpu of the buffer to copy.
+ *
+ * Flip the trace of a single CPU buffer between the @tr and the max_tr.
+ */
+void
+update_max_tr_single(struct trace_array *tr, struct task_struct *tsk, int cpu)
+{
+	int ret;
+
+	WARN_ON_ONCE(!irqs_disabled());
+	__raw_spin_lock(&ftrace_max_lock);
+
+	ftrace_disable_cpu();
+
+	ring_buffer_reset(max_tr.buffer);
+	ret = ring_buffer_swap_cpu(max_tr.buffer, tr->buffer, cpu);
+
+	ftrace_enable_cpu();
+
+	WARN_ON_ONCE(ret);
+
+	__update_max_tr(tr, tsk, cpu);
+	__raw_spin_unlock(&ftrace_max_lock);
+}
+
+#endif /* CONFIG_TRACER_MAX_TRACE */
+
+/**
  * trace_seq_printf - sequence printing of trace information
  * @s: trace sequence descriptor
  * @fmt: printf format string
@@ -447,63 +507,6 @@ trace_print_seq(struct seq_file *m, struct trace_seq *s)
 }
 
 /**
- * update_max_tr - snapshot all trace buffers from global_trace to max_tr
- * @tr: tracer
- * @tsk: the task with the latency
- * @cpu: The cpu that initiated the trace.
- *
- * Flip the buffers between the @tr and the max_tr and record information
- * about which task was the cause of this latency.
- */
-void
-update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu)
-{
-	struct ring_buffer *buf = tr->buffer;
-
-	WARN_ON_ONCE(!irqs_disabled());
-	__raw_spin_lock(&ftrace_max_lock);
-
-	tr->buffer = max_tr.buffer;
-	max_tr.buffer = buf;
-
-	ftrace_disable_cpu();
-	ring_buffer_reset(tr->buffer);
-	ftrace_enable_cpu();
-
-	__update_max_tr(tr, tsk, cpu);
-	__raw_spin_unlock(&ftrace_max_lock);
-}
-
-/**
- * update_max_tr_single - only copy one trace over, and reset the rest
- * @tr - tracer
- * @tsk - task with the latency
- * @cpu - the cpu of the buffer to copy.
- *
- * Flip the trace of a single CPU buffer between the @tr and the max_tr.
- */
-void
-update_max_tr_single(struct trace_array *tr, struct task_struct *tsk, int cpu)
-{
-	int ret;
-
-	WARN_ON_ONCE(!irqs_disabled());
-	__raw_spin_lock(&ftrace_max_lock);
-
-	ftrace_disable_cpu();
-
-	ring_buffer_reset(max_tr.buffer);
-	ret = ring_buffer_swap_cpu(max_tr.buffer, tr->buffer, cpu);
-
-	ftrace_enable_cpu();
-
-	WARN_ON_ONCE(ret);
-
-	__update_max_tr(tr, tsk, cpu);
-	__raw_spin_unlock(&ftrace_max_lock);
-}
-
-/**
  * register_tracer - register a tracer with the ftrace system.
  * @type - the plugin for the tracer
  *
@@ -2966,19 +2969,21 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
 			goto out;
 		}
 
-		ret = ring_buffer_resize(max_tr.buffer, val);
-		if (ret < 0) {
-			int r;
-			cnt = ret;
-			r = ring_buffer_resize(global_trace.buffer,
-					       global_trace.entries);
-			if (r < 0) {
-				/* AARGH! We are left with different
-				 * size max buffer!!!! */
-				WARN_ON(1);
-				tracing_disabled = 1;
+		if (max_tr.buffer) {
+			ret = ring_buffer_resize(max_tr.buffer, val);
+			if (ret < 0) {
+				int r;
+				cnt = ret;
+				r = ring_buffer_resize(global_trace.buffer,
+						       global_trace.entries);
+				if (r < 0) {
+					/* AARGH! We are left with different
+					 * size max buffer!!!! */
+					WARN_ON(1);
+					tracing_disabled = 1;
+				}
+				goto out;
 			}
-			goto out;
 		}
 
 		global_trace.entries = val;
-- 
1.5.6.5

-- 

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

* [PATCH 2/2] ftrace: show buffer size in kilobytes
  2008-11-13  5:09 [PATCH 0/2] ftrace patches for tip Steven Rostedt
  2008-11-13  5:09 ` [PATCH 1/2] ftrace: do not update max buffer with no users Steven Rostedt
@ 2008-11-13  5:09 ` Steven Rostedt
  2008-11-13  5:54   ` Arjan van de Ven
  1 sibling, 1 reply; 4+ messages in thread
From: Steven Rostedt @ 2008-11-13  5:09 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Arjan van de Ven,
	Frederic Weisbecker, Steven Rostedt

[-- Attachment #1: 0002-ftrace-show-buffer-size-in-kilobytes.patch --]
[-- Type: text/plain, Size: 3208 bytes --]

Impact: change the units of buffer_size to kilobytes

This patch changes the units of the buffer_size file to kilobytes.
Reading and writing to the file uses kilobytes as units. To help
users to know what units are used, the output of the file now
looks like:

  # cat /debug/tracing/buffer_size
  1408 (units kilobytes)

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 Documentation/ftrace.txt |   22 +++++-----------------
 kernel/trace/trace.c     |    5 ++++-
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/Documentation/ftrace.txt b/Documentation/ftrace.txt
index 01ac404..6fe99b4 100644
--- a/Documentation/ftrace.txt
+++ b/Documentation/ftrace.txt
@@ -94,10 +94,10 @@ of ftrace. Here is a list of some of the key files:
 		only be recorded if the latency is greater than
 		the value in this file. (in microseconds)
 
-  buffer_size: This sets or displays the number of bytes each CPU
+  buffer_size: This sets or displays the number of kilobytes each CPU
 		buffer can hold. The tracer buffers are the same size
 		for each CPU. The displayed number is the size of the
-		 CPU buffer and not total size of all buffers. The
+		CPU buffer and not total size of all buffers. The
 		trace buffers are allocated in pages (blocks of memory
 		that the kernel uses for allocation, usually 4 KB in size).
 		If the last page allocated has room for more bytes
@@ -1306,28 +1306,16 @@ the full size, multiply the number of possible CPUS with the
 number of entries.
 
  # cat /debug/tracing/buffer_size
-65620
+1408 (units kilobytes)
 
 Note, to modify this, you must have tracing completely disabled. To do that,
 echo "nop" into the current_tracer. If the current_tracer is not set
 to "nop", an EINVAL error will be returned.
 
  # echo nop > /debug/tracing/current_tracer
- # echo 100000 > /debug/tracing/buffer_size
+ # echo 10000 > /debug/tracing/buffer_size
  # cat /debug/tracing/buffer_size
-100045
-
-
-Notice that we echoed in 100,000 but the size is 100,045. The entries
-are held in individual pages. It allocates the number of pages it takes
-to fulfill the request. If more entries may fit on the last page
-then they will be added.
-
- # echo 1 > /debug/tracing/buffer_size
- # cat /debug/tracing/buffer_size
-85
-
-This shows us that 85 entries can fit in a single page.
+10000 (units kilobytes)
 
 The number of pages which will be allocated is limited to a percentage
 of available memory. Allocating too much will produce an error.
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 20fe77d..10f7de2 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2922,7 +2922,7 @@ tracing_entries_read(struct file *filp, char __user *ubuf,
 	char buf[64];
 	int r;
 
-	r = sprintf(buf, "%lu\n", tr->entries);
+	r = snprintf(buf, 64, "%lu (units kilobytes)\n", tr->entries >> 10);
 	return simple_read_from_buffer(ubuf, cnt, ppos, buf, r);
 }
 
@@ -2962,6 +2962,9 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
 			atomic_inc(&max_tr.data[cpu]->disabled);
 	}
 
+	/* value is in KB */
+	val <<= 10;
+
 	if (val != global_trace.entries) {
 		ret = ring_buffer_resize(global_trace.buffer, val);
 		if (ret < 0) {
-- 
1.5.6.5

-- 

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

* Re: [PATCH 2/2] ftrace: show buffer size in kilobytes
  2008-11-13  5:09 ` [PATCH 2/2] ftrace: show buffer size in kilobytes Steven Rostedt
@ 2008-11-13  5:54   ` Arjan van de Ven
  0 siblings, 0 replies; 4+ messages in thread
From: Arjan van de Ven @ 2008-11-13  5:54 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
	Frederic Weisbecker, Steven Rostedt

On Thu, 13 Nov 2008 00:09:35 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> Impact: change the units of buffer_size to kilobytes
> 
> This patch changes the units of the buffer_size file to kilobytes.
> Reading and writing to the file uses kilobytes as units. To help
> users to know what units are used, the output of the file now
> looks like:
> 
>   # cat /debug/tracing/buffer_size
>   1408 (units kilobytes)

I much rather would call the file

buffer_size_kb

and then be done with it... that seems to be the convention lately at
least


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

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

end of thread, other threads:[~2008-11-13  5:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-11-13  5:09 [PATCH 0/2] ftrace patches for tip Steven Rostedt
2008-11-13  5:09 ` [PATCH 1/2] ftrace: do not update max buffer with no users Steven Rostedt
2008-11-13  5:09 ` [PATCH 2/2] ftrace: show buffer size in kilobytes Steven Rostedt
2008-11-13  5:54   ` Arjan van de Ven

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.