Re: WARNING in tracepoint_probe_register_prio (2)

From: Mathieu Desnoyers
Date: Thu Mar 15 2018 - 07:52:30 EST


----- On Mar 15, 2018, at 5:19 AM, Jiri Olsa jolsa@xxxxxxxxxx wrote:

> On Thu, Mar 15, 2018 at 09:31:25AM +0100, Peter Zijlstra wrote:
>> On Wed, Mar 14, 2018 at 05:37:46PM -0400, Steven Rostedt wrote:
>> > On Tue, 13 Mar 2018 11:29:51 -0400 (EDT)
>> > Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> wrote:
>> >
>> > > Here is a WARN_ON() splat in tracepoint.c, which I suspect is caused
>> > > by perf trying to register the same probe twice to the tracepoint API.
>> > > We got another splat on unregister too, which I will forward in a
>> > > separate email.
>> > >
>> > > Thoughts ?
>> >
>> > Yes, it looks like it's perf not accounting for registered events
>> > properly.
>> >
>> > Peter?
>>
>> I've not yet managed to reproduce, but if you look at the provided
>> repro.c file, you'll see it opens two _different_ events.
>
> from the log it looks like they inject the slab error,
> and the allocation fails.. looks like we need to change
> the WARN to skip ENOMEM.. something like below?

Oh, I missed this important point. Then we should only
warn if !-ENOMEM for both tracepoint_add_func and tracepoint_remove_func,
because each performs memory allocation under the hood.
Like the following:

--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -207,7 +207,7 @@ static int tracepoint_add_func(struct tracepoint *tp,
lockdep_is_held(&tracepoints_mutex));
old = func_add(&tp_funcs, func, prio);
if (IS_ERR(old)) {
- WARN_ON_ONCE(1);
+ WARN_ON_ONCE(PTR_ERR(old) != -ENOMEM);
return PTR_ERR(old);
}

@@ -239,7 +239,7 @@ static int tracepoint_remove_func(struct tracepoint *tp,
lockdep_is_held(&tracepoints_mutex));
old = func_remove(&tp_funcs, func);
if (IS_ERR(old)) {
- WARN_ON_ONCE(1);
+ WARN_ON_ONCE(PTR_ERR(old) != -ENOMEM);
return PTR_ERR(old);
}

--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com