mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] trace: param: fix tp_printk option related with tp_printk_stop_on_boot
@ 2022-02-08 16:30 JaeSang Yoo
  2022-02-08 19:41 ` Steven Rostedt
  2022-02-08 20:57 ` kernel test robot
  0 siblings, 2 replies; 3+ messages in thread
From: JaeSang Yoo @ 2022-02-08 16:30 UTC (permalink / raw)
  To: rostedt; +Cc: mingo, linux-kernel, JaeSang Yoo

Kernel param "tp_printk_stop_on_boot" starts with "tp_printk" which is
the exact as the other kernel param "tp_printk".
In compile & build process, It may not guaranteed that
"tp_printk_stop_on_boot" always checked before "tp_printk".
(By swapping its __setup() macro order, it may not work as expected.)
Some kernel params which starts with other kernel params consider this
problem. See commit 745a600cf1a6 ("um: console: Ignore console= option")
or init/do_mounts.c:45 (setup function of "ro" kernel param)

Kernel param "tp_printk" can be handled with its value(0 or off) or
it can be handled without its value. (maybe it won't effect anything)
Fix setup function to ignore when the "tp_printk" becomes prefix of
other kernel param.

Signed-off-by: JaeSang Yoo <jsyoo5b@gmail.com>
---
 kernel/trace/trace.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c860f582b078..5c8a28d74cf8 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -252,6 +252,8 @@ __setup("trace_clock=", set_trace_boot_clock);
 
 static int __init set_tracepoint_printk(char *str)
 {
+	if (*str == '_')
+		return 0
 	if ((strcmp(str, "=0") != 0 && strcmp(str, "=off") != 0))
 		tracepoint_printk = 1;
 	return 1;
-- 
2.25.1


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

* Re: [PATCH v2] trace: param: fix tp_printk option related with tp_printk_stop_on_boot
  2022-02-08 16:30 [PATCH v2] trace: param: fix tp_printk option related with tp_printk_stop_on_boot JaeSang Yoo
@ 2022-02-08 19:41 ` Steven Rostedt
  2022-02-08 20:57 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: Steven Rostedt @ 2022-02-08 19:41 UTC (permalink / raw)
  To: JaeSang Yoo; +Cc: mingo, linux-kernel, JaeSang Yoo

On Wed,  9 Feb 2022 01:30:33 +0900
JaeSang Yoo <js.yoo.5b@gmail.com> wrote:

> Kernel param "tp_printk_stop_on_boot" starts with "tp_printk" which is
> the exact as the other kernel param "tp_printk".
> In compile & build process, It may not guaranteed that
> "tp_printk_stop_on_boot" always checked before "tp_printk".
> (By swapping its __setup() macro order, it may not work as expected.)
> Some kernel params which starts with other kernel params consider this
> problem. See commit 745a600cf1a6 ("um: console: Ignore console= option")
> or init/do_mounts.c:45 (setup function of "ro" kernel param)
> 
> Kernel param "tp_printk" can be handled with its value(0 or off) or
> it can be handled without its value. (maybe it won't effect anything)
> Fix setup function to ignore when the "tp_printk" becomes prefix of
> other kernel param.
> 
> Signed-off-by: JaeSang Yoo <jsyoo5b@gmail.com>
> ---
>  kernel/trace/trace.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index c860f582b078..5c8a28d74cf8 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -252,6 +252,8 @@ __setup("trace_clock=", set_trace_boot_clock);
>  
>  static int __init set_tracepoint_printk(char *str)
>  {
> +	if (*str == '_')
> +		return 0

Did you test this?

Need a space here (besides the obvious bug).

Also, it needs a comment above:

	/* Ignore the tp_printk_stop_on_boot */
	if (*str == '_')
		return 0;

-- Steve

>  	if ((strcmp(str, "=0") != 0 && strcmp(str, "=off") != 0))
>  		tracepoint_printk = 1;
>  	return 1;


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

* Re: [PATCH v2] trace: param: fix tp_printk option related with tp_printk_stop_on_boot
  2022-02-08 16:30 [PATCH v2] trace: param: fix tp_printk option related with tp_printk_stop_on_boot JaeSang Yoo
  2022-02-08 19:41 ` Steven Rostedt
@ 2022-02-08 20:57 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2022-02-08 20:57 UTC (permalink / raw)
  To: JaeSang Yoo, rostedt; +Cc: kbuild-all, mingo, linux-kernel, JaeSang Yoo

Hi JaeSang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on rostedt-trace/for-next]
[also build test ERROR on v5.17-rc3 next-20220208]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/JaeSang-Yoo/trace-param-fix-tp_printk-option-related-with-tp_printk_stop_on_boot/20220209-003316
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20220209/202202090421.PMWXYPxl-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/2f6eb784171798586c7fde6d2f2e445ac5a344c3
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review JaeSang-Yoo/trace-param-fix-tp_printk-option-related-with-tp_printk_stop_on_boot/20220209-003316
        git checkout 2f6eb784171798586c7fde6d2f2e445ac5a344c3
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   kernel/trace/trace.c: In function 'set_tracepoint_printk':
>> kernel/trace/trace.c:256:11: error: expected ';' before 'if'
     256 |   return 0
         |           ^
         |           ;
     257 |  if ((strcmp(str, "=0") != 0 && strcmp(str, "=off") != 0))
         |  ~~        
   kernel/trace/trace.c: In function 'trace_check_vprintf':
   kernel/trace/trace.c:3824:3: warning: function 'trace_check_vprintf' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
    3824 |   trace_seq_vprintf(&iter->seq, iter->fmt, ap);
         |   ^~~~~~~~~~~~~~~~~
   kernel/trace/trace.c:3891:3: warning: function 'trace_check_vprintf' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
    3891 |   trace_seq_vprintf(&iter->seq, p, ap);
         |   ^~~~~~~~~~~~~~~~~
   At top level:
   kernel/trace/trace.c:1672:37: warning: 'tracing_max_lat_fops' defined but not used [-Wunused-const-variable=]
    1672 | static const struct file_operations tracing_max_lat_fops;
         |                                     ^~~~~~~~~~~~~~~~~~~~


vim +256 kernel/trace/trace.c

   252	
   253	static int __init set_tracepoint_printk(char *str)
   254	{
   255		if (*str == '_')
 > 256			return 0
   257		if ((strcmp(str, "=0") != 0 && strcmp(str, "=off") != 0))
   258			tracepoint_printk = 1;
   259		return 1;
   260	}
   261	__setup("tp_printk", set_tracepoint_printk);
   262	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

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

end of thread, other threads:[~2022-02-08 22:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-08 16:30 [PATCH v2] trace: param: fix tp_printk option related with tp_printk_stop_on_boot JaeSang Yoo
2022-02-08 19:41 ` Steven Rostedt
2022-02-08 20:57 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®