[for-next][PATCH 0/7] tracing: Updates for 4.5 (and some stuff going into 4.4)

[for-next][PATCH 0/7] tracing: Updates for 4.5 (and some stuff going into 4.4)

[Steven Rostedt]

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

Head SHA1: ffaea3b8472da0f0e5a2b4fc9ccefb1ff460e824


Steven Rostedt (Red Hat) (6):
      ring-buffer: Update read stamp with first real commit on page
      ring-buffer: Put back the length if crossed page with add_timestamp
      ftracetest: Add instance create and delete test
      ring-buffer: Use READ_ONCE() for most tail_page access
      ring-buffer: Remove redundant update of page timestamp
      ring-buffer: Process commits whenever moving to a new page.

Tom Zanussi (1):
      tracing: Update cond flag when enabling or disabling a trigger

----
 kernel/module.c                                    | 18 ++++-
 kernel/trace/ring_buffer.c                         | 74 +++++++++---------
 kernel/trace/trace_events_trigger.c                | 10 ++-
 .../selftests/ftrace/test.d/instances/instance.tc  | 90 ++++++++++++++++++++++
 4 files changed, 147 insertions(+), 45 deletions(-)
 create mode 100644 tools/testing/selftests/ftrace/test.d/instances/instance.tc

[for-next][PATCH 1/7] ring-buffer: Update read stamp with first real commit on page

[Steven Rostedt]

[-- Attachment #1: 0001-ring-buffer-Update-read-stamp-with-first-real-commit.patch --]
[-- Type: text/plain, Size: 2105 bytes --]

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

Do not update the read stamp after swapping out the reader page from the
write buffer. If the reader page is swapped out of the buffer before an
event is written to it, then the read_stamp may get an out of date
timestamp, as the page timestamp is updated on the first commit to that
page.

rb_get_reader_page() only returns a page if it has an event on it, otherwise
it will return NULL. At that point, check if the page being returned has
events and has not been read yet. Then at that point update the read_stamp
to match the time stamp of the reader page.

Cc: stable@vger.kernel.org # 2.6.30+
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ring_buffer.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 75f1d05ea82d..4dd6d5bc4e11 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1887,12 +1887,6 @@ rb_event_index(struct ring_buffer_event *event)
 	return (addr & ~PAGE_MASK) - BUF_PAGE_HDR_SIZE;
 }
 
-static void rb_reset_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
-{
-	cpu_buffer->read_stamp = cpu_buffer->reader_page->page->time_stamp;
-	cpu_buffer->reader_page->read = 0;
-}
-
 static void rb_inc_iter(struct ring_buffer_iter *iter)
 {
 	struct ring_buffer_per_cpu *cpu_buffer = iter->cpu_buffer;
@@ -3626,7 +3620,7 @@ rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
 
 	/* Finally update the reader page to the new head */
 	cpu_buffer->reader_page = reader;
-	rb_reset_reader_page(cpu_buffer);
+	cpu_buffer->reader_page->read = 0;
 
 	if (overwrite != cpu_buffer->last_overrun) {
 		cpu_buffer->lost_events = overwrite - cpu_buffer->last_overrun;
@@ -3636,6 +3630,10 @@ rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
 	goto again;
 
  out:
+	/* Update the read_stamp on the first event */
+	if (reader && reader->read == 0)
+		cpu_buffer->read_stamp = reader->page->time_stamp;
+
 	arch_spin_unlock(&cpu_buffer->lock);
 	local_irq_restore(flags);
 
-- 
2.6.1

[for-next][PATCH 2/7] ring-buffer: Put back the length if crossed page with add_timestamp

[Steven Rostedt]

[-- Attachment #1: 0002-ring-buffer-Put-back-the-length-if-crossed-page-with.patch --]
[-- Type: text/plain, Size: 1399 bytes --]

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

Commit fcc742eaad7c "ring-buffer: Add event descriptor to simplify passing
data" added a descriptor that holds various data instead of passing around
several variables through parameters. The problem was that one of the
parameters was modified in a function and the code was designed not to have
an effect on that modified  parameter. Now that the parameter is a
descriptor and any modifications to it are non-volatile, the size of the
data could be unnecessarily expanded.

Remove the extra space added if a timestamp was added and the event went
across the page.

Cc: stable@vger.kernel.org # 4.3+
Fixes: fcc742eaad7c "ring-buffer: Add event descriptor to simplify passing data"
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ring_buffer.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 4dd6d5bc4e11..9c6045a27ba3 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2797,8 +2797,11 @@ rb_reserve_next_event(struct ring_buffer *buffer,
 
 	event = __rb_reserve_next(cpu_buffer, &info);
 
-	if (unlikely(PTR_ERR(event) == -EAGAIN))
+	if (unlikely(PTR_ERR(event) == -EAGAIN)) {
+		if (info.add_timestamp)
+			info.length -= RB_LEN_TIME_EXTEND;
 		goto again;
+	}
 
 	if (!event)
 		goto out_fail;
-- 
2.6.1

[for-next][PATCH 3/7] ftracetest: Add instance create and delete test

[Steven Rostedt]

[-- Attachment #1: 0003-ftracetest-Add-instance-create-and-delete-test.patch --]
[-- Type: text/plain, Size: 2211 bytes --]

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

Create a test to test instance creation and deletion. Several tasks are
created that create 3 directories and delete them. The tasks all create the
same directories. This places a stress on the code that creates and deletes
instances.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 .../selftests/ftrace/test.d/instances/instance.tc  | 90 ++++++++++++++++++++++
 1 file changed, 90 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/instances/instance.tc

diff --git a/tools/testing/selftests/ftrace/test.d/instances/instance.tc b/tools/testing/selftests/ftrace/test.d/instances/instance.tc
new file mode 100644
index 000000000000..773e276ff90b
--- /dev/null
+++ b/tools/testing/selftests/ftrace/test.d/instances/instance.tc
@@ -0,0 +1,90 @@
+#!/bin/sh
+# description: Test creation and deletion of trace instances
+
+if [ ! -d instances ] ; then
+    echo "no instance directory with this kernel"
+    exit_unsupported;
+fi
+
+fail() { # mesg
+    rmdir x y z 2>/dev/null
+    echo $1
+    set -e
+    exit $FAIL
+}
+
+cd instances
+
+# we don't want to fail on error
+set +e
+
+mkdir x
+rmdir x
+result=$?
+
+if [ $result -ne 0 ]; then
+    echo "instance rmdir not supported"
+    exit_unsupported
+fi
+
+instance_slam() {
+    while :; do
+	mkdir x
+	mkdir y
+	mkdir z
+	rmdir x
+	rmdir y
+	rmdir z
+    done 2>/dev/null
+}
+
+instance_slam &
+x=`jobs -l`
+p1=`echo $x | cut -d' ' -f2`
+echo $p1
+
+instance_slam &
+x=`jobs -l | tail -1`
+p2=`echo $x | cut -d' ' -f2`
+echo $p2
+
+instance_slam &
+x=`jobs -l | tail -1`
+p3=`echo $x | cut -d' ' -f2`
+echo $p3
+
+instance_slam &
+x=`jobs -l | tail -1`
+p4=`echo $x | cut -d' ' -f2`
+echo $p4
+
+instance_slam &
+x=`jobs -l | tail -1`
+p5=`echo $x | cut -d' ' -f2`
+echo $p5
+
+ls -lR >/dev/null
+sleep 1
+
+kill -1 $p1
+kill -1 $p2
+kill -1 $p3
+kill -1 $p4
+kill -1 $p5
+
+echo "Wait for processes to finish"
+wait $p1 $p2 $p3 $p4 $p5
+echo "all processes finished, wait for cleanup"
+
+mkdir x y z
+ls x y z
+rmdir x y z
+for d in x y z; do
+        if [ -d $d ]; then
+                fail "instance $d still exists"
+        fi
+done
+
+set -e
+
+exit 0
-- 
2.6.1

Re: [for-next][PATCH 3/7] ftracetest: Add instance create and delete test

[Steven Rostedt]

Shuah,

Sorry, I forgot to add you to the Cc of this patch. I have another test
to add, but I forgot to commit it into git ;-)

-- Steve


On Wed, 25 Nov 2015 11:08:41 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> 
> Create a test to test instance creation and deletion. Several tasks are
> created that create 3 directories and delete them. The tasks all create the
> same directories. This places a stress on the code that creates and deletes
> instances.
> 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
>  .../selftests/ftrace/test.d/instances/instance.tc  | 90 ++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
>  create mode 100644 tools/testing/selftests/ftrace/test.d/instances/instance.tc
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/instances/instance.tc b/tools/testing/selftests/ftrace/test.d/instances/instance.tc
> new file mode 100644
> index 000000000000..773e276ff90b
> --- /dev/null
> +++ b/tools/testing/selftests/ftrace/test.d/instances/instance.tc
> @@ -0,0 +1,90 @@
> +#!/bin/sh
> +# description: Test creation and deletion of trace instances
> +
> +if [ ! -d instances ] ; then
> +    echo "no instance directory with this kernel"
> +    exit_unsupported;
> +fi
> +
> +fail() { # mesg
> +    rmdir x y z 2>/dev/null
> +    echo $1
> +    set -e
> +    exit $FAIL
> +}
> +
> +cd instances
> +
> +# we don't want to fail on error
> +set +e
> +
> +mkdir x
> +rmdir x
> +result=$?
> +
> +if [ $result -ne 0 ]; then
> +    echo "instance rmdir not supported"
> +    exit_unsupported
> +fi
> +
> +instance_slam() {
> +    while :; do
> +	mkdir x
> +	mkdir y
> +	mkdir z
> +	rmdir x
> +	rmdir y
> +	rmdir z
> +    done 2>/dev/null
> +}
> +
> +instance_slam &
> +x=`jobs -l`
> +p1=`echo $x | cut -d' ' -f2`
> +echo $p1
> +
> +instance_slam &
> +x=`jobs -l | tail -1`
> +p2=`echo $x | cut -d' ' -f2`
> +echo $p2
> +
> +instance_slam &
> +x=`jobs -l | tail -1`
> +p3=`echo $x | cut -d' ' -f2`
> +echo $p3
> +
> +instance_slam &
> +x=`jobs -l | tail -1`
> +p4=`echo $x | cut -d' ' -f2`
> +echo $p4
> +
> +instance_slam &
> +x=`jobs -l | tail -1`
> +p5=`echo $x | cut -d' ' -f2`
> +echo $p5
> +
> +ls -lR >/dev/null
> +sleep 1
> +
> +kill -1 $p1
> +kill -1 $p2
> +kill -1 $p3
> +kill -1 $p4
> +kill -1 $p5
> +
> +echo "Wait for processes to finish"
> +wait $p1 $p2 $p3 $p4 $p5
> +echo "all processes finished, wait for cleanup"
> +
> +mkdir x y z
> +ls x y z
> +rmdir x y z
> +for d in x y z; do
> +        if [ -d $d ]; then
> +                fail "instance $d still exists"
> +        fi
> +done
> +
> +set -e
> +
> +exit 0

Re: [for-next][PATCH 3/7] ftracetest: Add instance create and delete test

[Michael Ellerman]

On Wed, 2015-11-25 at 11:08 -0500, Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> 
> Create a test to test instance creation and deletion. Several tasks are
> created that create 3 directories and delete them. The tasks all create the
> same directories. This places a stress on the code that creates and deletes
> instances.
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/instances/instance.tc b/tools/testing/selftests/ftrace/test.d/instances/instance.tc
> new file mode 100644
> index 000000000000..773e276ff90b
> --- /dev/null
> +++ b/tools/testing/selftests/ftrace/test.d/instances/instance.tc
> @@ -0,0 +1,90 @@

<snip>

> +instance_slam &
> +x=`jobs -l | tail -1`
> +p5=`echo $x | cut -d' ' -f2`
> +echo $p5
> +
> +ls -lR >/dev/null
> +sleep 1
> +
> +kill -1 $p1
> +kill -1 $p2
> +kill -1 $p3
> +kill -1 $p4
> +kill -1 $p5

Tells me:

./ftracetest: 72: kill: Usage: kill [-s sigspec | -signum | -sigspec] [pid | job]... or
kill -l [exitstatus]

And the test hangs forever.

I suspect because my /bin/sh is dash?

# /bin/dash
# kill -1
/bin/dash: 1: kill: Usage: kill [-s sigspec | -signum | -sigspec] [pid | job]... or
kill -l [exitstatus]


cheers

Re: [for-next][PATCH 3/7] ftracetest: Add instance create and delete test

[Steven Rostedt]

On Wed, 09 Dec 2015 16:09:38 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:

> I suspect because my /bin/sh is dash?
> 
> # /bin/dash
> # kill -1
> /bin/dash: 1: kill: Usage: kill [-s sigspec | -signum | -sigspec] [pid | job]... or
> kill -l [exitstatus]
> 

I did some digging and found a better way to do this. Does the belowe
patch fix things for you?

-- Steve


diff --git a/tools/testing/selftests/ftrace/test.d/instances/instance.tc b/tools/testing/selftests/ftrace/test.d/instances/instance.tc
index 773e276ff90b..1e1abe0ad354 100644
--- a/tools/testing/selftests/ftrace/test.d/instances/instance.tc
+++ b/tools/testing/selftests/ftrace/test.d/instances/instance.tc
@@ -39,28 +39,23 @@ instance_slam() {
 }
 
 instance_slam &
-x=`jobs -l`
-p1=`echo $x | cut -d' ' -f2`
+p1=$!
 echo $p1
 
 instance_slam &
-x=`jobs -l | tail -1`
-p2=`echo $x | cut -d' ' -f2`
+p2=$!
 echo $p2
 
 instance_slam &
-x=`jobs -l | tail -1`
-p3=`echo $x | cut -d' ' -f2`
+p3=$!
 echo $p3
 
 instance_slam &
-x=`jobs -l | tail -1`
-p4=`echo $x | cut -d' ' -f2`
+p4=$!
 echo $p4
 
 instance_slam &
-x=`jobs -l | tail -1`
-p5=`echo $x | cut -d' ' -f2`
+p5=$!
 echo $p5
 
 ls -lR >/dev/null

Re: [for-next][PATCH 3/7] ftracetest: Add instance create and delete test

[Michael Ellerman]

On Fri, 2015-12-11 at 14:36 -0500, Steven Rostedt wrote:
> On Wed, 09 Dec 2015 16:09:38 +1100
> Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
> > I suspect because my /bin/sh is dash?
> > 
> > # /bin/dash
> > # kill -1
> > /bin/dash: 1: kill: Usage: kill [-s sigspec | -signum | -sigspec] [pid | job]... or
> > kill -l [exitstatus]
> > 
> 
> I did some digging and found a better way to do this. Does the belowe
> patch fix things for you?

Yeah that works, thanks.

cheers

Re: [for-next][PATCH 3/7] ftracetest: Add instance create and delete test

[Steven Rostedt]

On Fri, 15 Jan 2016 16:08:41 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:

> On Fri, 2015-12-11 at 14:36 -0500, Steven Rostedt wrote:
> > On Wed, 09 Dec 2015 16:09:38 +1100
> > Michael Ellerman <mpe@ellerman.id.au> wrote:
> >   
> > > I suspect because my /bin/sh is dash?
> > > 
> > > # /bin/dash
> > > # kill -1
> > > /bin/dash: 1: kill: Usage: kill [-s sigspec | -signum | -sigspec] [pid | job]... or
> > > kill -l [exitstatus]
> > >   
> > 
> > I did some digging and found a better way to do this. Does the belowe
> > patch fix things for you?  
> 
> Yeah that works, thanks.
> 

Crap, I was waiting for a response, and forgot about this patch. I'll
go ahead and add this to my queue of fixes.

Thanks!

-- Steve

Re: [for-next][PATCH 3/7] ftracetest: Add instance create and delete test

[Michael Ellerman]

On Fri, 2016-01-15 at 12:50 -0500, Steven Rostedt wrote:
> On Fri, 15 Jan 2016 16:08:41 +1100
> Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
> > On Fri, 2015-12-11 at 14:36 -0500, Steven Rostedt wrote:
> > > On Wed, 09 Dec 2015 16:09:38 +1100
> > > Michael Ellerman <mpe@ellerman.id.au> wrote:
> > >   
> > > > I suspect because my /bin/sh is dash?
> > > > 
> > > > # /bin/dash
> > > > # kill -1
> > > > /bin/dash: 1: kill: Usage: kill [-s sigspec | -signum | -sigspec] [pid | job]... or
> > > > kill -l [exitstatus]
> > > >   
> > > 
> > > I did some digging and found a better way to do this. Does the belowe
> > > patch fix things for you?  
> > 
> > Yeah that works, thanks.
> 
> Crap, I was waiting for a response, and forgot about this patch. I'll
> go ahead and add this to my queue of fixes.

No worries, I was a bit slow replying :}

cheers

Re: [for-next][PATCH 3/7] ftracetest: Add instance create and delete test

[Michael Ellerman]

On Fri, 2016-01-15 at 12:50 -0500, Steven Rostedt wrote:
> On Fri, 15 Jan 2016 16:08:41 +1100
> Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
> > On Fri, 2015-12-11 at 14:36 -0500, Steven Rostedt wrote:
> > > On Wed, 09 Dec 2015 16:09:38 +1100
> > > Michael Ellerman <mpe@ellerman.id.au> wrote:
> > >   
> > > > I suspect because my /bin/sh is dash?
> > > > 
> > > > # /bin/dash
> > > > # kill -1
> > > > /bin/dash: 1: kill: Usage: kill [-s sigspec | -signum | -sigspec] [pid | job]... or
> > > > kill -l [exitstatus]
> > > >   
> > > 
> > > I did some digging and found a better way to do this. Does the belowe
> > > patch fix things for you?  
> > 
> > Yeah that works, thanks.
> > 
> 
> Crap, I was waiting for a response, and forgot about this patch. I'll
> go ahead and add this to my queue of fixes.

Hi Steve,

Looks like this got lost?

cheers

Re: [for-next][PATCH 3/7] ftracetest: Add instance create and delete test

[Steven Rostedt]

On Thu, 18 Feb 2016 23:37:36 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:

> On Fri, 2016-01-15 at 12:50 -0500, Steven Rostedt wrote:
> > On Fri, 15 Jan 2016 16:08:41 +1100
> > Michael Ellerman <mpe@ellerman.id.au> wrote:
> >   
> > > On Fri, 2015-12-11 at 14:36 -0500, Steven Rostedt wrote:  
> > > > On Wed, 09 Dec 2015 16:09:38 +1100
> > > > Michael Ellerman <mpe@ellerman.id.au> wrote:
> > > >     
> > > > > I suspect because my /bin/sh is dash?
> > > > > 
> > > > > # /bin/dash
> > > > > # kill -1
> > > > > /bin/dash: 1: kill: Usage: kill [-s sigspec | -signum | -sigspec] [pid | job]... or
> > > > > kill -l [exitstatus]
> > > > >     
> > > > 
> > > > I did some digging and found a better way to do this. Does the belowe
> > > > patch fix things for you?    
> > > 
> > > Yeah that works, thanks.
> > >   
> > 
> > Crap, I was waiting for a response, and forgot about this patch. I'll
> > go ahead and add this to my queue of fixes.  
> 
> Hi Steve,
> 
> Looks like this got lost?

No, not lost, just pushed down the priority list. I got inundated by
higher priority items since then :-/

I still need to do this though. Thanks for the reminder.

-- Steve

[for-next][PATCH 4/7] ring-buffer: Use READ_ONCE() for most tail_page access

[Steven Rostedt]

[-- Attachment #1: 0004-ring-buffer-Use-READ_ONCE-for-most-tail_page-access.patch --]
[-- Type: text/plain, Size: 3139 bytes --]

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

As cpu_buffer->tail_page may be modified by interrupts at almost any time,
the flow of logic is very important. Do not let gcc get smart with
re-reading cpu_buffer->tail_page by adding READ_ONCE() around most of its
accesses.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ring_buffer.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 9c6045a27ba3..ab102e6259bc 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1036,7 +1036,7 @@ static int rb_tail_page_update(struct ring_buffer_per_cpu *cpu_buffer,
 	 * it is, then it is up to us to update the tail
 	 * pointer.
 	 */
-	if (tail_page == cpu_buffer->tail_page) {
+	if (tail_page == READ_ONCE(cpu_buffer->tail_page)) {
 		/* Zero the write counter */
 		unsigned long val = old_write & ~RB_WRITE_MASK;
 		unsigned long eval = old_entries & ~RB_WRITE_MASK;
@@ -2036,12 +2036,15 @@ rb_handle_head_page(struct ring_buffer_per_cpu *cpu_buffer,
 	 * the tail page would have moved.
 	 */
 	if (ret == RB_PAGE_NORMAL) {
+		struct buffer_page *buffer_tail_page;
+
+		buffer_tail_page = READ_ONCE(cpu_buffer->tail_page);
 		/*
 		 * If the tail had moved passed next, then we need
 		 * to reset the pointer.
 		 */
-		if (cpu_buffer->tail_page != tail_page &&
-		    cpu_buffer->tail_page != next_page)
+		if (buffer_tail_page != tail_page &&
+		    buffer_tail_page != next_page)
 			rb_head_page_set_normal(cpu_buffer, new_head,
 						next_page,
 						RB_PAGE_HEAD);
@@ -2362,7 +2365,7 @@ rb_try_to_discard(struct ring_buffer_per_cpu *cpu_buffer,
 	addr = (unsigned long)event;
 	addr &= PAGE_MASK;
 
-	bpage = cpu_buffer->tail_page;
+	bpage = READ_ONCE(cpu_buffer->tail_page);
 
 	if (bpage->page == (void *)addr && rb_page_write(bpage) == old_index) {
 		unsigned long write_mask =
@@ -2410,7 +2413,7 @@ rb_set_commit_to_write(struct ring_buffer_per_cpu *cpu_buffer)
  again:
 	max_count = cpu_buffer->nr_pages * 100;
 
-	while (cpu_buffer->commit_page != cpu_buffer->tail_page) {
+	while (cpu_buffer->commit_page != READ_ONCE(cpu_buffer->tail_page)) {
 		if (RB_WARN_ON(cpu_buffer, !(--max_count)))
 			return;
 		if (RB_WARN_ON(cpu_buffer,
@@ -2443,7 +2446,7 @@ rb_set_commit_to_write(struct ring_buffer_per_cpu *cpu_buffer)
 	 * and pushed the tail page forward, we will be left with
 	 * a dangling commit that will never go forward.
 	 */
-	if (unlikely(cpu_buffer->commit_page != cpu_buffer->tail_page))
+	if (unlikely(cpu_buffer->commit_page != READ_ONCE(cpu_buffer->tail_page)))
 		goto again;
 }
 
@@ -2699,7 +2702,8 @@ __rb_reserve_next(struct ring_buffer_per_cpu *cpu_buffer,
 	if (unlikely(info->add_timestamp))
 		info->length += RB_LEN_TIME_EXTEND;
 
-	tail_page = info->tail_page = cpu_buffer->tail_page;
+	/* Don't let the compiler play games with cpu_buffer->tail_page */
+	tail_page = info->tail_page = READ_ONCE(cpu_buffer->tail_page);
 	write = local_add_return(info->length, &tail_page->write);
 
 	/* set write to only the index of the write */
-- 
2.6.1

[for-next][PATCH 5/7] ring-buffer: Remove redundant update of page timestamp

[Steven Rostedt]

[-- Attachment #1: 0005-ring-buffer-Remove-redundant-update-of-page-timestam.patch --]
[-- Type: text/plain, Size: 2946 bytes --]

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

The first commit of a buffer page updates the timestamp of that page. No
need to have the update to the next page add the timestamp too. It will only
be replaced by the first commit on that page anyway.

Only update to a page if it contains an event.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ring_buffer.c | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index ab102e6259bc..631541a53baf 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1001,17 +1001,13 @@ static int rb_head_page_replace(struct buffer_page *old,
 
 /*
  * rb_tail_page_update - move the tail page forward
- *
- * Returns 1 if moved tail page, 0 if someone else did.
  */
-static int rb_tail_page_update(struct ring_buffer_per_cpu *cpu_buffer,
+static void rb_tail_page_update(struct ring_buffer_per_cpu *cpu_buffer,
 			       struct buffer_page *tail_page,
 			       struct buffer_page *next_page)
 {
-	struct buffer_page *old_tail;
 	unsigned long old_entries;
 	unsigned long old_write;
-	int ret = 0;
 
 	/*
 	 * The tail page now needs to be moved forward.
@@ -1061,14 +1057,9 @@ static int rb_tail_page_update(struct ring_buffer_per_cpu *cpu_buffer,
 		 */
 		local_set(&next_page->page->commit, 0);
 
-		old_tail = cmpxchg(&cpu_buffer->tail_page,
-				   tail_page, next_page);
-
-		if (old_tail == tail_page)
-			ret = 1;
+		/* Again, either we update tail_page or an interrupt does */
+		(void)cmpxchg(&cpu_buffer->tail_page, tail_page, next_page);
 	}
-
-	return ret;
 }
 
 static int rb_check_bpage(struct ring_buffer_per_cpu *cpu_buffer,
@@ -2150,7 +2141,6 @@ rb_move_tail(struct ring_buffer_per_cpu *cpu_buffer,
 	struct ring_buffer *buffer = cpu_buffer->buffer;
 	struct buffer_page *next_page;
 	int ret;
-	u64 ts;
 
 	next_page = tail_page;
 
@@ -2224,15 +2214,7 @@ rb_move_tail(struct ring_buffer_per_cpu *cpu_buffer,
 		}
 	}
 
-	ret = rb_tail_page_update(cpu_buffer, tail_page, next_page);
-	if (ret) {
-		/*
-		 * Nested commits always have zero deltas, so
-		 * just reread the time stamp
-		 */
-		ts = rb_time_stamp(buffer);
-		next_page->page->time_stamp = ts;
-	}
+	rb_tail_page_update(cpu_buffer, tail_page, next_page);
 
  out_again:
 
@@ -2422,8 +2404,10 @@ rb_set_commit_to_write(struct ring_buffer_per_cpu *cpu_buffer)
 		local_set(&cpu_buffer->commit_page->page->commit,
 			  rb_page_write(cpu_buffer->commit_page));
 		rb_inc_page(cpu_buffer, &cpu_buffer->commit_page);
-		cpu_buffer->write_stamp =
-			cpu_buffer->commit_page->page->time_stamp;
+		/* Only update the write stamp if the page has an event */
+		if (rb_page_write(cpu_buffer->commit_page))
+			cpu_buffer->write_stamp =
+				cpu_buffer->commit_page->page->time_stamp;
 		/* add barrier to keep gcc from optimizing too much */
 		barrier();
 	}
-- 
2.6.1

[for-next][PATCH 6/7] ring-buffer: Process commits whenever moving to a new page.

[Steven Rostedt]

[-- Attachment #1: 0006-ring-buffer-Process-commits-whenever-moving-to-a-new.patch --]
[-- Type: text/plain, Size: 2105 bytes --]

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

When crossing over to a new page, commit the current work. This will allow
readers to get data with less latency, and also simplifies the work to get
timestamps working for interrupted events.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/module.c            | 18 ++++++++++++++++--
 kernel/trace/ring_buffer.c |  7 +++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 8f051a106676..c0ae7c18c2fa 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -284,11 +284,25 @@ static void module_assert_mutex(void)
 static void module_assert_mutex_or_preempt(void)
 {
 #ifdef CONFIG_LOCKDEP
+	static int once;
+
 	if (unlikely(!debug_locks))
 		return;
 
-	WARN_ON(!rcu_read_lock_sched_held() &&
-		!lockdep_is_held(&module_mutex));
+	/*
+	 * Would be nice to use WARN_ON_ONCE(), but the warning
+	 * that causes a stack trace may call __module_address()
+	 * which may call here, and we trigger the warning again,
+	 * before the WARN_ON_ONCE() updates its flag.
+	 * To prevent the recursion, we need to open code the
+	 * once logic.
+	 */
+	if (!once &&
+	    unlikely(!rcu_read_lock_sched_held() &&
+		     !lockdep_is_held(&module_mutex))) {
+		once++;
+		WARN_ON(1);
+	}
 #endif
 }
 
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 631541a53baf..95181e36891a 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2129,6 +2129,8 @@ rb_reset_tail(struct ring_buffer_per_cpu *cpu_buffer,
 	local_sub(length, &tail_page->write);
 }
 
+static inline void rb_end_commit(struct ring_buffer_per_cpu *cpu_buffer);
+
 /*
  * This is the slow path, force gcc not to inline it.
  */
@@ -2220,6 +2222,11 @@ rb_move_tail(struct ring_buffer_per_cpu *cpu_buffer,
 
 	rb_reset_tail(cpu_buffer, tail, info);
 
+	/* Commit what we have for now. */
+	rb_end_commit(cpu_buffer);
+	/* rb_end_commit() decs committing */
+	local_inc(&cpu_buffer->committing);
+
 	/* fail and let the caller try again */
 	return ERR_PTR(-EAGAIN);
 
-- 
2.6.1

Re: [for-next][PATCH 6/7] ring-buffer: Process commits whenever moving to a new page.

[Steven Rostedt]

> diff --git a/kernel/module.c b/kernel/module.c
> index 8f051a106676..c0ae7c18c2fa 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -284,11 +284,25 @@ static void module_assert_mutex(void)
>  static void module_assert_mutex_or_preempt(void)
>  {
>  #ifdef CONFIG_LOCKDEP
> +	static int once;
> +
>  	if (unlikely(!debug_locks))
>  		return;
>  
> -	WARN_ON(!rcu_read_lock_sched_held() &&
> -		!lockdep_is_held(&module_mutex));
> +	/*
> +	 * Would be nice to use WARN_ON_ONCE(), but the warning
> +	 * that causes a stack trace may call __module_address()
> +	 * which may call here, and we trigger the warning again,
> +	 * before the WARN_ON_ONCE() updates its flag.
> +	 * To prevent the recursion, we need to open code the
> +	 * once logic.
> +	 */
> +	if (!once &&
> +	    unlikely(!rcu_read_lock_sched_held() &&
> +		     !lockdep_is_held(&module_mutex))) {
> +		once++;
> +		WARN_ON(1);
> +	}
>  #endif
>  }
>  
>

Damn it. I added this via quilt to debug something (as this was causing
a crash), and forgot to pop it when doing my "git commit -a -s".

I'll have to nuke this part of the patch and rebase my tree :-(

-- Steve

[for-next][PATCH 7/7] tracing: Update cond flag when enabling or disabling a trigger

[Steven Rostedt]

[-- Attachment #1: 0007-tracing-Update-cond-flag-when-enabling-or-disabling-.patch --]
[-- Type: text/plain, Size: 2460 bytes --]

From: Tom Zanussi <tom.zanussi@linux.intel.com>

When a trigger is enabled, the cond flag should be set beforehand,
otherwise a trigger that's expecting to process a trace record
(e.g. one with post_trigger set) could be invoked without one.

Likewise a trigger's cond flag should be reset after it's disabled,
not before.

Link: http://lkml.kernel.org/r/a420b52a67b1c2d3cab017914362d153255acb99.1448303214.git.tom.zanussi@linux.intel.com

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Reviewed-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_events_trigger.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 42a4009fd75a..4d2f3ccc56f6 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -543,11 +543,12 @@ static int register_trigger(char *glob, struct event_trigger_ops *ops,
 	list_add_rcu(&data->list, &file->triggers);
 	ret++;
 
+	update_cond_flag(file);
 	if (trace_event_trigger_enable_disable(file, 1) < 0) {
 		list_del_rcu(&data->list);
+		update_cond_flag(file);
 		ret--;
 	}
-	update_cond_flag(file);
 out:
 	return ret;
 }
@@ -575,8 +576,8 @@ static void unregister_trigger(char *glob, struct event_trigger_ops *ops,
 		if (data->cmd_ops->trigger_type == test->cmd_ops->trigger_type) {
 			unregistered = true;
 			list_del_rcu(&data->list);
-			update_cond_flag(file);
 			trace_event_trigger_enable_disable(file, 0);
+			update_cond_flag(file);
 			break;
 		}
 	}
@@ -1319,11 +1320,12 @@ static int event_enable_register_trigger(char *glob,
 	list_add_rcu(&data->list, &file->triggers);
 	ret++;
 
+	update_cond_flag(file);
 	if (trace_event_trigger_enable_disable(file, 1) < 0) {
 		list_del_rcu(&data->list);
+		update_cond_flag(file);
 		ret--;
 	}
-	update_cond_flag(file);
 out:
 	return ret;
 }
@@ -1344,8 +1346,8 @@ static void event_enable_unregister_trigger(char *glob,
 		    (enable_data->file == test_enable_data->file)) {
 			unregistered = true;
 			list_del_rcu(&data->list);
-			update_cond_flag(file);
 			trace_event_trigger_enable_disable(file, 0);
+			update_cond_flag(file);
 			break;
 		}
 	}
-- 
2.6.1