All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Frank Ch. Eigler" <fche@redhat.com>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mel Gorman <mgorman@suse.de>, Ingo Molnar <mingo@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	SystemTap <systemtap@sourceware.org>
Subject: Re: systemtap broken by removal of register_timer_hook
Date: Fri, 19 Apr 2013 10:46:55 -0400	[thread overview]
Message-ID: <20130419144655.GD8308@redhat.com> (raw)
In-Reply-To: <CAFTL4hy+U-dLPwGR8BcZt=C8Tug-jvh+24epadfte7Ci2cDU5Q@mail.gmail.com>

Hi, Frederic -


> > How about this?
> >
> > Author: Frank Ch. Eigler <fche@redhat.com>
> > Date:   Wed Apr 3 10:35:21 2013 -0400
> >
> >     profiling: add profile_tick tracepoint
> > [...]

> It would be better not to tie this to CONFIG_PROFILING.
> A tracepoint in update_process_times() instead would be great but it's
> sometimes called several times in a tick from some archs.
> Probably we need something like:
> 
> static inline tick_trace(struct pt_regs *regs)
> {
>     trace_timer_tick(regs);
>     profile_tick(CPU_PROFILING);
> }

I looked into this, but found no natural place to define such an
inline function from which to call into a tracepoint, without having
to #include the <event/FOO.h> file many times.  Nor does it seem
appropriate to do the identical #define CREATE_TRACE_POINTS part from
all the different arch/.../*.c files that may call into that inline.
If you'd like to stick to this idea, please advise further where you
think the tracepoint definition & declarations should go.

In the alternative, here is v2 of the patch, just changing the
tracepoint-printing argument as suggested by jistone.

- FChE

---------------

Author: Frank Ch. Eigler <fche@redhat.com>
Date:   Wed Apr 3 10:35:21 2013 -0400

    profiling: add profile_tick tracepoint
    
    Commit ba6fdda4 removed the timer_hook mechanism for modules to listen
    to profiling timer ticks (without having to set up more complicated
    perf mechanisms).  To reduce the impact on out-of-tree users such as
    systemtap, a TRACE_EVENT-flavoured tracepoint is added in its place.
    Tested with perf and systemtap.
    
    Cc: Frederic Weisbecker <fweisbec@gmail.com>
    Cc: Ingo Molnar <mingo@kernel.org>
    Cc: Mel Gorman <mgorman@suse.de>
    Signed-off-by: Frank Ch. Eigler <fche@redhat.com>

diff --git a/include/trace/events/profile.h b/include/trace/events/profile.h
new file mode 100644
index 0000000..445aee7
--- /dev/null
+++ b/include/trace/events/profile.h
@@ -0,0 +1,37 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM profile
+
+#if !defined(_TRACE_PROFILE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_PROFILE_H
+
+#include <linux/tracepoint.h>
+
+
+struct pt_regs;
+
+/**
+ * profile_tick - called when the profiling timer ticks
+ * @type:	profiling tick type, generally @CPU_PROFILING
+ * @regs:	pointer to struct pt_regs*
+ */
+
+TRACE_EVENT(profile_tick,
+	TP_PROTO(int type, struct pt_regs *regs),
+	TP_ARGS(type, regs),
+	TP_STRUCT__entry(
+		__field( int,			type	)
+		__field( struct pt_regs*,	regs	)
+	),
+	TP_fast_assign(
+		__entry->type	= type;
+		__entry->regs	= regs;
+	),
+	TP_printk("type=%d ip=%p", __entry->type,
+		  instruction_pointer(__entry->regs))
+);
+
+
+#endif /*  _TRACE_PROFILE_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/kernel/profile.c b/kernel/profile.c
index dc3384e..d61f921 100644
--- a/kernel/profile.c
+++ b/kernel/profile.c
@@ -29,6 +29,9 @@
 #include <asm/irq_regs.h>
 #include <asm/ptrace.h>
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/profile.h>
+
 struct profile_hit {
 	u32 pc, hits;
 };
@@ -414,6 +417,8 @@ void profile_tick(int type)
 {
 	struct pt_regs *regs = get_irq_regs();
 
+	trace_profile_tick(type, regs);
+
 	if (!user_mode(regs) && prof_cpu_mask != NULL &&
 	    cpumask_test_cpu(smp_processor_id(), prof_cpu_mask))
 		profile_hit(type, (void *)profile_pc(regs));

  reply	other threads:[~2013-04-19 14:47 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-03  7:50 systemtap broken by removal of register_timer_hook Mel Gorman
2013-04-03 11:01 ` Frederic Weisbecker
     [not found] ` <CAFTL4hyYezB2ZxM-GJ70VoxOeRSG64V6u+nX2hTuhF30R-GdPg__32168.962484184$1364986928$gmane$org@mail.gmail.com>
2013-04-03 12:29   ` Frank Ch. Eigler
2013-04-03 12:49     ` Frederic Weisbecker
2013-04-03 14:44       ` Frank Ch. Eigler
2013-04-03 18:38         ` Josh Stone
2013-04-04 12:46         ` Frederic Weisbecker
2013-04-19 14:46           ` Frank Ch. Eigler [this message]
2013-04-30  0:19             ` Frederic Weisbecker
2013-04-30 17:27               ` Frank Ch. Eigler
2013-05-06 23:12                 ` Frederic Weisbecker
2013-04-10 11:24         ` Ingo Molnar

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=20130419144655.GD8308@redhat.com \
    --to=fche@redhat.com \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@kernel.org \
    --cc=systemtap@sourceware.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.