Re: [PATCH v10 1/4] trace: Add trace any kernel object

From: Jeff Xie
Date: Fri May 27 2022 - 08:21:49 EST


Hi Masami,

On Fri, May 27, 2022 at 7:42 AM Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
>
> Hi Jeff,
>
> On Wed, 18 May 2022 22:17:50 +0800
> Jeff Xie <xiehuan09@xxxxxxxxx> wrote:
>
> > Hi Masami,
> >
> > Thank you for your reminder, I feel very strange, this patch series
> > was always based on:
> > https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
> > ftrace/core
> >
> > These compile errors may be because other branches have not merged
> > into Tom's patches.
> > Steve told me before that this patch series should rely on Tom's patch.
>
> OK, when you send the next version, please describe this in the cover
> mail and point Tom's series if it is not merged yet.

Ok, thanks for the reminder.

> Thank you,
>
> >
> >
> > On Wed, May 18, 2022 at 9:48 PM Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
> > >
> > > Hi Jeff,
> > >
> > > Can you fix these errors, since these looks real bugs.
> > >
> > > Thank you,
> > >
> > > On Fri, 13 May 2022 10:01:48 +0800
> > > kernel test robot <lkp@xxxxxxxxx> wrote:
> > >
> > > > Hi Jeff,
> > > >
> > > > Thank you for the patch! Perhaps something to improve:
> > > >
> > > > [auto build test WARNING on rostedt-trace/for-next]
> > > > [also build test WARNING on v5.18-rc6 next-20220512]
> > > > [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/intel-lab-lkp/linux/commits/Jeff-Xie/trace-Introduce-objtrace-trigger-to-trace-the-kernel-object/20220513-010820
> > > > base: https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git for-next
> > > > config: i386-randconfig-c001 (https://download.01.org/0day-ci/archive/20220513/202205130923.SJJN7oez-lkp@xxxxxxxxx/config)
> > > > compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 9519dacab7b8afd537811fc2abaceb4d14f4e16a)
> > > > reproduce (this is a W=1 build):
> > > > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > > > chmod +x ~/bin/make.cross
> > > > # https://github.com/intel-lab-lkp/linux/commit/3c91aa291a0fe8b28b02a14827b0c4ca3ebda601
> > > > git remote add linux-review https://github.com/intel-lab-lkp/linux
> > > > git fetch --no-tags linux-review Jeff-Xie/trace-Introduce-objtrace-trigger-to-trace-the-kernel-object/20220513-010820
> > > > git checkout 3c91aa291a0fe8b28b02a14827b0c4ca3ebda601
> > > > # save the config file
> > > > mkdir build_dir && cp config build_dir/.config
> > > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash kernel/trace/
> > > >
> > > > If you fix the issue, kindly add following tag as appropriate
> > > > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > > >
> > > > All warnings (new ones prefixed by >>):
> > > >
> > > > kernel/trace/trace_object.c:259:13: error: incompatible function pointer types initializing 'int (*)(struct seq_file *, struct event_trigger_ops *, struct event_trigger_data *)' with an expression of type 'int (struct seq_file *, struct event_trigger_data *)' [-Werror,-Wincompatible-function-pointer-types]
> > > > .print = trace_object_trigger_print,
> > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > kernel/trace/trace_object.c:260:12: error: incompatible function pointer types initializing 'int (*)(struct event_trigger_ops *, struct event_trigger_data *)' with an expression of type 'int (struct event_trigger_data *)' [-Werror,-Wincompatible-function-pointer-types]
> > > > .init = event_object_trigger_init,
> > > > ^~~~~~~~~~~~~~~~~~~~~~~~~
> > > > kernel/trace/trace_object.c:261:12: error: incompatible function pointer types initializing 'void (*)(struct event_trigger_ops *, struct event_trigger_data *)' with an expression of type 'void (struct event_trigger_data *)' [-Werror,-Wincompatible-function-pointer-types]
> > > > .free = trace_object_trigger_free,
> > > > ^~~~~~~~~~~~~~~~~~~~~~~~~
> > > > kernel/trace/trace_object.c:266:13: error: incompatible function pointer types initializing 'int (*)(struct seq_file *, struct event_trigger_ops *, struct event_trigger_data *)' with an expression of type 'int (struct seq_file *, struct event_trigger_data *)' [-Werror,-Wincompatible-function-pointer-types]
> > > > .print = trace_object_trigger_print,
> > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > kernel/trace/trace_object.c:267:12: error: incompatible function pointer types initializing 'int (*)(struct event_trigger_ops *, struct event_trigger_data *)' with an expression of type 'int (struct event_trigger_data *)' [-Werror,-Wincompatible-function-pointer-types]
> > > > .init = event_object_trigger_init,
> > > > ^~~~~~~~~~~~~~~~~~~~~~~~~
> > > > kernel/trace/trace_object.c:268:12: error: incompatible function pointer types initializing 'void (*)(struct event_trigger_ops *, struct event_trigger_data *)' with an expression of type 'void (struct event_trigger_data *)' [-Werror,-Wincompatible-function-pointer-types]
> > > > .free = trace_object_trigger_free,
> > > > ^~~~~~~~~~~~~~~~~~~~~~~~~
> > > > >> kernel/trace/trace_object.c:335:61: warning: format specifies type 'long' but the argument has type 'unsigned int' [-Wformat]
> > > > pr_err("the size of the %s should be:%ld\n", field->name, sizeof(void *));
> > > > ~~~ ^~~~~~~~~~~~~~
> > > > %u
> > > > include/linux/printk.h:489:33: note: expanded from macro 'pr_err'
> > > > printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
> > > > ~~~ ^~~~~~~~~~~
> > > > include/linux/printk.h:446:60: note: expanded from macro 'printk'
> > > > #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
> > > > ~~~ ^~~~~~~~~~~
> > > > include/linux/printk.h:418:19: note: expanded from macro 'printk_index_wrap'
> > > > _p_func(_fmt, ##__VA_ARGS__); \
> > > > ~~~~ ^~~~~~~~~~~
> > > > kernel/trace/trace_object.c:356:3: error: implicit declaration of function 'event_trigger_unregister' [-Werror,-Wimplicit-function-declaration]
> > > > event_trigger_unregister(cmd_ops, file, glob+1, trigger_data);
> > > > ^
> > > > kernel/trace/trace_object.c:356:3: note: did you mean 'event_trigger_register'?
> > > > kernel/trace/trace.h:1650:12: note: 'event_trigger_register' declared here
> > > > extern int event_trigger_register(struct event_command *cmd_ops,
> > > > ^
> > > > kernel/trace/trace_object.c:370:64: error: too few arguments to function call, expected 7, have 4
> > > > ret = event_trigger_register(cmd_ops, file, glob, trigger_data);
> > > > ~~~~~~~~~~~~~~~~~~~~~~ ^
> > > > kernel/trace/trace.h:1650:12: note: 'event_trigger_register' declared here
> > > > extern int event_trigger_register(struct event_command *cmd_ops,
> > > > ^
> > > > 1 warning and 8 errors generated.
> > > >
> > > >
> > > > vim +335 kernel/trace/trace_object.c
> > > >
> > > > 296
> > > > 297 static int
> > > > 298 event_object_trigger_parse(struct event_command *cmd_ops,
> > > > 299 struct trace_event_file *file,
> > > > 300 char *glob, char *cmd, char *param_and_filter)
> > > > 301 {
> > > > 302 struct event_trigger_data *trigger_data;
> > > > 303 struct objtrace_trigger_data *obj_data;
> > > > 304 struct ftrace_event_field *field;
> > > > 305 char *objtrace_cmd, *arg;
> > > > 306 char *param, *filter;
> > > > 307 int ret;
> > > > 308 bool remove;
> > > > 309
> > > > 310 remove = event_trigger_check_remove(glob);
> > > > 311
> > > > 312 /*
> > > > 313 * separate the param and the filter:
> > > > 314 * objtrace:add:OBJ[:COUNT] [if filter]
> > > > 315 */
> > > > 316 ret = event_trigger_separate_filter(param_and_filter, &param, &filter, true);
> > > > 317 if (ret)
> > > > 318 return ret;
> > > > 319
> > > > 320 objtrace_cmd = strsep(&param, ":");
> > > > 321 if (!objtrace_cmd || strcmp(objtrace_cmd, "add")) {
> > > > 322 pr_err("error objtrace command\n");
> > > > 323 return -EINVAL;
> > > > 324 }
> > > > 325
> > > > 326 arg = strsep(&param, ":");
> > > > 327 if (!arg)
> > > > 328 return -EINVAL;
> > > > 329
> > > > 330 field = trace_find_event_field(file->event_call, arg);
> > > > 331 if (!field)
> > > > 332 return -EINVAL;
> > > > 333
> > > > 334 if (field->size != sizeof(void *)) {
> > > > > 335 pr_err("the size of the %s should be:%ld\n", field->name, sizeof(void *));
> > > > 336 return -EINVAL;
> > > > 337 }
> > > > 338
> > > > 339 if (remove && !field_exist(file, cmd_ops, field->name))
> > > > 340 return -EINVAL;
> > > > 341
> > > > 342 obj_data = kzalloc(sizeof(*obj_data), GFP_KERNEL);
> > > > 343 if (!obj_data)
> > > > 344 return -ENOMEM;
> > > > 345
> > > > 346 obj_data->field = field;
> > > > 347 obj_data->tr = file->tr;
> > > > 348 snprintf(obj_data->objtrace_cmd, OBJTRACE_CMD_LEN, objtrace_cmd);
> > > > 349
> > > > 350 trigger_data = event_trigger_alloc(cmd_ops, cmd, param, obj_data);
> > > > 351 if (!trigger_data) {
> > > > 352 kfree(obj_data);
> > > > 353 return -ENOMEM;
> > > > 354 }
> > > > 355 if (remove) {
> > > > 356 event_trigger_unregister(cmd_ops, file, glob+1, trigger_data);
> > > > 357 kfree(obj_data);
> > > > 358 kfree(trigger_data);
> > > > 359 return 0;
> > > > 360 }
> > > > 361
> > > > 362 ret = event_trigger_parse_num(param, trigger_data);
> > > > 363 if (ret)
> > > > 364 goto out_free;
> > > > 365
> > > > 366 ret = event_trigger_set_filter(cmd_ops, file, filter, trigger_data);
> > > > 367 if (ret < 0)
> > > > 368 goto out_free;
> > > > 369
> > > > 370 ret = event_trigger_register(cmd_ops, file, glob, trigger_data);
> > > > 371 if (ret)
> > > > 372 goto out_free;
> > > > 373
> > > > 374 return ret;
> > > > 375
> > > > 376 out_free:
> > > > 377 event_trigger_reset_filter(cmd_ops, trigger_data);
> > > > 378 kfree(obj_data);
> > > > 379 kfree(trigger_data);
> > > > 380 return ret;
> > > > 381 }
> > > > 382
> > > >
> > > > --
> > > > 0-DAY CI Kernel Test Service
> > > > https://01.org/lkp
> > >
> > >
> > > --
> > > Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> >
> > Thanks,
> > JeffXie
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>

Thanks,
JeffXie