(2013/08/01 22:34), Oleg Nesterov wrote:
> Just one off-topic note,
>
>> > @@ -632,7 +635,9 @@ static int release_all_trace_probes(void)
>> >/* TODO: Use batch unregistration */
>> >while (!list_empty(_list)) {
>> >tp = list_entry(probe_list.next, struct trace_probe, list);
>>
On 08/01, Steven Rostedt wrote:
>
> On Thu, 2013-08-01 at 16:33 +0200, Oleg Nesterov wrote:
> > On 08/01, Steven Rostedt wrote:
> > >
> > > On Thu, 2013-08-01 at 15:34 +0200, Oleg Nesterov wrote:
> > > > >
> > > > > __unregister_trace_probe(tp);
> > > > > list_del(>list)
> > > > >
On Thu, 2013-08-01 at 16:33 +0200, Oleg Nesterov wrote:
> On 08/01, Steven Rostedt wrote:
> >
> > On Thu, 2013-08-01 at 15:34 +0200, Oleg Nesterov wrote:
> > > >
> > > > __unregister_trace_probe(tp);
> > > > list_del(>list)
> > > > unregister_probe_event(tp) <-- fails!
> > > >
On 08/01, Steven Rostedt wrote:
>
> On Thu, 2013-08-01 at 15:34 +0200, Oleg Nesterov wrote:
> > >
> > > __unregister_trace_probe(tp);
> > > list_del(>list)
> > > unregister_probe_event(tp) <-- fails!
> > > free_trace_probe(tp)
> >
> > Yes. But again, this doesn't explain why
On Thu, 2013-08-01 at 15:34 +0200, Oleg Nesterov wrote:
> On 07/31, Steven Rostedt wrote:
> >
> > Subject: [PATCH] tracing/kprobes: Fail to unregister if probe event files
> > are
> > in use
> >
> > When a probe is being removed, it cleans up the event files that correspond
> > to the probe. But
On 08/01, Oleg Nesterov wrote:
>
> And just in case. I believe that the patch is fine.
>
> Just one off-topic note,
Forgot to mention,
> > @@ -632,7 +635,9 @@ static int release_all_trace_probes(void)
> > /* TODO: Use batch unregistration */
> > while (!list_empty(_list)) {
> >
On 07/31, Steven Rostedt wrote:
>
> Subject: [PATCH] tracing/kprobes: Fail to unregister if probe event files are
> in use
>
> When a probe is being removed, it cleans up the event files that correspond
> to the probe. But there is a race between writing to one of these files
> and deleting the
On 07/31, Steven Rostedt wrote:
>
> On Wed, 2013-07-31 at 22:40 +0200, Oleg Nesterov wrote:
> >
> > This should be fine. Either event_remove() path takes event_mutex
> > first and then ->write() fails, or ftrace_event_enable_disable()
> > actually disables this even successfully.
>
> Actually I
On 07/31, Steven Rostedt wrote:
On Wed, 2013-07-31 at 22:40 +0200, Oleg Nesterov wrote:
This should be fine. Either event_remove() path takes event_mutex
first and then -write() fails, or ftrace_event_enable_disable()
actually disables this even successfully.
Actually I meant while in
On 07/31, Steven Rostedt wrote:
Subject: [PATCH] tracing/kprobes: Fail to unregister if probe event files are
in use
When a probe is being removed, it cleans up the event files that correspond
to the probe. But there is a race between writing to one of these files
and deleting the probe.
On 08/01, Oleg Nesterov wrote:
And just in case. I believe that the patch is fine.
Just one off-topic note,
Forgot to mention,
@@ -632,7 +635,9 @@ static int release_all_trace_probes(void)
/* TODO: Use batch unregistration */
while (!list_empty(probe_list)) {
tp =
On Thu, 2013-08-01 at 15:34 +0200, Oleg Nesterov wrote:
On 07/31, Steven Rostedt wrote:
Subject: [PATCH] tracing/kprobes: Fail to unregister if probe event files
are
in use
When a probe is being removed, it cleans up the event files that correspond
to the probe. But there is a
On 08/01, Steven Rostedt wrote:
On Thu, 2013-08-01 at 15:34 +0200, Oleg Nesterov wrote:
__unregister_trace_probe(tp);
list_del(tp-list)
unregister_probe_event(tp) -- fails!
free_trace_probe(tp)
Yes. But again, this doesn't explain why unregister_probe_event()-
On Thu, 2013-08-01 at 16:33 +0200, Oleg Nesterov wrote:
On 08/01, Steven Rostedt wrote:
On Thu, 2013-08-01 at 15:34 +0200, Oleg Nesterov wrote:
__unregister_trace_probe(tp);
list_del(tp-list)
unregister_probe_event(tp) -- fails!
free_trace_probe(tp)
Yes. But
On 08/01, Steven Rostedt wrote:
On Thu, 2013-08-01 at 16:33 +0200, Oleg Nesterov wrote:
On 08/01, Steven Rostedt wrote:
On Thu, 2013-08-01 at 15:34 +0200, Oleg Nesterov wrote:
__unregister_trace_probe(tp);
list_del(tp-list)
unregister_probe_event(tp) -- fails!
(2013/08/01 22:34), Oleg Nesterov wrote:
Just one off-topic note,
@@ -632,7 +635,9 @@ static int release_all_trace_probes(void)
/* TODO: Use batch unregistration */
while (!list_empty(probe_list)) {
tp = list_entry(probe_list.next, struct trace_probe, list);
-
(2013/08/01 11:50), Steven Rostedt wrote:
> Here's the new change log, but the same patch. Does this sound ok to you
> guys?
Great! This looks good for me. ;)
Thank you very much!
>
> -- Steve
>
>>From 40c32592668b727cbfcf7b1c0567f581bd62a5e4 Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt
Here's the new change log, but the same patch. Does this sound ok to you
guys?
-- Steve
>From 40c32592668b727cbfcf7b1c0567f581bd62a5e4 Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (Red Hat)"
Date: Wed, 3 Jul 2013 23:33:50 -0400
Subject: [PATCH] tracing/kprobes: Fail to unregister if probe
On Wed, 2013-07-31 at 18:42 -0400, Steven Rostedt wrote:
>
> > This should be fine. Either event_remove() path takes event_mutex
> > first and then ->write() fails, or ftrace_event_enable_disable()
> > actually disables this even successfully.
>
> Actually I meant while in
On Wed, 2013-07-31 at 22:40 +0200, Oleg Nesterov wrote:
> On 07/31, Steven Rostedt wrote:
> >
> > On Wed, 2013-07-03 at 23:33 -0400, Steven Rostedt wrote:
> > > The above will corrupt the kprobe system, as the write to the enable
> > > file will happen after the kprobe was deleted.
> >
> > Oleg,
>
On 07/31, Steven Rostedt wrote:
>
> On Wed, 2013-07-03 at 23:33 -0400, Steven Rostedt wrote:
> > The above will corrupt the kprobe system, as the write to the enable
> > file will happen after the kprobe was deleted.
>
> Oleg,
>
> The above no longer triggers the bug due to your changes. The race
On Wed, 2013-07-03 at 23:33 -0400, Steven Rostedt wrote:
> plain text document attachment
> (0003-tracing-kprobes-Fail-to-unregister-if-probe-event-fi.patch)
> From: "Steven Rostedt (Red Hat)"
>
> When one of the event files is opened, we need to prevent them from
> being removed. Modules do
On Wed, 2013-07-03 at 23:33 -0400, Steven Rostedt wrote:
plain text document attachment
(0003-tracing-kprobes-Fail-to-unregister-if-probe-event-fi.patch)
From: Steven Rostedt (Red Hat) rost...@goodmis.org
When one of the event files is opened, we need to prevent them from
being removed.
On 07/31, Steven Rostedt wrote:
On Wed, 2013-07-03 at 23:33 -0400, Steven Rostedt wrote:
The above will corrupt the kprobe system, as the write to the enable
file will happen after the kprobe was deleted.
Oleg,
The above no longer triggers the bug due to your changes. The race is
much
On Wed, 2013-07-31 at 22:40 +0200, Oleg Nesterov wrote:
On 07/31, Steven Rostedt wrote:
On Wed, 2013-07-03 at 23:33 -0400, Steven Rostedt wrote:
The above will corrupt the kprobe system, as the write to the enable
file will happen after the kprobe was deleted.
Oleg,
The above no
On Wed, 2013-07-31 at 18:42 -0400, Steven Rostedt wrote:
This should be fine. Either event_remove() path takes event_mutex
first and then -write() fails, or ftrace_event_enable_disable()
actually disables this even successfully.
Actually I meant while in unregister_trace_probe(), it
Here's the new change log, but the same patch. Does this sound ok to you
guys?
-- Steve
From 40c32592668b727cbfcf7b1c0567f581bd62a5e4 Mon Sep 17 00:00:00 2001
From: Steven Rostedt (Red Hat) rost...@goodmis.org
Date: Wed, 3 Jul 2013 23:33:50 -0400
Subject: [PATCH] tracing/kprobes: Fail to
(2013/08/01 11:50), Steven Rostedt wrote:
Here's the new change log, but the same patch. Does this sound ok to you
guys?
Great! This looks good for me. ;)
Thank you very much!
-- Steve
From 40c32592668b727cbfcf7b1c0567f581bd62a5e4 Mon Sep 17 00:00:00 2001
From: Steven Rostedt (Red Hat)
(2013/07/04 12:33), Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)"
>
> When one of the event files is opened, we need to prevent them from
> being removed. Modules do with with the module owner set (automated
> from the VFS layer). The ftrace buffer instances have a ref count added
>
(2013/07/04 12:33), Steven Rostedt wrote:
From: Steven Rostedt (Red Hat) rost...@goodmis.org
When one of the event files is opened, we need to prevent them from
being removed. Modules do with with the module owner set (automated
from the VFS layer). The ftrace buffer instances have a ref
On 07/08, Masami Hiramatsu wrote:
>
> (2013/07/06 2:26), Oleg Nesterov wrote:
> (Perhaps, uprobes too.)
Sure, uprobes needs all the same fixes, lets ignore it for now.
> >> A safe way is to wait rcu always right after disable_*probe
> >> in disable_trace_probe. If we have an unused link, we can
On 07/08, Masami Hiramatsu wrote:
(2013/07/06 2:26), Oleg Nesterov wrote:
(Perhaps, uprobes too.)
Sure, uprobes needs all the same fixes, lets ignore it for now.
A safe way is to wait rcu always right after disable_*probe
in disable_trace_probe. If we have an unused link, we can
free it
(2013/07/06 2:26), Oleg Nesterov wrote:
> On 07/05, Masami Hiramatsu wrote:
>>
>> (2013/07/05 3:48), Oleg Nesterov wrote:
>>> On 07/04, Masami Hiramatsu wrote:
Actually disable_kprobe() doesn't ensure to finish the current running
kprobe handlers.
>>>
>>> Yes. in fact
(2013/07/06 2:26), Oleg Nesterov wrote:
On 07/05, Masami Hiramatsu wrote:
(2013/07/05 3:48), Oleg Nesterov wrote:
On 07/04, Masami Hiramatsu wrote:
Actually disable_kprobe() doesn't ensure to finish the current running
kprobe handlers.
Yes. in fact disable_trace_probe(file != NULL) does,
On 07/05, Masami Hiramatsu wrote:
>
> (2013/07/05 3:48), Oleg Nesterov wrote:
> > On 07/04, Masami Hiramatsu wrote:
> >>
> >> Actually disable_kprobe() doesn't ensure to finish the current running
> >> kprobe handlers.
> >
> > Yes. in fact disable_trace_probe(file != NULL) does, but perf doesn't.
On 07/05, Masami Hiramatsu wrote:
(2013/07/05 3:48), Oleg Nesterov wrote:
On 07/04, Masami Hiramatsu wrote:
Actually disable_kprobe() doesn't ensure to finish the current running
kprobe handlers.
Yes. in fact disable_trace_probe(file != NULL) does, but perf doesn't.
Ah, right. we
(2013/07/05 3:48), Oleg Nesterov wrote:
> On 07/04, Masami Hiramatsu wrote:
>>
>> (2013/07/04 12:33), Steven Rostedt wrote:
>>> + /* Will fail if probe is being used by ftrace or perf */
>>> + if (unregister_probe_event(tp))
>>> + return -EBUSY;
>>> +
>>>
On 07/04, Masami Hiramatsu wrote:
>
> (2013/07/04 12:33), Steven Rostedt wrote:
> > + /* Will fail if probe is being used by ftrace or perf */
> > + if (unregister_probe_event(tp))
> > + return -EBUSY;
> > +
> > __unregister_trace_probe(tp);
> > list_del(>list);
> > -
(2013/07/04 12:33), Steven Rostedt wrote:
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 7ed6976..ffcaf42 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -95,7 +95,7 @@ static __kprobes bool trace_probe_is_on_module(struct
>
(2013/07/04 12:33), Steven Rostedt wrote:
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 7ed6976..ffcaf42 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -95,7 +95,7 @@ static __kprobes bool trace_probe_is_on_module(struct
On 07/04, Masami Hiramatsu wrote:
(2013/07/04 12:33), Steven Rostedt wrote:
+ /* Will fail if probe is being used by ftrace or perf */
+ if (unregister_probe_event(tp))
+ return -EBUSY;
+
__unregister_trace_probe(tp);
list_del(tp-list);
-
(2013/07/05 3:48), Oleg Nesterov wrote:
On 07/04, Masami Hiramatsu wrote:
(2013/07/04 12:33), Steven Rostedt wrote:
+ /* Will fail if probe is being used by ftrace or perf */
+ if (unregister_probe_event(tp))
+ return -EBUSY;
+
__unregister_trace_probe(tp);
From: "Steven Rostedt (Red Hat)"
When one of the event files is opened, we need to prevent them from
being removed. Modules do with with the module owner set (automated
from the VFS layer). The ftrace buffer instances have a ref count added
to the trace_array when the enabled file is opened
From: Steven Rostedt (Red Hat) rost...@goodmis.org
When one of the event files is opened, we need to prevent them from
being removed. Modules do with with the module owner set (automated
from the VFS layer). The ftrace buffer instances have a ref count added
to the trace_array when the enabled
44 matches
Mail list logo
