[RFC PATCH 0/2] trace: Move trace data to new section _ftrace_data

[RFC PATCH 0/2] trace: Move trace data to new section _ftrace_data

[Joe Perches]

Move the trace data to a separate section to make it easier to
examine the amount of actual data in an object file.

Joe Perches (2):
  trace: Move data to new section _ftrace_data
  trace_events/trace_export: Use __section

 include/trace/trace_events.h | 36 +++++++++++++++++++++---------------
 kernel/trace/trace_export.c  | 13 ++++++++-----
 2 files changed, 29 insertions(+), 20 deletions(-)

-- 
2.24.0

[RFC PATCH 1/2] trace: Move data to new section _ftrace_data

[Joe Perches]

Move the data used by tracing to a separate section to
help identify and perhaps control access to tracing data.

Signed-off-by: Joe Perches <joe@perches.com>
---
 include/trace/trace_events.h | 31 ++++++++++++++++++++++---------
 kernel/trace/trace_export.c  | 10 +++++++---
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h
index 502c7b..361999f 100644
--- a/include/trace/trace_events.h
+++ b/include/trace/trace_events.h
@@ -369,7 +369,9 @@ trace_raw_output_##call(struct trace_iterator *iter, int flags,		\
 									\
 	return trace_handle_return(s);					\
 }									\
-static struct trace_event_functions trace_event_type_funcs_##call = {	\
+static struct trace_event_functions					\
+__attribute__((section("_ftrace_data")))				\
+trace_event_type_funcs_##call = {					\
 	.trace			= trace_raw_output_##call,		\
 };
 
@@ -439,9 +441,12 @@ static struct trace_event_functions trace_event_type_funcs_##call = {	\
 
 #undef DECLARE_EVENT_CLASS
 #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, func, print)	\
-static struct trace_event_fields trace_event_fields_##call[] = {	\
+static struct trace_event_fields					\
+__attribute__((section("_ftrace_data")))				\
+trace_event_fields_##call[] = {						\
 	tstruct								\
-	{} };
+	{}								\
+};
 
 #undef DEFINE_EVENT
 #define DEFINE_EVENT(template, name, proto, args)
@@ -746,7 +751,8 @@ static inline void ftrace_test_probe_##call(void)			\
 #undef DECLARE_EVENT_CLASS
 #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\
 _TRACE_PERF_PROTO(call, PARAMS(proto));					\
-static char print_fmt_##call[] = print;					\
+static char __attribute__((section("_ftrace_data")))			\
+print_fmt_##call[] = print;						\
 static struct trace_event_class __used __refdata event_class_##call = { \
 	.system			= TRACE_SYSTEM_STRING,			\
 	.fields_array		= trace_event_fields_##call,		\
@@ -760,7 +766,9 @@ static struct trace_event_class __used __refdata event_class_##call = { \
 #undef DEFINE_EVENT
 #define DEFINE_EVENT(template, call, proto, args)			\
 									\
-static struct trace_event_call __used event_##call = {			\
+static struct trace_event_call __used					\
+__attribute__((section("_ftrace_data")))				\
+event_##call = {							\
 	.class			= &event_class_##template,		\
 	{								\
 		.tp			= &__tracepoint_##call,		\
@@ -770,14 +778,18 @@ static struct trace_event_call __used event_##call = {			\
 	.flags			= TRACE_EVENT_FL_TRACEPOINT,		\
 };									\
 static struct trace_event_call __used					\
-__attribute__((section("_ftrace_events"))) *__event_##call = &event_##call
+__attribute__((section("_ftrace_events")))				\
+*__event_##call = &event_##call
 
 #undef DEFINE_EVENT_PRINT
 #define DEFINE_EVENT_PRINT(template, call, proto, args, print)		\
 									\
-static char print_fmt_##call[] = print;					\
+static char __attribute__((section("_ftrace_data")))			\
+print_fmt_##call[] = print;						\
 									\
-static struct trace_event_call __used event_##call = {			\
+static struct trace_event_call __used					\
+__attribute__((section("_ftrace_data")))				\
+event_##call = {							\
 	.class			= &event_class_##template,		\
 	{								\
 		.tp			= &__tracepoint_##call,		\
@@ -787,6 +799,7 @@ static struct trace_event_call __used event_##call = {			\
 	.flags			= TRACE_EVENT_FL_TRACEPOINT,		\
 };									\
 static struct trace_event_call __used					\
-__attribute__((section("_ftrace_events"))) *__event_##call = &event_##call
+__attribute__((section("_ftrace_events")))				\
+*__event_##call = &event_##call
 
 #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index 77ce5a..90af7c9 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -111,9 +111,12 @@ static void __always_unused ____ftrace_check_##name(void)		\
 
 #undef FTRACE_ENTRY
 #define FTRACE_ENTRY(name, struct_name, id, tstruct, print)		\
-static struct trace_event_fields ftrace_event_fields_##name[] = {	\
+static struct trace_event_fields					\
+__attribute__((section("_ftrace_data")))				\
+ftrace_event_fields_##name[] = {					\
 	tstruct								\
-	{} };
+	{}								\
+};
 
 #include "trace_entries.h"
 
@@ -150,7 +153,8 @@ static struct trace_event_class __refdata event_class_ftrace_##call = {	\
 	.reg			= regfn,				\
 };									\
 									\
-struct trace_event_call __used event_##call = {				\
+struct trace_event_call __used __attribute((section("_ftrace_data")))	\
+event_##call = {							\
 	.class			= &event_class_ftrace_##call,		\
 	{								\
 		.name			= #call,			\
-- 
2.24.0

[RFC PATCH 2/2] trace_events/trace_export: Use __section

[Joe Perches]

Convert __attribute__((section("foo"))) to __section(foo) to be a
bit more kernel style compliant and improve readability a tiny bit.

Signed-off-by: Joe Perches <joe@perches.com>
---
 include/trace/trace_events.h | 29 +++++++++++------------------
 kernel/trace/trace_export.c  |  9 ++++-----
 2 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h
index 361999f..a5b2eb 100644
--- a/include/trace/trace_events.h
+++ b/include/trace/trace_events.h
@@ -45,7 +45,7 @@ TRACE_MAKE_SYSTEM_STR();
 		.eval_value = a				\
 	};						\
 	static struct trace_eval_map __used		\
-	__attribute__((section("_ftrace_eval_map")))	\
+	__section(_ftrace_eval_map)			\
 	*TRACE_SYSTEM##_##a = &__##TRACE_SYSTEM##_##a
 
 #undef TRACE_DEFINE_SIZEOF
@@ -58,7 +58,7 @@ TRACE_MAKE_SYSTEM_STR();
 		.eval_value = sizeof(a)			\
 	};						\
 	static struct trace_eval_map __used		\
-	__attribute__((section("_ftrace_eval_map")))	\
+	__section(_ftrace_eval_map)			\
 	*TRACE_SYSTEM##_##a = &__##TRACE_SYSTEM##_##a
 
 /*
@@ -369,8 +369,7 @@ trace_raw_output_##call(struct trace_iterator *iter, int flags,		\
 									\
 	return trace_handle_return(s);					\
 }									\
-static struct trace_event_functions					\
-__attribute__((section("_ftrace_data")))				\
+static struct trace_event_functions __section(_ftrace_data)		\
 trace_event_type_funcs_##call = {					\
 	.trace			= trace_raw_output_##call,		\
 };
@@ -441,8 +440,7 @@ static struct trace_event_functions trace_event_type_funcs_##call = {	\
 
 #undef DECLARE_EVENT_CLASS
 #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, func, print)	\
-static struct trace_event_fields					\
-__attribute__((section("_ftrace_data")))				\
+static struct trace_event_fields __section(_ftrace_data)		\
 trace_event_fields_##call[] = {						\
 	tstruct								\
 	{}								\
@@ -624,7 +622,7 @@ static inline notrace int trace_event_get_offsets_##call(		\
  * // its only safe to use pointers when doing linker tricks to
  * // create an array.
  * static struct trace_event_call __used
- * __attribute__((section("_ftrace_events"))) *__event_<call> = &event_<call>;
+ * __section(_ftrace_events) *__event_<call> = &event_<call>;
  *
  */
 
@@ -751,7 +749,7 @@ static inline void ftrace_test_probe_##call(void)			\
 #undef DECLARE_EVENT_CLASS
 #define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\
 _TRACE_PERF_PROTO(call, PARAMS(proto));					\
-static char __attribute__((section("_ftrace_data")))			\
+static char __section(_ftrace_data)					\
 print_fmt_##call[] = print;						\
 static struct trace_event_class __used __refdata event_class_##call = { \
 	.system			= TRACE_SYSTEM_STRING,			\
@@ -766,8 +764,7 @@ static struct trace_event_class __used __refdata event_class_##call = { \
 #undef DEFINE_EVENT
 #define DEFINE_EVENT(template, call, proto, args)			\
 									\
-static struct trace_event_call __used					\
-__attribute__((section("_ftrace_data")))				\
+static struct trace_event_call __used __section(_ftrace_data)		\
 event_##call = {							\
 	.class			= &event_class_##template,		\
 	{								\
@@ -777,18 +774,15 @@ event_##call = {							\
 	.print_fmt		= print_fmt_##template,			\
 	.flags			= TRACE_EVENT_FL_TRACEPOINT,		\
 };									\
-static struct trace_event_call __used					\
-__attribute__((section("_ftrace_events")))				\
+static struct trace_event_call __used __section(_ftrace_events)		\
 *__event_##call = &event_##call
 
 #undef DEFINE_EVENT_PRINT
 #define DEFINE_EVENT_PRINT(template, call, proto, args, print)		\
-									\
-static char __attribute__((section("_ftrace_data")))			\
+static char __section(_ftrace_data)					\
 print_fmt_##call[] = print;						\
 									\
-static struct trace_event_call __used					\
-__attribute__((section("_ftrace_data")))				\
+static struct trace_event_call __used __section(_ftrace_data)		\
 event_##call = {							\
 	.class			= &event_class_##template,		\
 	{								\
@@ -798,8 +792,7 @@ event_##call = {							\
 	.print_fmt		= print_fmt_##call,			\
 	.flags			= TRACE_EVENT_FL_TRACEPOINT,		\
 };									\
-static struct trace_event_call __used					\
-__attribute__((section("_ftrace_events")))				\
+static struct trace_event_call __used __section(_ftrace_events)		\
 *__event_##call = &event_##call
 
 #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index 90af7c9..e38a672 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -111,8 +111,7 @@ static void __always_unused ____ftrace_check_##name(void)		\
 
 #undef FTRACE_ENTRY
 #define FTRACE_ENTRY(name, struct_name, id, tstruct, print)		\
-static struct trace_event_fields					\
-__attribute__((section("_ftrace_data")))				\
+static struct trace_event_fields __section(_ftrace_data)		\
 ftrace_event_fields_##name[] = {					\
 	tstruct								\
 	{}								\
@@ -153,7 +152,7 @@ static struct trace_event_class __refdata event_class_ftrace_##call = {	\
 	.reg			= regfn,				\
 };									\
 									\
-struct trace_event_call __used __attribute((section("_ftrace_data")))	\
+struct trace_event_call __used __section(_ftrace_data)			\
 event_##call = {							\
 	.class			= &event_class_ftrace_##call,		\
 	{								\
@@ -163,8 +162,8 @@ event_##call = {							\
 	.print_fmt		= print,				\
 	.flags			= TRACE_EVENT_FL_IGNORE_ENABLE,		\
 };									\
-static struct trace_event_call __used						\
-__attribute__((section("_ftrace_events"))) *__event_##call = &event_##call;
+static struct trace_event_call * __used __section(_ftrace_events)	\
+__event_##call = &event_##call;
 
 #undef FTRACE_ENTRY
 #define FTRACE_ENTRY(call, struct_name, etype, tstruct, print)		\
-- 
2.24.0

Re: [RFC PATCH 0/2] trace: Move trace data to new section _ftrace_data

[Steven Rostedt]

On Tue, 18 Feb 2020 18:03:16 -0800
Joe Perches <joe@perches.com> wrote:

> Move the trace data to a separate section to make it easier to
> examine the amount of actual data in an object file.

Not that I'm against this patch set, but can you elaborate more on
"make it easier to examine the amount of actual data in an object file".

Also, don't use "_ftrace" as the section name. "ftrace" should be
reserved for the function hook part of tracing, which trace events do
not apply to. "_trace_event_data" would be more appropriate.

-- Steve


> 
> Joe Perches (2):
>   trace: Move data to new section _ftrace_data
>   trace_events/trace_export: Use __section
> 
>  include/trace/trace_events.h | 36 +++++++++++++++++++++---------------
>  kernel/trace/trace_export.c  | 13 ++++++++-----
>  2 files changed, 29 insertions(+), 20 deletions(-)
> 

Re: [RFC PATCH 0/2] trace: Move trace data to new section _ftrace_data

[Joe Perches]

On Tue, 2020-02-18 at 21:53 -0500, Steven Rostedt wrote:
> On Tue, 18 Feb 2020 18:03:16 -0800
> Joe Perches <joe@perches.com> wrote:
> 
> > Move the trace data to a separate section to make it easier to
> > examine the amount of actual data in an object file.
> 
> Not that I'm against this patch set, but can you elaborate more on
> "make it easier to examine the amount of actual data in an object file".

size -A <object> would now show each section of an
object file separating normal data from trace data.

It makes it easier to identify data that could be const like
https://lore.kernel.org/lkml/60559197a1af9e0af7f329cc3427989e5756846f.camel@perches.com/

> Also, don't use "_ftrace" as the section name. "ftrace" should be
> reserved for the function hook part of tracing, which trace events do
> not apply to. "_trace_event_data" would be more appropriate.

I don't care about the section name at all.

I used that name for consistency with _ftrace_event
in the same file. 
Perhaps the _ftrace_event section
name could be renamed to something
intelligible too.

Re: [RFC PATCH 0/2] trace: Move trace data to new section _ftrace_data

[Steven Rostedt]

On Tue, 18 Feb 2020 19:09:10 -0800
Joe Perches <joe@perches.com> wrote:

> I don't care about the section name at all.
> 
> I used that name for consistency with _ftrace_event
> in the same file. 
> Perhaps the _ftrace_event section
> name could be renamed to something
> intelligible too.

Yes, that should probably get changed. That's a leftover when we just
called everything "ftrace", but it should have been changed when I
renamed the file from ftrace.h to trace_event.h.

-- Steve

Re: [RFC PATCH 0/2] trace: Move trace data to new section _ftrace_data

[Joe Perches]

On Tue, 2020-02-18 at 22:26 -0500, Steven Rostedt wrote:
> On Tue, 18 Feb 2020 19:09:10 -0800
> Joe Perches <joe@perches.com> wrote:
> 
> > I don't care about the section name at all.
> > 
> > I used that name for consistency with _ftrace_event
> > in the same file. 
> > Perhaps the _ftrace_event section
> > name could be renamed to something
> > intelligible too.
> 
> Yes, that should probably get changed. That's a leftover when we just
> called everything "ftrace", but it should have been changed when I
> renamed the file from ftrace.h to trace_event.h.

Pick a name.

btw: it's not used in an x86-64 allmodconfig or defconfig
as an actual separate section as far as I tell.

$ git grep _ftrace_events
include/asm-generic/vmlinux.lds.h:                      __start_ftrace_events = .;                      \
include/asm-generic/vmlinux.lds.h:                      KEEP(*(_ftrace_events))                         \
include/asm-generic/vmlinux.lds.h:                      __stop_ftrace_events = .;                       \
include/linux/syscalls.h:         __attribute__((section("_ftrace_events")))                    \
include/linux/syscalls.h:         __attribute__((section("_ftrace_events")))                    \
include/trace/trace_events.h: * __section(_ftrace_events) *__event_<call> = &event_<call>;
include/trace/trace_events.h:static struct trace_event_call __used __section(_ftrace_events)            \
include/trace/trace_events.h:static struct trace_event_call __used __section(_ftrace_events)            \
kernel/module.c:        mod->trace_events = section_objs(info, "_ftrace_events",
kernel/trace/trace_events.c:extern struct trace_event_call *__start_ftrace_events[];
kernel/trace/trace_events.c:extern struct trace_event_call *__stop_ftrace_events[];
kernel/trace/trace_events.c:    for_each_event(iter, __start_ftrace_events, __stop_ftrace_events) {
kernel/trace/trace_export.c:static struct trace_event_call * __used __section(_ftrace_events)   \