lkml.org 
[lkml]   [2015]   [Nov]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH perf/core 03/13] perf: Introduce generic refcount APIs with debug feature
    On Wed, Nov 18, 2015 at 03:40:16PM +0900, Masami Hiramatsu wrote:
    > This is a kind of debugging feature for atomic reference counter.
    > The reference counters are widely used in perf tools but not well
    > debugged. It sometimes causes memory leaks but no one has noticed
    > the issue, since it is hard to debug such un-reclaimed objects.
    >
    > This refcnt interface provides fully backtrace debug feature to
    > analyze such issue. User just replace atomic_t ops with refcnt
    > APIs and add refcnt__exit() where the object is released.
    >
    > /* At object initializing */
    > refcnt__init(obj, refcnt); /* <- atomic_set(&obj->refcnt, 1); */
    >
    > /* At object get method */
    > refcnt__get(obj, refcnt); /* <- atomic_inc(&obj->refcnt); */
    >
    > /* At object put method */
    > if (obj && refcnt__put(obj, refcnt)) /* <-atmoic_dec_and_test(&obj->refcnt)*/
    >
    > /* At object releasing */
    > refcnt__exit(obj, refcnt); /* <- Newly added */
    >
    > The debugging feature is enabled when building perf with
    > REFCNT_DEBUG=1. Otherwides it is just translated as normal
    > atomic ops.
    >
    > Debugging binary warns you if it finds leaks when the perf exits.
    > If you give -v (or --verbose) to the perf, it also shows backtrace
    > logs on all refcnt operations of leaked objects.
    >
    > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

    Looks really useful!

    Acked-by: Namhyung Kim <namhyung@kernel.org>

    Just a nitpick below..


    > ---

    [SNIP]
    > +void refcnt_object__record(void *obj, const char *name, int count)
    > +{
    > + struct refcnt_object *ref = refcnt__find_object(obj);
    > + struct refcnt_buffer *buf;
    > +
    > + /* If no entry, allocate new one */
    > + if (!ref) {
    > + ref = malloc(sizeof(*ref));
    > + if (!ref) {
    > + pr_debug("REFCNT: Out of memory for %p (%s)\n",
    > + obj, name);
    > + return;
    > + }
    > + INIT_LIST_HEAD(&ref->list);
    > + INIT_LIST_HEAD(&ref->head);
    > + ref->name = name;
    > + ref->obj = obj;
    > + list_add_tail(&ref->list, &refcnt_root);
    > + }
    > +
    > + buf = malloc(sizeof(*buf));
    > + if (!buf) {
    > + pr_debug("REFCNT: Out of memory for %p (%s)\n", obj, ref->name);
    > + return;
    > + }
    > +
    > + INIT_LIST_HEAD(&buf->list);
    > + buf->count = count;
    > + buf->nr = backtrace(buf->buf, BACKTRACE_SIZE);
    > + list_add_tail(&buf->list, &ref->head);
    > +}
    > +
    > +static void pr_refcnt_buffer(struct refcnt_buffer *buf)
    > +{
    > + char **symbuf;
    > + int i;
    > +
    > + if (!buf)
    > + return;
    > + symbuf = backtrace_symbols(buf->buf, buf->nr);

    It seems you need to check the return value. Maybe we can use
    backtrace_symbols_fd() instead, or just in case of an error.

    Thanks,
    Namhyung


    > + /* Skip the first one because it is always btrace__record */
    > + for (i = 1; i < buf->nr; i++)
    > + pr_debug(" %s\n", symbuf[i]);
    > + free(symbuf);
    > +}
    > +
    > +void refcnt__dump_unreclaimed(void) __attribute__((destructor));
    > +void refcnt__dump_unreclaimed(void)
    > +{
    > + struct refcnt_object *ref, *n;
    > + struct refcnt_buffer *buf;
    > + int i = 0;
    > +
    > + if (list_empty(&refcnt_root))
    > + return;
    > +
    > + pr_warning("REFCNT: BUG: Unreclaimed objects found.\n");
    > + list_for_each_entry_safe(ref, n, &refcnt_root, list) {
    > + pr_debug("==== [%d] ====\nUnreclaimed %s: %p\n", i,
    > + ref->name ? ref->name : "(object)", ref->obj);
    > + list_for_each_entry(buf, &ref->head, list) {
    > + pr_debug("Refcount %s => %d at\n",
    > + buf->count > 0 ? "+1" : "-1",
    > + buf->count > 0 ? buf->count : -buf->count - 1);
    > + pr_refcnt_buffer(buf);
    > + }
    > + __refcnt_object__delete(ref);
    > + i++;
    > + }
    > + pr_warning("REFCNT: Total %d objects are not reclaimed.\n", i);
    > + if (!verbose)
    > + pr_warning(" To see all backtraces, rerun with -v option\n");
    > +}


    \
     
     \ /
      Last update: 2015-11-20 04:01    [W:4.141 / U:0.244 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site