All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kernel: trace: remove __init from race_selftest_startup_function() and trace_selftest_startup_function_graph()
@ 2013-07-11  0:31 Chen Gang
  2013-07-11 16:47 ` Steven Rostedt
  0 siblings, 1 reply; 12+ messages in thread
From: Chen Gang @ 2013-07-11  0:31 UTC (permalink / raw)
  To: Steven Rostedt, Frederic Weisbecker; +Cc: mingo, linux-kernel

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 <gang.chen@asianux.com>
---
 kernel/trace/trace_selftest.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
index a7329b7..e341b9d 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -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)
 {
-- 
1.7.7.6

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

* Re: [PATCH] kernel: trace: remove __init from race_selftest_startup_function() and trace_selftest_startup_function_graph()
  2013-07-11  0:31 [PATCH] kernel: trace: remove __init from race_selftest_startup_function() and trace_selftest_startup_function_graph() Chen Gang
@ 2013-07-11 16:47 ` Steven Rostedt
  2013-07-11 23:51   ` Chen Gang
  0 siblings, 1 reply; 12+ messages in thread
From: Steven Rostedt @ 2013-07-11 16:47 UTC (permalink / raw)
  To: Chen Gang; +Cc: Frederic Weisbecker, mingo, linux-kernel

On Thu, 2013-07-11 at 08:31 +0800, Chen Gang wrote:
> 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
> 

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.

-- Steve

> 
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  kernel/trace/trace_selftest.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
> index a7329b7..e341b9d 100644
> --- a/kernel/trace/trace_selftest.c
> +++ b/kernel/trace/trace_selftest.c
> @@ -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)
>  {



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

* Re: [PATCH] kernel: trace: remove __init from race_selftest_startup_function() and trace_selftest_startup_function_graph()
  2013-07-11 16:47 ` Steven Rostedt
@ 2013-07-11 23:51   ` Chen Gang
  2013-07-12  1:41     ` Steven Rostedt
  0 siblings, 1 reply; 12+ messages in thread
From: Chen Gang @ 2013-07-11 23:51 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Frederic Weisbecker, mingo, linux-kernel

On 07/12/2013 12:47 AM, Steven Rostedt wrote:
> On Thu, 2013-07-11 at 08:31 +0800, Chen Gang wrote:
>> 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
>>
> 
> 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.
> 

Ok, thanks.

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


Thanks.
-- 
Chen Gang

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

* Re: [PATCH] kernel: trace: remove __init from race_selftest_startup_function() and trace_selftest_startup_function_graph()
  2013-07-11 23:51   ` Chen Gang
@ 2013-07-12  1:41     ` Steven Rostedt
  2013-07-12  1:58       ` Chen Gang
  0 siblings, 1 reply; 12+ messages in thread
From: Steven Rostedt @ 2013-07-12  1:41 UTC (permalink / raw)
  To: Chen Gang; +Cc: Frederic Weisbecker, mingo, linux-kernel

On Fri, 2013-07-12 at 07:51 +0800, Chen Gang wrote:

> 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.

-- Steve



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

* Re: [PATCH] kernel: trace: remove __init from race_selftest_startup_function() and trace_selftest_startup_function_graph()
  2013-07-12  1:41     ` Steven Rostedt
@ 2013-07-12  1:58       ` Chen Gang
  2013-07-12  2:38         ` Steven Rostedt
  0 siblings, 1 reply; 12+ messages in thread
From: Chen Gang @ 2013-07-12  1:58 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Frederic Weisbecker, mingo, linux-kernel

On 07/12/2013 09:41 AM, Steven Rostedt wrote:
> On Fri, 2013-07-12 at 07:51 +0800, Chen Gang wrote:
> 
>> > 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.

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) ?


Thanks.
-- 
Chen Gang

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

* Re: [PATCH] kernel: trace: remove __init from race_selftest_startup_function() and trace_selftest_startup_function_graph()
  2013-07-12  1:58       ` Chen Gang
@ 2013-07-12  2:38         ` Steven Rostedt
  2013-07-12  3:04           ` Chen Gang
  0 siblings, 1 reply; 12+ messages in thread
From: Steven Rostedt @ 2013-07-12  2:38 UTC (permalink / raw)
  To: Chen Gang; +Cc: Frederic Weisbecker, mingo, linux-kernel

On Fri, 2013-07-12 at 09:58 +0800, Chen Gang wrote:
> On 07/12/2013 09:41 AM, Steven Rostedt wrote:
> > On Fri, 2013-07-12 at 07:51 +0800, Chen Gang wrote:
> > 
> >> > 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.
> 
> 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.

> 
> 
> 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) ?

Does this patch fix your warning?

-- Steve

diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index a4ed382..5e794d1 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -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);
diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
index b863f93..38fe148 100644
--- a/kernel/trace/trace_functions.c
+++ b/kernel/trace/trace_functions.c
@@ -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,



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

* Re: [PATCH] kernel: trace: remove __init from race_selftest_startup_function() and trace_selftest_startup_function_graph()
  2013-07-12  2:38         ` Steven Rostedt
@ 2013-07-12  3:04           ` Chen Gang
  2013-07-12  7:20             ` Chen Gang
  0 siblings, 1 reply; 12+ messages in thread
From: Chen Gang @ 2013-07-12  3:04 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Frederic Weisbecker, mingo, linux-kernel

On 07/12/2013 10:38 AM, Steven Rostedt wrote:
> On Fri, 2013-07-12 at 09:58 +0800, Chen Gang wrote:
>> On 07/12/2013 09:41 AM, Steven Rostedt wrote:
>>> On Fri, 2013-07-12 at 07:51 +0800, Chen Gang wrote:
>>>
>>>>> 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.
>>
>> 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.
> 
>>
>>
>> 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) ?
> 
> 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.

> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index a4ed382..5e794d1 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -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);
> diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
> index b863f93..38fe148 100644
> --- a/kernel/trace/trace_functions.c
> +++ b/kernel/trace/trace_functions.c
> @@ -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,
> 
> 
> 
> 


-- 
Chen Gang

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

* Re: [PATCH] kernel: trace: remove __init from race_selftest_startup_function() and trace_selftest_startup_function_graph()
  2013-07-12  3:04           ` Chen Gang
@ 2013-07-12  7:20             ` Chen Gang
  2013-07-15  2:12               ` Chen Gang
  0 siblings, 1 reply; 12+ messages in thread
From: Chen Gang @ 2013-07-12  7:20 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Frederic Weisbecker, mingo, linux-kernel

On 07/12/2013 11:04 AM, Chen Gang wrote:
> On 07/12/2013 10:38 AM, Steven Rostedt wrote:
>> On Fri, 2013-07-12 at 09:58 +0800, Chen Gang wrote:
>>> On 07/12/2013 09:41 AM, Steven Rostedt wrote:
>>>> On Fri, 2013-07-12 at 07:51 +0800, Chen Gang wrote:
>>>>
>>>>>> 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.
>>>
>>> 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.
>>
>>>
>>>
>>> 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) ?
>>
>> 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.

> 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.
> 
>> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
>> index a4ed382..5e794d1 100644
>> --- a/kernel/trace/trace.h
>> +++ b/kernel/trace/trace.h
>> @@ -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);
>> diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
>> index b863f93..38fe148 100644
>> --- a/kernel/trace/trace_functions.c
>> +++ b/kernel/trace/trace_functions.c
>> @@ -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,
>>
>>
>>
>>
> 
> 


-- 
Chen Gang

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

* Re: [PATCH] kernel: trace: remove __init from race_selftest_startup_function() and trace_selftest_startup_function_graph()
  2013-07-12  7:20             ` Chen Gang
@ 2013-07-15  2:12               ` Chen Gang
  2013-07-15 16:20                 ` Steven Rostedt
  0 siblings, 1 reply; 12+ messages in thread
From: Chen Gang @ 2013-07-15  2:12 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Frederic Weisbecker, mingo, linux-kernel

On 07/12/2013 03:20 PM, Chen Gang wrote:
> On 07/12/2013 11:04 AM, Chen Gang wrote:
>> On 07/12/2013 10:38 AM, Steven Rostedt wrote:
>>> On Fri, 2013-07-12 at 09:58 +0800, Chen Gang wrote:
>>>> On 07/12/2013 09:41 AM, Steven Rostedt wrote:
>>>>> On Fri, 2013-07-12 at 07:51 +0800, Chen Gang wrote:
>>>>>
>>>>>>> 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.
>>>>
>>>> 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.
>>>
>>>>
>>>>
>>>> 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) ?
>>>
>>> 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.
> 

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).


Thanks.


>> 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.
>>
>>> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
>>> index a4ed382..5e794d1 100644
>>> --- a/kernel/trace/trace.h
>>> +++ b/kernel/trace/trace.h
>>> @@ -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);
>>> diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
>>> index b863f93..38fe148 100644
>>> --- a/kernel/trace/trace_functions.c
>>> +++ b/kernel/trace/trace_functions.c
>>> @@ -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,
>>>
>>>
>>>
>>>
>>
>>
> 
> 


-- 
Chen Gang

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

* Re: [PATCH] kernel: trace: remove __init from race_selftest_startup_function() and trace_selftest_startup_function_graph()
  2013-07-15  2:12               ` Chen Gang
@ 2013-07-15 16:20                 ` Steven Rostedt
  2013-07-16  0:22                   ` Chen Gang
  0 siblings, 1 reply; 12+ messages in thread
From: Steven Rostedt @ 2013-07-15 16:20 UTC (permalink / raw)
  To: Chen Gang; +Cc: Frederic Weisbecker, mingo, linux-kernel

On Mon, 2013-07-15 at 10:12 +0800, Chen Gang wrote:

> 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.

> 
> 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).

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.

-- Steve



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

* Re: [PATCH] kernel: trace: remove __init from race_selftest_startup_function() and trace_selftest_startup_function_graph()
  2013-07-15 16:20                 ` Steven Rostedt
@ 2013-07-16  0:22                   ` Chen Gang
  2013-07-17  0:52                     ` Chen Gang
  0 siblings, 1 reply; 12+ messages in thread
From: Chen Gang @ 2013-07-16  0:22 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Frederic Weisbecker, mingo, linux-kernel

On 07/16/2013 12:20 AM, Steven Rostedt wrote:
> On Mon, 2013-07-15 at 10:12 +0800, Chen Gang wrote:
> 
>> > 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.
> 

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.

>> > 
>> > 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).
> 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.
-- 
Chen Gang

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

* Re: [PATCH] kernel: trace: remove __init from race_selftest_startup_function() and trace_selftest_startup_function_graph()
  2013-07-16  0:22                   ` Chen Gang
@ 2013-07-17  0:52                     ` Chen Gang
  0 siblings, 0 replies; 12+ messages in thread
From: Chen Gang @ 2013-07-17  0:52 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Frederic Weisbecker, mingo, linux-kernel


Oh, sorry for my original impolite reply (at least it is not quite gentle).

:-)

On 07/16/2013 08:22 AM, Chen Gang wrote:
> On 07/16/2013 12:20 AM, Steven Rostedt wrote:
>> On Mon, 2013-07-15 at 10:12 +0800, Chen Gang wrote:
>>
>>>> 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.
>>
> 
> 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.
> 
>>>>
>>>> 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).
>> 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.
> 


-- 
Chen Gang

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

end of thread, other threads:[~2013-07-17  0:53 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-11  0:31 [PATCH] kernel: trace: remove __init from race_selftest_startup_function() and trace_selftest_startup_function_graph() Chen Gang
2013-07-11 16:47 ` Steven Rostedt
2013-07-11 23:51   ` Chen Gang
2013-07-12  1:41     ` Steven Rostedt
2013-07-12  1:58       ` Chen Gang
2013-07-12  2:38         ` Steven Rostedt
2013-07-12  3:04           ` Chen Gang
2013-07-12  7:20             ` Chen Gang
2013-07-15  2:12               ` Chen Gang
2013-07-15 16:20                 ` Steven Rostedt
2013-07-16  0:22                   ` Chen Gang
2013-07-17  0:52                     ` Chen Gang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.