Re: [PATCH 2/2] tracing: Export ftrace API for kernel modules

Steven Rostedt
2009-09-16T13:04:16Z

On Wed, 2009-09-16 at 15:09 +0900, Atsushi Tsuji wrote:

> > I don't want to disable preemption when I don't have to. The function
> > tracer that is called can. But actually, it's ever more that that. If
> > you only register a single function, it will call that function
> > directly. Then there will always be a race window between when the
> > function gets called and disabling preemption, even if the called
> > function disables preemption as the first thing it does.
>
> Thank you for detailed explanation.
>
> I may be wrong, but I think function_trace_probe_call using
> register_ftrace_function_probe is almost enough for modules,

Heh, I forgot about function_probe. Yeah, looking at that, it does seem
that it would be safe for modules.

> since it disables preemption while a probe is calling and it
> called every time even if only one probe function is registered.
> So is it enough to make a new registering function using
> it and upping module ref count for module safe?

Yes, upping the module ref count for every function probe would be
required. Unfortunately, this would require adding another variable to
struct ftrace_func_probe, which I hate to do.

>
> Or should I make another handler for modules not using
> function_trace_probe_call?

Maybe another handler might be better. But it may be similar to
function_probe.

>
> >>> It will still need to up the mod ref count when a probe is added, but it
> >>> can also remove it.
> >>>
> >>>
> >>> The problem with the current method, is that a probe can be executing at
> >>> anytime. Here's an example if we did it your way.
> >>>
> >>> 1. module installed
> >>> 2. module adds probe
> >>> 3. function X in kernel calls probe but gets preempted.
> >>> 4. module removes probe
> >>> 5. module unistalled
> >>> 6. function X in kernel continues to run probe but probe no longer
> >>> exists --- Oops!
> >> Agreed, if mcount doesn't disable preemption, this will happen.
> >
> > And it does not.
>
> I think the preemption is disabled in not register_ftrace_function
> but register_ftrace_function_probe, is that wrong?

No that's correct. It's been a while since I worked on the probe code,
so I forgot about it :-)

-- Steve