Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1532401 > unrolled thread

[for-next][PATCH 0/8] tracing: Some micro optimizations

Started bySteven Rostedt <rostedt@goodmis.org>
First post2016-11-29 16:20 +0100
Last post2016-11-29 16:20 +0100
Articles 5 — 1 participant

Back to article view | Back to linux.kernel


Contents

  [for-next][PATCH 0/8] tracing: Some micro optimizations Steven Rostedt <rostedt@goodmis.org> - 2016-11-29 16:20 +0100
    [for-next][PATCH 6/8] ring-buffer: Force inline of hotpath helper functions Steven Rostedt <rostedt@goodmis.org> - 2016-11-29 16:20 +0100
    [for-next][PATCH 3/8] ring-buffer: Always inline rb_event_data() Steven Rostedt <rostedt@goodmis.org> - 2016-11-29 16:20 +0100
    [for-next][PATCH 5/8] tracing: Make __buffer_unlock_commit() always_inline Steven Rostedt <rostedt@goodmis.org> - 2016-11-29 16:20 +0100
    [for-next][PATCH 2/8] ring-buffer: Make rb_reserve_next_event() always inlined Steven Rostedt <rostedt@goodmis.org> - 2016-11-29 16:20 +0100

#1532401 — [for-next][PATCH 0/8] tracing: Some micro optimizations

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-11-29 16:20 +0100
Subject[for-next][PATCH 0/8] tracing: Some micro optimizations
Message-ID<sIScF-1DN-9@gated-at.bofh.it>
Andi Kleen pointed out a trace that showed lots of functions being
called in the trace event hot path. These functions would help to
be inlined. More needs to be done, but this is just a start.

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

Head SHA1: 38e11df134297ea3860c7aad8263ece27db01308


Steven Rostedt (Red Hat) (8):
      tracing: Create a always_inlined __trace_buffer_lock_reserve()
      ring-buffer: Make rb_reserve_next_event() always inlined
      ring-buffer: Always inline rb_event_data()
      tracing: Make tracepoint_printk a static_key
      tracing: Make __buffer_unlock_commit() always_inline
      ring-buffer: Force inline of hotpath helper functions
      ring-buffer: Froce rb_update_write_stamp() to be inlined
      ring-buffer: Force rb_end_commit() and rb_set_commit_to_write() inline

----
 include/linux/ftrace.h               |   4 +
 kernel/sysctl.c                      |   2 +-
 kernel/trace/ring_buffer.c           |  28 ++---
 kernel/trace/trace.c                 | 206 ++++++++++++++++++++++++++---------
 kernel/trace/trace.h                 |   4 +-
 kernel/trace/trace_branch.c          |   2 +-
 kernel/trace/trace_events.c          |  40 -------
 kernel/trace/trace_functions_graph.c |   4 +-
 kernel/trace/trace_hwlat.c           |   2 +-
 9 files changed, 177 insertions(+), 115 deletions(-)

[toc] | [next] | [standalone]


#1532403 — [for-next][PATCH 6/8] ring-buffer: Force inline of hotpath helper functions

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-11-29 16:20 +0100
Subject[for-next][PATCH 6/8] ring-buffer: Force inline of hotpath helper functions
Message-ID<sISmm-1He-11@gated-at.bofh.it>
In reply to#1532401
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

There's several small helper functions in ring_buffer.c that are used in the
hot path. For some reason, even though they are marked inline, gcc tends not
to enforce it. Make sure these functions are always inlined.

Link: http://lkml.kernel.org/r/20161121183700.GW26852@two.firstfloor.org

Reported-by: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ring_buffer.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 2760aaca6d1b..04068c0ed927 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1829,48 +1829,48 @@ void ring_buffer_change_overwrite(struct ring_buffer *buffer, int val)
 }
 EXPORT_SYMBOL_GPL(ring_buffer_change_overwrite);
 
-static inline void *
+static __always_inline void *
 __rb_data_page_index(struct buffer_data_page *bpage, unsigned index)
 {
 	return bpage->data + index;
 }
 
-static inline void *__rb_page_index(struct buffer_page *bpage, unsigned index)
+static __always_inline void *__rb_page_index(struct buffer_page *bpage, unsigned index)
 {
 	return bpage->page->data + index;
 }
 
-static inline struct ring_buffer_event *
+static __always_inline struct ring_buffer_event *
 rb_reader_event(struct ring_buffer_per_cpu *cpu_buffer)
 {
 	return __rb_page_index(cpu_buffer->reader_page,
 			       cpu_buffer->reader_page->read);
 }
 
-static inline struct ring_buffer_event *
+static __always_inline struct ring_buffer_event *
 rb_iter_head_event(struct ring_buffer_iter *iter)
 {
 	return __rb_page_index(iter->head_page, iter->head);
 }
 
-static inline unsigned rb_page_commit(struct buffer_page *bpage)
+static __always_inline unsigned rb_page_commit(struct buffer_page *bpage)
 {
 	return local_read(&bpage->page->commit);
 }
 
 /* Size is determined by what has been committed */
-static inline unsigned rb_page_size(struct buffer_page *bpage)
+static __always_inline unsigned rb_page_size(struct buffer_page *bpage)
 {
 	return rb_page_commit(bpage);
 }
 
-static inline unsigned
+static __always_inline unsigned
 rb_commit_index(struct ring_buffer_per_cpu *cpu_buffer)
 {
 	return rb_page_commit(cpu_buffer->commit_page);
 }
 
-static inline unsigned
+static __always_inline unsigned
 rb_event_index(struct ring_buffer_event *event)
 {
 	unsigned long addr = (unsigned long)event;
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]


#1532404 — [for-next][PATCH 3/8] ring-buffer: Always inline rb_event_data()

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-11-29 16:20 +0100
Subject[for-next][PATCH 3/8] ring-buffer: Always inline rb_event_data()
Message-ID<sISmm-1He-5@gated-at.bofh.it>
In reply to#1532401
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

The rb_event_data() is the fast path of getting the ring buffer data from an
event. Externally, ring_buffer_event_data() is used to access this function.
But unfortunately, rb_event_data() is not inlined, and calling
ring_buffer_event_data() causes that function to be called again. Force
rb_event_data() to be inlined to lower the number of operations needed when
calling ring_buffer_event_data().

Link: http://lkml.kernel.org/r/20161121183700.GW26852@two.firstfloor.org

Reported-by: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ring_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 1f3580cee6cc..2760aaca6d1b 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -245,7 +245,7 @@ unsigned ring_buffer_event_length(struct ring_buffer_event *event)
 EXPORT_SYMBOL_GPL(ring_buffer_event_length);
 
 /* inline for ring buffer fast paths */
-static void *
+static __always_inline void *
 rb_event_data(struct ring_buffer_event *event)
 {
 	if (event->type_len == RINGBUF_TYPE_TIME_EXTEND)
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]


#1532406 — [for-next][PATCH 5/8] tracing: Make __buffer_unlock_commit() always_inline

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-11-29 16:20 +0100
Subject[for-next][PATCH 5/8] tracing: Make __buffer_unlock_commit() always_inline
Message-ID<sISmm-1He-19@gated-at.bofh.it>
In reply to#1532401
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

The function __buffer_unlock_commit() is called in a few places outside of
trace.c. But for the most part, it should really be inlined, as it is in the
hot path of the trace_events. For the callers outside of trace.c, create a
new function trace_buffer_unlock_commit_nostack(), as the reason it was used
was to avoid the stack tracing that trace_buffer_unlock_commit() could do.

Link: http://lkml.kernel.org/r/20161121183700.GW26852@two.firstfloor.org

Reported-by: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace.c                 | 41 +++++++++++++++++++++++-------------
 kernel/trace/trace.h                 |  4 ++--
 kernel/trace/trace_branch.c          |  2 +-
 kernel/trace/trace_functions_graph.c |  4 ++--
 kernel/trace/trace_hwlat.c           |  2 +-
 5 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 725e8b2c453f..60416bf7c591 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -794,6 +794,22 @@ void tracing_on(void)
 }
 EXPORT_SYMBOL_GPL(tracing_on);
 
+
+static __always_inline void
+__buffer_unlock_commit(struct ring_buffer *buffer, struct ring_buffer_event *event)
+{
+	__this_cpu_write(trace_cmdline_save, true);
+
+	/* If this is the temp buffer, we need to commit fully */
+	if (this_cpu_read(trace_buffered_event) == event) {
+		/* Length is in event->array[0] */
+		ring_buffer_write(buffer, event->array[0], &event->array[1]);
+		/* Release the temp buffer */
+		this_cpu_dec(trace_buffered_event_cnt);
+	} else
+		ring_buffer_unlock_commit(buffer, event);
+}
+
 /**
  * __trace_puts - write a constant string into the trace buffer.
  * @ip:	   The address of the caller
@@ -2059,21 +2075,6 @@ void trace_buffered_event_disable(void)
 	preempt_enable();
 }
 
-void
-__buffer_unlock_commit(struct ring_buffer *buffer, struct ring_buffer_event *event)
-{
-	__this_cpu_write(trace_cmdline_save, true);
-
-	/* If this is the temp buffer, we need to commit fully */
-	if (this_cpu_read(trace_buffered_event) == event) {
-		/* Length is in event->array[0] */
-		ring_buffer_write(buffer, event->array[0], &event->array[1]);
-		/* Release the temp buffer */
-		this_cpu_dec(trace_buffered_event_cnt);
-	} else
-		ring_buffer_unlock_commit(buffer, event);
-}
-
 static struct ring_buffer *temp_buffer;
 
 struct ring_buffer_event *
@@ -2214,6 +2215,16 @@ void trace_buffer_unlock_commit_regs(struct trace_array *tr,
 	ftrace_trace_userstack(buffer, flags, pc);
 }
 
+/*
+ * Similar to trace_buffer_unlock_commit_regs() but do not dump stack.
+ */
+void
+trace_buffer_unlock_commit_nostack(struct ring_buffer *buffer,
+				   struct ring_buffer_event *event)
+{
+	__buffer_unlock_commit(buffer, event);
+}
+
 static void
 trace_process_export(struct trace_export *export,
 	       struct ring_buffer_event *event)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 9294f8606ade..37602e722336 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -602,8 +602,8 @@ struct trace_entry *tracing_get_trace_entry(struct trace_array *tr,
 struct trace_entry *trace_find_next_entry(struct trace_iterator *iter,
 					  int *ent_cpu, u64 *ent_ts);
 
-void __buffer_unlock_commit(struct ring_buffer *buffer,
-			    struct ring_buffer_event *event);
+void trace_buffer_unlock_commit_nostack(struct ring_buffer *buffer,
+					struct ring_buffer_event *event);
 
 int trace_empty(struct trace_iterator *iter);
 
diff --git a/kernel/trace/trace_branch.c b/kernel/trace/trace_branch.c
index 3a2a73716a5b..75489de546b6 100644
--- a/kernel/trace/trace_branch.c
+++ b/kernel/trace/trace_branch.c
@@ -81,7 +81,7 @@ probe_likely_condition(struct ftrace_branch_data *f, int val, int expect)
 	entry->correct = val == expect;
 
 	if (!call_filter_check_discard(call, entry, buffer, event))
-		__buffer_unlock_commit(buffer, event);
+		trace_buffer_unlock_commit_nostack(buffer, event);
 
  out:
 	current->trace_recursion &= ~TRACE_BRANCH_BIT;
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 4e480e870474..8e1a115439fa 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -358,7 +358,7 @@ int __trace_graph_entry(struct trace_array *tr,
 	entry	= ring_buffer_event_data(event);
 	entry->graph_ent			= *trace;
 	if (!call_filter_check_discard(call, entry, buffer, event))
-		__buffer_unlock_commit(buffer, event);
+		trace_buffer_unlock_commit_nostack(buffer, event);
 
 	return 1;
 }
@@ -469,7 +469,7 @@ void __trace_graph_return(struct trace_array *tr,
 	entry	= ring_buffer_event_data(event);
 	entry->ret				= *trace;
 	if (!call_filter_check_discard(call, entry, buffer, event))
-		__buffer_unlock_commit(buffer, event);
+		trace_buffer_unlock_commit_nostack(buffer, event);
 }
 
 void trace_graph_return(struct ftrace_graph_ret *trace)
diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
index b97286c48735..775569ec50d0 100644
--- a/kernel/trace/trace_hwlat.c
+++ b/kernel/trace/trace_hwlat.c
@@ -127,7 +127,7 @@ static void trace_hwlat_sample(struct hwlat_sample *sample)
 	entry->nmi_count		= sample->nmi_count;
 
 	if (!call_filter_check_discard(call, entry, buffer, event))
-		__buffer_unlock_commit(buffer, event);
+		trace_buffer_unlock_commit_nostack(buffer, event);
 }
 
 /* Macros to encapsulate the time capturing infrastructure */
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]


#1532410 — [for-next][PATCH 2/8] ring-buffer: Make rb_reserve_next_event() always inlined

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-11-29 16:20 +0100
Subject[for-next][PATCH 2/8] ring-buffer: Make rb_reserve_next_event() always inlined
Message-ID<sISmm-1He-13@gated-at.bofh.it>
In reply to#1532401
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

The function rb_reserved_next_event() is called by two functions:
ring_buffer_lock_reserve() and ring_buffer_write(). This is in a very hot
path of the tracing code, and it is best that they are not functions. The
two callers are basically wrapers for rb_reserver_next_event(). Removing the
function calls can save execution time in the hotpath of tracing.

Link: http://lkml.kernel.org/r/20161121183700.GW26852@two.firstfloor.org

Reported-by: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/ring_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 9c143739b8d7..1f3580cee6cc 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2733,7 +2733,7 @@ __rb_reserve_next(struct ring_buffer_per_cpu *cpu_buffer,
 	return event;
 }
 
-static struct ring_buffer_event *
+static __always_inline struct ring_buffer_event *
 rb_reserve_next_event(struct ring_buffer *buffer,
 		      struct ring_buffer_per_cpu *cpu_buffer,
 		      unsigned long length)
-- 
2.10.2

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web