Message-ID: <20171018131018.692de636@gandalf.local.home>
Date:   Wed, 18 Oct 2017 13:10:18 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Jonathan Corbet <corbet@....net>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Stafford Horne <shorne@...il.com>,
        Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH] ftrace/docs: Add documentation on how to use ftrace
 from within the kernel

On Wed, 18 Oct 2017 10:25:48 -0600
Jonathan Corbet <corbet@....net> wrote:

> On Mon, 9 Oct 2017 15:32:30 -0400
> Steven Rostedt <rostedt@...dmis.org> wrote:
> 
> > With the coming removal of jprobes, using ftrace callbacks is one of the
> > utilities that replace the jprobes functionality. Having a document that
> > explains how to use ftrace as such will help in the transition from jprobes
> > to ftrace. This document is for kernel developers that require attaching a
> > callback to a function within the kernel.  
> 
> So I'm sorry, this kind of fell through the cracks.  There seem to be a lot
> of cracks recently...

No prob, I've been having cracks too. And I need to edit the content
anyway, because I just pushed to linux-next the removal of the PER_CPU
flag and its associated functions.

> 
> It generally looks good, and could be merged in this form, but I'm gonna
> poke at the RST side of it for just a bit.

Honestly, I have no idea how to write RST. I just looked at other files
to try to get a clue.

> 
> > Link: http://lkml.kernel.org/r/150724519527.5014.10207042218696587159.stgit@devbox
> > 
> > Signed-off-by: Steven Rostedt (VMware) <rostedt@...dmis.org>
> > ---
> >  Documentation/trace/ftrace-uses.rst | 297 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 297 insertions(+)
> >  create mode 100644 Documentation/trace/ftrace-uses.rst  
> 
> You've created an RST file here, but haven't hooked it into the overall
> document build.  It could go in the development tools book, or we could
> consider whether a separate tracing manual will eventually make sense?

Probably. I know some people here don't care for RST format (had a few
comments that I wrote this document that way). But I guess in the long
run it's the way to go.

> 
> > diff --git a/Documentation/trace/ftrace-uses.rst b/Documentation/trace/ftrace-uses.rst
> > new file mode 100644
> > index 0000000..5914d71
> > --- /dev/null
> > +++ b/Documentation/trace/ftrace-uses.rst
> > @@ -0,0 +1,297 @@
> > +		Using ftrace to hook to functions
> > +		=================================
> > +
> > +Copyright 2017 VMware Inc.
> > +   Author:   Steven Rostedt <srostedt@...dmis.org>
> > +  License:   The GNU Free Documentation License, Version 1.2
> > +               (dual licensed under the GPL v2)  
> 
> This isn't proper RST and may not format up the way you expect it to.

OK, what's the proper way? I didn't modify this from the .txt part. As
you said the document was already 99% there.

  http://lkml.kernel.org/r/20171009093350.2680fcdf@lwn.net

> 
> > +Written for: 4.14
> > +
> > +Introduction
> > +------------
> > +
> > +The ftrace infrastructure was originially created to attach callbacks to the
> > +beginning of functions in order to record and trace the flow of the kernel.
> > +But callbacks to the start of a function can have other use cases. Either
> > +for live kernel patching, or for security monitoring. This document describes
> > +how to use ftrace to implement your own function callbacks.
> > +
> > +
> > +The ftrace context
> > +==================  
> 
> In Documentation/doc-guide/sphinx.rst we have some recommended conventions

Ah is that where I find out how to do .rst?

> for subsection headers; it would be good to follow them.  This, again, will
> not format the way you expect.
> 
> > +WARNING: The ability to add a callback to almost any function within the
> > +kernel comes with risks. A callback can be called from any context
> > +(normal, softirq, irq, and NMI). Callbacks can also be called just before
> > +going to idle, during CPU bring up and takedown, or going to user space.
> > +This requires extra care to what can be done inside a callback. A callback
> > +can be called outside the protective scope of RCU.
> > +
> > +The ftrace infrastructure has some protections agains recursions and RCU
> > +but one must still be very careful how they use the callbacks.
> > +
> > +
> > +The ftrace_ops structure
> > +========================
> > +
> > +To register a function callback, a ftrace_ops is required. This structure
> > +is used to tell ftrace what function should be called as the callback
> > +as well as what protections the callback will perform and not require
> > +ftrace to handle.
> > +
> > +There is only one field that is needed to be set when registering
> > +an ftrace_ops with ftrace::
> > +
> > + struct ftrace_ops ops = {
> > +       .func			= my_callback_func,
> > +       .flags			= MY_FTRACE_FLAGS
> > +       .private			= any_private_data_structure,
> > + };
> > +
> > +Both .flags and .private are optional. Only .func is required.
> > +
> > +To enable tracing call::
> > +
> > +  register_ftrace_function(&ops);
> > +
> > +To disable tracing call::
> > +
> > +  unregister_ftrace_function(&ops);
> > +
> > +The above is defined by including the header::
> > +
> > + #include <linux/ftrace.h>
> > +
> > +The registered callback will start being called some time after the
> > +register_ftrace_function() is called and before it returns. The exact time
> > +that callbacks start being called is dependent upon architecture and scheduling
> > +of services. The callback itself will have to handle any synchronization if it
> > +must begin at an exact moment.
> > +
> > +The unregister_ftrace_function() will guarantee that the callback is
> > +no longer being called by functions after the unregister_ftrace_function()
> > +returns. Note that to perform this guarantee, the unregister_ftrace_function()
> > +may take some time to finish.
> > +
> > +
> > +The callback function
> > +=====================
> > +
> > +The prototype of the callback function is as follows (as of v4.14)::  
> 
> This would be nice to pull directly from the kerneldoc comments if
> possible. 

I guess I have to figure out how to do that. But first, I would have to
add that to kernel doc in the code. I guess I would put that next to
the typedef for the callback?

 typedef void (*ftrace_func_t)(unsigned long ip, unsigned long
 parent_ip,
			      struct ftrace_ops *op, struct pt_regs *regs);



> 
> > + void callback_func(unsigned long ip, unsigned long parent_ip,
> > +		    struct ftrace_ops *op, struct pt_regs *regs);
> > +
> > +@ip
> > +	 This is the instruction pointer of the function that is being traced.
> > +      	 (where the fentry or mcount is within the function)
> > +
> > +@...ent_ip
> > +	This is the instruction pointer of the function that called the
> > +	the function being traced (where the call of the function occurred).
> > +
> > +@op
> > +	This is a pointer to ftrace_ops that was used to register the callback.
> > +	This can be used to pass data to the callback via the private pointer.
> > +
> > +@...s
> > +	If the FTRACE_OPS_FL_SAVE_REGS or FTRACE_OPS_FL_SAVE_REGS_IF_SUPPORTED
> > +	flags are set in the ftrace_ops structure, then this will be pointing
> > +	to the pt_regs structure like it would be if an breakpoint was placed
> > +	at the start of the function where ftrace was tracing. Otherwise it
> > +	either contains garbage, or NULL.
> > +
> > +
> > +The ftrace FLAGS
> > +================
> > +
> > +The ftrace_ops flags are all defined and documented in include/linux/ftrace.h.
> > +Some of the flags are used for internal infrastructure of ftrace, but the
> > +ones that users should be aware of are the following:
> > +
> > +FTRACE_OPS_FL_PER_CPU
> > +	When set, the callback is only when the defined CPU has been
> > +	"enabled" to do so. After the callback is registered, all
> > +	CPUs are set to disable, and the callback is not called.
> > +	In order to enable or disalble a CPU to call a given callback,
> > +	use one of the following functions.
> > +
> > +::  
> 
> You could say "use one of the following functions::" (as you did above) and
> avoid the separate :: line.

This didn't work when I tried it in http://rst.ninjs.org/

But it's moot anyway, as this section is going to be deleted as it is
deleted in linux-next.

> 
> > +|    void ftrace_function_local_enable(struct ftrace_ops *ops);
> > +|    void ftrace_function_local_disable(struct ftrace_ops *ops);
> > +
> > +	These two functions must be called with preemption disabled.
> > +  
> 
> > [...]  
> 
> > +
> > +See Filter Commands in :file:`Documentation/trace/ftrace.txt`.  
> 
> So when do we get that RSTified so this can be a direct cross reference? :)

I guess.

> 
> > [...]  
> 
> Thanks for doing this,

Thanks for reviewing.

-- Steve

Powered by blists - more mailing lists