All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
To: Ingo Molnar <mingo@elte.hu>
Cc: "Frédéric Weisbecker" <fweisbec@gmail.com>,
	"Jason Baron" <jbaron@redhat.com>,
	"Tom Zanussi" <tzanussi@gmail.com>,
	linux-kernel@vger.kernel.org, laijs@cn.fujitsu.com,
	rostedt@goodmis.org, peterz@infradead.org, jiayingz@google.com,
	mbligh@google.com, roland@redhat.com, fche@redhat.com
Subject: Re: [RFC] convert ftrace syscall tracer to TRACE_EVENT()
Date: Sat, 9 May 2009 09:50:46 -0400	[thread overview]
Message-ID: <20090509135046.GA2344@Krystal> (raw)
In-Reply-To: <20090509133306.GA20684@elte.hu>

* Ingo Molnar (mingo@elte.hu) wrote:
> 
> * Frédéric Weisbecker <fweisbec@gmail.com> wrote:
> 
> > > Secondly, we should reuse the information we get in 
> > > SYSCALL_DEFINE, to construct the TRACE_EVENT tracepoints 
> > > directly - without having to list all syscalls again in a 
> > > separate file.
> > 
> > Indeed, that's not trivial though, but feasible. I'm not sure we 
> > can reuse the TRACE_EVENT macro directly inside SYSCALL_DEFINE. 
> > The resulting macro tempest effect that would occur confuses me 
> > and I have troubles to imagine the result.
> 
> Lets take an example. This syscall:
> 
> SYSCALL_DEFINE3(sched_setscheduler, pid_t, pid, int, policy,
>                 struct sched_param __user *, param)
> 
> Is equivalent to:
> 
> SYSCALL_DEFINE3(name, t1, v1, t2, v2, t3, v3)
> 
> ('t' for type, 'v' for variable/value).
> 
> This would transform into the following TRACE_EVENT() construct:
> 
> TRACE_EVENT_SYSCALL2():
> 
> TRACE_EVENT(sys_##name,
> 	TP_PROTO(t1 v1, t2 v2),
> 	TP_ARGS(v1, v2),
> 	TP_STRUCT__entry(
> 		__field(t1, v1)
> 		__field(t2, v2)
> 	),
> 	TP_fast_assign(
> 		__entry->v1 = v1;
> 		__entry->v2 = v2;
> 	),
> 	TP_printk("%016Lx %016Lx", (u64)__entry->v1, (u64)__entry->v2)
> );
> 
> We need TRACE_EVENT_SYSCALL[123456] definitions, and that's it.
> 
> The only place where we lose type information is the printk format - 
> but that's not a big issue, as i'd expect the event record to be the 
> main user of this.
> 
> [ In addition to this, we could extend DEFINE_SYSCALL[1..6] with a 
>   (optional) format string definition field, and fill that in for 
>   anything that matters. ]
> 
> Note, this assumes that all syscall types can be described via 
> __field() - i think that's correct. (we dont want to deref strings 
> as they are untrusted, and there are no arrays in syscall 
> parameters)
> 

I would expect to use copy_string_from_user (for strings)
and copy_from_user for structures, because without any strings
(especially), the trace information become much less useful.

This should probably be done at the TP_fast_assign level.

Note that ftrace fields do not support variable length strings, AFAIK.

Mathieu

> Can you see any complication?
> 
> 	Ingo

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

  reply	other threads:[~2009-05-09 13:50 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-08 21:03 [RFC] convert ftrace syscall tracer to TRACE_EVENT() Jason Baron
2009-05-09  8:37 ` Ingo Molnar
2009-05-09 12:53   ` Frédéric Weisbecker
2009-05-09 13:33     ` Ingo Molnar
2009-05-09 13:50       ` Mathieu Desnoyers [this message]
2009-05-09 14:06         ` Frédéric Weisbecker
2009-05-09 14:15           ` Ingo Molnar
2009-05-09 14:29             ` Mathieu Desnoyers
2009-05-09 15:01               ` Frédéric Weisbecker
2009-05-09 15:24                 ` Mathieu Desnoyers
2009-05-09 14:47             ` Frédéric Weisbecker
2009-05-09 17:44         ` David Wagner
2009-05-09 14:02       ` Frédéric Weisbecker
2009-05-09 14:07         ` Ingo Molnar
2009-05-09 14:12           ` Frédéric Weisbecker
2009-05-09 15:36   ` Frank Ch. Eigler
2009-05-09 15:57     ` Mathieu Desnoyers
2009-05-09 16:32     ` Mathieu Desnoyers
2009-05-10  6:59   ` Tom Zanussi
2009-05-11 22:16   ` Jason Baron
2009-05-12  2:44   ` Roland McGrath

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=20090509135046.GA2344@Krystal \
    --to=mathieu.desnoyers@polymtl.ca \
    --cc=fche@redhat.com \
    --cc=fweisbec@gmail.com \
    --cc=jbaron@redhat.com \
    --cc=jiayingz@google.com \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mbligh@google.com \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=roland@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=tzanussi@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 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.