lkml.org 
[lkml]   [2009]   [Oct]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: Possible bug in ftrace_profile_enable_event
    From
    2009/10/1 Paul Mackerras <paulus@samba.org>:
    > 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?


    Oh right. Good catch.
    I'll fix that soon.


    >
    > (Also, is kfree the appropriate counterpart to alloc_percpu?)

    No indeed.
    That said it looks harmless for now because percpu_free seem to just
    roughly wrap kfree. But
    its implementation may change later, so I'll fix that too.

    Thanks.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2009-10-02 14:17    [W:2.317 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site