linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH 0/4] tracing/perf: Use helper functions to help shrink kernel size
@ 2012-08-10  3:43 Steven Rostedt
  2012-08-10  3:43 ` [RFC][PATCH 1/4] tracing: Move print code from macro to standalone function Steven Rostedt
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Steven Rostedt @ 2012-08-10  3:43 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker

This patch set is another effort to bring down the size of trace events.

Helper functions are used to remove duplicate code that is created
for each event by the TRACE_EVENT() macros. With a config that enables
707 events in the core kernel, the size is brought down 91,774 bytes!


Steven Rostedt (4):
      tracing: Move print code from macro to standalone function
      tracing: Move event storage for array from macro to standalone function
      tracing: Use helper functions in event assignment to shrink macro size
      perf/events: Use helper functions in event assignment to shrink macro size

----
 include/linux/ftrace_event.h    |   40 +++++++++++++++--
 include/trace/ftrace.h          |   85 +++++++++---------------------------
 kernel/trace/trace_event_perf.c |   26 +++++++++++
 kernel/trace/trace_events.c     |    6 ---
 kernel/trace/trace_export.c     |   12 ++----
 kernel/trace/trace_output.c     |   90 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 176 insertions(+), 83 deletions(-)

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

* [RFC][PATCH 1/4] tracing: Move print code from macro to standalone function
  2012-08-10  3:43 [RFC][PATCH 0/4] tracing/perf: Use helper functions to help shrink kernel size Steven Rostedt
@ 2012-08-10  3:43 ` Steven Rostedt
  2012-08-10  3:43 ` [RFC][PATCH 2/4] tracing: Move event storage for array " Steven Rostedt
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2012-08-10  3:43 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker

[-- Attachment #1: 0001-tracing-Move-print-code-from-macro-to-standalone-fun.patch --]
[-- Type: text/plain, Size: 5027 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

The code for trace events to format the raw recorded event data
into human readable format in the 'trace' file is repeated for every
event in the system. When you have over 700 events, this can add up
quite a bit.

By making helper functions in the core kernel to do the work
instead, we can shrink the size of the kernel down a bit.

With a kernel configured with 707 events, the change in size was:

   text    data     bss     dec     hex filename
20016471        2594648 1945600 24556719        176b4af vmlinux-before
19994715        2594648 1945600 24534963        1765fb3 vmlinux-after

That's a total of 21756 bytes, which comes down to 30 bytes per event.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/ftrace_event.h |    5 +++++
 include/trace/ftrace.h       |   21 ++------------------
 kernel/trace/trace_output.c  |   44 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index af961d6..56d1e53 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -139,6 +139,11 @@ void trace_current_buffer_discard_commit(struct ring_buffer *buffer,
 
 void tracing_record_cmdline(struct task_struct *tsk);
 
+int ftrace_output_event(struct trace_iterator *iter, struct ftrace_event_call *event,
+			char *fmt, ...);
+
+int ftrace_output_call(struct trace_iterator *iter, char *name, char *fmt, ...);
+
 struct event_filter;
 
 enum trace_reg {
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index c6bc2fa..cb85747 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -228,11 +228,9 @@ ftrace_raw_output_##call(struct trace_iterator *iter, int flags,	\
 			 struct trace_event *trace_event)		\
 {									\
 	struct ftrace_event_call *event;				\
-	struct trace_seq *s = &iter->seq;				\
 	struct ftrace_raw_##call *field;				\
 	struct trace_entry *entry;					\
 	struct trace_seq *p = &iter->tmp_seq;				\
-	int ret;							\
 									\
 	event = container_of(trace_event, struct ftrace_event_call,	\
 			     event);					\
@@ -245,15 +243,8 @@ ftrace_raw_output_##call(struct trace_iterator *iter, int flags,	\
 	}								\
 									\
 	field = (typeof(field))entry;					\
-									\
 	trace_seq_init(p);						\
-	ret = trace_seq_printf(s, "%s: ", event->name);			\
-	if (ret)							\
-		ret = trace_seq_printf(s, print);			\
-	if (!ret)							\
-		return TRACE_TYPE_PARTIAL_LINE;				\
-									\
-	return TRACE_TYPE_HANDLED;					\
+	return ftrace_output_event(iter, event, print);			\
 }									\
 static struct trace_event_functions ftrace_event_type_funcs_##call = {	\
 	.trace			= ftrace_raw_output_##call,		\
@@ -265,11 +256,9 @@ static notrace enum print_line_t					\
 ftrace_raw_output_##call(struct trace_iterator *iter, int flags,	\
 			 struct trace_event *event)			\
 {									\
-	struct trace_seq *s = &iter->seq;				\
 	struct ftrace_raw_##template *field;				\
 	struct trace_entry *entry;					\
 	struct trace_seq *p = &iter->tmp_seq;				\
-	int ret;							\
 									\
 	entry = iter->ent;						\
 									\
@@ -281,13 +270,7 @@ ftrace_raw_output_##call(struct trace_iterator *iter, int flags,	\
 	field = (typeof(field))entry;					\
 									\
 	trace_seq_init(p);						\
-	ret = trace_seq_printf(s, "%s: ", #call);			\
-	if (ret)							\
-		ret = trace_seq_printf(s, print);			\
-	if (!ret)							\
-		return TRACE_TYPE_PARTIAL_LINE;				\
-									\
-	return TRACE_TYPE_HANDLED;					\
+	return ftrace_output_call(iter, #call, print);			\
 }									\
 static struct trace_event_functions ftrace_event_type_funcs_##call = {	\
 	.trace			= ftrace_raw_output_##call,		\
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 123b189..b927f45 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -397,6 +397,50 @@ ftrace_print_hex_seq(struct trace_seq *p, const unsigned char *buf, int buf_len)
 }
 EXPORT_SYMBOL(ftrace_print_hex_seq);
 
+static int ftrace_output_raw(struct trace_iterator *iter, char *name,
+			     char *fmt, va_list ap)
+{
+	struct trace_seq *s = &iter->seq;
+	int ret;
+
+	ret = trace_seq_printf(s, "%s: ", name);
+	if (!ret)
+		return TRACE_TYPE_PARTIAL_LINE;
+
+	ret = trace_seq_vprintf(s, fmt, ap);
+
+	if (!ret)
+		return TRACE_TYPE_PARTIAL_LINE;
+
+	return TRACE_TYPE_HANDLED;
+}
+
+int ftrace_output_call(struct trace_iterator *iter, char *name, char *fmt, ...)
+{
+	va_list ap;
+	int ret;
+
+	va_start(ap, fmt);
+	ret = ftrace_output_raw(iter, name, fmt, ap);
+	va_end(ap);
+
+	return ret;
+}
+
+int ftrace_output_event(struct trace_iterator *iter, struct ftrace_event_call *event,
+			char *fmt, ...)
+{
+	va_list ap;
+	int ret;
+
+	va_start(ap, fmt);
+	ret = ftrace_output_raw(iter, event->name, fmt, ap);
+	va_end(ap);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(ftrace_output_event);
+
 #ifdef CONFIG_KRETPROBES
 static inline const char *kretprobed(const char *name)
 {
-- 
1.7.10.4



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

* [RFC][PATCH 2/4] tracing: Move event storage for array from macro to standalone function
  2012-08-10  3:43 [RFC][PATCH 0/4] tracing/perf: Use helper functions to help shrink kernel size Steven Rostedt
  2012-08-10  3:43 ` [RFC][PATCH 1/4] tracing: Move print code from macro to standalone function Steven Rostedt
@ 2012-08-10  3:43 ` Steven Rostedt
  2012-08-10  3:43 ` [RFC][PATCH 3/4] tracing: Use helper functions in event assignment to shrink macro size Steven Rostedt
  2012-08-10  3:43 ` [RFC][PATCH 4/4] perf/events: " Steven Rostedt
  3 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2012-08-10  3:43 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker

[-- Attachment #1: 0002-tracing-Move-event-storage-for-array-from-macro-to-s.patch --]
[-- Type: text/plain, Size: 5580 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

The code that shows array fields for events is defined for all events.
This can add up quite a bit when you have over 700 events.

By making helper functions in the core kernel to do the work
instead, we can shrink the size of the kernel down a little.

With a kernel configured with 707 events, the change in size was:

   text    data     bss     dec     hex filename
19994715        2594648 1945600 24534963        1765fb3 vmlinux-before
19991705        2594648 1945600 24531953        17653f1 vmlinux-after

That's a total of 3010 bytes, which comes down to 4 bytes per event.
Although it's not much, this code is just called at initialization.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/ftrace_event.h |    8 ++++----
 include/trace/ftrace.h       |   12 ++++--------
 kernel/trace/trace_events.c  |    6 ------
 kernel/trace/trace_export.c  |   12 ++++--------
 kernel/trace/trace_output.c  |   20 ++++++++++++++++++++
 5 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 56d1e53..22f5fec 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -144,6 +144,10 @@ int ftrace_output_event(struct trace_iterator *iter, struct ftrace_event_call *e
 
 int ftrace_output_call(struct trace_iterator *iter, char *name, char *fmt, ...);
 
+int ftrace_event_define_field(struct ftrace_event_call *call,
+			      char *type, int len, char *item, int offset,
+			      int field_size, int sign, int filter);
+
 struct event_filter;
 
 enum trace_reg {
@@ -260,10 +264,6 @@ enum {
 	FILTER_TRACE_FN,
 };
 
-#define EVENT_STORAGE_SIZE 128
-extern struct mutex event_storage_mutex;
-extern char event_storage[EVENT_STORAGE_SIZE];
-
 extern int trace_event_raw_init(struct ftrace_event_call *call);
 extern int trace_define_field(struct ftrace_event_call *call, const char *type,
 			      const char *name, int offset, int size,
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index cb85747..95d6704 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -293,15 +293,11 @@ static struct trace_event_functions ftrace_event_type_funcs_##call = {	\
 #undef __array
 #define __array(type, item, len)					\
 	do {								\
-		mutex_lock(&event_storage_mutex);			\
 		BUILD_BUG_ON(len > MAX_FILTER_STR_VAL);			\
-		snprintf(event_storage, sizeof(event_storage),		\
-			 "%s[%d]", #type, len);				\
-		ret = trace_define_field(event_call, event_storage, #item, \
-				 offsetof(typeof(field), item),		\
-				 sizeof(field.item),			\
-				 is_signed_type(type), FILTER_OTHER);	\
-		mutex_unlock(&event_storage_mutex);			\
+		ret = ftrace_event_define_field(event_call, #type, len,	\
+				#item, offsetof(typeof(field), item),   \
+				sizeof(field.item),			\
+			 	is_signed_type(type), FILTER_OTHER); \
 		if (ret)						\
 			return ret;					\
 	} while (0);
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 6825d83..5c5dc1f 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -27,12 +27,6 @@
 
 DEFINE_MUTEX(event_mutex);
 
-DEFINE_MUTEX(event_storage_mutex);
-EXPORT_SYMBOL_GPL(event_storage_mutex);
-
-char event_storage[EVENT_STORAGE_SIZE];
-EXPORT_SYMBOL_GPL(event_storage);
-
 LIST_HEAD(ftrace_events);
 LIST_HEAD(ftrace_common_fields);
 
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index e039906..f837e35 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -96,14 +96,10 @@ static void __always_unused ____ftrace_check_##name(void)		\
 #define __array(type, item, len)					\
 	do {								\
 		BUILD_BUG_ON(len > MAX_FILTER_STR_VAL);			\
-		mutex_lock(&event_storage_mutex);			\
-		snprintf(event_storage, sizeof(event_storage),		\
-			 "%s[%d]", #type, len);				\
-		ret = trace_define_field(event_call, event_storage, #item, \
-				 offsetof(typeof(field), item),		\
-				 sizeof(field.item),			\
-				 is_signed_type(type), filter_type);	\
-		mutex_unlock(&event_storage_mutex);			\
+		ret = ftrace_event_define_field(event_call, #type, len,	\
+				#item, offsetof(typeof(field), item),   \
+				sizeof(field.item),			\
+			 	is_signed_type(type), filter_type);	\
 		if (ret)						\
 			return ret;					\
 	} while (0);
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index b927f45..06f27ab 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -20,6 +20,10 @@ static struct hlist_head event_hash[EVENT_HASHSIZE] __read_mostly;
 
 static int next_event_type = __TRACE_LAST_TYPE + 1;
 
+#define EVENT_STORAGE_SIZE 128
+static DEFINE_MUTEX(event_storage_mutex);
+static char event_storage[EVENT_STORAGE_SIZE];
+
 int trace_print_seq(struct seq_file *m, struct trace_seq *s)
 {
 	int len = s->len >= PAGE_SIZE ? PAGE_SIZE - 1 : s->len;
@@ -441,6 +445,22 @@ int ftrace_output_event(struct trace_iterator *iter, struct ftrace_event_call *e
 }
 EXPORT_SYMBOL_GPL(ftrace_output_event);
 
+int ftrace_event_define_field(struct ftrace_event_call *call,
+			      char *type, int len, char *item, int offset,
+			      int field_size, int sign, int filter)
+{
+	int ret;
+
+	mutex_lock(&event_storage_mutex);
+	snprintf(event_storage, sizeof(event_storage),
+		 "%s[%d]", type, len);
+	ret = trace_define_field(call, event_storage, item, offset,
+				 field_size, sign, filter);
+	mutex_unlock(&event_storage_mutex);
+
+	return ret;
+}
+
 #ifdef CONFIG_KRETPROBES
 static inline const char *kretprobed(const char *name)
 {
-- 
1.7.10.4



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

* [RFC][PATCH 3/4] tracing: Use helper functions in event assignment to shrink macro size
  2012-08-10  3:43 [RFC][PATCH 0/4] tracing/perf: Use helper functions to help shrink kernel size Steven Rostedt
  2012-08-10  3:43 ` [RFC][PATCH 1/4] tracing: Move print code from macro to standalone function Steven Rostedt
  2012-08-10  3:43 ` [RFC][PATCH 2/4] tracing: Move event storage for array " Steven Rostedt
@ 2012-08-10  3:43 ` Steven Rostedt
  2012-08-10  3:43 ` [RFC][PATCH 4/4] perf/events: " Steven Rostedt
  3 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2012-08-10  3:43 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker

[-- Attachment #1: 0003-tracing-Use-helper-functions-in-event-assignment-to-.patch --]
[-- Type: text/plain, Size: 4594 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

The functions that assign the contents for the ftrace events are
defined by the TRACE_EVENT() macros. Each event has its own unique
way to assign data to its buffer. When you have over 700 events,
that means there's 700 functions assigning data uniquely for each
event (not really that many, as DECLARE_EVENT_CLASS() and multiple
DEFINE_EVENT()s will only need a single function).

By making helper functions in the core kernel to do some of the work
instead, we can shrink the size of the kernel down a bit.

With a kernel configured with 707 events, the change in size was:

   text    data     bss     dec     hex filename
19991705        2594648 1945600 24531953        17653f1 vmlinux-before
19966937        2594648 1945600 24507185        175f331 vmlinux-after

That's a total of 24768 bytes, which comes down to 35 bytes per event.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/ftrace_event.h |   14 ++++++++++++++
 include/trace/ftrace.h       |   21 ++++++---------------
 kernel/trace/trace_output.c  |   26 ++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 22f5fec..b9f39d3 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -148,6 +148,20 @@ int ftrace_event_define_field(struct ftrace_event_call *call,
 			      char *type, int len, char *item, int offset,
 			      int field_size, int sign, int filter);
 
+struct ftrace_event_buffer {
+	struct ring_buffer		*buffer;
+	struct ring_buffer_event	*event;
+	unsigned long			flags;
+	int				pc;
+};
+
+void *ftrace_event_buffer_reserve(struct ftrace_event_buffer *fbuffer,
+				  int type, unsigned long len);
+
+void ftrace_event_buffer_commit(struct ftrace_event_buffer *fbuffer,
+				struct ftrace_event_call *event_call,
+				void *entry);
+
 struct event_filter;
 
 enum trace_reg {
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 95d6704..8118451 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -499,33 +499,24 @@ ftrace_raw_event_##call(void *__data, proto)				\
 {									\
 	struct ftrace_event_call *event_call = __data;			\
 	struct ftrace_data_offsets_##call __maybe_unused __data_offsets;\
-	struct ring_buffer_event *event;				\
+	struct ftrace_event_buffer fbuffer;				\
 	struct ftrace_raw_##call *entry;				\
-	struct ring_buffer *buffer;					\
-	unsigned long irq_flags;					\
 	int __data_size;						\
-	int pc;								\
-									\
-	local_save_flags(irq_flags);					\
-	pc = preempt_count();						\
 									\
 	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
 									\
-	event = trace_current_buffer_lock_reserve(&buffer,		\
+	entry = ftrace_event_buffer_reserve(&fbuffer,			\
 				 event_call->event.type,		\
-				 sizeof(*entry) + __data_size,		\
-				 irq_flags, pc);			\
-	if (!event)							\
+				 sizeof(*entry) + __data_size);		\
+									\
+	if (!entry)							\
 		return;							\
-	entry	= ring_buffer_event_data(event);			\
 									\
 	tstruct								\
 									\
 	{ assign; }							\
 									\
-	if (!filter_current_check_discard(buffer, event_call, entry, event)) \
-		trace_nowake_buffer_unlock_commit(buffer,		\
-						  event, irq_flags, pc); \
+	ftrace_event_buffer_commit(&fbuffer, event_call, entry);	\
 }
 /*
  * The ftrace_test_probe is compiled out, it is only here as a build time check
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 06f27ab..2babeef 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -461,6 +461,32 @@ int ftrace_event_define_field(struct ftrace_event_call *call,
 	return ret;
 }
 
+void *ftrace_event_buffer_reserve(struct ftrace_event_buffer *fbuffer,
+				  int type, unsigned long len)
+{
+	local_save_flags(fbuffer->flags);
+	fbuffer->pc = preempt_count();
+
+	fbuffer->event =
+		trace_current_buffer_lock_reserve(&fbuffer->buffer, type, len,
+						  fbuffer->flags, fbuffer->pc);
+	if (!fbuffer->event)
+		return NULL;
+
+	return ring_buffer_event_data(fbuffer->event);
+}
+
+void ftrace_event_buffer_commit(struct ftrace_event_buffer *fbuffer,
+				struct ftrace_event_call *event_call,
+				void *entry)
+{
+	if (!filter_current_check_discard(fbuffer->buffer, event_call,
+					  entry, fbuffer->event))
+		trace_nowake_buffer_unlock_commit(fbuffer->buffer,
+						  fbuffer->event,
+						  fbuffer->flags, fbuffer->pc);
+}
+
 #ifdef CONFIG_KRETPROBES
 static inline const char *kretprobed(const char *name)
 {
-- 
1.7.10.4



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

* [RFC][PATCH 4/4] perf/events: Use helper functions in event assignment to shrink macro size
  2012-08-10  3:43 [RFC][PATCH 0/4] tracing/perf: Use helper functions to help shrink kernel size Steven Rostedt
                   ` (2 preceding siblings ...)
  2012-08-10  3:43 ` [RFC][PATCH 3/4] tracing: Use helper functions in event assignment to shrink macro size Steven Rostedt
@ 2012-08-10  3:43 ` Steven Rostedt
  2012-08-13  8:03   ` Peter Zijlstra
  3 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2012-08-10  3:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker, Peter Zijlstra

[-- Attachment #1: 0004-perf-events-Use-helper-functions-in-event-assignment.patch --]
[-- Type: text/plain, Size: 5305 bytes --]

From: Steven Rostedt <srostedt@redhat.com>

The functions that assign the contents for the perf software events are
defined by the TRACE_EVENT() macros. Each event has its own unique
way to assign data to its buffer. When you have over 700 events,
that means there's 700 functions assigning data uniquely for each
event.

By making helper functions in the core kernel to do the work
instead, we can shrink the size of the kernel down a bit.

With a kernel configured with 707 events, the change in size was:

   text    data     bss     dec     hex filename
19966937        2594648 1945600 24507185        175f331 vmlinux-before
19924761        2594584 1945600 24464945        1754e31 vmlinux-after

That's a total of 42240 bytes, which comes down to 59 bytes per event.

Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/ftrace_event.h    |   13 +++++++++++++
 include/trace/ftrace.h          |   31 ++++++++-----------------------
 kernel/trace/trace_event_perf.c |   26 ++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index b9f39d3..06c6d04 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -313,6 +313,19 @@ struct perf_event;
 
 DECLARE_PER_CPU(struct pt_regs, perf_trace_regs);
 
+struct perf_trace_event {
+	struct pt_regs		regs;
+	u64			addr;
+	u64			count;
+	int			entry_size;
+	int			rctx;
+};
+
+extern void *perf_trace_event_setup(struct ftrace_event_call *event_call, 
+				    struct perf_trace_event *pe);
+extern void perf_trace_event_submit(void *raw_data, struct ftrace_event_call *event_call,
+				    struct perf_trace_event *pe);
+
 extern int  perf_trace_init(struct perf_event *event);
 extern void perf_trace_destroy(struct perf_event *event);
 extern int  perf_trace_add(struct perf_event *event, int flags);
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 8118451..a02d48d 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -677,10 +677,10 @@ __attribute__((section("_ftrace_events"))) *__event_##call = &event_##call
 #define __get_str(field) (char *)__get_dynamic_array(field)
 
 #undef __perf_addr
-#define __perf_addr(a) __addr = (a)
+#define __perf_addr(a) __pe.addr = (a)
 
 #undef __perf_count
-#define __perf_count(c) __count = (c)
+#define __perf_count(c) __pe.count = (c)
 
 #undef TP_perf_assign
 #define TP_perf_assign(args...) args
@@ -693,36 +693,21 @@ perf_trace_##call(void *__data, proto)					\
 	struct ftrace_event_call *event_call = __data;			\
 	struct ftrace_data_offsets_##call __maybe_unused __data_offsets;\
 	struct ftrace_raw_##call *entry;				\
-	struct pt_regs __regs;						\
-	u64 __addr = 0, __count = 1;					\
-	struct hlist_head *head;					\
-	int __entry_size;						\
+	struct perf_trace_event __pe;					\
 	int __data_size;						\
-	int rctx;							\
-									\
-	perf_fetch_caller_regs(&__regs);				\
 									\
 	__data_size = ftrace_get_offsets_##call(&__data_offsets, args); \
-	__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
-			     sizeof(u64));				\
-	__entry_size -= sizeof(u32);					\
-									\
-	if (WARN_ONCE(__entry_size > PERF_MAX_TRACE_SIZE,		\
-		      "profile buffer not large enough"))		\
-		return;							\
+	__pe.entry_size = __data_size + sizeof(*entry);			\
+	__pe.addr = 0;							\
+	__pe.count = 1;							\
 									\
-	entry = (struct ftrace_raw_##call *)perf_trace_buf_prepare(	\
-		__entry_size, event_call->event.type, &__regs, &rctx);	\
-	if (!entry)							\
-		return;							\
+	entry = perf_trace_event_setup(event_call, &__pe);		\
 									\
 	tstruct								\
 									\
 	{ assign; }							\
 									\
-	head = this_cpu_ptr(event_call->perf_events);			\
-	perf_trace_buf_submit(entry, __entry_size, rctx, __addr,	\
-		__count, &__regs, head);				\
+	perf_trace_event_submit(entry, event_call, &__pe);		\
 }
 
 /*
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index 9824419..75484aa 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -21,6 +21,32 @@ typedef typeof(unsigned long [PERF_MAX_TRACE_SIZE / sizeof(unsigned long)])
 /* Count the events in use (per event id, not per instance) */
 static int	total_ref_count;
 
+void *perf_trace_event_setup(struct ftrace_event_call *event_call, 
+			     struct perf_trace_event *pe)
+{
+	perf_fetch_caller_regs(&pe->regs);
+
+	pe->entry_size = ALIGN(pe->entry_size + sizeof(u32), sizeof(u64));
+	pe->entry_size -= sizeof(u32);
+
+	if (WARN_ONCE(pe->entry_size > PERF_MAX_TRACE_SIZE,
+		      "profile buffer not large enough"))
+		return NULL;
+
+	return perf_trace_buf_prepare(pe->entry_size,
+			event_call->event.type, &pe->regs, &pe->rctx);
+}
+
+void perf_trace_event_submit(void *raw_data, struct ftrace_event_call *event_call,
+			     struct perf_trace_event *pe)
+{
+	struct hlist_head *head;
+
+	head = this_cpu_ptr(event_call->perf_events);
+	perf_trace_buf_submit(raw_data, pe->entry_size, pe->rctx, pe->addr,
+			      pe->count, &pe->regs, head);
+}
+
 static int perf_trace_event_perm(struct ftrace_event_call *tp_event,
 				 struct perf_event *p_event)
 {
-- 
1.7.10.4



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

* Re: [RFC][PATCH 4/4] perf/events: Use helper functions in event assignment to shrink macro size
  2012-08-10  3:43 ` [RFC][PATCH 4/4] perf/events: " Steven Rostedt
@ 2012-08-13  8:03   ` Peter Zijlstra
  2012-08-13 13:03     ` Steven Rostedt
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Zijlstra @ 2012-08-13  8:03 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Frederic Weisbecker

On Thu, 2012-08-09 at 23:43 -0400, Steven Rostedt wrote:
> plain text document attachment
> (0004-perf-events-Use-helper-functions-in-event-assignment.patch)
> From: Steven Rostedt <srostedt@redhat.com>
> 
> The functions that assign the contents for the perf software events are
> defined by the TRACE_EVENT() macros. Each event has its own unique
> way to assign data to its buffer. When you have over 700 events,
> that means there's 700 functions assigning data uniquely for each
> event.
> 
> By making helper functions in the core kernel to do the work
> instead, we can shrink the size of the kernel down a bit.
> 
> With a kernel configured with 707 events, the change in size was:
> 
>    text    data     bss     dec     hex filename
> 19966937        2594648 1945600 24507185        175f331 vmlinux-before
> 19924761        2594584 1945600 24464945        1754e31 vmlinux-after
> 
> That's a total of 42240 bytes, which comes down to 59 bytes per event.

Seems like good stuff.. one nit..


> +void perf_trace_event_submit(void *raw_data, struct ftrace_event_call *event_call,
> +                            struct perf_trace_event *pe)
> +{
> +       struct hlist_head *head;
> +
> +       head = this_cpu_ptr(event_call->perf_events);
> +       perf_trace_buf_submit(raw_data, pe->entry_size, pe->rctx, pe->addr,
> +                             pe->count, &pe->regs, head);
> +}

Can you make perf_trace_buf_submit() go away? Its reduced to a simple
fwd function and layering another wrapper on top seems like pushing it.

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

* Re: [RFC][PATCH 4/4] perf/events: Use helper functions in event assignment to shrink macro size
  2012-08-13  8:03   ` Peter Zijlstra
@ 2012-08-13 13:03     ` Steven Rostedt
  2012-08-13 13:52       ` Peter Zijlstra
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2012-08-13 13:03 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Frederic Weisbecker

On Mon, 2012-08-13 at 10:03 +0200, Peter Zijlstra wrote:

> > +void perf_trace_event_submit(void *raw_data, struct ftrace_event_call *event_call,
> > +                            struct perf_trace_event *pe)
> > +{
> > +       struct hlist_head *head;
> > +
> > +       head = this_cpu_ptr(event_call->perf_events);
> > +       perf_trace_buf_submit(raw_data, pe->entry_size, pe->rctx, pe->addr,
> > +                             pe->count, &pe->regs, head);
> > +}
> 
> Can you make perf_trace_buf_submit() go away? Its reduced to a simple
> fwd function and layering another wrapper on top seems like pushing it.

You mean just have perf_trace_event_submit() call perf_tp_event()
directly?

I have no problem with that. Although I may make that into a separate
patch to keep this patch as a 'move' and the other patch as the change.

Looking at the history of perf_trace_buf_submit(), it use to be more
than one function call. But when you inlined
perf_swevent_put_recursion_context(), it became just a one2one mapping.

I'm assuming that we want to convert all calls to
perf_trace_buf_submit()s into perf_tp_event()?

-- Steve



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

* Re: [RFC][PATCH 4/4] perf/events: Use helper functions in event assignment to shrink macro size
  2012-08-13 13:03     ` Steven Rostedt
@ 2012-08-13 13:52       ` Peter Zijlstra
  2012-08-13 14:40         ` Steven Rostedt
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Zijlstra @ 2012-08-13 13:52 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Frederic Weisbecker

On Mon, 2012-08-13 at 09:03 -0400, Steven Rostedt wrote:
> On Mon, 2012-08-13 at 10:03 +0200, Peter Zijlstra wrote:
> 
> > > +void perf_trace_event_submit(void *raw_data, struct ftrace_event_call *event_call,
> > > +                            struct perf_trace_event *pe)
> > > +{
> > > +       struct hlist_head *head;
> > > +
> > > +       head = this_cpu_ptr(event_call->perf_events);
> > > +       perf_trace_buf_submit(raw_data, pe->entry_size, pe->rctx, pe->addr,
> > > +                             pe->count, &pe->regs, head);
> > > +}
> > 
> > Can you make perf_trace_buf_submit() go away? Its reduced to a simple
> > fwd function and layering another wrapper on top seems like pushing it.
> 
> You mean just have perf_trace_event_submit() call perf_tp_event()
> directly?
> 
> I have no problem with that. Although I may make that into a separate
> patch to keep this patch as a 'move' and the other patch as the change.
> 
> Looking at the history of perf_trace_buf_submit(), it use to be more
> than one function call. But when you inlined
> perf_swevent_put_recursion_context(), it became just a one2one mapping.

Right.

> I'm assuming that we want to convert all calls to
> perf_trace_buf_submit()s into perf_tp_event()?

Yeah.. I think you're referring to the {u,k}probes open-coded nonsense?
Should we make those use these new helpers you created as well?

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

* Re: [RFC][PATCH 4/4] perf/events: Use helper functions in event assignment to shrink macro size
  2012-08-13 13:52       ` Peter Zijlstra
@ 2012-08-13 14:40         ` Steven Rostedt
  2012-08-13 14:47           ` Peter Zijlstra
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2012-08-13 14:40 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Frederic Weisbecker

On Mon, 2012-08-13 at 15:52 +0200, Peter Zijlstra wrote:

> > Looking at the history of perf_trace_buf_submit(), it use to be more
> > than one function call. But when you inlined
> > perf_swevent_put_recursion_context(), it became just a one2one mapping.
> 
> Right.

OK, this should go as a separate change.

> 
> > I'm assuming that we want to convert all calls to
> > perf_trace_buf_submit()s into perf_tp_event()?
> 
> Yeah.. I think you're referring to the {u,k}probes open-coded nonsense?
> Should we make those use these new helpers you created as well?

They may not need it. The helpers I created were for the macros that are
created by all TRACE_EVENT() instances. If the uprobes and kprobes only
have a single instance, then they probably don't need the helpers. But
they still call the perf_trace_buf_submit(), which can can probably be
cleaned up.

-- Steve



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

* Re: [RFC][PATCH 4/4] perf/events: Use helper functions in event assignment to shrink macro size
  2012-08-13 14:40         ` Steven Rostedt
@ 2012-08-13 14:47           ` Peter Zijlstra
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Zijlstra @ 2012-08-13 14:47 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Ingo Molnar, Andrew Morton, Frederic Weisbecker

On Mon, 2012-08-13 at 10:40 -0400, Steven Rostedt wrote:
> > Yeah.. I think you're referring to the {u,k}probes open-coded nonsense?
> > Should we make those use these new helpers you created as well?
> 
> They may not need it. The helpers I created were for the macros that are
> created by all TRACE_EVENT() instances. If the uprobes and kprobes only
> have a single instance, then they probably don't need the helpers. But
> they still call the perf_trace_buf_submit(), which can can probably be
> cleaned up. 

Right, I realize that. I was just thinking it might be good to keep all
(3?) copies similar. Also avoiding two sets of wrappers seems like a
good idea.


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

* [RFC][PATCH 0/4] tracing/perf: Use helper functions to help shrink kernel size
@ 2014-02-06 17:39 Steven Rostedt
  0 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2014-02-06 17:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Peter Zijlstra,
	Frederic Weisbecker, Namhyung Kim, Oleg Nesterov, Li Zefan


I posted this a while ago (August 2012), and it seemed to have positive
feedback. But I forgot about it and it never went any further.

http://lkml.kernel.org/r/20120810034302.758092203@goodmis.org

It works to move the tracepoint code out of the macros and into reusable
functions that can save a whopping 73K from the kernel memory (with just the
modules I used compiled in).

There was some changes in mainline since I last posted this that helped
lower the tracepoint footprint, and that makes the first patch not as much
of an approvement that it was in the past.

Anyway, hopefully this can get into 3.15.

-- Steve


Steven Rostedt (4):
      tracing: Move raw output code from macro to standalone function
      tracing: Move event storage for array from macro to standalone function
      tracing: Use helper functions in event assignment to shrink macro size
      perf/events: Use helper functions in event assignment to shrink macro size

----
 include/linux/ftrace_event.h    | 46 +++++++++++++++++++++--
 include/trace/ftrace.h          | 75 ++++++++++++-------------------------
 kernel/trace/trace_event_perf.c | 51 +++++++++++++++++++++++++
 kernel/trace/trace_events.c     |  6 ---
 kernel/trace/trace_export.c     | 12 ++----
 kernel/trace/trace_output.c     | 83 +++++++++++++++++++++++++++++++++++++++++
 6 files changed, 203 insertions(+), 70 deletions(-)

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

end of thread, other threads:[~2014-02-06 18:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-10  3:43 [RFC][PATCH 0/4] tracing/perf: Use helper functions to help shrink kernel size Steven Rostedt
2012-08-10  3:43 ` [RFC][PATCH 1/4] tracing: Move print code from macro to standalone function Steven Rostedt
2012-08-10  3:43 ` [RFC][PATCH 2/4] tracing: Move event storage for array " Steven Rostedt
2012-08-10  3:43 ` [RFC][PATCH 3/4] tracing: Use helper functions in event assignment to shrink macro size Steven Rostedt
2012-08-10  3:43 ` [RFC][PATCH 4/4] perf/events: " Steven Rostedt
2012-08-13  8:03   ` Peter Zijlstra
2012-08-13 13:03     ` Steven Rostedt
2012-08-13 13:52       ` Peter Zijlstra
2012-08-13 14:40         ` Steven Rostedt
2012-08-13 14:47           ` Peter Zijlstra
  -- strict thread matches above, loose matches on Subject: below --
2014-02-06 17:39 [RFC][PATCH 0/4] tracing/perf: Use helper functions to help shrink kernel size Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).