* Re: ftrace/perf_event leak
2010-09-01 9:38 ` Li Zefan
@ 2010-09-01 9:41 ` Peter Zijlstra
2010-09-01 10:38 ` Avi Kivity
2010-09-09 19:45 ` [tip:perf/core] perf, trace: Fix module leak tip-bot for Li Zefan
2 siblings, 0 replies; 14+ messages in thread
From: Peter Zijlstra @ 2010-09-01 9:41 UTC (permalink / raw)
To: Li Zefan
Cc: Avi Kivity, Ingo Molnar, Frederic Weisbecker, Steven Rostedt,
kvm-devel, Linux Kernel Mailing List
On Wed, 2010-09-01 at 17:38 +0800, Li Zefan wrote:
> ---
> diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
> index 8a2b73f..9d1d1f2 100644
> --- a/kernel/trace/trace_event_perf.c
> +++ b/kernel/trace/trace_event_perf.c
> @@ -161,6 +161,7 @@ void perf_trace_destroy(struct perf_event *p_event)
> }
> }
> out:
> + module_put(tp_event->mod);
> mutex_unlock(&event_mutex);
> }
Ah, indeed, we so that try_module_get() for each reference to the
tracepoint, I guess we also should do the below, in case the
registration fails.
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index 000e6e8..31cc4cb 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -91,6 +91,8 @@ int perf_trace_init(struct perf_event *p_event)
tp_event->class && tp_event->class->reg &&
try_module_get(tp_event->mod)) {
ret = perf_trace_event_init(tp_event, p_event);
+ if (ret)
+ module_put(tp_event->mod);
break;
}
}
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: ftrace/perf_event leak
2010-09-01 9:38 ` Li Zefan
2010-09-01 9:41 ` Peter Zijlstra
@ 2010-09-01 10:38 ` Avi Kivity
2010-09-01 11:02 ` Peter Zijlstra
2010-09-09 19:45 ` [tip:perf/core] perf, trace: Fix module leak tip-bot for Li Zefan
2 siblings, 1 reply; 14+ messages in thread
From: Avi Kivity @ 2010-09-01 10:38 UTC (permalink / raw)
To: Li Zefan
Cc: Peter Zijlstra, Ingo Molnar, Frederic Weisbecker, Steven Rostedt,
kvm-devel, Linux Kernel Mailing List
On 09/01/2010 12:38 PM, Li Zefan wrote:
>
> Then try this:
Tested-by: Avi Kivity <avi@redhat.com>
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: ftrace/perf_event leak
2010-09-01 10:38 ` Avi Kivity
@ 2010-09-01 11:02 ` Peter Zijlstra
2010-09-01 11:07 ` Ingo Molnar
2010-09-01 12:15 ` Frederic Weisbecker
0 siblings, 2 replies; 14+ messages in thread
From: Peter Zijlstra @ 2010-09-01 11:02 UTC (permalink / raw)
To: Avi Kivity
Cc: Li Zefan, Ingo Molnar, Frederic Weisbecker, Steven Rostedt,
kvm-devel, Linux Kernel Mailing List
On Wed, 2010-09-01 at 13:38 +0300, Avi Kivity wrote:
> On 09/01/2010 12:38 PM, Li Zefan wrote:
> >
> > Then try this:
>
> Tested-by: Avi Kivity <avi@redhat.com>
>
Thanks, queued as:
---
Subject: perf, trace: Fix module leak
From: Li Zefan <lizf@cn.fujitsu.com>
Date: Wed Sep 01 12:58:43 CEST 2010
Commit 1c024eca (perf, trace: Optimize tracepoints by using
per-tracepoint-per-cpu hlist to track events) caused a module refcount
leak.
Tested-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <4C7E1F12.8030304@cn.fujitsu.com>
---
kernel/trace/trace_event_perf.c | 3 +++
1 file changed, 3 insertions(+)
Index: linux-2.6/kernel/trace/trace_event_perf.c
===================================================================
--- linux-2.6.orig/kernel/trace/trace_event_perf.c
+++ linux-2.6/kernel/trace/trace_event_perf.c
@@ -91,6 +91,8 @@ int perf_trace_init(struct perf_event *p
tp_event->class && tp_event->class->reg &&
try_module_get(tp_event->mod)) {
ret = perf_trace_event_init(tp_event, p_event);
+ if (ret)
+ module_put(tp_event->mod);
break;
}
}
@@ -147,6 +149,7 @@ void perf_trace_destroy(struct perf_even
}
}
out:
+ module_put(tp_event->mod);
mutex_unlock(&event_mutex);
}
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: ftrace/perf_event leak
2010-09-01 11:02 ` Peter Zijlstra
@ 2010-09-01 11:07 ` Ingo Molnar
2010-09-01 12:15 ` Frederic Weisbecker
1 sibling, 0 replies; 14+ messages in thread
From: Ingo Molnar @ 2010-09-01 11:07 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Avi Kivity, Li Zefan, Frederic Weisbecker, Steven Rostedt,
kvm-devel, Linux Kernel Mailing List
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, 2010-09-01 at 13:38 +0300, Avi Kivity wrote:
> > On 09/01/2010 12:38 PM, Li Zefan wrote:
> > >
> > > Then try this:
> >
> > Tested-by: Avi Kivity <avi@redhat.com>
> >
>
> Thanks, queued as:
>
> ---
> Subject: perf, trace: Fix module leak
> From: Li Zefan <lizf@cn.fujitsu.com>
> Date: Wed Sep 01 12:58:43 CEST 2010
>
> Commit 1c024eca (perf, trace: Optimize tracepoints by using
> per-tracepoint-per-cpu hlist to track events) caused a module refcount
> leak.
>
> Tested-by: Avi Kivity <avi@redhat.com>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> LKML-Reference: <4C7E1F12.8030304@cn.fujitsu.com>
Do we need this for -stable too?
Ingo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: ftrace/perf_event leak
2010-09-01 11:02 ` Peter Zijlstra
2010-09-01 11:07 ` Ingo Molnar
@ 2010-09-01 12:15 ` Frederic Weisbecker
2010-09-01 13:59 ` Steven Rostedt
2010-09-02 1:20 ` Li Zefan
1 sibling, 2 replies; 14+ messages in thread
From: Frederic Weisbecker @ 2010-09-01 12:15 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Avi Kivity, Li Zefan, Ingo Molnar, Steven Rostedt, kvm-devel,
Linux Kernel Mailing List
On Wed, Sep 01, 2010 at 01:02:57PM +0200, Peter Zijlstra wrote:
> On Wed, 2010-09-01 at 13:38 +0300, Avi Kivity wrote:
> > On 09/01/2010 12:38 PM, Li Zefan wrote:
> > >
> > > Then try this:
> >
> > Tested-by: Avi Kivity <avi@redhat.com>
> >
>
> Thanks, queued as:
>
> ---
> Subject: perf, trace: Fix module leak
> From: Li Zefan <lizf@cn.fujitsu.com>
> Date: Wed Sep 01 12:58:43 CEST 2010
>
> Commit 1c024eca (perf, trace: Optimize tracepoints by using
> per-tracepoint-per-cpu hlist to track events) caused a module refcount
> leak.
>
> Tested-by: Avi Kivity <avi@redhat.com>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> LKML-Reference: <4C7E1F12.8030304@cn.fujitsu.com>
> ---
> kernel/trace/trace_event_perf.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> Index: linux-2.6/kernel/trace/trace_event_perf.c
> ===================================================================
> --- linux-2.6.orig/kernel/trace/trace_event_perf.c
> +++ linux-2.6/kernel/trace/trace_event_perf.c
> @@ -91,6 +91,8 @@ int perf_trace_init(struct perf_event *p
> tp_event->class && tp_event->class->reg &&
> try_module_get(tp_event->mod)) {
> ret = perf_trace_event_init(tp_event, p_event);
> + if (ret)
> + module_put(tp_event->mod);
> break;
> }
> }
> @@ -147,6 +149,7 @@ void perf_trace_destroy(struct perf_even
> }
> }
> out:
> + module_put(tp_event->mod);
> mutex_unlock(&event_mutex);
> }
>
>
Thanks for fixing this.
However, can we split this in two patches to ease the backport?
The lack of a module_put() after perf_trace_init() failure is there for a while
(the backport needs to start in 2.6.32).
But the lack of a module_put in the destroy path needs a .35 backport only.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: ftrace/perf_event leak
2010-09-01 12:15 ` Frederic Weisbecker
@ 2010-09-01 13:59 ` Steven Rostedt
2010-09-01 17:32 ` Ingo Molnar
2010-09-02 1:20 ` Li Zefan
1 sibling, 1 reply; 14+ messages in thread
From: Steven Rostedt @ 2010-09-01 13:59 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Peter Zijlstra, Avi Kivity, Li Zefan, Ingo Molnar, kvm-devel,
Linux Kernel Mailing List
On Wed, 2010-09-01 at 14:15 +0200, Frederic Weisbecker wrote:
> Thanks for fixing this.
>
> However, can we split this in two patches to ease the backport?
>
> The lack of a module_put() after perf_trace_init() failure is there for a while
> (the backport needs to start in 2.6.32).
>
> But the lack of a module_put in the destroy path needs a .35 backport only.
I don't think it really needs two patches. Just notify stable (and
Greg KH in particular) about the backport requirements. Greg can handle
it ;)
-- Steve
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: ftrace/perf_event leak
2010-09-01 13:59 ` Steven Rostedt
@ 2010-09-01 17:32 ` Ingo Molnar
0 siblings, 0 replies; 14+ messages in thread
From: Ingo Molnar @ 2010-09-01 17:32 UTC (permalink / raw)
To: Steven Rostedt
Cc: Frederic Weisbecker, Peter Zijlstra, Avi Kivity, Li Zefan,
kvm-devel, Linux Kernel Mailing List
* Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 2010-09-01 at 14:15 +0200, Frederic Weisbecker wrote:
>
> > Thanks for fixing this.
> >
> > However, can we split this in two patches to ease the backport?
> >
> > The lack of a module_put() after perf_trace_init() failure is there for a while
> > (the backport needs to start in 2.6.32).
> >
> > But the lack of a module_put in the destroy path needs a .35 backport only.
>
> I don't think it really needs two patches. Just notify stable (and
> Greg KH in particular) about the backport requirements. Greg can
> handle it ;)
Well, Greg certainly has more than enough to handle, so if there's
different chunks with different -stable vectors then it would be most
helpful to him to split things up!
Manually trying to split up patches is both error-prone and
stress-inducing.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: ftrace/perf_event leak
2010-09-01 12:15 ` Frederic Weisbecker
2010-09-01 13:59 ` Steven Rostedt
@ 2010-09-02 1:20 ` Li Zefan
1 sibling, 0 replies; 14+ messages in thread
From: Li Zefan @ 2010-09-02 1:20 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Peter Zijlstra, Avi Kivity, Ingo Molnar, Steven Rostedt,
kvm-devel, Linux Kernel Mailing List
>> Subject: perf, trace: Fix module leak
>> From: Li Zefan <lizf@cn.fujitsu.com>
>> Date: Wed Sep 01 12:58:43 CEST 2010
>>
>> Commit 1c024eca (perf, trace: Optimize tracepoints by using
>> per-tracepoint-per-cpu hlist to track events) caused a module refcount
>> leak.
>>
>> Tested-by: Avi Kivity <avi@redhat.com>
>> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
>> LKML-Reference: <4C7E1F12.8030304@cn.fujitsu.com>
>> ---
>> kernel/trace/trace_event_perf.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> Index: linux-2.6/kernel/trace/trace_event_perf.c
>> ===================================================================
>> --- linux-2.6.orig/kernel/trace/trace_event_perf.c
>> +++ linux-2.6/kernel/trace/trace_event_perf.c
>> @@ -91,6 +91,8 @@ int perf_trace_init(struct perf_event *p
>> tp_event->class && tp_event->class->reg &&
>> try_module_get(tp_event->mod)) {
>> ret = perf_trace_event_init(tp_event, p_event);
>> + if (ret)
>> + module_put(tp_event->mod);
>> break;
>> }
>> }
>> @@ -147,6 +149,7 @@ void perf_trace_destroy(struct perf_even
>> }
>> }
>> out:
>> + module_put(tp_event->mod);
>> mutex_unlock(&event_mutex);
>> }
>>
>>
>
> Thanks for fixing this.
>
> However, can we split this in two patches to ease the backport?
>
> The lack of a module_put() after perf_trace_init() failure is there for a while
> (the backport needs to start in 2.6.32).
The failure should be a rare case, I don't think this has to be backported?
>
> But the lack of a module_put in the destroy path needs a .35 backport only.
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [tip:perf/core] perf, trace: Fix module leak
2010-09-01 9:38 ` Li Zefan
2010-09-01 9:41 ` Peter Zijlstra
2010-09-01 10:38 ` Avi Kivity
@ 2010-09-09 19:45 ` tip-bot for Li Zefan
2 siblings, 0 replies; 14+ messages in thread
From: tip-bot for Li Zefan @ 2010-09-09 19:45 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, a.p.zijlstra, lizf, tglx, avi, mingo
Commit-ID: 9cb627d5f38830ca19aa0dca52d1d3a633018bf7
Gitweb: http://git.kernel.org/tip/9cb627d5f38830ca19aa0dca52d1d3a633018bf7
Author: Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Wed, 1 Sep 2010 12:58:43 +0200
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Thu, 9 Sep 2010 20:38:51 +0200
perf, trace: Fix module leak
Commit 1c024eca (perf, trace: Optimize tracepoints by using
per-tracepoint-per-cpu hlist to track events) caused a module
refcount leak.
Reported-And-Tested-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <4C7E1F12.8030304@cn.fujitsu.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
kernel/trace/trace_event_perf.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index 000e6e8..31cc4cb 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -91,6 +91,8 @@ int perf_trace_init(struct perf_event *p_event)
tp_event->class && tp_event->class->reg &&
try_module_get(tp_event->mod)) {
ret = perf_trace_event_init(tp_event, p_event);
+ if (ret)
+ module_put(tp_event->mod);
break;
}
}
@@ -146,6 +148,7 @@ void perf_trace_destroy(struct perf_event *p_event)
}
}
out:
+ module_put(tp_event->mod);
mutex_unlock(&event_mutex);
}
^ permalink raw reply related [flat|nested] 14+ messages in thread