Messages in this thread | | | Date | Thu, 1 Oct 2009 14:50:22 +1000 | From | Paul Mackerras <> | Subject | Possible bug in ftrace_profile_enable_event |
| |
I was looking through kernel/trace/trace_event_profile.c and I saw this code:
static int ftrace_profile_enable_event(struct ftrace_event_call *event) { char *buf; int ret = -ENOMEM;
if (atomic_inc_return(&event->profile_count)) return 0;
if (!total_profile_count++) { buf = (char *)alloc_percpu(profile_buf_t); if (!buf) goto fail_buf;
rcu_assign_pointer(trace_profile_buf, buf);
buf = (char *)alloc_percpu(profile_buf_t); if (!buf) goto fail_buf_nmi;
rcu_assign_pointer(trace_profile_buf_nmi, buf); }
ret = event->profile_enable(); if (!ret) return 0;
kfree(trace_profile_buf_nmi); fail_buf_nmi: kfree(trace_profile_buf); fail_buf: total_profile_count--;
...
So we only allocate trace_profile_buf and trace_profile_buf_nmi if total_profile_count was zero on entry, but if we get an error returned from event->profile_enable(), we free them both unconditionally, regardless of the value of total_profile_count. That seems wrong. Is there a subtle reason why that is the right thing to do?
(Also, is kfree the appropriate counterpart to alloc_percpu?)
Paul.
| |