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: default avatarMasami 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.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent b6847d2c
...@@ -1744,7 +1744,7 @@ static void intel_pt_free(struct perf_session *session) ...@@ -1744,7 +1744,7 @@ static void intel_pt_free(struct perf_session *session)
auxtrace_heap__free(&pt->heap); auxtrace_heap__free(&pt->heap);
intel_pt_free_events(session); intel_pt_free_events(session);
session->auxtrace = NULL; session->auxtrace = NULL;
thread__delete(pt->unknown_thread); thread__put(pt->unknown_thread);
free(pt); free(pt);
} }
...@@ -2153,7 +2153,7 @@ int intel_pt_process_auxtrace_info(union perf_event *event, ...@@ -2153,7 +2153,7 @@ int intel_pt_process_auxtrace_info(union perf_event *event,
return 0; return 0;
err_delete_thread: err_delete_thread:
thread__delete(pt->unknown_thread); thread__zput(pt->unknown_thread);
err_free_queues: err_free_queues:
intel_pt_log_disable(); intel_pt_log_disable();
auxtrace_queues__free(&pt->queues); auxtrace_queues__free(&pt->queues);
......
...@@ -352,13 +352,18 @@ static void machine__update_thread_pid(struct machine *machine, ...@@ -352,13 +352,18 @@ static void machine__update_thread_pid(struct machine *machine,
} }
th->mg = map_groups__get(leader->mg); th->mg = map_groups__get(leader->mg);
out_put:
thread__put(leader);
return; return;
out_err: out_err:
pr_err("Failed to join map groups for %d:%d\n", th->pid_, th->tid); 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, static struct thread *____machine__findnew_thread(struct machine *machine,
pid_t pid, pid_t tid, pid_t pid, pid_t tid,
bool create) bool create)
...@@ -376,7 +381,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine, ...@@ -376,7 +381,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine,
if (th != NULL) { if (th != NULL) {
if (th->tid == tid) { if (th->tid == tid) {
machine__update_thread_pid(machine, th, pid); machine__update_thread_pid(machine, th, pid);
return th; return thread__get(th);
} }
machine->last_match = NULL; machine->last_match = NULL;
...@@ -389,7 +394,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine, ...@@ -389,7 +394,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine,
if (th->tid == tid) { if (th->tid == tid) {
machine->last_match = th; machine->last_match = th;
machine__update_thread_pid(machine, th, pid); machine__update_thread_pid(machine, th, pid);
return th; return thread__get(th);
} }
if (tid < th->tid) if (tid < th->tid)
...@@ -417,7 +422,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine, ...@@ -417,7 +422,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine,
if (thread__init_map_groups(th, machine)) { if (thread__init_map_groups(th, machine)) {
rb_erase_init(&th->rb_node, &machine->threads); rb_erase_init(&th->rb_node, &machine->threads);
RB_CLEAR_NODE(&th->rb_node); RB_CLEAR_NODE(&th->rb_node);
thread__delete(th); thread__put(th);
return NULL; return NULL;
} }
/* /*
...@@ -441,7 +446,7 @@ struct thread *machine__findnew_thread(struct machine *machine, pid_t pid, ...@@ -441,7 +446,7 @@ struct thread *machine__findnew_thread(struct machine *machine, pid_t pid,
struct thread *th; struct thread *th;
pthread_rwlock_wrlock(&machine->threads_lock); 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); pthread_rwlock_unlock(&machine->threads_lock);
return th; return th;
} }
...@@ -451,7 +456,7 @@ struct thread *machine__find_thread(struct machine *machine, pid_t pid, ...@@ -451,7 +456,7 @@ struct thread *machine__find_thread(struct machine *machine, pid_t pid,
{ {
struct thread *th; struct thread *th;
pthread_rwlock_rdlock(&machine->threads_lock); 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); pthread_rwlock_unlock(&machine->threads_lock);
return th; return th;
} }
......
...@@ -19,8 +19,10 @@ int thread__init_map_groups(struct thread *thread, struct machine *machine) ...@@ -19,8 +19,10 @@ int thread__init_map_groups(struct thread *thread, struct machine *machine)
thread->mg = map_groups__new(machine); thread->mg = map_groups__new(machine);
} else { } else {
leader = __machine__findnew_thread(machine, pid, pid); leader = __machine__findnew_thread(machine, pid, pid);
if (leader) if (leader) {
thread->mg = map_groups__get(leader->mg); thread->mg = map_groups__get(leader->mg);
thread__put(leader);
}
} }
return thread->mg ? 0 : -1; return thread->mg ? 0 : -1;
...@@ -53,7 +55,7 @@ struct thread *thread__new(pid_t pid, pid_t tid) ...@@ -53,7 +55,7 @@ struct thread *thread__new(pid_t pid, pid_t tid)
goto err_thread; goto err_thread;
list_add(&comm->list, &thread->comm_list); list_add(&comm->list, &thread->comm_list);
atomic_set(&thread->refcnt, 0); atomic_set(&thread->refcnt, 1);
RB_CLEAR_NODE(&thread->rb_node); RB_CLEAR_NODE(&thread->rb_node);
} }
...@@ -95,6 +97,10 @@ struct thread *thread__get(struct thread *thread) ...@@ -95,6 +97,10 @@ struct thread *thread__get(struct thread *thread)
void thread__put(struct thread *thread) void thread__put(struct thread *thread)
{ {
if (thread && atomic_dec_and_test(&thread->refcnt)) { 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); list_del_init(&thread->node);
thread__delete(thread); thread__delete(thread);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment