LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>
Cc: rostedt@goodmis.org, linux-trace-devel@vger.kernel.org,
	linux-kernel@vger.kernel.org, tom.zanussi@linux.intel.com
Subject: Re: [PATCH v4] [RFC] trace: Add kprobe on tracepoint
Date: Thu, 12 Aug 2021 00:03:43 +0900	[thread overview]
Message-ID: <20210812000343.887f0084ff1c48de8c47ec90@kernel.org> (raw)
In-Reply-To: <20210811141433.1976072-1-tz.stoyanov@gmail.com>

On Wed, 11 Aug 2021 17:14:33 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> From: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> 
> A new dynamic event is introduced: event probe. The event is attached
> to an existing tracepoint and uses its fields as arguments. The user
> can specify custom format string of the new event, select what tracepoint
> arguments will be printed and how to print them.
> An event probe is created by writing configuration string in
> 'dynamic_events' ftrace file:
>  e:GNAME/ENAME SYSTEM.EVENT [FETCHARGS]	- Set an event probe

Hmm, this inconsistency looks not good to me.

"GNAME/ENAME" "SYSTEM.EVENT"
 - GNAME is "group name" but SYSTEM is "system name" but both must point
   same idea.
 - Delimiter character is different.

Aren't these confusing users?

One idea is adding a patch for kprobe and uprobe events to accept new
delimiter '.'. This will give a consistency with hist triggers too.

Also, you can add a note about that the system and group is same
meaning in events.

>  -:GNAME/ENAME				- Delete an event probe
> 
> Where:
>  GNAME	- Group name, if omitted 'eprobes' is used.

If this is not mandatory, you should write it as 

e:[GNAME/]ENAME SYSTEM.EVENT [FETCHARGS]


>  ENAME	- Name of the new event in GNAME, mandatory.
>  SYSTEM	- Name of the system, where the tracepoint is defined, mandatory.
>  EVENT	- Name of the tracepoint event in SYSTEM, mandatory.
>  FETCHARGS - Arguments:
>   <name>=$<field>[:TYPE] - Fetch given filed of the tracepoint and print
> 			   it as given TYPE with given name. Supported
> 			   types are:
> 	                    (u8/u16/u32/u64/s8/s16/s32/s64), basic type
>         	            (x8/x16/x32/x64), hexadecimal types
> 			    "string", "ustring" and bitfield.
> 
> Example, attach an event probe on openat system call and print name of the
> file that will be opened:
>  echo "e:esys/eopen syscalls.sys_enter_openat file=\$filename:string" >> dynamic_events
> A new dynamic event is created in events/esys/eopen/ directory. It
> can be deleted with:
>  echo "-:esys/eopen" >> dynamic_events
> 
> Filters, triggers and histograms can be attached to the new event, it can
> be matched in synthetic events. There is one limitation - no synthetic
> events can be attached to an event probe.

I'm not sure what the "no synthetic events can be attached to an event probe"
means.
Can you show an example command what this means?

> 
> Co-developed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
> Subject of the patch is still related to kprobe, though the design no
> more depends on kprobe. I did not change it for consistency with the
> first patch version. 

OK, thanks for moving onto the dynevent. Let me check the code in another
mail.
Anyway, I think this is good starting point. 

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

  reply	other threads:[~2021-08-11 15:03 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-11 14:14 Tzvetomir Stoyanov (VMware)
2021-08-11 15:03 ` Masami Hiramatsu [this message]
2021-08-11 15:22   ` Steven Rostedt
2021-08-12  1:27     ` Masami Hiramatsu
2021-08-12  3:46       ` Steven Rostedt
2021-08-12  9:44         ` Masami Hiramatsu
2021-08-12 11:14           ` Masami Hiramatsu
2021-08-12  4:02       ` Steven Rostedt
2021-08-12 11:15         ` Masami Hiramatsu
2021-08-12 11:31       ` Masami Hiramatsu
2021-08-12 13:44         ` Steven Rostedt
2021-08-12 15:06           ` Masami Hiramatsu
2021-08-12 15:44 ` Masami Hiramatsu
2021-08-16 21:40   ` Steven Rostedt
2021-08-17 11:52     ` Masami Hiramatsu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210812000343.887f0084ff1c48de8c47ec90@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tom.zanussi@linux.intel.com \
    --cc=tz.stoyanov@gmail.com \
    --subject='Re: [PATCH v4] [RFC] trace: Add kprobe on tracepoint' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).