Message-ID: <20171018102548.02fb1289@lwn.net>
Date:   Wed, 18 Oct 2017 10:25:48 -0600
From:   Jonathan Corbet <corbet@....net>
To:     Steven Rostedt <rostedt@...dmis.org>
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 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...

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.

> 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?

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

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

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

> +|    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? :)

> [...]

Thanks for doing this,

jon

Powered by blists - more mailing lists