lkml.org 
[lkml]   [2014]   [Dec]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 15/37] perf tools: Introduce machine__find*_thread_time()
On 12/24/14 12:15 AM, Namhyung Kim wrote:
> @@ -61,12 +61,12 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)
> __attribute__ ((noinline))
> static int unwind_thread(struct thread *thread)
> {
> - struct perf_sample sample;
> + struct perf_sample sample = {
> + .time = -1ULL,
> + };

1-liner like the others?



> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 582e011adc92..2cc088d71922 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -431,6 +431,103 @@ struct thread *machine__find_thread(struct machine *machine, pid_t pid,
> return __machine__findnew_thread(machine, pid, tid, false);
> }
>
> +static void machine__remove_thread(struct machine *machine, struct thread *th);

Why is this declaration needed?

> +
> +static struct thread *__machine__findnew_thread_time(struct machine *machine,
> + pid_t pid, pid_t tid,
> + u64 timestamp, bool create)
> +{
> + struct thread *curr, *pos, *new;
> + struct thread *th = NULL;
> + struct rb_node **p;
> + struct rb_node *parent = NULL;
> + bool initial = timestamp == (u64)0;
> +
> + curr = __machine__findnew_thread(machine, pid, tid, initial);

What if create arg is false? Should initial arg also be false?

> + if (curr && timestamp >= curr->start_time)
> + return curr;
> +
> + p = &machine->dead_threads.rb_node;
> + while (*p != NULL) {
> + parent = *p;
> + th = rb_entry(parent, struct thread, rb_node);
> +
> + if (th->tid == tid) {
> + list_for_each_entry(pos, &th->node, node) {
> + if (timestamp >= pos->start_time &&
> + pos->start_time > th->start_time) {
> + th = pos;
> + break;
> + }
> + }
> +
> + if (timestamp >= th->start_time) {
> + machine__update_thread_pid(machine, th, pid);
> + return th;
> + }
> + break;
> + }
> +
> + if (tid < th->tid)
> + p = &(*p)->rb_left;
> + else
> + p = &(*p)->rb_right;
> + }

Can the dead_threads search be put in a separate function --
machine__find_dead_thread?

> +
> + if (!create)
> + return NULL;
> +
> + if (!curr)
> + return __machine__findnew_thread(machine, pid, tid, true);
> +
> + new = thread__new(pid, tid);
> + if (new == NULL)
> + return NULL;
> +
> + new->start_time = timestamp;
> +
> + if (*p) {
> + list_for_each_entry(pos, &th->node, node) {
> + /* sort by time */
> + if (timestamp >= pos->start_time) {
> + th = pos;
> + break;
> + }
> + }
> + list_add_tail(&new->node, &th->node);
> + } else {
> + rb_link_node(&new->rb_node, parent, p);
> + rb_insert_color(&new->rb_node, &machine->dead_threads);
> + }

Why insert this unknown tid on the dead_threads list?

> +
> + /*
> + * We have to initialize map_groups separately
> + * after rb tree is updated.
> + *
> + * The reason is that we call machine__findnew_thread
> + * within thread__init_map_groups to find the thread
> + * leader and that would screwed the rb tree.
> + */
> + if (thread__init_map_groups(new, machine)) {
> + thread__delete(new);
> + return NULL;
> + }
> +
> + return new;
> +}

---8<---

> @@ -1265,6 +1362,16 @@ static void machine__remove_thread(struct machine *machine, struct thread *th)
> pos = rb_entry(parent, struct thread, rb_node);
>
> if (pos->tid == th->tid) {
> + struct thread *old;
> +
> + /* sort by time */
> + list_for_each_entry(old, &pos->node, node) {
> + if (th->start_time >= old->start_time) {
> + pos = old;
> + break;
> + }
> + }
> +

this change seems unrelated to the patch subject --
machine__find*_thread_time.

David


\
 
 \ /
  Last update: 2014-12-27 18:01    [W:0.350 / U:0.344 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site