All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@redhat.com>
To: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Hideo AOKI <haoki@redhat.com>,
	mingo@elte.hu, linux-kernel@vger.kernel.org,
	Steven Rostedt <rostedt@goodmis.org>
Subject: Re: Kernel marker has no performance impact on ia64.
Date: Thu, 12 Jun 2008 11:31:28 -0400	[thread overview]
Message-ID: <48514150.6090001@redhat.com> (raw)
In-Reply-To: <20080612140415.GC22348@Krystal>

Hi Mathieu,

Mathieu Desnoyers wrote:
>> If so, I'd like to suggest below changes,
>>
>> - introduce below macro in marker.h
>>
>> #define DEFINE_TRACE(name, vargs, args...)	\
>> static inline void trace_##name vargs		\
>> {						\
>> 	trace_mark(name, #vargs, ##args);	\
>> }
>>
>> - remove __marker_check_format from __trace_mark
>> - and write a definition in sched_trace.h
>>
>> DEFINE_TRACE(sched_switch, (struct task_struct *prev, struct task_struct *next),
>> 	     prev, next);
>>
>> Thus, we can remove fmt string and also ensure the type checking, because;
>> - Type checking at the trace point is done by the compiler.
>> - Type checking of probe handler can be done by comparing #vargs strings.
>>
> 
> Hrm, interesting! The only problem I see with this is that it won't
> allow a tracer to efficiently parse the "format information". Parsing C
> code is not as straightforward and compact as parsing a format string.

Sure, Parsing C code is not a good idea. I think each tracer can have
a lookup table to get a printf-style format corresponding to each
"regular" marking point. Maintaining this lookup table is not so hard,
because these "regular" marking points should be enough stable.

> However, Peter and you are about to convince me that an hybrid between
> the solution you propose here and the marker scheme could be used.
> 
> Your scheme could be used to declare the markers and probes
> (DEFINE_TRACE()) in header files. It would declare a static inline
> function called at the instrumentation site and a probe prototype
> that could then be used in the probe module to declare the probe that
> will have to be connected to the marker. This part would allow
> custom-made probes.
> 
> Within the tracer, we would declare custom-made probes for each of these
> DEFINE_TRACE statements and associate them with format strings. Because
> the probe has to match the prototype, type correctness is ensured. The
> format strings would at that point be the exact same as the current
> trace_mark() statements. The information passed to trace_mark() would be
> send for direct interpretation or serialization with only basic types
> available, similar to printk().

If the tracer including systemtap introduces above the lookup table,
that can translate "name(arguments)" to "format" easily, and can continue
to use its format string parser.

> We sould leave the trace_mark() statements available for people who want
> to add their own debug-style instrumentation to their kernel without
> having to add DEFINE_TRACE() statements and modify the tracer
> accordingly.

I agree, trace_mark() still useful for "non-regular" markers temporarily
inserted to the developing code by individual developers.

> I guess a bit of polishing will come with the implementation, which I
> plan to start soon.
> 
> Thanks!
> 
> Mathieu
> 

Thank you!

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com


  reply	other threads:[~2008-06-12 15:33 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-02 22:12 Kernel marker has no performance impact on ia64 Hideo AOKI
2008-06-02 22:32 ` Peter Zijlstra
2008-06-02 23:21   ` Mathieu Desnoyers
2008-06-03  6:07     ` Takashi Nishiie
2008-06-04  4:58     ` Masami Hiramatsu
2008-06-04 23:26       ` Mathieu Desnoyers
2008-06-04 23:40         ` Masami Hiramatsu
2008-06-04 22:27     ` Peter Zijlstra
2008-06-04 23:22       ` Mathieu Desnoyers
2008-06-05  8:12         ` Peter Zijlstra
2008-06-05 14:28           ` Masami Hiramatsu
2008-06-12 14:04             ` Mathieu Desnoyers
2008-06-12 15:31               ` Masami Hiramatsu [this message]
2008-06-12 13:53           ` Mathieu Desnoyers
2008-06-12 14:27             ` Peter Zijlstra
2008-06-12 15:53               ` Frank Ch. Eigler
2008-06-12 16:16                 ` Masami Hiramatsu
2008-06-12 16:43                   ` Frank Ch. Eigler
2008-06-12 16:56                     ` Peter Zijlstra
2008-06-12 22:10                       ` Mathieu Desnoyers
2008-06-12 17:05                     ` Masami Hiramatsu
2008-06-12 17:48                       ` Frank Ch. Eigler
2008-06-12 19:34                         ` Masami Hiramatsu
2008-06-13  4:19                           ` Takashi Nishiie
2008-06-13 18:02                             ` Masami Hiramatsu
2008-06-16  2:58                               ` Takashi Nishiie
2008-06-12 16:53                 ` Peter Zijlstra
2008-06-12 17:38                   ` Frank Ch. Eigler
2008-06-13 11:01                     ` Peter Zijlstra
2008-06-13 14:17                       ` Frank Ch. Eigler

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=48514150.6090001@redhat.com \
    --to=mhiramat@redhat.com \
    --cc=haoki@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@polymtl.ca \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    /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.