On Fri, Nov 20, 2015 at 04:12:06AM +0000, 平松雅巳 / HIRAMATU,MASAMI wrote: > From: Namhyung Kim [mailto:namhy...@kernel.org] > > > >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...@hitachi.com> > > > >Looks really useful! > > > >Acked-by: Namhyung Kim <namhy...@kernel.org> > > > >Just a nitpick below.. > > Thanks! > > >> --- > > > >[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. > > Yeah, it should be checked and in that case we can fall back to > backtrace_symbols_fd(as the last resort), but I don’t like > backtrace_symbols_fd replacing because it doesn't allow us to > indent the backtrace result.
OK, I think we need to improve the backtrace code in general. I'll send a related patch soon. 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"); > >> +} -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/