linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tracing: Export key trace event symbols
@ 2015-04-20 21:38 Ron Rechenmacher
  2015-04-21  6:10 ` Christoph Hellwig
  2015-04-24 21:24 ` Steven Rostedt
  0 siblings, 2 replies; 28+ messages in thread
From: Ron Rechenmacher @ 2015-04-20 21:38 UTC (permalink / raw)
  To: linux-kernel; +Cc: Steven Rostedt

If symbols are not exported, modules can no longer register additional
(module specified) tracepoints like they use to be able to (i.e linux-3.15.x).
Somewhere on or about commit de7b2973903c6cc50b31ee5682a69b2219b9919d
(Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date:   Tue Apr 8 17:26:21 2014 -0400
tracepoint: Use struct pointer instead of name hash for reg/unreg tracepoints)
modules which attempted to register additional tracing functions would
get "Unknown symbol" errors. For example: "... Unknown symbol
__tracepoint_sched_switch (err 0)"
Symbols can be exported using the kernel's EXPORT_TRACEPOINT_SYMBOL_GPL macro
to allow modules to once again register their own tracing functions (for at
least some key points in the kernel as provided by this patch).

Signed-off-by: Ron Rechenmacher <ron@fnal.gov>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=96051
---
  kernel/irq/handle.c               |    6 ++++++
  kernel/softirq.c                  |    6 ++++++
  kernel/trace/trace_sched_switch.c |    5 +++++
  kernel/trace/trace_syscalls.c     |    6 ++++++
  4 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
index 470d08c..a98d763 100644
--- a/kernel/irq/handle.c
+++ b/kernel/irq/handle.c
@@ -20,6 +20,12 @@

  #include "internals.h"

+/*
+ * Allow modules to register additional trace routines
+ */
+EXPORT_TRACEPOINT_SYMBOL_GPL(irq_handler_entry);
+EXPORT_TRACEPOINT_SYMBOL_GPL(irq_handler_exit);
+
  /**
   * handle_bad_irq - handle spurious and unhandled irqs
   * @irq:       the interrupt number
diff --git a/kernel/softirq.c b/kernel/softirq.c
index fca82c3..af6fa2e 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -52,6 +52,12 @@ irq_cpustat_t irq_stat[NR_CPUS] ____cacheline_aligned;
  EXPORT_SYMBOL(irq_stat);
  #endif

+/*
+ * Allow modules to register additional trace routines
+ */
+EXPORT_TRACEPOINT_SYMBOL_GPL(softirq_entry);
+EXPORT_TRACEPOINT_SYMBOL_GPL(softirq_exit);
+
  static struct softirq_action softirq_vec[NR_SOFTIRQS] __cacheline_aligned_in_smp;

  DEFINE_PER_CPU(struct task_struct *, ksoftirqd);
diff --git a/kernel/trace/trace_sched_switch.c b/kernel/trace/trace_sched_switch.c
index 7e62c0a..2ea1a6a 100644
--- a/kernel/trace/trace_sched_switch.c
+++ b/kernel/trace/trace_sched_switch.c
@@ -14,6 +14,11 @@

  #include "trace.h"

+/*
+ * Allow modules to register additional trace routines
+ */
+EXPORT_TRACEPOINT_SYMBOL_GPL(sched_switch);
+
  static struct trace_array      *ctx_trace;
  static int __read_mostly       tracer_enabled;
  static int                     sched_ref;
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index ee7b5a0..6c3bbd0 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -9,6 +9,12 @@
  #include "trace_output.h"
  #include "trace.h"

+/*
+ * Allow modules to register additional trace routines
+ */
+EXPORT_TRACEPOINT_SYMBOL_GPL(sys_enter);
+EXPORT_TRACEPOINT_SYMBOL_GPL(sys_exit);
+
  static DEFINE_MUTEX(syscall_trace_lock);
  static int sys_refcount_enter;
  static int sys_refcount_exit;

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-20 21:38 [PATCH] tracing: Export key trace event symbols Ron Rechenmacher
@ 2015-04-21  6:10 ` Christoph Hellwig
  2015-04-21 12:04   ` Ron Rechenmacher
  2015-04-24 21:24 ` Steven Rostedt
  1 sibling, 1 reply; 28+ messages in thread
From: Christoph Hellwig @ 2015-04-21  6:10 UTC (permalink / raw)
  To: Ron Rechenmacher; +Cc: linux-kernel, Steven Rostedt

On Mon, Apr 20, 2015 at 04:38:11PM -0500, Ron Rechenmacher wrote:
> If symbols are not exported, modules can no longer register additional
> (module specified) tracepoints like they use to be able to (i.e linux-3.15.x).
> Somewhere on or about commit de7b2973903c6cc50b31ee5682a69b2219b9919d
> (Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Date:   Tue Apr 8 17:26:21 2014 -0400
> tracepoint: Use struct pointer instead of name hash for reg/unreg tracepoints)
> modules which attempted to register additional tracing functions would
> get "Unknown symbol" errors. For example: "... Unknown symbol
> __tracepoint_sched_switch (err 0)"
> Symbols can be exported using the kernel's EXPORT_TRACEPOINT_SYMBOL_GPL macro
> to allow modules to once again register their own tracing functions (for at
> least some key points in the kernel as provided by this patch).

Which (in-tree) module fails with this?  I don't think anyone should
actually register a symbol.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-21  6:10 ` Christoph Hellwig
@ 2015-04-21 12:04   ` Ron Rechenmacher
  2015-04-21 12:19     ` Ron Rechenmacher
  2015-04-21 12:22     ` Christoph Hellwig
  0 siblings, 2 replies; 28+ messages in thread
From: Ron Rechenmacher @ 2015-04-21 12:04 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-kernel, Steven Rostedt



Christoph Hellwig wrote on 04/21/15 01:10:
>
> Which (in-tree) module fails with this?  I don't think anyone should
> actually register a symbol.
>

I see you (Christoph Hellwig) have asked this question in a similar context
(see https://patches.linaro.org/28821/).
This question does not seem to make sense because:
1) the external module is not registering a _symbol_ but more
precisely a tracepoint _function_ as the whole tracepoint system allows for
_multiple_ functions to be called for each tracepoint declared in the kernel.
2) It's not the point that an in-tree module would fail.  Again, the tracepoint
system allows for _multiple_functions_ to be defined/registered for each tracepoint
and _in_the_earlier_kernels_(i.e. 3.10.x and many others),_external_modules_could_
_register_ one or more _additional_functions_ to be called.

IF you're specifically saying that external modules should not register additional
tracepoint functions, my question would simply be: why do you think this?

To give you an example of the usefulness of continuing to allow this (continuation
from earlier kernels): the kernel scheduling has a tracepoint defined; of course a
critical operation for any kernel. I use to be able to insert a module which would
collect my own statistics on when and what switching was going on on what CPU cores.
I can think of many other potential reasons that this would be useful for external
modules. To think that tracepoints would only be useful for in-tree development is,
perhaps, (not meaning to offend) short sighted.

-- 
Ron Rechenmacher
Engineer
Fermi National Accelerator Laboratory
Batavia, IL  60510

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-21 12:04   ` Ron Rechenmacher
@ 2015-04-21 12:19     ` Ron Rechenmacher
  2015-04-21 13:38       ` Steven Rostedt
  2015-04-21 12:22     ` Christoph Hellwig
  1 sibling, 1 reply; 28+ messages in thread
From: Ron Rechenmacher @ 2015-04-21 12:19 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-kernel, Steven Rostedt

I see in the reference I mentioned below (https://patches.linaro.org/28821/),
and in the current mm source, that some tracepoint symbols are already EXPORTed,
but not _GPL. I do not know the fine points between "GPL-ed" and "non-GPL-ed" symbol
exporting.  Would it make a difference if my patch proposed non-GPL exporting?

Ron Rechenmacher wrote on 04/21/15 07:04:
>
>
> Christoph Hellwig wrote on 04/21/15 01:10:
>>
>> Which (in-tree) module fails with this?  I don't think anyone should
>> actually register a symbol.
>>
>
> I see you (Christoph Hellwig) have asked this question in a similar context
> (see https://patches.linaro.org/28821/).
> This question does not seem to make sense because:
> 1) the external module is not registering a _symbol_ but more
> precisely a tracepoint _function_ as the whole tracepoint system allows for
> _multiple_ functions to be called for each tracepoint declared in the kernel.
> 2) It's not the point that an in-tree module would fail.  Again, the tracepoint
> system allows for _multiple_functions_ to be defined/registered for each tracepoint
> and _in_the_earlier_kernels_(i.e. 3.10.x and many others),_external_modules_could_
> _register_ one or more _additional_functions_ to be called.
>
> IF you're specifically saying that external modules should not register additional
> tracepoint functions, my question would simply be: why do you think this?
>
> To give you an example of the usefulness of continuing to allow this (continuation
> from earlier kernels): the kernel scheduling has a tracepoint defined; of course a
> critical operation for any kernel. I use to be able to insert a module which would
> collect my own statistics on when and what switching was going on on what CPU cores.
> I can think of many other potential reasons that this would be useful for external
> modules. To think that tracepoints would only be useful for in-tree development is,
> perhaps, (not meaning to offend) short sighted.
>

-- 
Ron Rechenmacher
Engineer
Fermi National Accelerator Laboratory
Batavia, IL  60510

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-21 12:04   ` Ron Rechenmacher
  2015-04-21 12:19     ` Ron Rechenmacher
@ 2015-04-21 12:22     ` Christoph Hellwig
  2015-04-21 13:13       ` Ron Rechenmacher
  1 sibling, 1 reply; 28+ messages in thread
From: Christoph Hellwig @ 2015-04-21 12:22 UTC (permalink / raw)
  To: Ron Rechenmacher; +Cc: Christoph Hellwig, linux-kernel, Steven Rostedt

Hi Ron,

the Linux kernel is not a shared library, but an integrated project
that happens to be split into loadable modules.

Please send your module that uses the tracepoints and we can start
discussing if it makes sense to exports the symbols for it, or if
we can implement it in a better way.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-21 12:22     ` Christoph Hellwig
@ 2015-04-21 13:13       ` Ron Rechenmacher
  2015-04-21 13:23         ` Christoph Hellwig
  0 siblings, 1 reply; 28+ messages in thread
From: Ron Rechenmacher @ 2015-04-21 13:13 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-kernel, Steven Rostedt

Hi Christoph,

My module, and information about it, can be found at:
https://cdcvs.fnal.gov/redmine/projects/trace
https://cdcvs.fnal.gov/redmine/projects/trace/repository/show/src_module
with the header at
https://cdcvs.fnal.gov/redmine/projects/trace/repository/raw/include/trace.h

It seems that you are the person that I have to convince in order to get
my patch accepted. Is this true?

Can you tell me how:
./kernel/trace/power-traces.c:17:EXPORT_TRACEPOINT_SYMBOL_GPL(power_start);
./kernel/trace/power-traces.c:19:EXPORT_TRACEPOINT_SYMBOL_GPL(cpu_idle);
should be allowed (how it fits with your definition of the Linux kernel below) and
my proposed EXPORTS not?
I hope that you will not suggest removing the above 2 exports.

Thanks,
Ron

Christoph Hellwig wrote on 04/21/15 07:22:
> Hi Ron,
>
> the Linux kernel is not a shared library, but an integrated project
> that happens to be split into loadable modules.
>
> Please send your module that uses the tracepoints and we can start
> discussing if it makes sense to exports the symbols for it, or if
> we can implement it in a better way.
>

-- 
Ron Rechenmacher
Engineer
Fermi National Accelerator Laboratory
Batavia, IL  60510

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-21 13:13       ` Ron Rechenmacher
@ 2015-04-21 13:23         ` Christoph Hellwig
  2015-04-21 13:26           ` Ron Rechenmacher
  2015-04-21 13:31           ` Steven Rostedt
  0 siblings, 2 replies; 28+ messages in thread
From: Christoph Hellwig @ 2015-04-21 13:23 UTC (permalink / raw)
  To: Ron Rechenmacher; +Cc: Christoph Hellwig, linux-kernel, Steven Rostedt

On Tue, Apr 21, 2015 at 08:13:24AM -0500, Ron Rechenmacher wrote:
> Hi Christoph,
> 
> My module, and information about it, can be found at:
> https://cdcvs.fnal.gov/redmine/projects/trace
> https://cdcvs.fnal.gov/redmine/projects/trace/repository/show/src_module
> with the header at
> https://cdcvs.fnal.gov/redmine/projects/trace/repository/raw/include/trace.h

And now send a patch including a rationale on why we need it.  I haven't
looked deep enough, but so far I can't see any real value add in it.

> 
> It seems that you are the person that I have to convince in order to get
> my patch accepted. Is this true?

In the end you'll need to convince a maintainer to apply it.

> Can you tell me how:
> ./kernel/trace/power-traces.c:17:EXPORT_TRACEPOINT_SYMBOL_GPL(power_start);

I can't find a tracepoint with that name in the current tree.

> ./kernel/trace/power-traces.c:19:EXPORT_TRACEPOINT_SYMBOL_GPL(cpu_idle);

This one is used by the potentially modular intel_idle (now cpuids)
driver, so it's exported for a reason.

> should be allowed (how it fits with your definition of the Linux kernel below) and
> my proposed EXPORTS not?
> I hope that you will not suggest removing the above 2 exports.

It's perfectly fine to remove unused exports, and we do it regularly

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-21 13:23         ` Christoph Hellwig
@ 2015-04-21 13:26           ` Ron Rechenmacher
  2015-04-21 13:53             ` Steven Rostedt
  2015-04-21 13:31           ` Steven Rostedt
  1 sibling, 1 reply; 28+ messages in thread
From: Ron Rechenmacher @ 2015-04-21 13:26 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-kernel, Steven Rostedt

But why export anything _GPL in the first place?


Christoph Hellwig wrote on 04/21/15 08:23:
> On Tue, Apr 21, 2015 at 08:13:24AM -0500, Ron Rechenmacher wrote:
>> Hi Christoph,
>>
>> My module, and information about it, can be found at:
>> https://cdcvs.fnal.gov/redmine/projects/trace
>> https://cdcvs.fnal.gov/redmine/projects/trace/repository/show/src_module
>> with the header at
>> https://cdcvs.fnal.gov/redmine/projects/trace/repository/raw/include/trace.h
>
> And now send a patch including a rationale on why we need it.  I haven't
> looked deep enough, but so far I can't see any real value add in it.
>
>>
>> It seems that you are the person that I have to convince in order to get
>> my patch accepted. Is this true?
>
> In the end you'll need to convince a maintainer to apply it.
>
>> Can you tell me how:
>> ./kernel/trace/power-traces.c:17:EXPORT_TRACEPOINT_SYMBOL_GPL(power_start);
>
> I can't find a tracepoint with that name in the current tree.
>
>> ./kernel/trace/power-traces.c:19:EXPORT_TRACEPOINT_SYMBOL_GPL(cpu_idle);
>
> This one is used by the potentially modular intel_idle (now cpuids)
> driver, so it's exported for a reason.
>
>> should be allowed (how it fits with your definition of the Linux kernel below) and
>> my proposed EXPORTS not?
>> I hope that you will not suggest removing the above 2 exports.
>
> It's perfectly fine to remove unused exports, and we do it regularly
>

-- 
Ron Rechenmacher
Engineer
Fermi National Accelerator Laboratory
Batavia, IL  60510

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-21 13:23         ` Christoph Hellwig
  2015-04-21 13:26           ` Ron Rechenmacher
@ 2015-04-21 13:31           ` Steven Rostedt
  1 sibling, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2015-04-21 13:31 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Ron Rechenmacher, linux-kernel

On Tue, 21 Apr 2015 06:23:55 -0700
Christoph Hellwig <hch@infradead.org> wrote:

> > 
> > It seems that you are the person that I have to convince in order to get
> > my patch accepted. Is this true?
> 
> In the end you'll need to convince a maintainer to apply it.

But a Nack from you will keep us from applying it ;-)

-- Steve

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-21 12:19     ` Ron Rechenmacher
@ 2015-04-21 13:38       ` Steven Rostedt
  0 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2015-04-21 13:38 UTC (permalink / raw)
  To: Ron Rechenmacher; +Cc: Christoph Hellwig, linux-kernel

On Tue, 21 Apr 2015 07:19:30 -0500
Ron Rechenmacher <ron@fnal.gov> wrote:

> I see in the reference I mentioned below (https://patches.linaro.org/28821/),
> and in the current mm source, that some tracepoint symbols are already EXPORTed,
> but not _GPL. I do not know the fine points between "GPL-ed" and "non-GPL-ed" symbol
> exporting.  Would it make a difference if my patch proposed non-GPL exporting?

The only reason there's non GPL exports for tracepoints is because
tracepoints were used in header files, and called by static inlines
like kfree() and such that were already exported non-gpl.

I believe we removed all tracepoints from static inlines and headers
because they were causing other problems (you don't want tracepoint
code inlined all over the place, it bloats the kernel).

I may look into cleaning up the kernel and removing the
EXPORT_TRACEPOINT_SYMBOL().

-- Steve

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-21 13:26           ` Ron Rechenmacher
@ 2015-04-21 13:53             ` Steven Rostedt
  2015-04-21 15:00               ` Ron Rechenmacher
  0 siblings, 1 reply; 28+ messages in thread
From: Steven Rostedt @ 2015-04-21 13:53 UTC (permalink / raw)
  To: Ron Rechenmacher; +Cc: Christoph Hellwig, linux-kernel

On Tue, 21 Apr 2015 08:26:26 -0500
Ron Rechenmacher <ron@fnal.gov> wrote:

> But why export anything _GPL in the first place?
> 

To make it clear that they are derivatives of the kernel. An export is
just to allow modules access to the functions, they were not added for
out of tree modules. Out of tree modules just happen to be lucky to
have them available.

Note, if you can present a good case to why they should be exported,
then we should have no problem exporting them. But I just looked at
your code, and I have some questions about it.

EXPORT_SYMBOL( traceControl_p );
EXPORT_SYMBOL( traceEntries_p );
EXPORT_SYMBOL( traceNamLvls_p );
EXPORT_SYMBOL( trace_allow_printk );

You have non GPL exported functions. Why? Is this used by non GPL code?

MODULE_LICENSE("GPL"); /* Get rid of taint message by declaring code as GPL */

That comment is very telling.

Maybe I'm reading into things, but until we understand exactly why you
need these symbols exported, we wont exported them. There's no kernel
ABI that we must stick to. Only the user space ABI is what we keep
stable.

-- Steve

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-21 13:53             ` Steven Rostedt
@ 2015-04-21 15:00               ` Ron Rechenmacher
  2015-04-21 15:49                 ` Steven Rostedt
  0 siblings, 1 reply; 28+ messages in thread
From: Ron Rechenmacher @ 2015-04-21 15:00 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Christoph Hellwig, linux-kernel

Thanks for looking at the code and providing the feedback.

Using EXPORT_SYMBOL was a mistake. I just changed them to EXPORT_SYMBOL_GPL.
I've been doing this sort of "tracing" (which I consider the "minimum level of
complexity" tracing) for about 30 years. I've never made any money on this, nor do
I care to. For the last 25 years, I've worked at a US Dept. of Energy research facility
and consider all the stuff I do to be in the public domain. I, in no way, want to suggest
or even care if other want to use it or not. I won't even suggest that you might need it.
It has allowed me to do great work/research and I will patch the kernel manually it need be.
But if there is anything else I should do to the code to make is more public, please let me
know.

Exactly why I _need_ these symbols exported?:

When data acquistion systems I develop or am involved with at Fermilab, have near-realtime
performance issues, _my_ tracing system is what _I_ use. There is a user-space component to
it that can be used with or without the kernel module. Using the module allows me to
see interactions with the Linux OS.

Sure, there are other tools -- none which provide me "the minimum level of complexity"
( About once a year I take a week or 2 and learn the latest of what's available. )

There has been times in the past, when I've had a major experiment collaboration
load a patched kernel, but there is a certain level of inconvenience associated with that.


Currently, the major experiments are using a derivative of Redhat EL6 and there is not
a problem with creating and loading my module. I'm optimistic that there will not
be a problem with EL7 which has a base kernel derived form Linux-3.10.

I am, however, doing research on high speed networking and we are using the Linux-3.16.1
kernel, which I am currently patching.  In the future, it would be nice not to have to
patch the kernel.

The kernel currently exports:
      EXPORT_TRACEPOINT_SYMBOL_GPL(power_start)
and  EXPORT_TRACEPOINT_SYMBOL_GPL(cpu_idle)

I understand that "Only the user space ABI is what we keep stable" and that before
commit de7b2973903c (tracepoint: Use struct pointer instead of name hash for reg/unreg
tracepoints) it was just "lucky" that I could have my module work.
But, I can image how these (the above EXPORTs) would be very useful for external
modules to help with custom power management. And, I suspect that if they went away,
someone would complain.

I would like to have my proposed patch included to provide similar
usefulness for my custom tracing (for the sake of science).

Please accept my patch.

Thanks for you consideration and if there is specific help/suggestion you can offer to
get my patch accepted, please let me know.

Thanks again for the discussion.

--Ron


Steven Rostedt wrote on 04/21/15 08:53:
> On Tue, 21 Apr 2015 08:26:26 -0500
> Ron Rechenmacher <ron@fnal.gov> wrote:
>
>> But why export anything _GPL in the first place?
>>
>
> To make it clear that they are derivatives of the kernel. An export is
> just to allow modules access to the functions, they were not added for
> out of tree modules. Out of tree modules just happen to be lucky to
> have them available.
>
> Note, if you can present a good case to why they should be exported,
> then we should have no problem exporting them. But I just looked at
> your code, and I have some questions about it.
>
> EXPORT_SYMBOL( traceControl_p );
> EXPORT_SYMBOL( traceEntries_p );
> EXPORT_SYMBOL( traceNamLvls_p );
> EXPORT_SYMBOL( trace_allow_printk );
>
> You have non GPL exported functions. Why? Is this used by non GPL code?
>
> MODULE_LICENSE("GPL"); /* Get rid of taint message by declaring code as GPL */
>
> That comment is very telling.
>
> Maybe I'm reading into things, but until we understand exactly why you
> need these symbols exported, we wont exported them. There's no kernel
> ABI that we must stick to. Only the user space ABI is what we keep
> stable.
>
> -- Steve
>

-- 
Ron Rechenmacher
Engineer
Fermi National Accelerator Laboratory
Batavia, IL  60510

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-21 15:00               ` Ron Rechenmacher
@ 2015-04-21 15:49                 ` Steven Rostedt
  2015-04-21 22:23                   ` Ron Rechenmacher
  0 siblings, 1 reply; 28+ messages in thread
From: Steven Rostedt @ 2015-04-21 15:49 UTC (permalink / raw)
  To: Ron Rechenmacher; +Cc: Christoph Hellwig, linux-kernel

On Tue, 21 Apr 2015 10:00:25 -0500
Ron Rechenmacher <ron@fnal.gov> wrote:

> Thanks for looking at the code and providing the feedback.
> 
> Using EXPORT_SYMBOL was a mistake. I just changed them to EXPORT_SYMBOL_GPL.
> I've been doing this sort of "tracing" (which I consider the "minimum level of
> complexity" tracing) for about 30 years. I've never made any money on this, nor do
> I care to. For the last 25 years, I've worked at a US Dept. of Energy research facility
> and consider all the stuff I do to be in the public domain. I, in no way, want to suggest
> or even care if other want to use it or not. I won't even suggest that you might need it.
> It has allowed me to do great work/research and I will patch the kernel manually it need be.
> But if there is anything else I should do to the code to make is more public, please let me
> know.

Heh, OK, I believe you :-)  it's just that we need to be careful about
this, as Christoph is even in the middle of litigation about GPL abuses.

> 
> Exactly why I _need_ these symbols exported?:
> 
> When data acquistion systems I develop or am involved with at Fermilab, have near-realtime
> performance issues, _my_ tracing system is what _I_ use. There is a user-space component to
> it that can be used with or without the kernel module. Using the module allows me to
> see interactions with the Linux OS.

I understand the _my_ tracing system. Before ftrace, I used logdev. Not
quite 30 years, but it started sometime in '98.

> 
> Sure, there are other tools -- none which provide me "the minimum level of complexity"
> ( About once a year I take a week or 2 and learn the latest of what's available. )

If you have any suggestions to make the in kernel tracer better, please
feel free to make comments.

> 
> There has been times in the past, when I've had a major experiment collaboration
> load a patched kernel, but there is a certain level of inconvenience associated with that.
> 
> 
> Currently, the major experiments are using a derivative of Redhat EL6 and there is not
> a problem with creating and loading my module. I'm optimistic that there will not
> be a problem with EL7 which has a base kernel derived form Linux-3.10.
> 
> I am, however, doing research on high speed networking and we are using the Linux-3.16.1
> kernel, which I am currently patching.  In the future, it would be nice not to have to
> patch the kernel.
> 
> The kernel currently exports:
>       EXPORT_TRACEPOINT_SYMBOL_GPL(power_start)
> and  EXPORT_TRACEPOINT_SYMBOL_GPL(cpu_idle)
> 
> I understand that "Only the user space ABI is what we keep stable" and that before
> commit de7b2973903c (tracepoint: Use struct pointer instead of name hash for reg/unreg
> tracepoints) it was just "lucky" that I could have my module work.
> But, I can image how these (the above EXPORTs) would be very useful for external
> modules to help with custom power management. And, I suspect that if they went away,
> someone would complain.
> 
> I would like to have my proposed patch included to provide similar
> usefulness for my custom tracing (for the sake of science).
> 
> Please accept my patch.

OK, lets see what you are doing in your patch:

+/*
+ * Allow modules to register additional trace routines
+ */
+EXPORT_TRACEPOINT_SYMBOL_GPL(irq_handler_entry);
+EXPORT_TRACEPOINT_SYMBOL_GPL(irq_handler_exit);
+

Instead of just saying "Allow modules to register additional trace
routines", please explain what its used for. What are you measuring
externally? Just the timings of the interrupts?

I'm also curious to know why ftrace isn't good enough.

Maybe the better solution is to add something to ftrace that does what
your tracer currently does?

-- Steve

> 
> Thanks for you consideration and if there is specific help/suggestion you can offer to
> get my patch accepted, please let me know.
> 
> Thanks again for the discussion.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-21 15:49                 ` Steven Rostedt
@ 2015-04-21 22:23                   ` Ron Rechenmacher
  2015-04-21 22:44                     ` Steven Rostedt
  0 siblings, 1 reply; 28+ messages in thread
From: Ron Rechenmacher @ 2015-04-21 22:23 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Christoph Hellwig, linux-kernel


Steven Rostedt wrote on 04/21/15 10:49:
[snip]
>
> OK, lets see what you are doing in your patch:

Thanks for helping/discussing!

>
> +/*
> + * Allow modules to register additional trace routines
> + */
> +EXPORT_TRACEPOINT_SYMBOL_GPL(irq_handler_entry);
> +EXPORT_TRACEPOINT_SYMBOL_GPL(irq_handler_exit);
> +
>
> Instead of just saying "Allow modules to register additional trace
> routines", please explain what its used for. What are you measuring
> externally? Just the timings of the interrupts?

How about "Allow for module (internal or external) to register trace function which
shows (or measures) interactions with user-space executables more directly.  Knowing
when interrupts happen and how they can subtly impact user-space application timing
is important in some application."

>
> I'm also curious to know why ftrace isn't good enough.

ftrace (if one considers ltt-ng) is probably capable of doing what my trace does
except that the timestamp is not TOD (Time Of Day) -- but probably/maybe could
be made to do so???

It's a matter of complexity or "start up cost" (or entry barrier).

I'm not the best at articulating all there is to TRACE succinctly, but lets
start with the fact that the user-space component of TRACE is basically just a header
file that defines the TRACE printf-like macro and handles the initialization which is
basically mmap-ing a file (as a trace buffer (with control structure)) and is,
for the most part, Unix OS independent.

The TRACE module, in addition to registering TRACE compatible tracing functions,
also creates a virtual file that the user-space TRACE-enable program(s) can mmap just
like another regular file (it doesn't know it's a kernel virtual file).
[As a side point, it's just for convenience that one or more user-space programs are
configured (via env. var.) to use the kernel virtual file as the buffer; they could use
any other regular file or each program could use a separate file.]

The module becomes critical when tracing OS key interactions (scheduling and interrupts)
with the userspace application(s) is necessary.


>
> Maybe the better solution is to add something to ftrace that does what
> your tracer currently does?

Although there is significant overlap between my TRACE and ftrace, the key
design criteria is such that trying to make ftrace do what is desired of TRACE
would be impractical.  Ftrace is more of a developer tool while TRACE is more of
an end-user utility, where simplicity of use is paramount.

Thanks for your help,
Ron

PS. Below, I've included a portion of a print out from the user-space TRACE control
program printing the buffer and showing the exact impact that an interrupt
has on a program looping.  Just a contrived example.

>
> -- Steve
>
>>
>> Thanks for you consideration and if there is specific help/suggestion you can offer to
>> get my patch accepted, please let me know.
>>
>> Thanks again for the discussion.
/home/ron/work/tracePrj/trace
cluck :^) TRACE_SHOW=HxNTiICLR
--04/21_16:42:24--
/home/ron/work/tracePrj/trace
cluck :^) tshow 10000 | tdelta -stats | { for xx in 1 2;do read h; echo "$h";done;tail;}
idx           us_tod     delta   tid TID cpu lv r msg
---- ---------------- --------- ----- --- --- -- - -----------------------------
9996 1429652195997224         1 40744  18  17  0 . Hello 9996. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9997 1429652195997223         1 40744  18  17  0 . Hello 9997. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9998 1429652195997222         1 40744  18  17  0 . Hello 9998. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9999 1429652195997221         1 40744  18  17  0 . Hello 9999. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
cpu="0"
                   min       -13
                   max        17
                   tot      7872
                   ave 0.7872787
                   cnt      9999
--04/21_16:42:27--
/home/ron/work/tracePrj/trace
cluck :^) tshow 10000 | tdelta -stats | { for xx in 1 2;do read h; echo "$h";done;grep -C15 '2        17';}
idx           us_tod     delta   tid TID cpu lv r msg
---- ---------------- --------- ----- --- --- -- - -----------------------------
9438 1429652195997676         1 40744  18  17  0 . Hello 9470. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9439 1429652195997676         0 40744  18  17  0 . Hello 9471. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9440 1429652195997675         1 40744  18  17  0 . Hello 9472. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9441 1429652195997674         1 40744  18  17  0 . Hello 9473. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9442 1429652195997673         1 40744  18  17  0 . Hello 9474. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9443 1429652195997672         1 40744  18  17  0 . Hello 9475. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9444 1429652195997672         0 40744  18  17  0 . Hello 9476. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9445 1429652195997671         1 40744  18  17  0 . Hello 9477. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9446 1429652195997670         1 40744  18  17  0 . Hello 9478. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9447 1429652195997667         3 40744   0  17 28 . sirqexit: cpu=17 vec_nr=9
9448 1429652195997664         3 40744   0  17 27 . sirqenter: cpu=17 vec_nr=9
9449 1429652195997662         2 40744   0  17 28 . sirqexit: cpu=17 vec_nr=1
9450 1429652195997660         2 40744   0  17 27 . sirqenter: cpu=17 vec_nr=1
9451 1429652195997656         4     0   0  13 28 . sirqexit: cpu=13 vec_nr=9
9452 1429652195997669       -13 40744  18  17  0 . Hello 9479. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9453 1429652195997652        17     0   0  13 27 1 sirqenter: cpu=13 vec_nr=9
9454 1429652195997652         0 40744  18  17  0 . Hello 9480. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9455 1429652195997651         1 40744  18  17  0 . Hello 9481. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9456 1429652195997649         2     0   0  13 28 1 sirqexit: cpu=13 vec_nr=1
9457 1429652195997649         0 40744  18  17  0 . Hello 9482. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9458 1429652195997648         1     0   0  12 28 . sirqexit: cpu=12 vec_nr=9
9459 1429652195997648         0 40744  18  17  0 . Hello 9483. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9460 1429652195997647         1 40744  18  17  0 . Hello 9484. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9461 1429652195997646         1 40744  18  17  0 . Hello 9485. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9462 1429652195997645         1     0   0  13 27 1 sirqenter: cpu=13 vec_nr=1
9463 1429652195997645         0 40744  18  17  0 1 Hello 9486. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9464 1429652195997644         1     0   0  12 27 . sirqenter: cpu=12 vec_nr=9
9465 1429652195997643         1 40744  18  17  0 . Hello 9487. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9466 1429652195997642         1 40744  18  17  0 . Hello 9488. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
9467 1429652195997642         0     0   0  12 28 . sirqexit: cpu=12 vec_nr=1
9468 1429652195997641         1 40744  18  17  0 . Hello 9489. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
5000000000 15
--04/21_17:15:52--



-- 
Ron Rechenmacher
Engineer
Fermi National Accelerator Laboratory
Batavia, IL  60510

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-21 22:23                   ` Ron Rechenmacher
@ 2015-04-21 22:44                     ` Steven Rostedt
  2015-04-22  2:24                       ` Ron Rechenmacher
  0 siblings, 1 reply; 28+ messages in thread
From: Steven Rostedt @ 2015-04-21 22:44 UTC (permalink / raw)
  To: Ron Rechenmacher
  Cc: Christoph Hellwig, linux-kernel, Arnaldo Carvalho de Melo,
	Arjan van de Ven, Namhyung Kim


[ Added a bunch of people that use perf ;-) ]

On Tue, 21 Apr 2015 17:23:45 -0500
Ron Rechenmacher <ron@fnal.gov> wrote:

> 
> Steven Rostedt wrote on 04/21/15 10:49:
> [snip]
> >
> > OK, lets see what you are doing in your patch:
> 
> Thanks for helping/discussing!
> 
> >
> > +/*
> > + * Allow modules to register additional trace routines
> > + */
> > +EXPORT_TRACEPOINT_SYMBOL_GPL(irq_handler_entry);
> > +EXPORT_TRACEPOINT_SYMBOL_GPL(irq_handler_exit);
> > +
> >
> > Instead of just saying "Allow modules to register additional trace
> > routines", please explain what its used for. What are you measuring
> > externally? Just the timings of the interrupts?
> 
> How about "Allow for module (internal or external) to register trace function which
> shows (or measures) interactions with user-space executables more directly.  Knowing
> when interrupts happen and how they can subtly impact user-space application timing
> is important in some application."
> 
> >
> > I'm also curious to know why ftrace isn't good enough.
> 
> ftrace (if one considers ltt-ng) is probably capable of doing what my trace does
> except that the timestamp is not TOD (Time Of Day) -- but probably/maybe could
> be made to do so???
> 
> It's a matter of complexity or "start up cost" (or entry barrier).
> 
> I'm not the best at articulating all there is to TRACE succinctly, but lets
> start with the fact that the user-space component of TRACE is basically just a header
> file that defines the TRACE printf-like macro and handles the initialization which is
> basically mmap-ing a file (as a trace buffer (with control structure)) and is,
> for the most part, Unix OS independent.

Heh, that sounds like the perf interface. Have you thought about using
that instead?

Look at what PowerTop does: https://01.org/powertop

You can use the perf interface to mmap a memory buffer and read the
data in its raw format. To parse the data, you need to read the format
files in /sys/kernel/debug/tracing/events, but there's a library that
does that. Unfortunately, that library isn't in distros yet (we really
need to get that done). But you can just copy the code from the kernel
source tree from tools/lib/traceevents/ and use that as perf does (as
well as powertop does too).

The format files tells you how to get the data from the raw binary
format.

If you use the perf interface, you can have userspace tools do custom
things with the events as they happen. This may make your utility more
robust in the long run.

I'd be happy to help you understand how to use the traceevent library. I
don't use the perf interface, but you can get examples from the
PowerTop code.

-- Steve


> 
> The TRACE module, in addition to registering TRACE compatible tracing functions,
> also creates a virtual file that the user-space TRACE-enable program(s) can mmap just
> like another regular file (it doesn't know it's a kernel virtual file).
> [As a side point, it's just for convenience that one or more user-space programs are
> configured (via env. var.) to use the kernel virtual file as the buffer; they could use
> any other regular file or each program could use a separate file.]
> 
> The module becomes critical when tracing OS key interactions (scheduling and interrupts)
> with the userspace application(s) is necessary.
> 
> 
> >
> > Maybe the better solution is to add something to ftrace that does what
> > your tracer currently does?
> 
> Although there is significant overlap between my TRACE and ftrace, the key
> design criteria is such that trying to make ftrace do what is desired of TRACE
> would be impractical.  Ftrace is more of a developer tool while TRACE is more of
> an end-user utility, where simplicity of use is paramount.
> 
> Thanks for your help,
> Ron
> 
> PS. Below, I've included a portion of a print out from the user-space TRACE control
> program printing the buffer and showing the exact impact that an interrupt
> has on a program looping.  Just a contrived example.
> 
> >
> > -- Steve
> >
> >>
> >> Thanks for you consideration and if there is specific help/suggestion you can offer to
> >> get my patch accepted, please let me know.
> >>
> >> Thanks again for the discussion.
> /home/ron/work/tracePrj/trace
> cluck :^) TRACE_SHOW=HxNTiICLR
> --04/21_16:42:24--
> /home/ron/work/tracePrj/trace
> cluck :^) tshow 10000 | tdelta -stats | { for xx in 1 2;do read h; echo "$h";done;tail;}
> idx           us_tod     delta   tid TID cpu lv r msg
> ---- ---------------- --------- ----- --- --- -- - -----------------------------
> 9996 1429652195997224         1 40744  18  17  0 . Hello 9996. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9997 1429652195997223         1 40744  18  17  0 . Hello 9997. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9998 1429652195997222         1 40744  18  17  0 . Hello 9998. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9999 1429652195997221         1 40744  18  17  0 . Hello 9999. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> cpu="0"
>                    min       -13
>                    max        17
>                    tot      7872
>                    ave 0.7872787
>                    cnt      9999
> --04/21_16:42:27--
> /home/ron/work/tracePrj/trace
> cluck :^) tshow 10000 | tdelta -stats | { for xx in 1 2;do read h; echo "$h";done;grep -C15 '2        17';}
> idx           us_tod     delta   tid TID cpu lv r msg
> ---- ---------------- --------- ----- --- --- -- - -----------------------------
> 9438 1429652195997676         1 40744  18  17  0 . Hello 9470. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9439 1429652195997676         0 40744  18  17  0 . Hello 9471. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9440 1429652195997675         1 40744  18  17  0 . Hello 9472. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9441 1429652195997674         1 40744  18  17  0 . Hello 9473. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9442 1429652195997673         1 40744  18  17  0 . Hello 9474. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9443 1429652195997672         1 40744  18  17  0 . Hello 9475. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9444 1429652195997672         0 40744  18  17  0 . Hello 9476. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9445 1429652195997671         1 40744  18  17  0 . Hello 9477. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9446 1429652195997670         1 40744  18  17  0 . Hello 9478. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9447 1429652195997667         3 40744   0  17 28 . sirqexit: cpu=17 vec_nr=9
> 9448 1429652195997664         3 40744   0  17 27 . sirqenter: cpu=17 vec_nr=9
> 9449 1429652195997662         2 40744   0  17 28 . sirqexit: cpu=17 vec_nr=1
> 9450 1429652195997660         2 40744   0  17 27 . sirqenter: cpu=17 vec_nr=1
> 9451 1429652195997656         4     0   0  13 28 . sirqexit: cpu=13 vec_nr=9
> 9452 1429652195997669       -13 40744  18  17  0 . Hello 9479. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9453 1429652195997652        17     0   0  13 27 1 sirqenter: cpu=13 vec_nr=9
> 9454 1429652195997652         0 40744  18  17  0 . Hello 9480. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9455 1429652195997651         1 40744  18  17  0 . Hello 9481. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9456 1429652195997649         2     0   0  13 28 1 sirqexit: cpu=13 vec_nr=1
> 9457 1429652195997649         0 40744  18  17  0 . Hello 9482. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9458 1429652195997648         1     0   0  12 28 . sirqexit: cpu=12 vec_nr=9
> 9459 1429652195997648         0 40744  18  17  0 . Hello 9483. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9460 1429652195997647         1 40744  18  17  0 . Hello 9484. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9461 1429652195997646         1 40744  18  17  0 . Hello 9485. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9462 1429652195997645         1     0   0  13 27 1 sirqenter: cpu=13 vec_nr=1
> 9463 1429652195997645         0 40744  18  17  0 1 Hello 9486. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9464 1429652195997644         1     0   0  12 27 . sirqenter: cpu=12 vec_nr=9
> 9465 1429652195997643         1 40744  18  17  0 . Hello 9487. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9466 1429652195997642         1 40744  18  17  0 . Hello 9488. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> 9467 1429652195997642         0     0   0  12 28 . sirqexit: cpu=12 vec_nr=1
> 9468 1429652195997641         1 40744  18  17  0 . Hello 9489. "c 2.5 5 5000000000 15" should be repeated here: c 2.5 5 
> 5000000000 15
> --04/21_17:15:52--
> 
> 
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-21 22:44                     ` Steven Rostedt
@ 2015-04-22  2:24                       ` Ron Rechenmacher
  2015-04-22 12:53                         ` Steven Rostedt
  0 siblings, 1 reply; 28+ messages in thread
From: Ron Rechenmacher @ 2015-04-22  2:24 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Christoph Hellwig, linux-kernel, Arnaldo Carvalho de Melo,
	Arjan van de Ven, Namhyung Kim

Steven Rostedt wrote on 04/21/15 17:44:
>
> [ Added a bunch of people that use perf ;-) ]
>

Thanks. I hope they can help me get the patch accepted.

[snip]
>>>
>>> +/*
>>> + * Allow modules to register additional trace routines
>>> + */
>>> +EXPORT_TRACEPOINT_SYMBOL_GPL(irq_handler_entry);
>>> +EXPORT_TRACEPOINT_SYMBOL_GPL(irq_handler_exit);
>>> +
>>>
>>> Instead of just saying "Allow modules to register additional trace
>>> routines", please explain what its used for. What are you measuring
>>> externally? Just the timings of the interrupts?
>>
>> How about "Allow for module (internal or external) to register trace function which
>> shows (or measures) interactions with user-space executables more directly.  Knowing
>> when interrupts happen and how they can subtly impact user-space application timing
>> is important in some application."
>>
>>>
>>> I'm also curious to know why ftrace isn't good enough.
>>
>> ftrace (if one considers ltt-ng) is probably capable of doing what my trace does
>> except that the timestamp is not TOD (Time Of Day) -- but probably/maybe could
>> be made to do so???
>>
>> It's a matter of complexity or "start up cost" (or entry barrier).
>>
>> I'm not the best at articulating all there is to TRACE succinctly, but lets
>> start with the fact that the user-space component of TRACE is basically just a header
>> file that defines the TRACE printf-like macro and handles the initialization which is
>> basically mmap-ing a file (as a trace buffer (with control structure)) and is,
>> for the most part, Unix OS independent.
>
> Heh, that sounds like the perf interface. Have you thought about using
> that instead?
>
> Look at what PowerTop does: https://01.org/powertop
>
> You can use the perf interface to mmap a memory buffer and read the
> data in its raw format. To parse the data, you need to read the format

I've looked at the above reference briefly and it appears that user-space
would be mmapping the buffer read-only. Is that correct?
I've also looked at lttng and I've said before, the "start-up cost" is too high.
My TRACE module creates the virtual file and allows the user-space program(s) to
mmap a portion of it read-only (to protect the kernel) and then the buffer portion
read-write - so, yes, any user-space program _could_ simply scribble over the
buffer-portion, but what they usually end up doing it _writing_ their traces into
the appropriate "slot" memory along side of slots that the kernel could be writing the
key event TRACEs to.
I'm not trying to promote the use of my TRACE, I'm just trying to show
that there is a set of valid reasons (current and potential future) for accepting my patch.

> files in /sys/kernel/debug/tracing/events, but there's a library that
> does that. Unfortunately, that library isn't in distros yet (we really
> need to get that done). But you can just copy the code from the kernel
> source tree from tools/lib/traceevents/ and use that as perf does (as
> well as powertop does too).
>
> The format files tells you how to get the data from the raw binary
> format.
>
> If you use the perf interface, you can have userspace tools do custom
> things with the events as they happen. This may make your utility more
> robust in the long run.
>
> I'd be happy to help you understand how to use the traceevent library. I
> don't use the perf interface, but you can get examples from the
> PowerTop code.

I'd rather not get into the "you should stop doing your thing and just/only do
something else" mode, if you don't mind. As I've said before, I believe my TRACEing
is just the right level of complexity for the total system analysis that I do and
I have been periodically checking/searching what's out there.

A college of mine and I actually used perf, just today, to gain an understanding
of what is going on in our "Nova data acquistion" system at fermilab. While it was
useful, it is not a replacement for TRACE.

Thanks,
Ron

>
> -- Steve
>
[snip]

-- 
Ron Rechenmacher
Engineer
Fermi National Accelerator Laboratory
Batavia, IL  60510

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-22  2:24                       ` Ron Rechenmacher
@ 2015-04-22 12:53                         ` Steven Rostedt
  2015-04-22 12:55                           ` Steven Rostedt
  2015-04-22 14:47                           ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 28+ messages in thread
From: Steven Rostedt @ 2015-04-22 12:53 UTC (permalink / raw)
  To: Ron Rechenmacher
  Cc: Christoph Hellwig, linux-kernel, Arnaldo Carvalho de Melo,
	Arjan van de Ven, Namhyung Kim

On Tue, 21 Apr 2015 21:24:51 -0500
Ron Rechenmacher <ron@fnal.gov> wrote:


> I've looked at the above reference briefly and it appears that user-space
> would be mmapping the buffer read-only. Is that correct?

Correct, but I'm sure we could still add something (if it doesn't
already exist) to have userspace write into the buffer. Ftrace has that
with the trace_marker file.

> I've also looked at lttng and I've said before, the "start-up cost" is too high.

I'm not sure what you mean by "start-up cost". Note, I'm not actually
talking about using userspace perf. I'm talking about using the perf
interface directly with your program like powertop does.

> My TRACE module creates the virtual file and allows the user-space program(s) to
> mmap a portion of it read-only (to protect the kernel) and then the buffer portion
> read-write - so, yes, any user-space program _could_ simply scribble over the
> buffer-portion, but what they usually end up doing it _writing_ their traces into
> the appropriate "slot" memory along side of slots that the kernel could be writing the
> key event TRACEs to.
> I'm not trying to promote the use of my TRACE, I'm just trying to show
> that there is a set of valid reasons (current and potential future) for accepting my patch.
> 


> 
> I'd rather not get into the "you should stop doing your thing and just/only do
> something else" mode, if you don't mind. As I've said before, I believe my TRACEing
> is just the right level of complexity for the total system analysis that I do and
> I have been periodically checking/searching what's out there.

Understand. This was my feeling with logdev. I ported it year after
year (from '98 all the way to 2007 - when ftrace made it obsolete, I
even ported it to the xen hypervisor!).

What I learned was that something that was so useful to me could also
be very useful for others. You got my attention. I want to understand
why this works for you so well, and perhaps we can modify the existing
tools to make them handle all the requirements that your TRACE tool
does or at the minimum, have something your TRACE tool can hook into.

ftrace is the result of logdev being merged with the latency-tracer
that was in the -rt patch. I'm not saying stop doing your thing, I'm
asking what is different about your thing that we can make our tools
better to do it too. Your thing is obviously beneficial to you. I'm
guessing it can be beneficial to others outside the fermilab.


> 
> A college of mine and I actually used perf, just today, to gain an understanding
> of what is going on in our "Nova data acquistion" system at fermilab. While it was
> useful, it is not a replacement for TRACE.

Again, I did not mean to use the perf user space tool. I was wondering
if it would be possible to have TRACE use the perf kernel interface.

-- Steve

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-22 12:53                         ` Steven Rostedt
@ 2015-04-22 12:55                           ` Steven Rostedt
  2015-04-22 14:47                           ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2015-04-22 12:55 UTC (permalink / raw)
  To: Ron Rechenmacher
  Cc: Christoph Hellwig, linux-kernel, Arnaldo Carvalho de Melo,
	Arjan van de Ven, Namhyung Kim

On Wed, 22 Apr 2015 08:53:14 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
 
> > 
> > I'd rather not get into the "you should stop doing your thing and just/only do
> > something else" mode, if you don't mind. As I've said before, I believe my TRACEing
> > is just the right level of complexity for the total system analysis that I do and
> > I have been periodically checking/searching what's out there.
> 
> Understand. This was my feeling with logdev. I ported it year after

That was suppose to be "I understand.", that is, I understand what you
meant. It was not suppose to sound like I'm telling you to "understand".

-- Steve

> year (from '98 all the way to 2007 - when ftrace made it obsolete, I
> even ported it to the xen hypervisor!).

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-22 12:53                         ` Steven Rostedt
  2015-04-22 12:55                           ` Steven Rostedt
@ 2015-04-22 14:47                           ` Arnaldo Carvalho de Melo
  2015-04-22 15:36                             ` David Ahern
  1 sibling, 1 reply; 28+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-04-22 14:47 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Ron Rechenmacher, Christoph Hellwig, linux-kernel,
	Arjan van de Ven, Namhyung Kim, David Ahern

Em Wed, Apr 22, 2015 at 08:53:14AM -0400, Steven Rostedt escreveu:
> On Tue, 21 Apr 2015 21:24:51 -0500
> Ron Rechenmacher <ron@fnal.gov> wrote:
> > I've looked at the above reference briefly and it appears that user-space
> > would be mmapping the buffer read-only. Is that correct?
> 
> Correct, but I'm sure we could still add something (if it doesn't
> already exist) to have userspace write into the buffer. Ftrace has that
> with the trace_marker file.

There is something in the works, I guess Pawell Moll (sp) was working on it, and
David Ahern (CCed) should know, David?
 
> > I've also looked at lttng and I've said before, the "start-up cost" is too high.
> 
> I'm not sure what you mean by "start-up cost". Note, I'm not actually
> talking about using userspace perf. I'm talking about using the perf
> interface directly with your program like powertop does.
> 
> > My TRACE module creates the virtual file and allows the user-space program(s) to
> > mmap a portion of it read-only (to protect the kernel) and then the buffer portion
> > read-write - so, yes, any user-space program _could_ simply scribble over the
> > buffer-portion, but what they usually end up doing it _writing_ their traces into
> > the appropriate "slot" memory along side of slots that the kernel could be writing the
> > key event TRACEs to.
> > I'm not trying to promote the use of my TRACE, I'm just trying to show
> > that there is a set of valid reasons (current and potential future) for accepting my patch.

> > I'd rather not get into the "you should stop doing your thing and just/only do
> > something else" mode, if you don't mind. As I've said before, I believe my TRACEing
> > is just the right level of complexity for the total system analysis that I do and
> > I have been periodically checking/searching what's out there.
 
> Understand. This was my feeling with logdev. I ported it year after
> year (from '98 all the way to 2007 - when ftrace made it obsolete, I
> even ported it to the xen hypervisor!).
 
> What I learned was that something that was so useful to me could also
> be very useful for others. You got my attention. I want to understand
> why this works for you so well, and perhaps we can modify the existing
> tools to make them handle all the requirements that your TRACE tool
> does or at the minimum, have something your TRACE tool can hook into.
> 
> ftrace is the result of logdev being merged with the latency-tracer
> that was in the -rt patch. I'm not saying stop doing your thing, I'm
> asking what is different about your thing that we can make our tools
> better to do it too. Your thing is obviously beneficial to you. I'm
> guessing it can be beneficial to others outside the fermilab.
 
> > A college of mine and I actually used perf, just today, to gain an understanding
> > of what is going on in our "Nova data acquistion" system at fermilab. While it was
> > useful, it is not a replacement for TRACE.
 
> Again, I did not mean to use the perf user space tool. I was wondering
> if it would be possible to have TRACE use the perf kernel interface.

Well, but then it can influence how perf works, be it in the kernel (this
userspace event insertion in the ring buffer, in the works, may be suitable
here, may be not), be it in the tooling side, where there is a strace like tool
that could do parts of what you want.

- Arnaldo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-22 14:47                           ` Arnaldo Carvalho de Melo
@ 2015-04-22 15:36                             ` David Ahern
  2015-04-22 15:44                               ` Steven Rostedt
  2015-04-23 15:28                               ` Pawel Moll
  0 siblings, 2 replies; 28+ messages in thread
From: David Ahern @ 2015-04-22 15:36 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Steven Rostedt
  Cc: Ron Rechenmacher, Christoph Hellwig, linux-kernel,
	Arjan van de Ven, Namhyung Kim, Pawel Moll, Stephane Eranian

On 4/22/15 8:47 AM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Apr 22, 2015 at 08:53:14AM -0400, Steven Rostedt escreveu:
>> >On Tue, 21 Apr 2015 21:24:51 -0500
>> >Ron Rechenmacher<ron@fnal.gov>  wrote:
>>> > >I've looked at the above reference briefly and it appears that user-space
>>> > >would be mmapping the buffer read-only. Is that correct?
>> >
>> >Correct, but I'm sure we could still add something (if it doesn't
>> >already exist) to have userspace write into the buffer. Ftrace has that
>> >with the trace_marker file.
> There is something in the works, I guess Pawell Moll (sp) was working on it, and
> David Ahern (CCed) should know, David?
>

I played around with generating perf events in userspace with the 
intention of having the userspace events get merged with kernel events 
during the processing stage, but I did not take it to the point of 
integrating into perf. This was around October 2013. I got distracted 
with other topics and have not come back to it.

Pawel has a patch that allows userspace to inject events into the stream 
via ioctl calls.

Stephane also injects events for JIT.

One of the key requirements is a common time basis (e.g., 
CLOCK_MONOTONIC or PERF_CLOCK) to be able to merge the events properly. 
I have a kernel module that exports perf_clock to userspace via 
clock_gettime; the 4.1 kernel should have the code that allows the clock 
id to be specified providing a solution to this problem.

David

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-22 15:36                             ` David Ahern
@ 2015-04-22 15:44                               ` Steven Rostedt
  2015-04-22 16:35                                 ` Ron Rechenmacher
  2015-04-23 15:28                               ` Pawel Moll
  1 sibling, 1 reply; 28+ messages in thread
From: Steven Rostedt @ 2015-04-22 15:44 UTC (permalink / raw)
  To: Ron Rechenmacher
  Cc: David Ahern, Arnaldo Carvalho de Melo, Christoph Hellwig,
	linux-kernel, Arjan van de Ven, Namhyung Kim, Pawel Moll,
	Stephane Eranian

On Wed, 22 Apr 2015 09:36:20 -0600
David Ahern <dsahern@gmail.com> wrote:

> One of the key requirements is a common time basis (e.g., 
> CLOCK_MONOTONIC or PERF_CLOCK) to be able to merge the events properly. 
> I have a kernel module that exports perf_clock to userspace via 
> clock_gettime; the 4.1 kernel should have the code that allows the clock 
> id to be specified providing a solution to this problem.

Ron,

I should have warned you. This is normal Linux kernel development.
Someone posts a simple patch to make something of there's work better,
and it ends up becoming a massive undertaking to come up with a solution
that makes Linux in general a better operating system ;-)

-- Steve

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-22 15:44                               ` Steven Rostedt
@ 2015-04-22 16:35                                 ` Ron Rechenmacher
  2015-04-22 17:00                                   ` Steven Rostedt
  0 siblings, 1 reply; 28+ messages in thread
From: Ron Rechenmacher @ 2015-04-22 16:35 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: David Ahern, Arnaldo Carvalho de Melo, Christoph Hellwig,
	linux-kernel, Arjan van de Ven, Namhyung Kim, Pawel Moll,
	Stephane Eranian

Thanks, Steve :)

I was working on a reply to your previous email but I'll reply here.
I'm happy with how this is going and I'm really appreciative of how
you have/are handling (managing) this. Thanks!

Assuming I'm only getting the tip of the ice berg of "normal Linux kernel
development," I don't know if it's reasonable to achieve a balance between
discussing "accepting the patch" and "what I'm doing with TRACE."

If I may, I can separate the two as follows (and focusing more on "accepting the patch"):
In my 30 year evolution of TRACE, a significant point came (somewhere in the
1990's) when, with Vxworks, I noticed that they had a "task switch hook add" routine
and I was able to incorporate kernel task switching events in with the application
traces. My first patches to the linux-2.2 kernel included a patch to the
scheduler and interrupts came quickly there after.  I was always wondering
if Linux would one day provide "task switch hook" capability.
In early 2013, when I started the v3 of TRACE, I was extremely happy to find
the symbol "register_trace_sched_switch" --- this was all I needed and I thought
"they did it."

With the understanding, as has been mentioned before, that "only the user space ABI is
what we keep stable" and that before commit de7b2973903c (tracepoint: Use struct pointer
instead of name hash for reg/unreg tracepoints) it was just "lucky" that I could use
the register_tracepoint_* routines as somewhat generic "add hook" routines (from
external modules), is it now reasonable to accept my patch to allow external modules
to "add hooks" to key events in Linux?  Why or why not?

Thanks,
Ron


Steven Rostedt wrote on 04/22/15 10:44:
> On Wed, 22 Apr 2015 09:36:20 -0600
> David Ahern <dsahern@gmail.com> wrote:
>
>> One of the key requirements is a common time basis (e.g.,
>> CLOCK_MONOTONIC or PERF_CLOCK) to be able to merge the events properly.
>> I have a kernel module that exports perf_clock to userspace via
>> clock_gettime; the 4.1 kernel should have the code that allows the clock
>> id to be specified providing a solution to this problem.
>
> Ron,
>
> I should have warned you. This is normal Linux kernel development.
> Someone posts a simple patch to make something of there's work better,
> and it ends up becoming a massive undertaking to come up with a solution
> that makes Linux in general a better operating system ;-)
>
> -- Steve
>

-- 
Ron Rechenmacher
Engineer
Fermi National Accelerator Laboratory
Batavia, IL  60510

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-22 16:35                                 ` Ron Rechenmacher
@ 2015-04-22 17:00                                   ` Steven Rostedt
  0 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2015-04-22 17:00 UTC (permalink / raw)
  To: Ron Rechenmacher
  Cc: David Ahern, Arnaldo Carvalho de Melo, Christoph Hellwig,
	linux-kernel, Arjan van de Ven, Namhyung Kim, Pawel Moll,
	Stephane Eranian

On Wed, 22 Apr 2015 11:35:41 -0500
Ron Rechenmacher <ron@fnal.gov> wrote:

 
> With the understanding, as has been mentioned before, that "only the user space ABI is
> what we keep stable" and that before commit de7b2973903c (tracepoint: Use struct pointer
> instead of name hash for reg/unreg tracepoints) it was just "lucky" that I could use
> the register_tracepoint_* routines as somewhat generic "add hook" routines (from
> external modules), is it now reasonable to accept my patch to allow external modules
> to "add hooks" to key events in Linux?  Why or why not?
> 

As has been mentioned before, the EXPORT_SYMBOL*() calls is for in-tree
modules that need them. If there's no in-tree users of the exported
symbol, that can be grounds for removing them.

But that is more of a guideline than a rule. Preferably, we want people
using the in-tree mechanisms, because as I have said before, if
something is useful for you, it is most likely useful for others. Even
if it is your own home grown utility. You've been keeping it around for
30 years and that says something. It means that it is probably useful
for other people as well.

We resist making small changes for out-of-tree modules because we want
that functionality in tree. Now, if for some reason, we agree that the
functionality does not belong in tree, and all maintainers involved are
OK, we can add a hook for you. But that wont happen until we are
convinced that the functionality you want belongs out of tree and the
in tree mechanisms are not good enough for you.

I'll admit it adds more burden on you. But the net goal of Linux kernel
development is not it improve usability for each individual, but to
improve the infrastructure as a whole, such that the community benefits
in the end, and not just for a few individuals. Yes, it takes more
time, thought and effort, but in the long run, it's a net gain for
everyone (including you :-).

-- Steve

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-22 15:36                             ` David Ahern
  2015-04-22 15:44                               ` Steven Rostedt
@ 2015-04-23 15:28                               ` Pawel Moll
  2015-04-23 15:33                                 ` Pawel Moll
  1 sibling, 1 reply; 28+ messages in thread
From: Pawel Moll @ 2015-04-23 15:28 UTC (permalink / raw)
  To: David Ahern
  Cc: Arnaldo Carvalho de Melo, Steven Rostedt, Ron Rechenmacher,
	Christoph Hellwig, linux-kernel, Arjan van de Ven, Namhyung Kim,
	Stephane Eranian

On Wed, 2015-04-22 at 16:36 +0100, David Ahern wrote:
> On 4/22/15 8:47 AM, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Apr 22, 2015 at 08:53:14AM -0400, Steven Rostedt escreveu:
> >> >On Tue, 21 Apr 2015 21:24:51 -0500
> >> >Ron Rechenmacher<ron@fnal.gov>  wrote:
> >>> > >I've looked at the above reference briefly and it appears that user-space
> >>> > >would be mmapping the buffer read-only. Is that correct?
> >> >
> >> >Correct, but I'm sure we could still add something (if it doesn't
> >> >already exist) to have userspace write into the buffer. Ftrace has that
> >> >with the trace_marker file.
> > There is something in the works, I guess Pawell Moll (sp) was working on it, and
> > David Ahern (CCed) should know, David?
> >
> 
> I played around with generating perf events in userspace with the 
> intention of having the userspace events get merged with kernel events 
> during the processing stage, but I did not take it to the point of 
> integrating into perf. This was around October 2013. I got distracted 
> with other topics and have not come back to it.
> 
> Pawel has a patch that allows userspace to inject events into the stream 
> via ioctl calls.

In the last version it was even a prctl - no need for a perf file
descriptor any more :-)

But the patch requires more care if it's to go in, so I'm open to people
screaming "yes, we need it!" ;-)

> Stephane also injects events for JIT.

But that, as far as I understand, happens in userspace (as in: nothing
goes down to kernel)?

> One of the key requirements is a common time basis (e.g., 
> CLOCK_MONOTONIC or PERF_CLOCK) to be able to merge the events properly. 
> I have a kernel module that exports perf_clock to userspace via 
> clock_gettime; the 4.1 kernel should have the code that allows the clock 
> id to be specified providing a solution to this problem.

It's in! :-) After all these years...

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=34f439278cef7b1177f8ce24f9fc81dfc6221d3b

Pawel


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-23 15:28                               ` Pawel Moll
@ 2015-04-23 15:33                                 ` Pawel Moll
  0 siblings, 0 replies; 28+ messages in thread
From: Pawel Moll @ 2015-04-23 15:33 UTC (permalink / raw)
  To: David Ahern
  Cc: Arnaldo Carvalho de Melo, Steven Rostedt, Ron Rechenmacher,
	Christoph Hellwig, linux-kernel, Arjan van de Ven, Namhyung Kim,
	Stephane Eranian

On Thu, 2015-04-23 at 16:28 +0100, Pawel Moll wrote:
> On Wed, 2015-04-22 at 16:36 +0100, David Ahern wrote:
> > On 4/22/15 8:47 AM, Arnaldo Carvalho de Melo wrote:
> > > Em Wed, Apr 22, 2015 at 08:53:14AM -0400, Steven Rostedt escreveu:
> > >> >On Tue, 21 Apr 2015 21:24:51 -0500
> > >> >Ron Rechenmacher<ron@fnal.gov>  wrote:
> > >>> > >I've looked at the above reference briefly and it appears that user-space
> > >>> > >would be mmapping the buffer read-only. Is that correct?
> > >> >
> > >> >Correct, but I'm sure we could still add something (if it doesn't
> > >> >already exist) to have userspace write into the buffer. Ftrace has that
> > >> >with the trace_marker file.
> > > There is something in the works, I guess Pawell Moll (sp) was working on it, and
> > > David Ahern (CCed) should know, David?
> > >
> > 
> > I played around with generating perf events in userspace with the 
> > intention of having the userspace events get merged with kernel events 
> > during the processing stage, but I did not take it to the point of 
> > integrating into perf. This was around October 2013. I got distracted 
> > with other topics and have not come back to it.
> > 
> > Pawel has a patch that allows userspace to inject events into the stream 
> > via ioctl calls.
> 
> In the last version it was even a prctl - no need for a perf file
> descriptor any more :-)
> 
> But the patch requires more care if it's to go in, so I'm open to people
> screaming "yes, we need it!" ;-)

Forgot to quote the link:

http://article.gmane.org/gmane.linux.kernel.api/5851

Pawel


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-20 21:38 [PATCH] tracing: Export key trace event symbols Ron Rechenmacher
  2015-04-21  6:10 ` Christoph Hellwig
@ 2015-04-24 21:24 ` Steven Rostedt
  2015-04-24 21:39   ` Mathieu Desnoyers
  2015-04-27 19:12   ` Ron Rechenmacher
  1 sibling, 2 replies; 28+ messages in thread
From: Steven Rostedt @ 2015-04-24 21:24 UTC (permalink / raw)
  To: Ron Rechenmacher; +Cc: linux-kernel, Mathieu Desnoyers

On Mon, 20 Apr 2015 16:38:11 -0500
Ron Rechenmacher <ron@fnal.gov> wrote:

> If symbols are not exported, modules can no longer register additional
> (module specified) tracepoints like they use to be able to (i.e linux-3.15.x).
> Somewhere on or about commit de7b2973903c6cc50b31ee5682a69b2219b9919d
> (Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Date:   Tue Apr 8 17:26:21 2014 -0400
> tracepoint: Use struct pointer instead of name hash for reg/unreg tracepoints)
> modules which attempted to register additional tracing functions would
> get "Unknown symbol" errors. For example: "... Unknown symbol
> __tracepoint_sched_switch (err 0)"
> Symbols can be exported using the kernel's EXPORT_TRACEPOINT_SYMBOL_GPL macro
> to allow modules to once again register their own tracing functions (for at
> least some key points in the kernel as provided by this patch).
> 
> Signed-off-by: Ron Rechenmacher <ron@fnal.gov>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=96051

Hi Ron,

I was talking with Mathieu on IRC and asked him how LTTng gets its
kernel tracepoints, and he told me he uses
for_each_kernel_tracepoint(). That will iterate over all tracepoints
that have been added in the kernel (and is exported GPL).

You can still use that to get the handle onto any tracepoint you need.
It's pretty straight forward (I just wrote a simple module to test it
out), and just compare against the tp->name, to find what you want.

I still would like to get more usage out of the internal code, but this
is your work around you wanted. No need to export new symbols. Just a
little more setup time on module load.

-- Steve

Here's my mod....


#include <linux/module.h>
#include <linux/ftrace.h>
#include <linux/tracepoint.h>

static func(struct tracepoint *tp, void *ignore)
{
	printk("tracepoint: %s\n", tp->name);
}

static int __init my_tp_init(void)
{
	for_each_kernel_tracepoint(func, NULL);
	return 0;
}

static void __exit my_tp_exit(void)
{
}

module_init(my_tp_init);
module_exit(my_tp_exit);

MODULE_AUTHOR("My name here");
MODULE_DESCRIPTION("Me!");
MODULE_LICENSE("GPL");

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-24 21:24 ` Steven Rostedt
@ 2015-04-24 21:39   ` Mathieu Desnoyers
  2015-04-27 19:12   ` Ron Rechenmacher
  1 sibling, 0 replies; 28+ messages in thread
From: Mathieu Desnoyers @ 2015-04-24 21:39 UTC (permalink / raw)
  To: Ron Rechenmacher; +Cc: linux-kernel, Thomas Gleixner, Steven Rostedt

----- Original Message -----
> On Mon, 20 Apr 2015 16:38:11 -0500
> Ron Rechenmacher <ron@fnal.gov> wrote:
> 
> > If symbols are not exported, modules can no longer register additional
> > (module specified) tracepoints like they use to be able to (i.e
> > linux-3.15.x).
> > Somewhere on or about commit de7b2973903c6cc50b31ee5682a69b2219b9919d
> > (Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > Date:   Tue Apr 8 17:26:21 2014 -0400
> > tracepoint: Use struct pointer instead of name hash for reg/unreg
> > tracepoints)
> > modules which attempted to register additional tracing functions would
> > get "Unknown symbol" errors. For example: "... Unknown symbol
> > __tracepoint_sched_switch (err 0)"
> > Symbols can be exported using the kernel's EXPORT_TRACEPOINT_SYMBOL_GPL
> > macro
> > to allow modules to once again register their own tracing functions (for at
> > least some key points in the kernel as provided by this patch).
> > 
> > Signed-off-by: Ron Rechenmacher <ron@fnal.gov>
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=96051
> 
> Hi Ron,
> 
> I was talking with Mathieu on IRC and asked him how LTTng gets its
> kernel tracepoints, and he told me he uses
> for_each_kernel_tracepoint(). That will iterate over all tracepoints
> that have been added in the kernel (and is exported GPL).
> 
> You can still use that to get the handle onto any tracepoint you need.
> It's pretty straight forward (I just wrote a simple module to test it
> out), and just compare against the tp->name, to find what you want.
> 
> I still would like to get more usage out of the internal code, but this
> is your work around you wanted. No need to export new symbols. Just a
> little more setup time on module load.

Hi Ron,

Quoting a snippet of your earlier emails:

"ftrace (if one considers ltt-ng) is probably capable of doing what my trace does
except that the timestamp is not TOD (Time Of Day) -- but probably/maybe could
be made to do so???"

FYI, LTTng and Ftrace are two different projects. LTTng features
am out-of-tree kernel and a user-space tracer, whereas Ftrace features
a kernel tracer, readily available from the Linux kernel sources.

The timestamps can now be the monotonic clock for each of perf, ftrace,
and lttng, thanks to the work on nmi-safe clock source done by Thomas
Gleixner (merged in 3.17).

With the nmi-safe clocks, you'll be able to correlate samples
and traces taken by perf, ftrace, and lttng-modules with LTTng
userspace traces (LTTng-UST). This has been one of the goals of the
Common Trace Format (CTF) since its creation. You might want to look
at the perf-to-ctf conversion feature merged in Linux 4.0.

Best regards,

Mathieu



> 
> -- Steve
> 
> Here's my mod....
> 
> 
> #include <linux/module.h>
> #include <linux/ftrace.h>
> #include <linux/tracepoint.h>
> 
> static func(struct tracepoint *tp, void *ignore)
> {
> 	printk("tracepoint: %s\n", tp->name);
> }
> 
> static int __init my_tp_init(void)
> {
> 	for_each_kernel_tracepoint(func, NULL);
> 	return 0;
> }
> 
> static void __exit my_tp_exit(void)
> {
> }
> 
> module_init(my_tp_init);
> module_exit(my_tp_exit);
> 
> MODULE_AUTHOR("My name here");
> MODULE_DESCRIPTION("Me!");
> MODULE_LICENSE("GPL");
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] tracing: Export key trace event symbols
  2015-04-24 21:24 ` Steven Rostedt
  2015-04-24 21:39   ` Mathieu Desnoyers
@ 2015-04-27 19:12   ` Ron Rechenmacher
  1 sibling, 0 replies; 28+ messages in thread
From: Ron Rechenmacher @ 2015-04-27 19:12 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, Mathieu Desnoyers

Steven Rostedt wrote on 04/24/15 16:24:
> On Mon, 20 Apr 2015 16:38:11 -0500
> Ron Rechenmacher <ron@fnal.gov> wrote:
>
>> If symbols are not exported, modules can no longer register additional
>> (module specified) tracepoints like they use to be able to (i.e linux-3.15.x).
>> Somewhere on or about commit de7b2973903c6cc50b31ee5682a69b2219b9919d
>> (Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> Date:   Tue Apr 8 17:26:21 2014 -0400
>> tracepoint: Use struct pointer instead of name hash for reg/unreg tracepoints)
>> modules which attempted to register additional tracing functions would
>> get "Unknown symbol" errors. For example: "... Unknown symbol
>> __tracepoint_sched_switch (err 0)"
>> Symbols can be exported using the kernel's EXPORT_TRACEPOINT_SYMBOL_GPL macro
>> to allow modules to once again register their own tracing functions (for at
>> least some key points in the kernel as provided by this patch).
>>
>> Signed-off-by: Ron Rechenmacher <ron@fnal.gov>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=96051
>
> Hi Ron,
>
> I was talking with Mathieu on IRC and asked him how LTTng gets its
> kernel tracepoints, and he told me he uses
> for_each_kernel_tracepoint(). That will iterate over all tracepoints
> that have been added in the kernel (and is exported GPL).
>
> You can still use that to get the handle onto any tracepoint you need.
> It's pretty straight forward (I just wrote a simple module to test it
> out), and just compare against the tp->name, to find what you want.

Excellent. Thanks Steve. I was able to get this to work 100% for me.
What should be done about the bug report (96051)?
Will you handle it or should I change it to "resolved as invalid"?

>
> I still would like to get more usage out of the internal code, but this
> is your work around you wanted. No need to export new symbols. Just a
> little more setup time on module load.

Do you think for_each_kernel_tracepoint() and, additionally, the
tracepoint_probe_register() and tracepoint_probe_unregister() functions
will be around for quite some time? I would imagine as long as
LTTng uses them.

Thanks,
Ron

>
> -- Steve
>
> Here's my mod....
>
>
> #include <linux/module.h>
> #include <linux/ftrace.h>
> #include <linux/tracepoint.h>
>
> static func(struct tracepoint *tp, void *ignore)
> {
> 	printk("tracepoint: %s\n", tp->name);
> }
>
> static int __init my_tp_init(void)
> {
> 	for_each_kernel_tracepoint(func, NULL);
> 	return 0;
> }
>
> static void __exit my_tp_exit(void)
> {
> }
>
> module_init(my_tp_init);
> module_exit(my_tp_exit);
>
> MODULE_AUTHOR("My name here");
> MODULE_DESCRIPTION("Me!");
> MODULE_LICENSE("GPL");
>

-- 
Ron Rechenmacher
Engineer
Fermi National Accelerator Laboratory
Batavia, IL  60510

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2015-04-27 19:17 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-20 21:38 [PATCH] tracing: Export key trace event symbols Ron Rechenmacher
2015-04-21  6:10 ` Christoph Hellwig
2015-04-21 12:04   ` Ron Rechenmacher
2015-04-21 12:19     ` Ron Rechenmacher
2015-04-21 13:38       ` Steven Rostedt
2015-04-21 12:22     ` Christoph Hellwig
2015-04-21 13:13       ` Ron Rechenmacher
2015-04-21 13:23         ` Christoph Hellwig
2015-04-21 13:26           ` Ron Rechenmacher
2015-04-21 13:53             ` Steven Rostedt
2015-04-21 15:00               ` Ron Rechenmacher
2015-04-21 15:49                 ` Steven Rostedt
2015-04-21 22:23                   ` Ron Rechenmacher
2015-04-21 22:44                     ` Steven Rostedt
2015-04-22  2:24                       ` Ron Rechenmacher
2015-04-22 12:53                         ` Steven Rostedt
2015-04-22 12:55                           ` Steven Rostedt
2015-04-22 14:47                           ` Arnaldo Carvalho de Melo
2015-04-22 15:36                             ` David Ahern
2015-04-22 15:44                               ` Steven Rostedt
2015-04-22 16:35                                 ` Ron Rechenmacher
2015-04-22 17:00                                   ` Steven Rostedt
2015-04-23 15:28                               ` Pawel Moll
2015-04-23 15:33                                 ` Pawel Moll
2015-04-21 13:31           ` Steven Rostedt
2015-04-24 21:24 ` Steven Rostedt
2015-04-24 21:39   ` Mathieu Desnoyers
2015-04-27 19:12   ` Ron Rechenmacher

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).