[PATCH 0/5] ftrace updates for tip

[PATCH 0/5] ftrace updates for tip

[Steven Rostedt]

Ingo,

Here's some more ftrace updates.

The first should help fix the bug in lockdep.

The next are changes to the ring buffer to get ready to have it
work with splice.

The last are some clean ups for function graph.


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 (5):
      ftrace: replace raw_local_irq_save with local_irq_save
      ring-buffer: move some metadata into buffer page
      ring-buffer: read page interface
      ftrace: clean up function graph asm
      ftrace: have function graph use mcount caller address

----
 arch/x86/kernel/entry_32.S  |    1 +
 arch/x86/kernel/entry_64.S  |    1 +
 arch/x86/kernel/ftrace.c    |   20 +---
 include/linux/ring_buffer.h |    5 +
 kernel/lockdep.c            |    1 +
 kernel/trace/ring_buffer.c  |  229 ++++++++++++++++++++++++++++++++++++++-----
 kernel/trace/trace.c        |   12 +-
 kernel/trace/trace_branch.c |    4 +-
 kernel/trace/trace_stack.c  |    8 +-
 9 files changed, 226 insertions(+), 55 deletions(-)
-- 

[PATCH 1/5] ftrace: replace raw_local_irq_save with local_irq_save

[Steven Rostedt]

[-- Attachment #1: 0001-ftrace-replace-raw_local_irq_save-with-local_irq_sa.patch --]
[-- Type: text/plain, Size: 4158 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Impact: fix for lockdep and ftrace

The raw_local_irq_save/restore confuses lockdep. This patch
converts them to the local_irq_save/restore variants.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 kernel/lockdep.c            |    1 +
 kernel/trace/trace.c        |   12 ++++++------
 kernel/trace/trace_branch.c |    4 ++--
 kernel/trace/trace_stack.c  |    8 ++++----
 4 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/kernel/lockdep.c b/kernel/lockdep.c
index 670aba8..1eb7c0b 100644
--- a/kernel/lockdep.c
+++ b/kernel/lockdep.c
@@ -25,6 +25,7 @@
  * Thanks to Arjan van de Ven for coming up with the initial idea of
  * mapping lock dependencies runtime.
  */
+#define DISABLE_BRANCH_PROFILING
 #include <linux/mutex.h>
 #include <linux/sched.h>
 #include <linux/delay.h>
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 91887a2..380de63 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1209,7 +1209,7 @@ void trace_graph_entry(struct ftrace_graph_ent *trace)
 	int cpu;
 	int pc;
 
-	raw_local_irq_save(flags);
+	local_irq_save(flags);
 	cpu = raw_smp_processor_id();
 	data = tr->data[cpu];
 	disabled = atomic_inc_return(&data->disabled);
@@ -1218,7 +1218,7 @@ void trace_graph_entry(struct ftrace_graph_ent *trace)
 		__trace_graph_entry(tr, data, trace, flags, pc);
 	}
 	atomic_dec(&data->disabled);
-	raw_local_irq_restore(flags);
+	local_irq_restore(flags);
 }
 
 void trace_graph_return(struct ftrace_graph_ret *trace)
@@ -1230,7 +1230,7 @@ void trace_graph_return(struct ftrace_graph_ret *trace)
 	int cpu;
 	int pc;
 
-	raw_local_irq_save(flags);
+	local_irq_save(flags);
 	cpu = raw_smp_processor_id();
 	data = tr->data[cpu];
 	disabled = atomic_inc_return(&data->disabled);
@@ -1239,7 +1239,7 @@ void trace_graph_return(struct ftrace_graph_ret *trace)
 		__trace_graph_return(tr, data, trace, flags, pc);
 	}
 	atomic_dec(&data->disabled);
-	raw_local_irq_restore(flags);
+	local_irq_restore(flags);
 }
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
 
@@ -2645,7 +2645,7 @@ tracing_cpumask_write(struct file *filp, const char __user *ubuf,
 	if (err)
 		goto err_unlock;
 
-	raw_local_irq_disable();
+	local_irq_disable();
 	__raw_spin_lock(&ftrace_max_lock);
 	for_each_tracing_cpu(cpu) {
 		/*
@@ -2662,7 +2662,7 @@ tracing_cpumask_write(struct file *filp, const char __user *ubuf,
 		}
 	}
 	__raw_spin_unlock(&ftrace_max_lock);
-	raw_local_irq_enable();
+	local_irq_enable();
 
 	tracing_cpumask = tracing_cpumask_new;
 
diff --git a/kernel/trace/trace_branch.c b/kernel/trace/trace_branch.c
index bc97275..6c00feb 100644
--- a/kernel/trace/trace_branch.c
+++ b/kernel/trace/trace_branch.c
@@ -42,7 +42,7 @@ probe_likely_condition(struct ftrace_branch_data *f, int val, int expect)
 	if (unlikely(!tr))
 		return;
 
-	raw_local_irq_save(flags);
+	local_irq_save(flags);
 	cpu = raw_smp_processor_id();
 	if (atomic_inc_return(&tr->data[cpu]->disabled) != 1)
 		goto out;
@@ -74,7 +74,7 @@ probe_likely_condition(struct ftrace_branch_data *f, int val, int expect)
 
  out:
 	atomic_dec(&tr->data[cpu]->disabled);
-	raw_local_irq_restore(flags);
+	local_irq_restore(flags);
 }
 
 static inline
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index fde3be1..06a1611 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -48,7 +48,7 @@ static inline void check_stack(void)
 	if (!object_is_on_stack(&this_size))
 		return;
 
-	raw_local_irq_save(flags);
+	local_irq_save(flags);
 	__raw_spin_lock(&max_stack_lock);
 
 	/* a race could have already updated it */
@@ -96,7 +96,7 @@ static inline void check_stack(void)
 
  out:
 	__raw_spin_unlock(&max_stack_lock);
-	raw_local_irq_restore(flags);
+	local_irq_restore(flags);
 }
 
 static void
@@ -162,11 +162,11 @@ stack_max_size_write(struct file *filp, const char __user *ubuf,
 	if (ret < 0)
 		return ret;
 
-	raw_local_irq_save(flags);
+	local_irq_save(flags);
 	__raw_spin_lock(&max_stack_lock);
 	*ptr = val;
 	__raw_spin_unlock(&max_stack_lock);
-	raw_local_irq_restore(flags);
+	local_irq_restore(flags);
 
 	return count;
 }
-- 
1.5.6.5

-- 

Re: [PATCH 1/5] ftrace: replace raw_local_irq_save with local_irq_save

[Wang Liming]

hi Ingo,

Where could I find define of "local_irq_save" for x86 if 
CONFIG_TRACE_IRQFLAGS_SUPPORT is not defined?


Liming Wang

[PATCH 2/5] ring-buffer: move some metadata into buffer page

[Steven Rostedt]

[-- Attachment #1: 0002-ring-buffer-move-some-metadata-into-buffer-page.patch --]
[-- Type: text/plain, Size: 7779 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Impact: get ready for splice changes

This patch moves the commit and timestamp into the beginning of each
data page of the buffer. This change will allow the page to be moved
to another location (disk, network, etc) and still have information
in the page to be able to read it.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 kernel/trace/ring_buffer.c |   63 +++++++++++++++++++++++++-------------------
 1 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index e206951..8619c53 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -195,20 +195,24 @@ void *ring_buffer_event_data(struct ring_buffer_event *event)
 #define TS_MASK		((1ULL << TS_SHIFT) - 1)
 #define TS_DELTA_TEST	(~TS_MASK)
 
-/*
- * This hack stolen from mm/slob.c.
- * We can store per page timing information in the page frame of the page.
- * Thanks to Peter Zijlstra for suggesting this idea.
- */
-struct buffer_page {
+struct buffer_data_page {
 	u64		 time_stamp;	/* page time stamp */
-	local_t		 write;		/* index for next write */
 	local_t		 commit;	/* write commited index */
+	unsigned char	 data[];	/* data of buffer page */
+};
+
+struct buffer_page {
+	local_t		 write;		/* index for next write */
 	unsigned	 read;		/* index for next read */
 	struct list_head list;		/* list of free pages */
-	void *page;			/* Actual data page */
+	struct buffer_data_page *page;	/* Actual data page */
 };
 
+static void rb_init_page(struct buffer_data_page *page)
+{
+	local_set(&page->commit, 0);
+}
+
 /*
  * Also stolen from mm/slob.c. Thanks to Mathieu Desnoyers for pointing
  * this issue out.
@@ -230,7 +234,7 @@ static inline int test_time_stamp(u64 delta)
 	return 0;
 }
 
-#define BUF_PAGE_SIZE PAGE_SIZE
+#define BUF_PAGE_SIZE (PAGE_SIZE - sizeof(struct buffer_data_page))
 
 /*
  * head_page == tail_page && head == tail then buffer is empty.
@@ -333,6 +337,7 @@ static int rb_allocate_pages(struct ring_buffer_per_cpu *cpu_buffer,
 		if (!addr)
 			goto free_pages;
 		page->page = (void *)addr;
+		rb_init_page(page->page);
 	}
 
 	list_splice(&pages, head);
@@ -378,6 +383,7 @@ rb_allocate_cpu_buffer(struct ring_buffer *buffer, int cpu)
 	if (!addr)
 		goto fail_free_reader;
 	page->page = (void *)addr;
+	rb_init_page(page->page);
 
 	INIT_LIST_HEAD(&cpu_buffer->reader_page->list);
 
@@ -647,6 +653,7 @@ int ring_buffer_resize(struct ring_buffer *buffer, unsigned long size)
 			if (!addr)
 				goto free_pages;
 			page->page = (void *)addr;
+			rb_init_page(page->page);
 		}
 	}
 
@@ -682,7 +689,7 @@ static inline int rb_null_event(struct ring_buffer_event *event)
 
 static inline void *__rb_page_index(struct buffer_page *page, unsigned index)
 {
-	return page->page + index;
+	return page->page->data + index;
 }
 
 static inline struct ring_buffer_event *
@@ -712,7 +719,7 @@ static inline unsigned rb_page_write(struct buffer_page *bpage)
 
 static inline unsigned rb_page_commit(struct buffer_page *bpage)
 {
-	return local_read(&bpage->commit);
+	return local_read(&bpage->page->commit);
 }
 
 /* Size is determined by what has been commited */
@@ -804,14 +811,15 @@ rb_set_commit_event(struct ring_buffer_per_cpu *cpu_buffer,
 		if (RB_WARN_ON(cpu_buffer,
 			  cpu_buffer->commit_page == cpu_buffer->tail_page))
 			return;
-		cpu_buffer->commit_page->commit =
+		cpu_buffer->commit_page->page->commit =
 			cpu_buffer->commit_page->write;
 		rb_inc_page(cpu_buffer, &cpu_buffer->commit_page);
-		cpu_buffer->write_stamp = cpu_buffer->commit_page->time_stamp;
+		cpu_buffer->write_stamp =
+			cpu_buffer->commit_page->page->time_stamp;
 	}
 
 	/* Now set the commit to the event's index */
-	local_set(&cpu_buffer->commit_page->commit, index);
+	local_set(&cpu_buffer->commit_page->page->commit, index);
 }
 
 static inline void
@@ -826,16 +834,17 @@ rb_set_commit_to_write(struct ring_buffer_per_cpu *cpu_buffer)
 	 * assign the commit to the tail.
 	 */
 	while (cpu_buffer->commit_page != cpu_buffer->tail_page) {
-		cpu_buffer->commit_page->commit =
+		cpu_buffer->commit_page->page->commit =
 			cpu_buffer->commit_page->write;
 		rb_inc_page(cpu_buffer, &cpu_buffer->commit_page);
-		cpu_buffer->write_stamp = cpu_buffer->commit_page->time_stamp;
+		cpu_buffer->write_stamp =
+			cpu_buffer->commit_page->page->time_stamp;
 		/* add barrier to keep gcc from optimizing too much */
 		barrier();
 	}
 	while (rb_commit_index(cpu_buffer) !=
 	       rb_page_write(cpu_buffer->commit_page)) {
-		cpu_buffer->commit_page->commit =
+		cpu_buffer->commit_page->page->commit =
 			cpu_buffer->commit_page->write;
 		barrier();
 	}
@@ -843,7 +852,7 @@ rb_set_commit_to_write(struct ring_buffer_per_cpu *cpu_buffer)
 
 static void rb_reset_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
 {
-	cpu_buffer->read_stamp = cpu_buffer->reader_page->time_stamp;
+	cpu_buffer->read_stamp = cpu_buffer->reader_page->page->time_stamp;
 	cpu_buffer->reader_page->read = 0;
 }
 
@@ -862,7 +871,7 @@ static inline void rb_inc_iter(struct ring_buffer_iter *iter)
 	else
 		rb_inc_page(cpu_buffer, &iter->head_page);
 
-	iter->read_stamp = iter->head_page->time_stamp;
+	iter->read_stamp = iter->head_page->page->time_stamp;
 	iter->head = 0;
 }
 
@@ -998,12 +1007,12 @@ __rb_reserve_next(struct ring_buffer_per_cpu *cpu_buffer,
 		 */
 		if (tail_page == cpu_buffer->tail_page) {
 			local_set(&next_page->write, 0);
-			local_set(&next_page->commit, 0);
+			local_set(&next_page->page->commit, 0);
 			cpu_buffer->tail_page = next_page;
 
 			/* reread the time stamp */
 			*ts = ring_buffer_time_stamp(cpu_buffer->cpu);
-			cpu_buffer->tail_page->time_stamp = *ts;
+			cpu_buffer->tail_page->page->time_stamp = *ts;
 		}
 
 		/*
@@ -1048,7 +1057,7 @@ __rb_reserve_next(struct ring_buffer_per_cpu *cpu_buffer,
 	 * this page's time stamp.
 	 */
 	if (!tail && rb_is_commit(cpu_buffer, event))
-		cpu_buffer->commit_page->time_stamp = *ts;
+		cpu_buffer->commit_page->page->time_stamp = *ts;
 
 	return event;
 
@@ -1099,7 +1108,7 @@ rb_add_time_stamp(struct ring_buffer_per_cpu *cpu_buffer,
 			event->time_delta = *delta & TS_MASK;
 			event->array[0] = *delta >> TS_SHIFT;
 		} else {
-			cpu_buffer->commit_page->time_stamp = *ts;
+			cpu_buffer->commit_page->page->time_stamp = *ts;
 			event->time_delta = 0;
 			event->array[0] = 0;
 		}
@@ -1552,7 +1561,7 @@ static void rb_iter_reset(struct ring_buffer_iter *iter)
 	if (iter->head)
 		iter->read_stamp = cpu_buffer->read_stamp;
 	else
-		iter->read_stamp = iter->head_page->time_stamp;
+		iter->read_stamp = iter->head_page->page->time_stamp;
 }
 
 /**
@@ -1696,7 +1705,7 @@ rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
 	cpu_buffer->reader_page->list.prev = reader->list.prev;
 
 	local_set(&cpu_buffer->reader_page->write, 0);
-	local_set(&cpu_buffer->reader_page->commit, 0);
+	local_set(&cpu_buffer->reader_page->page->commit, 0);
 
 	/* Make the reader page now replace the head */
 	reader->list.prev->next = &cpu_buffer->reader_page->list;
@@ -2088,7 +2097,7 @@ rb_reset_cpu(struct ring_buffer_per_cpu *cpu_buffer)
 	cpu_buffer->head_page
 		= list_entry(cpu_buffer->pages.next, struct buffer_page, list);
 	local_set(&cpu_buffer->head_page->write, 0);
-	local_set(&cpu_buffer->head_page->commit, 0);
+	local_set(&cpu_buffer->head_page->page->commit, 0);
 
 	cpu_buffer->head_page->read = 0;
 
@@ -2097,7 +2106,7 @@ rb_reset_cpu(struct ring_buffer_per_cpu *cpu_buffer)
 
 	INIT_LIST_HEAD(&cpu_buffer->reader_page->list);
 	local_set(&cpu_buffer->reader_page->write, 0);
-	local_set(&cpu_buffer->reader_page->commit, 0);
+	local_set(&cpu_buffer->reader_page->page->commit, 0);
 	cpu_buffer->reader_page->read = 0;
 
 	cpu_buffer->overrun = 0;
-- 
1.5.6.5

-- 

[PATCH 3/5] ring-buffer: read page interface

[Steven Rostedt]

[-- Attachment #1: 0003-ring-buffer-read-page-interface.patch --]
[-- Type: text/plain, Size: 6978 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Impact: new API to ring buffer

This patch adds a new interface into the ring buffer that allows a
page to be read from the ring buffer on a given CPU. For every page
read, one must also be given to allow for a "swap" of the pages.

 rpage = ring_buffer_alloc_read_page(buffer);
 if (!rpage)
	goto err;
 ret = ring_buffer_read_page(buffer, &rpage, cpu, full);
 if (!ret)
	goto empty;
 process_page(rpage);
 ring_buffer_free_read_page(rpage);

The caller of these functions must handle any waits that are
needed to wait for new data. The ring_buffer_read_page will simply
return 0 if there is no data, or if "full" is set and the writer
is still on the current page.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 include/linux/ring_buffer.h |    5 ++
 kernel/trace/ring_buffer.c  |  166 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 171 insertions(+), 0 deletions(-)

diff --git a/include/linux/ring_buffer.h b/include/linux/ring_buffer.h
index 3bb87a7..1a350a8 100644
--- a/include/linux/ring_buffer.h
+++ b/include/linux/ring_buffer.h
@@ -124,6 +124,11 @@ void tracing_on(void);
 void tracing_off(void);
 void tracing_off_permanent(void);
 
+void *ring_buffer_alloc_read_page(struct ring_buffer *buffer);
+void ring_buffer_free_read_page(struct ring_buffer *buffer, void *data);
+int ring_buffer_read_page(struct ring_buffer *buffer,
+			  void **data_page, int cpu, int full);
+
 enum ring_buffer_flags {
 	RB_FL_OVERWRITE		= 1 << 0,
 };
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 8619c53..50b74d3 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -687,6 +687,12 @@ static inline int rb_null_event(struct ring_buffer_event *event)
 	return event->type == RINGBUF_TYPE_PADDING;
 }
 
+static inline void *
+__rb_data_page_index(struct buffer_data_page *page, unsigned index)
+{
+	return page->data + index;
+}
+
 static inline void *__rb_page_index(struct buffer_page *page, unsigned index)
 {
 	return page->page->data + index;
@@ -2232,6 +2238,166 @@ int ring_buffer_swap_cpu(struct ring_buffer *buffer_a,
 	return 0;
 }
 
+static void rb_remove_entries(struct ring_buffer_per_cpu *cpu_buffer,
+			      struct buffer_data_page *page)
+{
+	struct ring_buffer_event *event;
+	unsigned long head;
+
+	__raw_spin_lock(&cpu_buffer->lock);
+	for (head = 0; head < local_read(&page->commit);
+	     head += rb_event_length(event)) {
+
+		event = __rb_data_page_index(page, head);
+		if (RB_WARN_ON(cpu_buffer, rb_null_event(event)))
+			return;
+		/* Only count data entries */
+		if (event->type != RINGBUF_TYPE_DATA)
+			continue;
+		cpu_buffer->entries--;
+	}
+	__raw_spin_unlock(&cpu_buffer->lock);
+}
+
+/**
+ * ring_buffer_alloc_read_page - allocate a page to read from buffer
+ * @buffer: the buffer to allocate for.
+ *
+ * This function is used in conjunction with ring_buffer_read_page.
+ * When reading a full page from the ring buffer, these functions
+ * can be used to speed up the process. The calling function should
+ * allocate a few pages first with this function. Then when it
+ * needs to get pages from the ring buffer, it passes the result
+ * of this function into ring_buffer_read_page, which will swap
+ * the page that was allocated, with the read page of the buffer.
+ *
+ * Returns:
+ *  The page allocated, or NULL on error.
+ */
+void *ring_buffer_alloc_read_page(struct ring_buffer *buffer)
+{
+	unsigned long addr;
+	struct buffer_data_page *page;
+
+	addr = __get_free_page(GFP_KERNEL);
+	if (!addr)
+		return NULL;
+
+	page = (void *)addr;
+
+	return page;
+}
+
+/**
+ * ring_buffer_free_read_page - free an allocated read page
+ * @buffer: the buffer the page was allocate for
+ * @data: the page to free
+ *
+ * Free a page allocated from ring_buffer_alloc_read_page.
+ */
+void ring_buffer_free_read_page(struct ring_buffer *buffer, void *data)
+{
+	free_page((unsigned long)data);
+}
+
+/**
+ * ring_buffer_read_page - extract a page from the ring buffer
+ * @buffer: buffer to extract from
+ * @data_page: the page to use allocated from ring_buffer_alloc_read_page
+ * @cpu: the cpu of the buffer to extract
+ * @full: should the extraction only happen when the page is full.
+ *
+ * This function will pull out a page from the ring buffer and consume it.
+ * @data_page must be the address of the variable that was returned
+ * from ring_buffer_alloc_read_page. This is because the page might be used
+ * to swap with a page in the ring buffer.
+ *
+ * for example:
+ *	rpage = ring_buffer_alloc_page(buffer);
+ *	if (!rpage)
+ *		return error;
+ *	ret = ring_buffer_read_page(buffer, &rpage, cpu, 0);
+ *	if (ret)
+ *		process_page(rpage);
+ *
+ * When @full is set, the function will not return true unless
+ * the writer is off the reader page.
+ *
+ * Note: it is up to the calling functions to handle sleeps and wakeups.
+ *  The ring buffer can be used anywhere in the kernel and can not
+ *  blindly call wake_up. The layer that uses the ring buffer must be
+ *  responsible for that.
+ *
+ * Returns:
+ *  1 if data has been transferred
+ *  0 if no data has been transferred.
+ */
+int ring_buffer_read_page(struct ring_buffer *buffer,
+			    void **data_page, int cpu, int full)
+{
+	struct ring_buffer_per_cpu *cpu_buffer = buffer->buffers[cpu];
+	struct ring_buffer_event *event;
+	struct buffer_data_page *page;
+	unsigned long flags;
+	int ret = 0;
+
+	if (!data_page)
+		return 0;
+
+	page = *data_page;
+	if (!page)
+		return 0;
+
+	spin_lock_irqsave(&cpu_buffer->reader_lock, flags);
+
+	/*
+	 * rb_buffer_peek will get the next ring buffer if
+	 * the current reader page is empty.
+	 */
+	event = rb_buffer_peek(buffer, cpu, NULL);
+	if (!event)
+		goto out;
+
+	/* check for data */
+	if (!local_read(&cpu_buffer->reader_page->page->commit))
+		goto out;
+	/*
+	 * If the writer is already off of the read page, then simply
+	 * switch the read page with the given page. Otherwise
+	 * we need to copy the data from the reader to the writer.
+	 */
+	if (cpu_buffer->reader_page == cpu_buffer->commit_page) {
+		unsigned int read = cpu_buffer->reader_page->read;
+
+		if (full)
+			goto out;
+		/* The writer is still on the reader page, we must copy */
+		page = cpu_buffer->reader_page->page;
+		memcpy(page->data,
+		       cpu_buffer->reader_page->page->data + read,
+		       local_read(&page->commit) - read);
+
+		/* consume what was read */
+		cpu_buffer->reader_page += read;
+
+	} else {
+		/* swap the pages */
+		rb_init_page(page);
+		page = cpu_buffer->reader_page->page;
+		cpu_buffer->reader_page->page = *data_page;
+		cpu_buffer->reader_page->read = 0;
+		*data_page = page;
+	}
+	ret = 1;
+
+	/* update the entry counter */
+	rb_remove_entries(cpu_buffer, page);
+ out:
+	spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
+
+	return ret;
+}
+
 static ssize_t
 rb_simple_read(struct file *filp, char __user *ubuf,
 	       size_t cnt, loff_t *ppos)
-- 
1.5.6.5

-- 

Re: [PATCH 3/5] ring-buffer: read page interface

[Andrew Morton]

On Tue, 02 Dec 2008 15:34:07 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <srostedt@redhat.com>
> 
> Impact: new API to ring buffer
> 
> This patch adds a new interface into the ring buffer that allows a
> page to be read from the ring buffer on a given CPU. For every page
> read, one must also be given to allow for a "swap" of the pages.
> 
>  rpage = ring_buffer_alloc_read_page(buffer);
>  if (!rpage)
> 	goto err;
>  ret = ring_buffer_read_page(buffer, &rpage, cpu, full);
>  if (!ret)
> 	goto empty;
>  process_page(rpage);
>  ring_buffer_free_read_page(rpage);
> 
> The caller of these functions must handle any waits that are
> needed to wait for new data. The ring_buffer_read_page will simply
> return 0 if there is no data, or if "full" is set and the writer
> is still on the current page.
> 
>  ...
>
> +static inline void *
> +__rb_data_page_index(struct buffer_data_page *page, unsigned index)
> +{
> +	return page->data + index;
> +}

Many kernel developers will fall over stunned and confused when they
see a variable called `page' which does not have type `struct page *'. 
This is a very strong kernel convention and this code tromps all over
it :(

How's about renaming all of these everywhere to `bpage' or whatever?

>
> ...
>
> +	if (!local_read(&cpu_buffer->reader_page->page->commit))
>

Rusty (I think it was Rusty) has convincingly argued that local_t is
irredeemably busted.  I forget the details?

Re: [PATCH 3/5] ring-buffer: read page interface

[Steven Rostedt]

On Tue, 2 Dec 2008, Andrew Morton wrote:

> On Tue, 02 Dec 2008 15:34:07 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > From: Steven Rostedt <srostedt@redhat.com>
> > 
> > Impact: new API to ring buffer
> > 
> > This patch adds a new interface into the ring buffer that allows a
> > page to be read from the ring buffer on a given CPU. For every page
> > read, one must also be given to allow for a "swap" of the pages.
> > 
> >  rpage = ring_buffer_alloc_read_page(buffer);
> >  if (!rpage)
> > 	goto err;
> >  ret = ring_buffer_read_page(buffer, &rpage, cpu, full);
> >  if (!ret)
> > 	goto empty;
> >  process_page(rpage);
> >  ring_buffer_free_read_page(rpage);
> > 
> > The caller of these functions must handle any waits that are
> > needed to wait for new data. The ring_buffer_read_page will simply
> > return 0 if there is no data, or if "full" is set and the writer
> > is still on the current page.
> > 
> >  ...
> >
> > +static inline void *
> > +__rb_data_page_index(struct buffer_data_page *page, unsigned index)
> > +{
> > +	return page->data + index;
> > +}
> 
> Many kernel developers will fall over stunned and confused when they
> see a variable called `page' which does not have type `struct page *'. 
> This is a very strong kernel convention and this code tromps all over
> it :(
> 
> How's about renaming all of these everywhere to `bpage' or whatever?

OK, I'll send a patch to Ingo to make this update.

> 
> >
> > ...
> >
> > +	if (!local_read(&cpu_buffer->reader_page->page->commit))
> >
> 
> Rusty (I think it was Rusty) has convincingly argued that local_t is
> irredeemably busted.  I forget the details?

Really?? I've been using local for quite a bit. Using atomic seems 
overkill, since I only want to be able to protect against updates by 
interrupts. Perhaps what needs to be done is to fix local?

-- Steve

Re: [PATCH 3/5] ring-buffer: read page interface

[Rusty Russell]

On Wednesday 03 December 2008 08:06:54 Andrew Morton wrote:
> Steven Rostedt <rostedt@goodmis.org> wrote:
> > +	if (!local_read(&cpu_buffer->reader_page->page->commit))
>
> Rusty (I think it was Rusty) has convincingly argued that local_t is
> irredeemably busted.  I forget the details?

That was probably Christoph Lameter.  The x86 implementation needs some love, 
but the API seems reasonable.

On x86-32, the cpu_local_inc should be (as intended) turned into one insn.  
That would be possible on 64 bit once we get zero-based percpu (as will World 
Peace).

Cheers,
Rusty.

[PATCH 4/5] ftrace: clean up function graph asm

[Steven Rostedt]

[-- Attachment #1: 0004-ftrace-clean-up-function-graph-asm.patch --]
[-- Type: text/plain, Size: 1419 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Impact: clean up

There exists macros for x86 asm to handle x86_64 and i386.
This patch updates function graph asm to use them.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 arch/x86/kernel/ftrace.c |   20 ++++----------------
 1 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 5883247..1a5b8f8 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -467,28 +467,16 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
 	 * ignore such a protection.
 	 */
 	asm volatile(
-#ifdef CONFIG_X86_64
-		"1: movq (%[parent_old]), %[old]\n"
-		"2: movq %[return_hooker], (%[parent_replaced])\n"
-#else
-		"1: movl (%[parent_old]), %[old]\n"
-		"2: movl %[return_hooker], (%[parent_replaced])\n"
-#endif
+		"1: " _ASM_MOV " (%[parent_old]), %[old]\n"
+		"2: " _ASM_MOV " %[return_hooker], (%[parent_replaced])\n"
 		"   movl $0, %[faulted]\n"
 
 		".section .fixup, \"ax\"\n"
 		"3: movl $1, %[faulted]\n"
 		".previous\n"
 
-		".section __ex_table, \"a\"\n"
-#ifdef CONFIG_X86_64
-		"   .quad 1b, 3b\n"
-		"   .quad 2b, 3b\n"
-#else
-		"   .long 1b, 3b\n"
-		"   .long 2b, 3b\n"
-#endif
-		".previous\n"
+		_ASM_EXTABLE(1b, 3b)
+		_ASM_EXTABLE(2b, 3b)
 
 		: [parent_replaced] "=r" (parent), [old] "=r" (old),
 		  [faulted] "=r" (faulted)
-- 
1.5.6.5

-- 

Re: [PATCH 4/5] ftrace: clean up function graph asm

[Frédéric Weisbecker]

2008/12/2 Steven Rostedt <rostedt@goodmis.org>:
> From: Steven Rostedt <srostedt@redhat.com>
>
> Impact: clean up
>
> There exists macros for x86 asm to handle x86_64 and i386.
> This patch updates function graph asm to use them.
>
> Signed-off-by: Steven Rostedt <srostedt@redhat.com>
> ---
>  arch/x86/kernel/ftrace.c |   20 ++++----------------
>  1 files changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 5883247..1a5b8f8 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -467,28 +467,16 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
>         * ignore such a protection.
>         */
>        asm volatile(
> -#ifdef CONFIG_X86_64
> -               "1: movq (%[parent_old]), %[old]\n"
> -               "2: movq %[return_hooker], (%[parent_replaced])\n"
> -#else
> -               "1: movl (%[parent_old]), %[old]\n"
> -               "2: movl %[return_hooker], (%[parent_replaced])\n"
> -#endif
> +               "1: " _ASM_MOV " (%[parent_old]), %[old]\n"
> +               "2: " _ASM_MOV " %[return_hooker], (%[parent_replaced])\n"
>                "   movl $0, %[faulted]\n"
>
>                ".section .fixup, \"ax\"\n"
>                "3: movl $1, %[faulted]\n"
>                ".previous\n"
>
> -               ".section __ex_table, \"a\"\n"
> -#ifdef CONFIG_X86_64
> -               "   .quad 1b, 3b\n"
> -               "   .quad 2b, 3b\n"
> -#else
> -               "   .long 1b, 3b\n"
> -               "   .long 2b, 3b\n"
> -#endif
> -               ".previous\n"
> +               _ASM_EXTABLE(1b, 3b)
> +               _ASM_EXTABLE(2b, 3b)
>
>                : [parent_replaced] "=r" (parent), [old] "=r" (old),
>                  [faulted] "=r" (faulted)
> --

Thanks Steven. I wasn't aware of _ASM_MOV.

[PATCH 5/5] ftrace: have function graph use mcount caller address

[Steven Rostedt]

[-- Attachment #1: 0005-ftrace-have-function-graph-use-mcount-caller-addres.patch --]
[-- Type: text/plain, Size: 1073 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

Impact: consistency change for function graph

This patch makes function graph record the mcount caller address
the same way the function tracer does.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 arch/x86/kernel/entry_32.S |    1 +
 arch/x86/kernel/entry_64.S |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index f484e9a..3497321 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -1027,6 +1027,7 @@ ENTRY(ftrace_graph_caller)
 	pushl %edx
 	movl 0xc(%esp), %edx
 	lea 0x4(%ebp), %eax
+	subl $MCOUNT_INSN_SIZE, %edx
 	call prepare_ftrace_return
 	popl %edx
 	popl %ecx
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index c433d86..47aa5f0 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -172,6 +172,7 @@ ENTRY(ftrace_graph_caller)
 
 	leaq 8(%rbp), %rdi
 	movq 0x38(%rsp), %rsi
+	subq $MCOUNT_INSN_SIZE, %rsi
 
 	call	prepare_ftrace_return
 
-- 
1.5.6.5

-- 

Re: [PATCH 0/5] ftrace updates for tip

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> Ingo,
> 
> Here's some more ftrace updates.
> 
> The first should help fix the bug in lockdep.

ok.

> The next are changes to the ring buffer to get ready to have it
> work with splice.

great!

> The last are some clean ups for function graph.

ok. I'll also pull the followup cleanups that you did based on Andrew's 
suggestions.

> 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 (5):
>       ftrace: replace raw_local_irq_save with local_irq_save
>       ring-buffer: move some metadata into buffer page
>       ring-buffer: read page interface
>       ftrace: clean up function graph asm
>       ftrace: have function graph use mcount caller address
> 
> ----
>  arch/x86/kernel/entry_32.S  |    1 +
>  arch/x86/kernel/entry_64.S  |    1 +
>  arch/x86/kernel/ftrace.c    |   20 +---
>  include/linux/ring_buffer.h |    5 +
>  kernel/lockdep.c            |    1 +
>  kernel/trace/ring_buffer.c  |  229 ++++++++++++++++++++++++++++++++++++++-----
>  kernel/trace/trace.c        |   12 +-
>  kernel/trace/trace_branch.c |    4 +-
>  kernel/trace/trace_stack.c  |    8 +-
>  9 files changed, 226 insertions(+), 55 deletions(-)

pulled into tip/tracing/ftrace, thanks Steve!

	Ingo

[PATCH 0/5] ftrace updates for tip, and ground work for other architectures

[Steven Rostedt]

Ingo,

Patch 2 is a change in dynamic ftrace interface between the generic
code and the arch dependent code.  The original code did not allow
for the flexibility to handle some architecture limitations. I ran
into this when trying to port to PPC32 and realized that the
modules used trampolines to make the jumps to mcount.

I have successfully ported PPC64 and PPC32 and will send those patches
out later. But this patch series sets up the ground work for other
architectures to port dynamic ftrace.

The main changes to the code is:

 1) I removed the ftarce_modify_code and replaced it with two functions:
 	ftrace_make_nop() and ftrace_make_call. The former converts
	the code into a nop. The later converts a nop into a call.

 2) instead of passing just the address and expected and replace in
 	I pass in the dynamic ftrace record itself. This allows
	the architecture code save data per record if needed.
	To do this, there is now a dyn_arch_ftrace structure
	in the dyn_ftrace record.

 3) On intialization, ftrace_make_nop gets passed in the module that
 	created it or NULL if it is core kernel code. This allows
	the archicecture code do special things for modules, and
	also use the module sturcture to store data for later use
	with the conversions.

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 (5):
      ftrace: replace raw_local_irq_save with local_irq_save
      ftrace: pass module struct to arch dynamic ftrace functions
      ftrace: allow NULL pointers in mcount_loc
      ftrace: fix dyn ftrace filter
      ftrace: make filtered functions effective on setting

----
 arch/x86/include/asm/ftrace.h |    8 ++
 arch/x86/kernel/ftrace.c      |   29 +++++++-
 include/linux/ftrace.h        |   53 +++++++++++----
 kernel/module.c               |    2 +-
 kernel/trace/ftrace.c         |  147 ++++++++++++++++++-----------------------
 kernel/trace/trace.c          |    4 +-
 kernel/trace/trace_selftest.c |    4 +-
 7 files changed, 145 insertions(+), 102 deletions(-)

[PATCH 1/5] ftrace: replace raw_local_irq_save with local_irq_save

[Steven Rostedt]

[-- Attachment #1: 0001-ftrace-replace-raw_local_irq_save-with-local_irq_sa.patch --]
[-- Type: text/plain, Size: 1826 bytes --]

Impact: let logdev work when function tracing is enabled

The raw_local_irq_saves used in ftrace is causing problems with
lockdep. They are not needed, and the normal local_irq_save is fine.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 kernel/trace/trace.c          |    4 ++--
 kernel/trace/trace_selftest.c |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 4a90462..dff4bee 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1051,7 +1051,7 @@ function_trace_call(unsigned long ip, unsigned long parent_ip)
 	 * Need to use raw, since this must be called before the
 	 * recursive protection is performed.
 	 */
-	raw_local_irq_save(flags);
+	local_irq_save(flags);
 	cpu = raw_smp_processor_id();
 	data = tr->data[cpu];
 	disabled = atomic_inc_return(&data->disabled);
@@ -1062,7 +1062,7 @@ function_trace_call(unsigned long ip, unsigned long parent_ip)
 	}
 
 	atomic_dec(&data->disabled);
-	raw_local_irq_restore(flags);
+	local_irq_restore(flags);
 }
 
 #ifdef CONFIG_FUNCTION_RET_TRACER
diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
index 24e6e07..5cb64ea 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -52,7 +52,7 @@ static int trace_test_buffer(struct trace_array *tr, unsigned long *count)
 	int cpu, ret = 0;
 
 	/* Don't allow flipping of max traces now */
-	raw_local_irq_save(flags);
+	local_irq_save(flags);
 	__raw_spin_lock(&ftrace_max_lock);
 
 	cnt = ring_buffer_entries(tr->buffer);
@@ -63,7 +63,7 @@ static int trace_test_buffer(struct trace_array *tr, unsigned long *count)
 			break;
 	}
 	__raw_spin_unlock(&ftrace_max_lock);
-	raw_local_irq_restore(flags);
+	local_irq_restore(flags);
 
 	if (count)
 		*count = cnt;
-- 
1.5.6.5

--