linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>,
	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 20:14:47 +0900	[thread overview]
Message-ID: <20210812201447.0b04d20abacb84ecec1ad6b5@kernel.org> (raw)
In-Reply-To: <20210812184429.176d1416ff922ade4b5342fb@kernel.org>

On Thu, 12 Aug 2021 18:44:29 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> On Wed, 11 Aug 2021 23:46:48 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Thu, 12 Aug 2021 10:27:35 +0900
> > Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > 
> > > Let me confirm this, so eprobes can be attached to synthetic event?
> > > IMHO, I rather like to prevent attaching eprobe_event on the other
> > > dynamic events. It makes hard to check when removing the base dynamic
> > > events...
> > > 
> > > For the above example, we can rewrite it as below to trace filename
> > > without attaching eprobe_events on the synthetic event.
> > > 
> > >   echo 'my_open pid_t pid; char file[]' > synthetic_events
> > > 
> > >   echo 'e:myopen syscalls.sys_enter_open file=+0($filename):ustring' > dynamic_events
> > >   echo 'e:myopen_ret syscalls.sys_exit_open ret=$ret' > dynamic_events
> > >  
> > >   echo 'hist:keys=common_pid:fname=file' > events/eprobes/myopen/trigger
> > >   echo 'hist:keys=common_pid:fname=$fname:onmatch(eprobes.myopen).trace(my_open,common_pid,$fname)' > events/eprobes/myopen_ret
> > > 
> > 
> > The problem is that the above wont work :-(
> > 
> > For example, I can use this program:
> > 
> > #include <stdio.h>
> > #include <unistd.h>
> > #include <fcntl.h>
> > #include <sys/types.h>
> > 
> > static const char *file = "/etc/passwd";
> > 
> > int main (int argc, char **argv)
> > {
> > 	int fd;
> > 
> > 	fd = open(file, O_RDONLY);
> > 	if (fd < 0)
> > 		perror(file);
> > 	close(fd);
> > 	return 0;
> > }
> > 
> > Which if you do the above, all you'll get from the myopen is "(null)".
> > 
> > That's because the "/etc/passwd" is not paged in at the start of the
> > system call, and because tracepoints can not fault, the "ustring" will
> > not be mapped yet, it can not give you the content of the file pointer.
> > This was the entire reason we are working on eprobes to attach to
> > synthetic events in the first place.
> 
> I think that is another limitation. If you run this program,
> 
> static const char *file = "/etc/passwd";
> 
> int main (int argc, char **argv)
> {
> 	char buf[BUFSIZE];
> 	int fd;
> 
> 	strlcpy(buf, file, BUFSIZE);
> 	fd = open(buf, O_RDONLY);
> 	if (fd < 0)
> 		perror(file);
> 	read(fd, buf, BUFSIZE);
> 	close(fd);
> 	return 0;
> }
> 
> you'll not see any filename from the "myopen_ret" or the synthetic event.
> Thus, the user-space page fault must be handled by the other way. (e.g.
> making a special worker thread and run it before the task returns to
> user space.)
> Using eprobe over synthetic event does not solve the root cause (and
> it can introduce another issue.)

Oops, I missed that is the exit of open(), not close(). OK so filename 
should be accessible at that point.

> > 
> > The trick is to use the synthetic event to pass the filename pointer to
> > the exit of the system call, which the system call itself would map the
> > pointer to "file", and when the eprobe reads it with ":ustring" from
> > the exit of the system call it gets "/etc/passwd" instead of "(null)".
> > 
> > Your above example doesn't fix this.

OK, I got it.

Thanks, 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-11 14:14 [PATCH v4] [RFC] trace: Add kprobe on tracepoint Tzvetomir Stoyanov (VMware)
2021-08-11 15:03 ` Masami Hiramatsu
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 [this message]
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=20210812201447.0b04d20abacb84ecec1ad6b5@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 \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).