From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
"zhangwei(Jovi)" <jovi.zhangwei@huawei.com>,
Jiri Olsa <jolsa@redhat.com>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
Frederic Weisbecker <fweisbec@gmail.com>,
Ingo Molnar <mingo@redhat.com>,
Andrew Morton <akpm@linux-foundation.org>,
linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 0/4] tracing: fix open/delete fixes
Date: Thu, 18 Jul 2013 17:18:23 +0900 [thread overview]
Message-ID: <51E7A4CF.2050404@hitachi.com> (raw)
In-Reply-To: <20130717144357.GA7358@redhat.com>
(2013/07/17 23:43), Oleg Nesterov wrote:
> On 07/17, Masami Hiramatsu wrote:
>>
>> At a glance, you're trying to change which operation will be
>> failed. Currently, user can not remove an event while someone
>> opens files which related to the event. And this approach
>> changes that the someone can remove the event even if the
>> files are opened (and read/write operation will be failed).
>> Am I understand correctly?
>
> Yes.
>
> Once again, I am still not sure and I am asking for your review.
OK,
> But to me this looks much better. To simplify the discussion, lets
> consider ftrace_enable_fops in particular.
>
> - Why should .open() block rmdir or unregister_uprobe_event?
Because it is opened and under preparing for use. :)
But, yeah, if we expect there is only one user using
ftrace, accessing the removing event file is meaningless.
It should be failed.
> - Why do we need .open() at all? Whatever it can do to
> validate file/call/etc, .read/write can do the same.
Currently, just for preparing and reserving.
> - If we kill .open/release, we do not need the nontrivial
> refcounting. Everything becomes simple, no need to keep
> the state "in between".
That also means to refrain checking existence under locking mutex
in all operations. And we have to check it, which I actually concern.
refcounting is not so small and itself is complex, but it just
needs to inc/dec on open/close.
> We need event_mutex anyway (and note that other f_op's can
> also rely on other locks taken by trace_remove_event_call),
> the validation degrades to the trivial != NULL check.
>
> - This also simplifies trace_remove_event_call() paths, we
> know that once it takes event_mutex nobody can play with
> ftrace_event_file/ftrace_event_call we are going to free.
Hmm, it seems that we can remove only refcount check, or more?
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
next prev parent reply other threads:[~2013-07-18 8:18 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-16 18:56 [RFC PATCH 0/4] tracing: fix open/delete fixes Oleg Nesterov
2013-07-16 18:57 ` [RFC PATCH 1/4] tracing: Change remove_event_from_tracers() to clear d_subdirs->i_private Oleg Nesterov
2013-07-16 18:57 ` [RFC PATCH 2/4] tracing: Turn "id"->i_private into call->event.type Oleg Nesterov
2013-07-16 19:49 ` Steven Rostedt
2013-07-17 19:39 ` Oleg Nesterov
2013-07-16 18:57 ` [RFC PATCH 3/4] tracing: Kill tracing_open/release_generic_file Oleg Nesterov
2013-07-16 19:51 ` Steven Rostedt
2013-07-17 19:19 ` Oleg Nesterov
2013-07-18 10:56 ` Masami Hiramatsu
2013-07-18 14:55 ` Oleg Nesterov
2013-07-16 18:57 ` [RFC PATCH 4/4] tracing: Change ftrace_event_filter_fops to rely on event_mutex/i_private Oleg Nesterov
2013-07-17 2:36 ` [RFC PATCH 0/4] tracing: fix open/delete fixes Masami Hiramatsu
2013-07-17 14:43 ` Oleg Nesterov
2013-07-18 8:18 ` Masami Hiramatsu [this message]
2013-07-18 15:27 ` Oleg Nesterov
2013-07-17 19:30 ` [RFC PATCH 5/4] tracing: Simplify the ftrace_event_field iteration in f_next/f_show Oleg Nesterov
2013-07-17 19:37 ` Oleg Nesterov
2013-07-17 19:30 ` [RFC PATCH 6/4] tracing: Change f_start() to verify i_private under event_mutex Oleg Nesterov
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=51E7A4CF.2050404@hitachi.com \
--to=masami.hiramatsu.pt@hitachi.com \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@ghostprotocols.net \
--cc=akpm@linux-foundation.org \
--cc=fweisbec@gmail.com \
--cc=jolsa@redhat.com \
--cc=jovi.zhangwei@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=oleg@redhat.com \
--cc=rostedt@goodmis.org \
--cc=srikar@linux.vnet.ibm.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.