2018-03-06 03:48:48

by Du, Changbin

[permalink] [raw]
Subject: [PATCH v2 0/2] perf sched map: re-annotate shortname if thread comm changed

From: Changbin Du <[email protected]>

v2:
o add a patch to move thread::shortname to thread_runtime
o add function perf_sched__process_comm() to process PERF_RECORD_COMM event.

Changbin Du (2):
perf sched: move thread::shortname to thread_runtime
perf sched map: re-annotate shortname if thread comm changed

tools/perf/builtin-sched.c | 132 ++++++++++++++++++++++++++++++---------------
tools/perf/util/thread.h | 1 -
2 files changed, 90 insertions(+), 43 deletions(-)

--
2.7.4



2018-03-06 03:49:05

by Du, Changbin

[permalink] [raw]
Subject: [PATCH v2 2/2] perf sched map: re-annotate shortname if thread comm changed

From: Changbin Du <[email protected]>

This is to show the real name of thread that created via fork-exec.
See below example for shortname *A0*.

$ sudo ./perf sched map
*A0 80393.050639 secs A0 => perf:22368
*. A0 80393.050748 secs . => swapper:0
. *. 80393.050887 secs
*B0 . . 80393.052735 secs B0 => rcu_sched:8
*. . . 80393.052743 secs
. *C0 . 80393.056264 secs C0 => kworker/2:1H:287
. *A0 . 80393.056270 secs
. *D0 . 80393.056769 secs D0 => ksoftirqd/2:22
- . *A0 . 80393.056804 secs
+ . *A0 . 80393.056804 secs A0 => pi:22368
. *. . 80393.056854 secs
*B0 . . 80393.060727 secs
...

Cc: Namhyung Kim <[email protected]>
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Changbin Du <[email protected]>

---
v2: add function perf_sched__process_comm() to process PERF_RECORD_COMM event.
---
tools/perf/builtin-sched.c | 37 +++++++++++++++++++++++++++++++++++--
1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 5bfc8d5..7aa0600 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -257,6 +257,7 @@ struct thread_runtime {
u64 migrations;

char shortname[3];
+ bool comm_changed;
};

/* per event run time data */
@@ -1626,7 +1627,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,

timestamp__scnprintf_usec(timestamp, stimestamp, sizeof(stimestamp));
color_fprintf(stdout, color, " %12s secs ", stimestamp);
- if (new_shortname || (verbose > 0 && sched_in->tid)) {
+ if (new_shortname || tr->comm_changed || (verbose > 0 && sched_in->tid)) {
const char *pid_color = color;

if (thread__has_color(sched_in))
@@ -1634,6 +1635,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,

color_fprintf(stdout, pid_color, "%s => %s:%d",
tr->shortname, thread__comm_str(sched_in), sched_in->tid);
+ tr->comm_changed = false;
}

if (sched->map.comp && new_cpu)
@@ -1737,6 +1739,37 @@ static int perf_sched__process_tracepoint_sample(struct perf_tool *tool __maybe_
return err;
}

+static int perf_sched__process_comm(struct perf_tool *tool __maybe_unused,
+ union perf_event *event,
+ struct perf_sample *sample,
+ struct machine *machine)
+{
+ struct thread *thread;
+ struct thread_runtime *tr;
+ int err;
+
+ err = perf_event__process_comm(tool, event, sample, machine);
+ if (err)
+ return err;
+
+ thread = machine__find_thread(machine, sample->pid, sample->tid);
+ if (!thread) {
+ pr_err("Internal error: can't find thread\n");
+ return -1;
+ }
+
+ tr = thread__get_runtime(thread);
+ if (tr == NULL) {
+ thread__put(thread);
+ return -1;
+ }
+
+ tr->comm_changed = true;
+ thread__put(thread);
+
+ return 0;
+}
+
static int perf_sched__read_events(struct perf_sched *sched)
{
const struct perf_evsel_str_handler handlers[] = {
@@ -3306,7 +3339,7 @@ int cmd_sched(int argc, const char **argv)
struct perf_sched sched = {
.tool = {
.sample = perf_sched__process_tracepoint_sample,
- .comm = perf_event__process_comm,
+ .comm = perf_sched__process_comm,
.namespaces = perf_event__process_namespaces,
.lost = perf_event__process_lost,
.fork = perf_sched__process_fork_event,
--
2.7.4


2018-03-06 03:49:18

by Du, Changbin

[permalink] [raw]
Subject: [PATCH v2 1/2] perf sched: move thread::shortname to thread_runtime

From: Changbin Du <[email protected]>

The thread::shortname only used by sched command, so move it
to sched private structure.

Signed-off-by: Changbin Du <[email protected]>
---
tools/perf/builtin-sched.c | 95 +++++++++++++++++++++++++++-------------------
tools/perf/util/thread.h | 1 -
2 files changed, 55 insertions(+), 41 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 83283fe..5bfc8d5 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -255,6 +255,8 @@ struct thread_runtime {

int last_state;
u64 migrations;
+
+ char shortname[3];
};

/* per event run time data */
@@ -897,6 +899,37 @@ struct sort_dimension {
struct list_head list;
};

+/*
+ * handle runtime stats saved per thread
+ */
+static struct thread_runtime *thread__init_runtime(struct thread *thread)
+{
+ struct thread_runtime *r;
+
+ r = zalloc(sizeof(struct thread_runtime));
+ if (!r)
+ return NULL;
+
+ init_stats(&r->run_stats);
+ thread__set_priv(thread, r);
+
+ return r;
+}
+
+static struct thread_runtime *thread__get_runtime(struct thread *thread)
+{
+ struct thread_runtime *tr;
+
+ tr = thread__priv(thread);
+ if (tr == NULL) {
+ tr = thread__init_runtime(thread);
+ if (tr == NULL)
+ pr_debug("Failed to malloc memory for runtime data.\n");
+ }
+
+ return tr;
+}
+
static int
thread_lat_cmp(struct list_head *list, struct work_atoms *l, struct work_atoms *r)
{
@@ -1480,6 +1513,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
{
const u32 next_pid = perf_evsel__intval(evsel, sample, "next_pid");
struct thread *sched_in;
+ struct thread_runtime *tr;
int new_shortname;
u64 timestamp0, timestamp = sample->time;
s64 delta;
@@ -1519,22 +1553,28 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
if (sched_in == NULL)
return -1;

+ tr = thread__get_runtime(sched_in);
+ if (tr == NULL) {
+ thread__put(sched_in);
+ return -1;
+ }
+
sched->curr_thread[this_cpu] = thread__get(sched_in);

printf(" ");

new_shortname = 0;
- if (!sched_in->shortname[0]) {
+ if (!tr->shortname[0]) {
if (!strcmp(thread__comm_str(sched_in), "swapper")) {
/*
* Don't allocate a letter-number for swapper:0
* as a shortname. Instead, we use '.' for it.
*/
- sched_in->shortname[0] = '.';
- sched_in->shortname[1] = ' ';
+ tr->shortname[0] = '.';
+ tr->shortname[1] = ' ';
} else {
- sched_in->shortname[0] = sched->next_shortname1;
- sched_in->shortname[1] = sched->next_shortname2;
+ tr->shortname[0] = sched->next_shortname1;
+ tr->shortname[1] = sched->next_shortname2;

if (sched->next_shortname1 < 'Z') {
sched->next_shortname1++;
@@ -1552,6 +1592,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
for (i = 0; i < cpus_nr; i++) {
int cpu = sched->map.comp ? sched->map.comp_cpus[i] : i;
struct thread *curr_thread = sched->curr_thread[cpu];
+ struct thread_runtime *curr_tr;
const char *pid_color = color;
const char *cpu_color = color;

@@ -1569,9 +1610,14 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
else
color_fprintf(stdout, cpu_color, "*");

- if (sched->curr_thread[cpu])
- color_fprintf(stdout, pid_color, "%2s ", sched->curr_thread[cpu]->shortname);
- else
+ if (sched->curr_thread[cpu]) {
+ curr_tr = thread__get_runtime(sched->curr_thread[cpu]);
+ if (curr_tr == NULL) {
+ thread__put(sched_in);
+ return -1;
+ }
+ color_fprintf(stdout, pid_color, "%2s ", curr_tr->shortname);
+ } else
color_fprintf(stdout, color, " ");
}

@@ -1587,7 +1633,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
pid_color = COLOR_PIDS;

color_fprintf(stdout, pid_color, "%s => %s:%d",
- sched_in->shortname, thread__comm_str(sched_in), sched_in->tid);
+ tr->shortname, thread__comm_str(sched_in), sched_in->tid);
}

if (sched->map.comp && new_cpu)
@@ -2200,37 +2246,6 @@ static void save_idle_callchain(struct idle_thread_runtime *itr,
callchain_cursor__copy(&itr->cursor, &callchain_cursor);
}

-/*
- * handle runtime stats saved per thread
- */
-static struct thread_runtime *thread__init_runtime(struct thread *thread)
-{
- struct thread_runtime *r;
-
- r = zalloc(sizeof(struct thread_runtime));
- if (!r)
- return NULL;
-
- init_stats(&r->run_stats);
- thread__set_priv(thread, r);
-
- return r;
-}
-
-static struct thread_runtime *thread__get_runtime(struct thread *thread)
-{
- struct thread_runtime *tr;
-
- tr = thread__priv(thread);
- if (tr == NULL) {
- tr = thread__init_runtime(thread);
- if (tr == NULL)
- pr_debug("Failed to malloc memory for runtime data.\n");
- }
-
- return tr;
-}
-
static struct thread *timehist_get_thread(struct perf_sched *sched,
struct perf_sample *sample,
struct machine *machine,
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 40cfa36..14d44c3 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -26,7 +26,6 @@ struct thread {
pid_t ppid;
int cpu;
refcount_t refcnt;
- char shortname[3];
bool comm_set;
int comm_len;
bool dead; /* if set thread has exited */
--
2.7.4


2018-03-06 07:54:42

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] perf sched map: re-annotate shortname if thread comm changed

On Tue, Mar 06, 2018 at 11:37:35AM +0800, [email protected] wrote:
> From: Changbin Du <[email protected]>
>
> v2:
> o add a patch to move thread::shortname to thread_runtime
> o add function perf_sched__process_comm() to process PERF_RECORD_COMM event.
>
> Changbin Du (2):
> perf sched: move thread::shortname to thread_runtime
> perf sched map: re-annotate shortname if thread comm changed

Acked-by: Jiri Olsa <[email protected]>

thanks,
jirka

>
> tools/perf/builtin-sched.c | 132 ++++++++++++++++++++++++++++++---------------
> tools/perf/util/thread.h | 1 -
> 2 files changed, 90 insertions(+), 43 deletions(-)
>
> --
> 2.7.4
>

2018-03-06 14:12:36

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] perf sched: move thread::shortname to thread_runtime

Em Tue, Mar 06, 2018 at 11:37:36AM +0800, [email protected] escreveu:
> From: Changbin Du <[email protected]>
>
> The thread::shortname only used by sched command, so move it
> to sched private structure.
>
> Signed-off-by: Changbin Du <[email protected]>
> ---
> tools/perf/builtin-sched.c | 95 +++++++++++++++++++++++++++-------------------
> tools/perf/util/thread.h | 1 -
> 2 files changed, 55 insertions(+), 41 deletions(-)
>
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 83283fe..5bfc8d5 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -255,6 +255,8 @@ struct thread_runtime {
>
> int last_state;
> u64 migrations;
> +
> + char shortname[3];
> };

So, what we have for this struct right now is:

[root@jouet perf]# pahole -C thread_runtime ~/bin/perf
struct thread_runtime {
u64 last_time; /* 0 8 */
u64 dt_run; /* 8 8 */
u64 dt_sleep; /* 16 8 */
u64 dt_iowait; /* 24 8 */
u64 dt_preempt; /* 32 8 */
u64 dt_delay; /* 40 8 */
u64 ready_to_run; /* 48 8 */
struct stats run_stats; /* 56 40 */
/* --- cacheline 1 boundary (64 bytes) was 32 bytes ago --- */
u64 total_run_time; /* 96 8 */
u64 total_sleep_time; /* 104 8 */
u64 total_iowait_time; /* 112 8 */
u64 total_preempt_time; /* 120 8 */
/* --- cacheline 2 boundary (128 bytes) --- */
u64 total_delay_time; /* 128 8 */
int last_state; /* 136 4 */

/* XXX 4 bytes hole, try to pack */

u64 migrations; /* 144 8 */

/* size: 152, cachelines: 3, members: 15 */
/* sum members: 148, holes: 1, sum holes: 4 */
/* last cacheline: 24 bytes */
};
[root@jouet perf]#

So I'll move this shortname member to just before 'migrations', then the
bool you'll add on the second patch to right after 'shortname' so that
this gets optimally packed.

Thanks,

- Arnaldo

> /* per event run time data */
> @@ -897,6 +899,37 @@ struct sort_dimension {
> struct list_head list;
> };
>
> +/*
> + * handle runtime stats saved per thread
> + */
> +static struct thread_runtime *thread__init_runtime(struct thread *thread)
> +{
> + struct thread_runtime *r;
> +
> + r = zalloc(sizeof(struct thread_runtime));
> + if (!r)
> + return NULL;
> +
> + init_stats(&r->run_stats);
> + thread__set_priv(thread, r);
> +
> + return r;
> +}
> +
> +static struct thread_runtime *thread__get_runtime(struct thread *thread)
> +{
> + struct thread_runtime *tr;
> +
> + tr = thread__priv(thread);
> + if (tr == NULL) {
> + tr = thread__init_runtime(thread);
> + if (tr == NULL)
> + pr_debug("Failed to malloc memory for runtime data.\n");
> + }
> +
> + return tr;
> +}
> +
> static int
> thread_lat_cmp(struct list_head *list, struct work_atoms *l, struct work_atoms *r)
> {
> @@ -1480,6 +1513,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
> {
> const u32 next_pid = perf_evsel__intval(evsel, sample, "next_pid");
> struct thread *sched_in;
> + struct thread_runtime *tr;
> int new_shortname;
> u64 timestamp0, timestamp = sample->time;
> s64 delta;
> @@ -1519,22 +1553,28 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
> if (sched_in == NULL)
> return -1;
>
> + tr = thread__get_runtime(sched_in);
> + if (tr == NULL) {
> + thread__put(sched_in);
> + return -1;
> + }
> +
> sched->curr_thread[this_cpu] = thread__get(sched_in);
>
> printf(" ");
>
> new_shortname = 0;
> - if (!sched_in->shortname[0]) {
> + if (!tr->shortname[0]) {
> if (!strcmp(thread__comm_str(sched_in), "swapper")) {
> /*
> * Don't allocate a letter-number for swapper:0
> * as a shortname. Instead, we use '.' for it.
> */
> - sched_in->shortname[0] = '.';
> - sched_in->shortname[1] = ' ';
> + tr->shortname[0] = '.';
> + tr->shortname[1] = ' ';
> } else {
> - sched_in->shortname[0] = sched->next_shortname1;
> - sched_in->shortname[1] = sched->next_shortname2;
> + tr->shortname[0] = sched->next_shortname1;
> + tr->shortname[1] = sched->next_shortname2;
>
> if (sched->next_shortname1 < 'Z') {
> sched->next_shortname1++;
> @@ -1552,6 +1592,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
> for (i = 0; i < cpus_nr; i++) {
> int cpu = sched->map.comp ? sched->map.comp_cpus[i] : i;
> struct thread *curr_thread = sched->curr_thread[cpu];
> + struct thread_runtime *curr_tr;
> const char *pid_color = color;
> const char *cpu_color = color;
>
> @@ -1569,9 +1610,14 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
> else
> color_fprintf(stdout, cpu_color, "*");
>
> - if (sched->curr_thread[cpu])
> - color_fprintf(stdout, pid_color, "%2s ", sched->curr_thread[cpu]->shortname);
> - else
> + if (sched->curr_thread[cpu]) {
> + curr_tr = thread__get_runtime(sched->curr_thread[cpu]);
> + if (curr_tr == NULL) {
> + thread__put(sched_in);
> + return -1;
> + }
> + color_fprintf(stdout, pid_color, "%2s ", curr_tr->shortname);
> + } else
> color_fprintf(stdout, color, " ");
> }
>
> @@ -1587,7 +1633,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
> pid_color = COLOR_PIDS;
>
> color_fprintf(stdout, pid_color, "%s => %s:%d",
> - sched_in->shortname, thread__comm_str(sched_in), sched_in->tid);
> + tr->shortname, thread__comm_str(sched_in), sched_in->tid);
> }
>
> if (sched->map.comp && new_cpu)
> @@ -2200,37 +2246,6 @@ static void save_idle_callchain(struct idle_thread_runtime *itr,
> callchain_cursor__copy(&itr->cursor, &callchain_cursor);
> }
>
> -/*
> - * handle runtime stats saved per thread
> - */
> -static struct thread_runtime *thread__init_runtime(struct thread *thread)
> -{
> - struct thread_runtime *r;
> -
> - r = zalloc(sizeof(struct thread_runtime));
> - if (!r)
> - return NULL;
> -
> - init_stats(&r->run_stats);
> - thread__set_priv(thread, r);
> -
> - return r;
> -}
> -
> -static struct thread_runtime *thread__get_runtime(struct thread *thread)
> -{
> - struct thread_runtime *tr;
> -
> - tr = thread__priv(thread);
> - if (tr == NULL) {
> - tr = thread__init_runtime(thread);
> - if (tr == NULL)
> - pr_debug("Failed to malloc memory for runtime data.\n");
> - }
> -
> - return tr;
> -}
> -
> static struct thread *timehist_get_thread(struct perf_sched *sched,
> struct perf_sample *sample,
> struct machine *machine,
> diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> index 40cfa36..14d44c3 100644
> --- a/tools/perf/util/thread.h
> +++ b/tools/perf/util/thread.h
> @@ -26,7 +26,6 @@ struct thread {
> pid_t ppid;
> int cpu;
> refcount_t refcnt;
> - char shortname[3];
> bool comm_set;
> int comm_len;
> bool dead; /* if set thread has exited */
> --
> 2.7.4

2018-03-06 14:18:21

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] perf sched map: re-annotate shortname if thread comm changed

Em Tue, Mar 06, 2018 at 08:53:02AM +0100, Jiri Olsa escreveu:
> On Tue, Mar 06, 2018 at 11:37:35AM +0800, [email protected] wrote:
> > From: Changbin Du <[email protected]>
> >
> > v2:
> > o add a patch to move thread::shortname to thread_runtime
> > o add function perf_sched__process_comm() to process PERF_RECORD_COMM event.
> >
> > Changbin Du (2):
> > perf sched: move thread::shortname to thread_runtime
> > perf sched map: re-annotate shortname if thread comm changed
>
> Acked-by: Jiri Olsa <[email protected]>

Thanks, applied both, the final layout for 'struct thread_runtime':

[root@jouet perf]# pahole -C thread_runtime ~/bin/perf
struct thread_runtime {
u64 last_time; /* 0 8 */
u64 dt_run; /* 8 8 */
u64 dt_sleep; /* 16 8 */
u64 dt_iowait; /* 24 8 */
u64 dt_preempt; /* 32 8 */
u64 dt_delay; /* 40 8 */
u64 ready_to_run; /* 48 8 */
struct stats run_stats; /* 56 40 */
/* --- cacheline 1 boundary (64 bytes) was 32 bytes ago --- */
u64 total_run_time; /* 96 8 */
u64 total_sleep_time; /* 104 8 */
u64 total_iowait_time; /* 112 8 */
u64 total_preempt_time; /* 120 8 */
/* --- cacheline 2 boundary (128 bytes) --- */
u64 total_delay_time; /* 128 8 */
int last_state; /* 136 4 */
char shortname[3]; /* 140 3 */
_Bool comm_changed; /* 143 1 */
u64 migrations; /* 144 8 */

/* size: 152, cachelines: 3, members: 17 */
/* last cacheline: 24 bytes */
};
[root@jouet perf]#

2018-03-07 02:39:12

by Du, Changbin

[permalink] [raw]
Subject: Re: [PATCH v2 0/2] perf sched map: re-annotate shortname if thread comm changed

On Tue, Mar 06, 2018 at 11:17:07AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 06, 2018 at 08:53:02AM +0100, Jiri Olsa escreveu:
> > On Tue, Mar 06, 2018 at 11:37:35AM +0800, [email protected] wrote:
> > > From: Changbin Du <[email protected]>
> > >
> > > v2:
> > > o add a patch to move thread::shortname to thread_runtime
> > > o add function perf_sched__process_comm() to process PERF_RECORD_COMM event.
> > >
> > > Changbin Du (2):
> > > perf sched: move thread::shortname to thread_runtime
> > > perf sched map: re-annotate shortname if thread comm changed
> >
> > Acked-by: Jiri Olsa <[email protected]>
>
> Thanks, applied both, the final layout for 'struct thread_runtime':
>
> [root@jouet perf]# pahole -C thread_runtime ~/bin/perf
> struct thread_runtime {
> u64 last_time; /* 0 8 */
> u64 dt_run; /* 8 8 */
> u64 dt_sleep; /* 16 8 */
> u64 dt_iowait; /* 24 8 */
> u64 dt_preempt; /* 32 8 */
> u64 dt_delay; /* 40 8 */
> u64 ready_to_run; /* 48 8 */
> struct stats run_stats; /* 56 40 */
> /* --- cacheline 1 boundary (64 bytes) was 32 bytes ago --- */
> u64 total_run_time; /* 96 8 */
> u64 total_sleep_time; /* 104 8 */
> u64 total_iowait_time; /* 112 8 */
> u64 total_preempt_time; /* 120 8 */
> /* --- cacheline 2 boundary (128 bytes) --- */
> u64 total_delay_time; /* 128 8 */
> int last_state; /* 136 4 */
> char shortname[3]; /* 140 3 */
> _Bool comm_changed; /* 143 1 */
> u64 migrations; /* 144 8 */
>
> /* size: 152, cachelines: 3, members: 17 */
> /* last cacheline: 24 bytes */
> };
> [root@jouet perf]#

Hi Arnaldo, thanks for your patient optimization for this!

--
Thanks,
Changbin Du

2018-03-07 02:49:15

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] perf sched: move thread::shortname to thread_runtime

Hi,

On Tue, Mar 06, 2018 at 11:37:36AM +0800, [email protected] wrote:
> From: Changbin Du <[email protected]>
>
> The thread::shortname only used by sched command, so move it
> to sched private structure.
>
> Signed-off-by: Changbin Du <[email protected]>
> ---

[SNIP]
> diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
> index 40cfa36..14d44c3 100644
> --- a/tools/perf/util/thread.h
> +++ b/tools/perf/util/thread.h
> @@ -26,7 +26,6 @@ struct thread {
> pid_t ppid;
> int cpu;
> refcount_t refcnt;
> - char shortname[3];
> bool comm_set;
> int comm_len;
> bool dead; /* if set thread has exited */

I also suggest to move the 'dead' field to pack two bools.

Thanks,
Namhyung

Subject: [tip:perf/core] perf sched: Move thread::shortname to thread_runtime

Commit-ID: 8640da9f4fea88c8fbb44ff63fde4000203cb7d1
Gitweb: https://git.kernel.org/tip/8640da9f4fea88c8fbb44ff63fde4000203cb7d1
Author: Changbin Du <[email protected]>
AuthorDate: Tue, 6 Mar 2018 11:37:36 +0800
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 7 Mar 2018 10:22:26 -0300

perf sched: Move thread::shortname to thread_runtime

The thread::shortname only used by sched command, so move it to sched
private structure.

Signed-off-by: Changbin Du <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-sched.c | 95 +++++++++++++++++++++++++++-------------------
tools/perf/util/thread.h | 1 -
2 files changed, 55 insertions(+), 41 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 83283fedb00f..0a632a6b6228 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -254,6 +254,8 @@ struct thread_runtime {
u64 total_delay_time;

int last_state;
+
+ char shortname[3];
u64 migrations;
};

@@ -897,6 +899,37 @@ struct sort_dimension {
struct list_head list;
};

+/*
+ * handle runtime stats saved per thread
+ */
+static struct thread_runtime *thread__init_runtime(struct thread *thread)
+{
+ struct thread_runtime *r;
+
+ r = zalloc(sizeof(struct thread_runtime));
+ if (!r)
+ return NULL;
+
+ init_stats(&r->run_stats);
+ thread__set_priv(thread, r);
+
+ return r;
+}
+
+static struct thread_runtime *thread__get_runtime(struct thread *thread)
+{
+ struct thread_runtime *tr;
+
+ tr = thread__priv(thread);
+ if (tr == NULL) {
+ tr = thread__init_runtime(thread);
+ if (tr == NULL)
+ pr_debug("Failed to malloc memory for runtime data.\n");
+ }
+
+ return tr;
+}
+
static int
thread_lat_cmp(struct list_head *list, struct work_atoms *l, struct work_atoms *r)
{
@@ -1480,6 +1513,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
{
const u32 next_pid = perf_evsel__intval(evsel, sample, "next_pid");
struct thread *sched_in;
+ struct thread_runtime *tr;
int new_shortname;
u64 timestamp0, timestamp = sample->time;
s64 delta;
@@ -1519,22 +1553,28 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
if (sched_in == NULL)
return -1;

+ tr = thread__get_runtime(sched_in);
+ if (tr == NULL) {
+ thread__put(sched_in);
+ return -1;
+ }
+
sched->curr_thread[this_cpu] = thread__get(sched_in);

printf(" ");

new_shortname = 0;
- if (!sched_in->shortname[0]) {
+ if (!tr->shortname[0]) {
if (!strcmp(thread__comm_str(sched_in), "swapper")) {
/*
* Don't allocate a letter-number for swapper:0
* as a shortname. Instead, we use '.' for it.
*/
- sched_in->shortname[0] = '.';
- sched_in->shortname[1] = ' ';
+ tr->shortname[0] = '.';
+ tr->shortname[1] = ' ';
} else {
- sched_in->shortname[0] = sched->next_shortname1;
- sched_in->shortname[1] = sched->next_shortname2;
+ tr->shortname[0] = sched->next_shortname1;
+ tr->shortname[1] = sched->next_shortname2;

if (sched->next_shortname1 < 'Z') {
sched->next_shortname1++;
@@ -1552,6 +1592,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
for (i = 0; i < cpus_nr; i++) {
int cpu = sched->map.comp ? sched->map.comp_cpus[i] : i;
struct thread *curr_thread = sched->curr_thread[cpu];
+ struct thread_runtime *curr_tr;
const char *pid_color = color;
const char *cpu_color = color;

@@ -1569,9 +1610,14 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
else
color_fprintf(stdout, cpu_color, "*");

- if (sched->curr_thread[cpu])
- color_fprintf(stdout, pid_color, "%2s ", sched->curr_thread[cpu]->shortname);
- else
+ if (sched->curr_thread[cpu]) {
+ curr_tr = thread__get_runtime(sched->curr_thread[cpu]);
+ if (curr_tr == NULL) {
+ thread__put(sched_in);
+ return -1;
+ }
+ color_fprintf(stdout, pid_color, "%2s ", curr_tr->shortname);
+ } else
color_fprintf(stdout, color, " ");
}

@@ -1587,7 +1633,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,
pid_color = COLOR_PIDS;

color_fprintf(stdout, pid_color, "%s => %s:%d",
- sched_in->shortname, thread__comm_str(sched_in), sched_in->tid);
+ tr->shortname, thread__comm_str(sched_in), sched_in->tid);
}

if (sched->map.comp && new_cpu)
@@ -2200,37 +2246,6 @@ static void save_idle_callchain(struct idle_thread_runtime *itr,
callchain_cursor__copy(&itr->cursor, &callchain_cursor);
}

-/*
- * handle runtime stats saved per thread
- */
-static struct thread_runtime *thread__init_runtime(struct thread *thread)
-{
- struct thread_runtime *r;
-
- r = zalloc(sizeof(struct thread_runtime));
- if (!r)
- return NULL;
-
- init_stats(&r->run_stats);
- thread__set_priv(thread, r);
-
- return r;
-}
-
-static struct thread_runtime *thread__get_runtime(struct thread *thread)
-{
- struct thread_runtime *tr;
-
- tr = thread__priv(thread);
- if (tr == NULL) {
- tr = thread__init_runtime(thread);
- if (tr == NULL)
- pr_debug("Failed to malloc memory for runtime data.\n");
- }
-
- return tr;
-}
-
static struct thread *timehist_get_thread(struct perf_sched *sched,
struct perf_sample *sample,
struct machine *machine,
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 40cfa36c022a..14d44c3235b8 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -26,7 +26,6 @@ struct thread {
pid_t ppid;
int cpu;
refcount_t refcnt;
- char shortname[3];
bool comm_set;
int comm_len;
bool dead; /* if set thread has exited */

Subject: [tip:perf/core] perf sched map: Re-annotate shortname if thread comm changed

Commit-ID: 99a3c3a91382a7e5e841a98467a8409b47b540f0
Gitweb: https://git.kernel.org/tip/99a3c3a91382a7e5e841a98467a8409b47b540f0
Author: Changbin Du <[email protected]>
AuthorDate: Tue, 6 Mar 2018 11:37:37 +0800
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 7 Mar 2018 10:22:26 -0300

perf sched map: Re-annotate shortname if thread comm changed

This is to show the real name of thread that created via fork-exec. See
below example for shortname *A0*.

$ sudo ./perf sched map
*A0 80393.050639 secs A0 => perf:22368
*. A0 80393.050748 secs . => swapper:0
. *. 80393.050887 secs
*B0 . . 80393.052735 secs B0 => rcu_sched:8
*. . . 80393.052743 secs
. *C0 . 80393.056264 secs C0 => kworker/2:1H:287
. *A0 . 80393.056270 secs
. *D0 . 80393.056769 secs D0 => ksoftirqd/2:22
- . *A0 . 80393.056804 secs
+ . *A0 . 80393.056804 secs A0 => pi:22368
. *. . 80393.056854 secs
*B0 . . 80393.060727 secs
...

Signed-off-by: Changbin Du <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
[ Optimally pack struct thread_runtime when adding the new bool member ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-sched.c | 38 ++++++++++++++++++++++++++++++++++++--
1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 0a632a6b6228..4dfdee668b0c 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -256,6 +256,8 @@ struct thread_runtime {
int last_state;

char shortname[3];
+ bool comm_changed;
+
u64 migrations;
};

@@ -1626,7 +1628,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,

timestamp__scnprintf_usec(timestamp, stimestamp, sizeof(stimestamp));
color_fprintf(stdout, color, " %12s secs ", stimestamp);
- if (new_shortname || (verbose > 0 && sched_in->tid)) {
+ if (new_shortname || tr->comm_changed || (verbose > 0 && sched_in->tid)) {
const char *pid_color = color;

if (thread__has_color(sched_in))
@@ -1634,6 +1636,7 @@ static int map_switch_event(struct perf_sched *sched, struct perf_evsel *evsel,

color_fprintf(stdout, pid_color, "%s => %s:%d",
tr->shortname, thread__comm_str(sched_in), sched_in->tid);
+ tr->comm_changed = false;
}

if (sched->map.comp && new_cpu)
@@ -1737,6 +1740,37 @@ static int perf_sched__process_tracepoint_sample(struct perf_tool *tool __maybe_
return err;
}

+static int perf_sched__process_comm(struct perf_tool *tool __maybe_unused,
+ union perf_event *event,
+ struct perf_sample *sample,
+ struct machine *machine)
+{
+ struct thread *thread;
+ struct thread_runtime *tr;
+ int err;
+
+ err = perf_event__process_comm(tool, event, sample, machine);
+ if (err)
+ return err;
+
+ thread = machine__find_thread(machine, sample->pid, sample->tid);
+ if (!thread) {
+ pr_err("Internal error: can't find thread\n");
+ return -1;
+ }
+
+ tr = thread__get_runtime(thread);
+ if (tr == NULL) {
+ thread__put(thread);
+ return -1;
+ }
+
+ tr->comm_changed = true;
+ thread__put(thread);
+
+ return 0;
+}
+
static int perf_sched__read_events(struct perf_sched *sched)
{
const struct perf_evsel_str_handler handlers[] = {
@@ -3306,7 +3340,7 @@ int cmd_sched(int argc, const char **argv)
struct perf_sched sched = {
.tool = {
.sample = perf_sched__process_tracepoint_sample,
- .comm = perf_event__process_comm,
+ .comm = perf_sched__process_comm,
.namespaces = perf_event__process_namespaces,
.lost = perf_event__process_lost,
.fork = perf_sched__process_fork_event,