All of lore.kernel.org
 help / color / mirror / Atom feed
From: Atsushi Tsuji <a-tsuji@bk.jp.nec.com>
To: rostedt@goodmis.org
Cc: Masami Hiramatsu <mhiramat@redhat.com>,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@elte.hu>,
	fweisbec@gmail.com, "Frank Ch. Eigler" <fche@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	paulus@samba.org, systemtap@sources.redhat.com
Subject: Re: [PATCH 2/2] tracing: Export ftrace API for kernel modules
Date: Wed, 16 Sep 2009 15:09:55 +0900	[thread overview]
Message-ID: <4AB08133.4070504@bk.jp.nec.com> (raw)
In-Reply-To: <1253024944.20020.109.camel@gandalf.stny.rr.com>

Steven Rostedt wrote:
> On Tue, 2009-09-15 at 10:01 -0400, Masami Hiramatsu wrote:
>> Steven Rostedt wrote:
> 
>>> Now I know of two ways to fix this.
>>>
>>> 1) The simple way. Up the module ref count so once it registers a
>>> function it can never be disabled. Of course there's the "force module
>>> unload" but people should not do that anyway.
>>>
>>> 2) Create a second hook handler for modules. That is the function caller
>>> for modules will go to a wrapper first. This wrapper could disable
>>> interrupts or grab a lock or something that would prevent a module from
>>> being unloaded as the hooks are being called. Perhaps even disabling
>>> preemption while calling the hooks will be enough (this is not something
>>> I want the normal function caller to do).
>> I think this is better solution.
>> Out of curiously, is disabling preemption so harmful?
> 
> Yes ;-)
> 
> 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,
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?  

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

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

Thanks,
Atsushi




  reply	other threads:[~2009-09-16  6:10 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-15 10:06 [PATCH 2/2] tracing: Export ftrace API for kernel modules Atsushi Tsuji
2009-09-15 10:11 ` Peter Zijlstra
2009-09-15 13:43 ` Steven Rostedt
2009-09-15 14:01   ` Masami Hiramatsu
2009-09-15 14:29     ` Steven Rostedt
2009-09-16  6:09       ` Atsushi Tsuji [this message]
2009-09-16 13:04         ` Steven Rostedt
2009-09-17  0:42           ` Atsushi Tsuji
2009-09-15 23:17 ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4AB08133.4070504@bk.jp.nec.com \
    --to=a-tsuji@bk.jp.nec.com \
    --cc=fche@redhat.com \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@redhat.com \
    --cc=mingo@elte.hu \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=systemtap@sources.redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.