linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] perf sched map: re-annotate shortname if thread comm changed
@ 2018-03-06  3:37 changbin.du
  2018-03-06  3:37 ` [PATCH v2 1/2] perf sched: move thread::shortname to thread_runtime changbin.du
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: changbin.du @ 2018-03-06  3:37 UTC (permalink / raw)
  To: acme, jolsa
  Cc: peterz, mingo, namhyung, linux-kernel, linux-perf-users,
	Changbin Du

From: Changbin Du <changbin.du@intel.com>

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/2] perf sched: move thread::shortname to thread_runtime
  2018-03-06  3:37 [PATCH v2 0/2] perf sched map: re-annotate shortname if thread comm changed changbin.du
@ 2018-03-06  3:37 ` changbin.du
  2018-03-06 14:09   ` Arnaldo Carvalho de Melo
  2018-03-07  2:44   ` Namhyung Kim
  2018-03-06  3:37 ` [PATCH v2 2/2] perf sched map: re-annotate shortname if thread comm changed changbin.du
  2018-03-06  7:53 ` [PATCH v2 0/2] " Jiri Olsa
  2 siblings, 2 replies; 8+ messages in thread
From: changbin.du @ 2018-03-06  3:37 UTC (permalink / raw)
  To: acme, jolsa
  Cc: peterz, mingo, namhyung, linux-kernel, linux-perf-users,
	Changbin Du

From: Changbin Du <changbin.du@intel.com>

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

Signed-off-by: Changbin Du <changbin.du@intel.com>
---
 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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/2] perf sched map: re-annotate shortname if thread comm changed
  2018-03-06  3:37 [PATCH v2 0/2] perf sched map: re-annotate shortname if thread comm changed changbin.du
  2018-03-06  3:37 ` [PATCH v2 1/2] perf sched: move thread::shortname to thread_runtime changbin.du
@ 2018-03-06  3:37 ` changbin.du
  2018-03-06  7:53 ` [PATCH v2 0/2] " Jiri Olsa
  2 siblings, 0 replies; 8+ messages in thread
From: changbin.du @ 2018-03-06  3:37 UTC (permalink / raw)
  To: acme, jolsa
  Cc: peterz, mingo, namhyung, linux-kernel, linux-perf-users,
	Changbin Du

From: Changbin Du <changbin.du@intel.com>

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 <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Changbin Du <changbin.du@intel.com>

---
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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/2] perf sched map: re-annotate shortname if thread comm changed
  2018-03-06  3:37 [PATCH v2 0/2] perf sched map: re-annotate shortname if thread comm changed changbin.du
  2018-03-06  3:37 ` [PATCH v2 1/2] perf sched: move thread::shortname to thread_runtime changbin.du
  2018-03-06  3:37 ` [PATCH v2 2/2] perf sched map: re-annotate shortname if thread comm changed changbin.du
@ 2018-03-06  7:53 ` Jiri Olsa
  2018-03-06 14:17   ` Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2018-03-06  7:53 UTC (permalink / raw)
  To: changbin.du; +Cc: acme, peterz, mingo, namhyung, linux-kernel, linux-perf-users

On Tue, Mar 06, 2018 at 11:37:35AM +0800, changbin.du@intel.com wrote:
> From: Changbin Du <changbin.du@intel.com>
> 
> 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 <jolsa@kernel.org>

thanks,
jirka

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] perf sched: move thread::shortname to thread_runtime
  2018-03-06  3:37 ` [PATCH v2 1/2] perf sched: move thread::shortname to thread_runtime changbin.du
@ 2018-03-06 14:09   ` Arnaldo Carvalho de Melo
  2018-03-07  2:44   ` Namhyung Kim
  1 sibling, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-03-06 14:09 UTC (permalink / raw)
  To: changbin.du
  Cc: jolsa, peterz, mingo, namhyung, linux-kernel, linux-perf-users

Em Tue, Mar 06, 2018 at 11:37:36AM +0800, changbin.du@intel.com escreveu:
> From: Changbin Du <changbin.du@intel.com>
> 
> The thread::shortname only used by sched command, so move it
> to sched private structure.
> 
> Signed-off-by: Changbin Du <changbin.du@intel.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/2] perf sched map: re-annotate shortname if thread comm changed
  2018-03-06  7:53 ` [PATCH v2 0/2] " Jiri Olsa
@ 2018-03-06 14:17   ` Arnaldo Carvalho de Melo
  2018-03-07  2:28     ` Du, Changbin
  0 siblings, 1 reply; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-03-06 14:17 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: changbin.du, peterz, mingo, namhyung, linux-kernel,
	linux-perf-users

Em Tue, Mar 06, 2018 at 08:53:02AM +0100, Jiri Olsa escreveu:
> On Tue, Mar 06, 2018 at 11:37:35AM +0800, changbin.du@intel.com wrote:
> > From: Changbin Du <changbin.du@intel.com>
> > 
> > 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 <jolsa@kernel.org>

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]#

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/2] perf sched map: re-annotate shortname if thread comm changed
  2018-03-06 14:17   ` Arnaldo Carvalho de Melo
@ 2018-03-07  2:28     ` Du, Changbin
  0 siblings, 0 replies; 8+ messages in thread
From: Du, Changbin @ 2018-03-07  2:28 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, changbin.du, peterz, mingo, namhyung, linux-kernel,
	linux-perf-users

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, changbin.du@intel.com wrote:
> > > From: Changbin Du <changbin.du@intel.com>
> > > 
> > > 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 <jolsa@kernel.org>
> 
> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/2] perf sched: move thread::shortname to thread_runtime
  2018-03-06  3:37 ` [PATCH v2 1/2] perf sched: move thread::shortname to thread_runtime changbin.du
  2018-03-06 14:09   ` Arnaldo Carvalho de Melo
@ 2018-03-07  2:44   ` Namhyung Kim
  1 sibling, 0 replies; 8+ messages in thread
From: Namhyung Kim @ 2018-03-07  2:44 UTC (permalink / raw)
  To: changbin.du
  Cc: acme, jolsa, peterz, mingo, linux-kernel, linux-perf-users,
	kernel-team

Hi,

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

[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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-03-07  2:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-06  3:37 [PATCH v2 0/2] perf sched map: re-annotate shortname if thread comm changed changbin.du
2018-03-06  3:37 ` [PATCH v2 1/2] perf sched: move thread::shortname to thread_runtime changbin.du
2018-03-06 14:09   ` Arnaldo Carvalho de Melo
2018-03-07  2:44   ` Namhyung Kim
2018-03-06  3:37 ` [PATCH v2 2/2] perf sched map: re-annotate shortname if thread comm changed changbin.du
2018-03-06  7:53 ` [PATCH v2 0/2] " Jiri Olsa
2018-03-06 14:17   ` Arnaldo Carvalho de Melo
2018-03-07  2:28     ` Du, Changbin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).