Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1464601

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

From Steven Rostedt <rostedt@goodmis.org>
Newsgroups linux.kernel
Subject Re: [PATCH V4 1/3] tracing: add a possibility of exporting function trace to other places instead of ring buffer only
Date 2016-08-17 16:20 +0200
Message-ID <s79Rf-29m-9@gated-at.bofh.it> (permalink)
References <s6oIF-56q-11@gated-at.bofh.it> <s6oIF-56q-9@gated-at.bofh.it> <s6r3Q-6HX-13@gated-at.bofh.it> <s78s9-15B-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Wed, 17 Aug 2016 20:48:07 +0800
Chunyan Zhang <zhang.chunyan@linaro.org> wrote:


> Ok, will do.
> 
> But I guess 'ftrace_ops_list' in ftrace.c [1] also should be annotated as rcu?
> 
> [1] http://lxr.free-electrons.com/source/kernel/trace/ftrace.c#L460

Hmm, perhaps it should. I wonder if sparse complains about this?




> >  
> >> +
> >> +     mutex_lock(&trace_export_lock);
> >> +
> >> +     export->tr = trace_exports_list->tr;  
> >
> > I don't see where tr is ever assigned.
> >  
> >> +     export->commit = trace_generic_commit;  
> >
> > Shouldn't the caller pass in the commit function too?  
> 
> The trace_export::write() callback is for caller, commit function
> mainly deal with traces, is it better to keep 'trace_generic_commit'
> in trace.c, i.e don't expose it to callers?

It's fine to be external if it's only declared in kernel/trace/trace.h.
I would think anything using a different "write" would require a
different "commit".

But maybe I'm misunderstanding your objective. See below.

> 
> >  
> >> +
> >> +     add_trace_export(&trace_exports_list, export);
> >> +
> >> +     mutex_unlock(&trace_export_lock);
> >> +
> >> +     return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(register_trace_export);
> >> +
> >> +int unregister_trace_export(struct trace_export *export)
> >> +{
> >> +     int ret;
> >> +
> >> +     mutex_lock(&trace_export_lock);
> >> +
> >> +     ret = rm_trace_export(&trace_exports_list, export);
> >> +
> >> +     mutex_unlock(&trace_export_lock);
> >> +
> >> +     return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(unregister_trace_export);
> >> +
> >>  void
> >>  trace_function(struct trace_array *tr,
> >>              unsigned long ip, unsigned long parent_ip, unsigned long flags,
> >> @@ -2147,7 +2269,7 @@ trace_function(struct trace_array *tr,
> >>       entry->parent_ip                = parent_ip;
> >>
> >>       if (!call_filter_check_discard(call, entry, buffer, event))  
> >
> > How do you handle the discard? If the entry doesn't match the filter,
> > it will try to discard the event. I don't see how the "trace_exports"
> > handle that.  
> 
> I directly used the entries which had already been filtered.
> Humm.. sorry, actually you lost me here.

I'm assuming this entire patch is to have the events written to
something other than the ftrace ring buffer, correct?

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.

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.

-- Steve

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH V4 0/3] Integration of function trace with System Trace IP blocks Chunyan Zhang <zhang.chunyan@linaro.org> - 2016-08-15 14:00 +0200
  [PATCH V4 1/3] tracing: add a possibility of exporting function trace to other places instead of ring buffer only Chunyan Zhang <zhang.chunyan@linaro.org> - 2016-08-15 14:00 +0200
    Re: [PATCH V4 1/3] tracing: add a possibility of exporting function  trace to other places instead of ring buffer only kbuild test robot <lkp@intel.com> - 2016-08-15 15:10 +0200
      Re: [PATCH V4 1/3] tracing: add a possibility of exporting function  trace to other places instead of ring buffer only Chunyan Zhang <zhang.chunyan@linaro.org> - 2016-08-18 09:50 +0200
    Re: [PATCH V4 1/3] tracing: add a possibility of exporting function  trace to other places instead of ring buffer only Steven Rostedt <rostedt@goodmis.org> - 2016-08-15 16:30 +0200
      Re: [PATCH V4 1/3] tracing: add a possibility of exporting function  trace to other places instead of ring buffer only Chunyan Zhang <zhang.chunyan@linaro.org> - 2016-08-17 14:50 +0200
        Re: [PATCH V4 1/3] tracing: add a possibility of exporting function  trace to other places instead of ring buffer only Steven Rostedt <rostedt@goodmis.org> - 2016-08-17 16:20 +0200
          Re: [PATCH V4 1/3] tracing: add a possibility of exporting function  trace to other places instead of ring buffer only Chunyan Zhang <zhang.chunyan@linaro.org> - 2016-08-18 11:30 +0200
            Re: [PATCH V4 1/3] tracing: add a possibility of exporting function  trace to other places instead of ring buffer only Steven Rostedt <rostedt@goodmis.org> - 2016-08-19 03:00 +0200
              Re: [PATCH V4 1/3] tracing: add a possibility of exporting function  trace to other places instead of ring buffer only Chunyan Zhang <zhang.chunyan@linaro.org> - 2016-08-23 11:00 +0200
  [PATCH V4 2/3] stm class: ftrace: Add ftrace-export-over-stm driver Chunyan Zhang <zhang.chunyan@linaro.org> - 2016-08-15 14:00 +0200
  [PATCH V4 3/3] stm: Mark the functions of writing buffer with notrace Chunyan Zhang <zhang.chunyan@linaro.org> - 2016-08-15 14:00 +0200

csiph-web