[PATCH V4 1/3] tracing: add a possibility of exporting function trace to other places instead of ring buffer only

On 19 August 2016 at 00:12, Steven Rostedt [off-list ref] wrote:

On Thu, 18 Aug 2016 17:22:11 +0800
Chunyan Zhang [off-list ref] wrote:

quoted

quoted

Or is this just trying to hook into the tracing that is happening? That
is, this isn't replacing writing into the ftrace ring buffer, but it is
just adding a way to write to someplace in addition to the ftrace ring
buffer. Where you still write to the ftrace ring buffer, but then you
can add a hook to copy someplace else as well.

Yes, this is what this patch is trying to implement.

quoted

I was looking at this as a way that you are adding a replacement, not
only an addition to. If that's the case, I think there may be a easier
way to do this.

I want to know how it would be in the easier way you mentioned here.

I was trying to add a ftrace_ops before, but with that way, I have to
deal with a lot of trace or ring buffer stuff including the sort of
discard things like you mentioned, which the existed ftrace code does.
And if I choose to implement a new ftrace_ops, I'm only able to get
the function trace support for STM and have to do many things which
would be overlap with the current ftrace subsystem.

Adding your own ftrace_ops is a way for replacing, not just adding a
hook into.

quoted

So in order to reuse the existed code and architecture, I chose to add
a trace_export interface for Ftrace subsytem, and in this way I'm
using in this patch, I will get all supports of traces which are dealt
with trace_function();

Actually, a trace_export() should only be called if there's been
something added. And that should be done with a static_key_false()
branch (which is dynamically enabled, and does not use a comparison
branch).

That is, something like this instead:

        if (!call_filter_check_discard(call, entry, buffer, event)) {
                if (static_key_false(&ftrace_trace_exports_enabled))
                        ftrace_exports(tr, event);
                __buffer_unlock_commit(buffer, event);
        }

Thanks for the sample code, I got it, will do like this.

Don't touch the current logic. Just have your code hook into the
ftrace_exports (note I use "ftrace_exports" and not trace_exports()
because it's the function tracer, which has stricter requirements than
events do. If you add a hook for tracepoints later, use trace_exports()
and have a different list for that).

quoted

Another benefit of adding a trace_export is, if there will be other
subsystem would like to use the processed traces, it only needs to
register a trace_export and provides a .write() function call back or
together with a commit function, although from what I can see now
.write() is enough since my purpose was the processed traces I don't
need 'ring_buffer_event' so long as I had trace entries.

I'm saying if you don't mind the ring buffer being used along with
your own code (which seems to be what's happening), then just add a
call back to your code. Don't monkey with the current logic.

I think that will simplify things tremendously.

Thanks for your comments and detailed explanation,
Chunyan

-- Steve