perf thread: Fix reference count initial state

We should always return from thread__new(), the constructor, with the
object with a reference count of one, so that:

     struct thread *thread = thread__new();
     thread__put(thread);

Will call thread__delete().

If any reference is made to that 'thread' variable, it better use
thread__get(thread) to hold a reference.

We were returning with thread->refcnt set to zero, fix it and some cases
where thread__delete() was being called, which were not a problem
because just one reference was being used, now that we set it to 1, use
thread__put() instead.

Reported-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-4b9mkuk66to4ecckpmpvqx6s@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
index 97f963a3dcb95157f8dcaf3e568d3ac487989608..81a2eb77ba7ff56f7558328437fa32e8345051cd 100644 (file)
--- a/tools/perf/util/intel-pt.c
+++ b/tools/perf/util/intel-pt.c
@@ -1744,7 +1744,7 @@ static void intel_pt_free(struct perf_session *session)
        auxtrace_heap__free(&pt->heap);
        intel_pt_free_events(session);
        session->auxtrace = NULL;
-       thread__delete(pt->unknown_thread);
+       thread__put(pt->unknown_thread);
        free(pt);
 }
 
@@ -2153,7 +2153,7 @@ int intel_pt_process_auxtrace_info(union perf_event *event,
        return 0;
 
 err_delete_thread:
-       thread__delete(pt->unknown_thread);
+       thread__zput(pt->unknown_thread);
 err_free_queues:
        intel_pt_log_disable();
        auxtrace_queues__free(&pt->queues);

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 1407d5107480bb8c082ce2121a59c5fe6c397c76..ad79297c76c87533fb961ac5f8e3daee58f2fda6 100644 (file)
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -352,13 +352,18 @@ static void machine__update_thread_pid(struct machine *machine,
        }
 
        th->mg = map_groups__get(leader->mg);
-
+out_put:
+       thread__put(leader);
        return;
-
 out_err:
        pr_err("Failed to join map groups for %d:%d\n", th->pid_, th->tid);
+       goto out_put;
 }
 
+/*
+ * Caller must eventually drop thread->refcnt returned with a successfull
+ * lookup/new thread inserted.
+ */
 static struct thread *____machine__findnew_thread(struct machine *machine,
                                                  pid_t pid, pid_t tid,
                                                  bool create)
@@ -376,7 +381,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine,
        if (th != NULL) {
                if (th->tid == tid) {
                        machine__update_thread_pid(machine, th, pid);
-                       return th;
+                       return thread__get(th);
                }
 
                machine->last_match = NULL;
@@ -389,7 +394,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine,
                if (th->tid == tid) {
                        machine->last_match = th;
                        machine__update_thread_pid(machine, th, pid);
-                       return th;
+                       return thread__get(th);
                }
 
                if (tid < th->tid)
@@ -417,7 +422,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine,
                if (thread__init_map_groups(th, machine)) {
                        rb_erase_init(&th->rb_node, &machine->threads);
                        RB_CLEAR_NODE(&th->rb_node);
-                       thread__delete(th);
+                       thread__put(th);
                        return NULL;
                }
                /*
@@ -441,7 +446,7 @@ struct thread *machine__findnew_thread(struct machine *machine, pid_t pid,
        struct thread *th;
 
        pthread_rwlock_wrlock(&machine->threads_lock);
-       th = thread__get(__machine__findnew_thread(machine, pid, tid));
+       th = __machine__findnew_thread(machine, pid, tid);
        pthread_rwlock_unlock(&machine->threads_lock);
        return th;
 }
@@ -451,7 +456,7 @@ struct thread *machine__find_thread(struct machine *machine, pid_t pid,
 {
        struct thread *th;
        pthread_rwlock_rdlock(&machine->threads_lock);
-       th =  thread__get(____machine__findnew_thread(machine, pid, tid, false));
+       th =  ____machine__findnew_thread(machine, pid, tid, false);
        pthread_rwlock_unlock(&machine->threads_lock);
        return th;
 }

diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 0a9ae8014729c085ffd2872e2bc13871f79c9908..dfd00c6dad6e68ad3dfde68cbb9e0b16a95f1fee 100644 (file)
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -19,8 +19,10 @@ int thread__init_map_groups(struct thread *thread, struct machine *machine)
                thread->mg = map_groups__new(machine);
        } else {
                leader = __machine__findnew_thread(machine, pid, pid);
-               if (leader)
+               if (leader) {
                        thread->mg = map_groups__get(leader->mg);
+                       thread__put(leader);
+               }
        }
 
        return thread->mg ? 0 : -1;
@@ -53,7 +55,7 @@ struct thread *thread__new(pid_t pid, pid_t tid)
                        goto err_thread;
 
                list_add(&comm->list, &thread->comm_list);
-               atomic_set(&thread->refcnt, 0);
+               atomic_set(&thread->refcnt, 1);
                RB_CLEAR_NODE(&thread->rb_node);
        }
 
@@ -95,6 +97,10 @@ struct thread *thread__get(struct thread *thread)
 void thread__put(struct thread *thread)
 {
        if (thread && atomic_dec_and_test(&thread->refcnt)) {
+               /*
+                * Remove it from the dead_threads list, as last reference
+                * is gone.
+                */
                list_del_init(&thread->node);
                thread__delete(thread);
        }