kernel: trace: remove __init from race_selftest_startup_function() and trace_selftest_startup_function_graph()

Like other trace_selftest_startup_*, trace_selftest_startup_function() and trace_selftest_startup_function_graph() need in normal section, or may cause section mismatch.

The related warnings:

LD kernel/trace/built-in.o
WARNING: kernel/trace/built-in.o(.data+0x154c): Section mismatch in reference from the variable function_trace to the function .init.text:trace_selftest_startup_function() The variable function_trace references
the function __init trace_selftest_startup_function()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console

Signed-off-by Chen Gang <[email protected]>

---
 kernel/trace/trace_selftest.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

kernel/trace/trace_selftest.c +2 -2

index a7329b7..e341b9d 100644
@@ -640,7 +640,7 @@ out:
  * Enable ftrace, sleep 1/10 second, and then read the trace
  * buffer to see if all is in order.
  */
-__init int
+int
 trace_selftest_startup_function(struct tracer *trace, struct trace_array *tr)
 {
 	int save_ftrace_enabled = ftrace_enabled;

@@ -734,7 +734,7 @@ static int trace_graph_entry_watchdog(struct ftrace_graph_ent *trace)
  * Pretty much the same than for the function tracer from which the selftest
  * has been borrowed.
  */
-__init int
+int
 trace_selftest_startup_function_graph(struct tracer *trace,
 					struct trace_array *tr)
 {

Quoted 6 lines · Chen Gang

Like other trace_selftest_startup_*, trace_selftest_startup_function() and trace_selftest_startup_function_graph() need in normal section, or may cause section mismatch.

The related warnings:

Quoted 8 lines

LD kernel/trace/built-in.o
WARNING: kernel/trace/built-in.o(.data+0x154c): Section mismatch in reference from the variable function_trace to the function .init.text:trace_selftest_startup_function() The variable function_trace references
the function __init trace_selftest_startup_function()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console

No the fix is to add ref_data to the user. The selftest are only called at boot up. No need to waste memory keeping them around.

Hmm, can all trace_selftest_startup_* (*selftest* in trace_selftest.c) use '__init', so not waste memory keeping them around ?

Quoted 2 lines · Chen Gang

Quoted 2 lines

Hmm, can all trace_selftest_startup_* (*selftest* in trace_selftest.c) use '__init', so not waste memory keeping them around ?

Yeah, they should all be set to __init, but that's pretty low on the totem poll, as distros don't enable selftests in their main kernels.

Quoted 4 lines · Steven Rostedt

Quoted 2 lines · Chen Gang

Quoted 2 lines

Hmm, can all trace_selftest_startup_* (*selftest* in trace_selftest.c) use '__init', so not waste memory keeping them around ?

Quoted 2 lines

Yeah, they should all be set to __init, but that's pretty low on the totem poll, as distros don't enable selftests in their main kernels.

Excuse me, my English is not quite well, I guess your meaning is:

they should all be set to '__init', although it is minor in real world.

Is it correct ?

For me, I recommend to let all *selftest* as the same: "all add ' __init' or none add '__init'" (if choose add, all report warnings).

Is it suitable to still send new related patch for it ? If so, could you provide your suggesting choice (all add, or none add) ?

Quoted 5 lines · Chen Gang

Quoted 4 lines · Steven Rostedt

Quoted 2 lines · Chen Gang

Quoted 2 lines

Hmm, can all trace_selftest_startup_* (*selftest* in trace_selftest.c) use '__init', so not waste memory keeping them around ?

Quoted 8 lines

Quoted 2 lines

Yeah, they should all be set to __init, but that's pretty low on the totem poll, as distros don't enable selftests in their main kernels.

Excuse me, my English is not quite well, I guess your meaning is:

they should all be set to '__init', although it is minor in real world.

Is it correct ?

Correct.

Quoted 5 lines

For me, I recommend to let all *selftest* as the same: "all add ' __init' or none add '__init'" (if choose add, all report warnings).

Quoted 2 lines

Is it suitable to still send new related patch for it ? If so, could you provide your suggesting choice (all add, or none add) ?

Does this patch fix your warning?

-- Steve

kernel/trace/trace.h +9 -0

index a4ed382..5e794d1 100644
@@ -680,6 +680,15 @@ extern int trace_selftest_startup_sched_switch(struct tracer *trace,
 					       struct trace_array *tr);
 extern int trace_selftest_startup_branch(struct tracer *trace,
 					 struct trace_array *tr);
+/*
+ * Tracer data references selftest functions that only occur
+ * on boot up. These can be __init functions. Thus, when selftests
+ * are enabled, then the tracers need to reference __init functions.
+ */
+#define __tracer_data		__refdata
+#else
+/* Tracers are seldom changed. Optimize when selftests are disabled. */
+#define __tracer_data		__read_mostly
 #endif /* CONFIG_FTRACE_STARTUP_TEST */
 
 extern void *head_page(struct trace_array_cpu *data);

kernel/trace/trace_functions.c +1 -1

index b863f93..38fe148 100644
@@ -199,7 +199,7 @@ static int func_set_flag(u32 old_flags, u32 bit, int set)
 	return 0;
 }
 
-static struct tracer function_trace __read_mostly =
+static struct tracer function_trace __tracer_data =
 {
 	.name		= "function",
 	.init		= function_trace_init,

Quoted 22 lines · Steven Rostedt

Quoted 5 lines · Chen Gang

Quoted 4 lines · Steven Rostedt

Quoted 2 lines · Chen Gang

Quoted 2 lines

Hmm, can all trace_selftest_startup_* (*selftest* in trace_selftest.c) use '__init', so not waste memory keeping them around ?

Quoted 8 lines

Quoted 2 lines

Yeah, they should all be set to __init, but that's pretty low on the totem poll, as distros don't enable selftests in their main kernels.

Excuse me, my English is not quite well, I guess your meaning is:

they should all be set to '__init', although it is minor in real world.

Is it correct ?

Correct.

Quoted 5 lines

For me, I recommend to let all *selftest* as the same: "all add ' __init' or none add '__init'" (if choose add, all report warnings).

Quoted 7 lines

Quoted 2 lines

Is it suitable to still send new related patch for it ? If so, could you provide your suggesting choice (all add, or none add) ?

Does this patch fix your warning?

-- Steve

I guess it can (although I do not give a compiling test), it seems a better fixing.

And is it suitable to let all *selftest* as the same ? (all add, or none add '__init').

Thanks.

Quoted 37 lines

kernel/trace/trace.h +9 -0

index a4ed382..5e794d1 100644
@@ -680,6 +680,15 @@ extern int trace_selftest_startup_sched_switch(struct tracer *trace,
 					       struct trace_array *tr);
 extern int trace_selftest_startup_branch(struct tracer *trace,
 					 struct trace_array *tr);
+/*
+ * Tracer data references selftest functions that only occur
+ * on boot up. These can be __init functions. Thus, when selftests
+ * are enabled, then the tracers need to reference __init functions.
+ */
+#define __tracer_data		__refdata
+#else
+/* Tracers are seldom changed. Optimize when selftests are disabled. */
+#define __tracer_data		__read_mostly
 #endif /* CONFIG_FTRACE_STARTUP_TEST */
 
 extern void *head_page(struct trace_array_cpu *data);

kernel/trace/trace_functions.c +1 -1

index b863f93..38fe148 100644
@@ -199,7 +199,7 @@ static int func_set_flag(u32 old_flags, u32 bit, int set)
 	return 0;
 }
 
-static struct tracer function_trace __read_mostly =
+static struct tracer function_trace __tracer_data =
 {
 	.name		= "function",
 	.init		= function_trace_init,

Quoted 23 lines · Chen Gang

Quoted 22 lines · Steven Rostedt

Quoted 5 lines · Chen Gang

Quoted 4 lines · Steven Rostedt

Quoted 2 lines · Chen Gang

Quoted 2 lines

Hmm, can all trace_selftest_startup_* (*selftest* in trace_selftest.c) use '__init', so not waste memory keeping them around ?

Quoted 8 lines

Quoted 2 lines

Yeah, they should all be set to __init, but that's pretty low on the totem poll, as distros don't enable selftests in their main kernels.

Excuse me, my English is not quite well, I guess your meaning is:

they should all be set to '__init', although it is minor in real world.

Is it correct ?

Correct.

Quoted 5 lines

For me, I recommend to let all *selftest* as the same: "all add ' __init' or none add '__init'" (if choose add, all report warnings).

Quoted 8 lines

Quoted 7 lines

Quoted 2 lines

Is it suitable to still send new related patch for it ? If so, could you provide your suggesting choice (all add, or none add) ?

Does this patch fix your warning?

-- Steve

After the test, they will not report the related warning.

Hmm..., but that will let another none *selftest* functions miss '__read_mostly'.

Do the original *selftest* intend to have no '__init' so can fit other none *selftest* with '__read_mostly', and without warnings ?

Welcome any members' suggestions or completions.

Thanks.

Quoted 47 lines

I guess it can (although I do not give a compiling test), it seems a better fixing.

And is it suitable to let all *selftest* as the same ? (all add, or none add '__init').

Thanks.

Quoted 37 lines

kernel/trace/trace.h +9 -0

index a4ed382..5e794d1 100644
@@ -680,6 +680,15 @@ extern int trace_selftest_startup_sched_switch(struct tracer *trace,
 					       struct trace_array *tr);
 extern int trace_selftest_startup_branch(struct tracer *trace,
 					 struct trace_array *tr);
+/*
+ * Tracer data references selftest functions that only occur
+ * on boot up. These can be __init functions. Thus, when selftests
+ * are enabled, then the tracers need to reference __init functions.
+ */
+#define __tracer_data		__refdata
+#else
+/* Tracers are seldom changed. Optimize when selftests are disabled. */
+#define __tracer_data		__read_mostly
 #endif /* CONFIG_FTRACE_STARTUP_TEST */
 
 extern void *head_page(struct trace_array_cpu *data);

kernel/trace/trace_functions.c +1 -1

index b863f93..38fe148 100644
@@ -199,7 +199,7 @@ static int func_set_flag(u32 old_flags, u32 bit, int set)
 	return 0;
 }
 
-static struct tracer function_trace __read_mostly =
+static struct tracer function_trace __tracer_data =
 {
 	.name		= "function",
 	.init		= function_trace_init,

Hello Frederic and Ingo:

Could you provide your suggestions or completions for it ?

The trace_selftest_startup_* funcitons are mostly added by you without '__init', do you have additional considerations about it (intend to have no '__init') ?

If no reply, I recommend to keep no '__init': apply this patch or regress part of the patch "f1ed7c7 ftrace: Do not run selftest if command line parameter is set" (at least, it can avoid related warnings and treat all *selftest* fair).

Quoted 1 line · Chen Gang

Quoted 1 line

Hello Frederic and Ingo:

Are you trying to go around me? I wrote this code and I'm one of the maintainers for it. Your issue is very minor, and can wait till other things get done first.

You said my previous patch fixed your problem, right? Then I'll add your tested by and push it in due course.

I'll also get around to adding __init's to other functions too. But it is *very* low on the totem pole of importance.

Quoted 7 lines

Quoted 7 lines

Could you provide your suggestions or completions for it ?

The trace_selftest_startup_* funcitons are mostly added by you without '__init', do you have additional considerations about it (intend to have no '__init') ?

Quoted 4 lines

Quoted 4 lines

If no reply, I recommend to keep no '__init': apply this patch or regress part of the patch "f1ed7c7 ftrace: Do not run selftest if command line parameter is set" (at least, it can avoid related warnings and treat all *selftest* fair).

It's a compile time warning that's a false positive. Not a run time crash or other issue of importance. It can wait, relax. Otherwise you are starting to become annoying.

Quoted 7 lines · Steven Rostedt

Quoted 1 line · Chen Gang

Quoted 1 line

Hello Frederic and Ingo:

Are you trying to go around me? I wrote this code and I'm one of the maintainers for it. Your issue is very minor, and can wait till other things get done first.

Quoted 6 lines

You said my previous patch fixed your problem, right? Then I'll add your tested by and push it in due course.

I'll also get around to adding __init's to other functions too. But it is *very* low on the totem pole of importance.

I only provide my suggestions (or recommendations) which I think might be useful for us, and I don't care about whether you accept or not.

If you want discuss, we can continue, if you won't (now, I guess so), you can just provide your choice is OK.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Could you provide your suggestions or completions for it ?

The trace_selftest_startup_* funcitons are mostly added by you without '__init', do you have additional considerations about it (intend to have no '__init') ?

Quoted 7 lines

Quoted 4 lines

Quoted 4 lines

If no reply, I recommend to keep no '__init': apply this patch or regress part of the patch "f1ed7c7 ftrace: Do not run selftest if command line parameter is set" (at least, it can avoid related warnings and treat all *selftest* fair).

It's a compile time warning that's a false positive. Not a run time crash or other issue of importance. It can wait, relax. Otherwise you are starting to become annoying.

At least this patch is not 'urgent' (not a run time crash, or other issue of 'urgent'), but every members have their own opinions to treat this issue whether 'important' or not ('important' != 'urgent').

And every members' time resources are expensive (not only you, but also me, and other members).

When I got none-reply, I don't know what happened (whether you agree, or not, what I said correct or incorrect ?), it is a polite to give a confirmation reply to tell whether you accept or not.

Thanks.