All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 1/2] tools/perf: Include global and local variants for p_stage_cyc sort key
@ 2021-12-03  2:20 ` Athira Rajeev
  0 siblings, 0 replies; 12+ messages in thread
From: Athira Rajeev @ 2021-12-03  2:20 UTC (permalink / raw)
  To: acme, jolsa
  Cc: mpe, linux-perf-users, linuxppc-dev, maddy, rnsastry, kjain, namhyung

Sort key p_stage_cyc is used to present the latency
cycles spend in pipeline stages. perf tool has local
p_stage_cyc sort key to display this info. There is no
global variant available for this sort key. local variant
shows latency in a sinlge sample, whereas, global value
will be useful to present the total latency (sum of
latencies) in the hist entry. It represents latency
number multiplied by the number of samples.

Add global (p_stage_cyc) and local variant
(local_p_stage_cyc) for this sort key. Use the
local_p_stage_cyc as default option for "mem" sort mode.
Also add this to list of dynamic sort keys and made the
"dynamic_headers" and "arch_specific_sort_keys" as static.

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Reported-by: Namhyung Kim <namhyung@kernel.org>
---
Changelog:
v1 -> v2:
 Addressed review comments from Jiri by making the
 "dynamic_headers" and "arch_specific_sort_keys"
 as static.

 tools/perf/util/hist.c |  4 +++-
 tools/perf/util/hist.h |  3 ++-
 tools/perf/util/sort.c | 34 +++++++++++++++++++++++++---------
 tools/perf/util/sort.h |  3 ++-
 4 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b776465e04ef..0a8033b09e28 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -211,7 +211,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 	hists__new_col_len(hists, HISTC_MEM_BLOCKED, 10);
 	hists__new_col_len(hists, HISTC_LOCAL_INS_LAT, 13);
 	hists__new_col_len(hists, HISTC_GLOBAL_INS_LAT, 13);
-	hists__new_col_len(hists, HISTC_P_STAGE_CYC, 13);
+	hists__new_col_len(hists, HISTC_LOCAL_P_STAGE_CYC, 13);
+	hists__new_col_len(hists, HISTC_GLOBAL_P_STAGE_CYC, 13);
+
 	if (symbol_conf.nanosecs)
 		hists__new_col_len(hists, HISTC_TIME, 16);
 	else
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 621f35ae1efa..2a15e22fb89c 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -75,7 +75,8 @@ enum hist_column {
 	HISTC_MEM_BLOCKED,
 	HISTC_LOCAL_INS_LAT,
 	HISTC_GLOBAL_INS_LAT,
-	HISTC_P_STAGE_CYC,
+	HISTC_LOCAL_P_STAGE_CYC,
+	HISTC_GLOBAL_P_STAGE_CYC,
 	HISTC_NR_COLS, /* Last entry */
 };
 
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index a111065b484e..e417e47f51b9 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -37,7 +37,7 @@ const char	default_parent_pattern[] = "^sys_|^do_page_fault";
 const char	*parent_pattern = default_parent_pattern;
 const char	*default_sort_order = "comm,dso,symbol";
 const char	default_branch_sort_order[] = "comm,dso_from,symbol_from,symbol_to,cycles";
-const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,p_stage_cyc";
+const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,local_p_stage_cyc";
 const char	default_top_sort_order[] = "dso,symbol";
 const char	default_diff_sort_order[] = "dso,symbol";
 const char	default_tracepoint_sort_order[] = "trace";
@@ -46,8 +46,8 @@ const char	*field_order;
 regex_t		ignore_callees_regex;
 int		have_ignore_callees = 0;
 enum sort_mode	sort__mode = SORT_MODE__NORMAL;
-const char	*dynamic_headers[] = {"local_ins_lat", "p_stage_cyc"};
-const char	*arch_specific_sort_keys[] = {"p_stage_cyc"};
+static const char *const dynamic_headers[] = {"local_ins_lat", "ins_lat", "local_p_stage_cyc", "p_stage_cyc"};
+static const char *const arch_specific_sort_keys[] = {"local_p_stage_cyc", "p_stage_cyc"};
 
 /*
  * Replaces all occurrences of a char used with the:
@@ -1392,22 +1392,37 @@ struct sort_entry sort_global_ins_lat = {
 };
 
 static int64_t
-sort__global_p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
+sort__p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
 {
 	return left->p_stage_cyc - right->p_stage_cyc;
 }
 
+static int hist_entry__global_p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
+					size_t size, unsigned int width)
+{
+	return repsep_snprintf(bf, size, "%-*u", width,
+			he->p_stage_cyc * he->stat.nr_events);
+}
+
+
 static int hist_entry__p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
 					size_t size, unsigned int width)
 {
 	return repsep_snprintf(bf, size, "%-*u", width, he->p_stage_cyc);
 }
 
-struct sort_entry sort_p_stage_cyc = {
-	.se_header      = "Pipeline Stage Cycle",
-	.se_cmp         = sort__global_p_stage_cyc_cmp,
+struct sort_entry sort_local_p_stage_cyc = {
+	.se_header      = "Local Pipeline Stage Cycle",
+	.se_cmp         = sort__p_stage_cyc_cmp,
 	.se_snprintf	= hist_entry__p_stage_cyc_snprintf,
-	.se_width_idx	= HISTC_P_STAGE_CYC,
+	.se_width_idx	= HISTC_LOCAL_P_STAGE_CYC,
+};
+
+struct sort_entry sort_global_p_stage_cyc = {
+	.se_header      = "Pipeline Stage Cycle",
+	.se_cmp         = sort__p_stage_cyc_cmp,
+	.se_snprintf    = hist_entry__global_p_stage_cyc_snprintf,
+	.se_width_idx   = HISTC_GLOBAL_P_STAGE_CYC,
 };
 
 struct sort_entry sort_mem_daddr_sym = {
@@ -1858,7 +1873,8 @@ static struct sort_dimension common_sort_dimensions[] = {
 	DIM(SORT_CODE_PAGE_SIZE, "code_page_size", sort_code_page_size),
 	DIM(SORT_LOCAL_INS_LAT, "local_ins_lat", sort_local_ins_lat),
 	DIM(SORT_GLOBAL_INS_LAT, "ins_lat", sort_global_ins_lat),
-	DIM(SORT_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_p_stage_cyc),
+	DIM(SORT_LOCAL_PIPELINE_STAGE_CYC, "local_p_stage_cyc", sort_local_p_stage_cyc),
+	DIM(SORT_GLOBAL_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_global_p_stage_cyc),
 };
 
 #undef DIM
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 7b7145501933..f994261888e1 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -235,7 +235,8 @@ enum sort_type {
 	SORT_CODE_PAGE_SIZE,
 	SORT_LOCAL_INS_LAT,
 	SORT_GLOBAL_INS_LAT,
-	SORT_PIPELINE_STAGE_CYC,
+	SORT_LOCAL_PIPELINE_STAGE_CYC,
+	SORT_GLOBAL_PIPELINE_STAGE_CYC,
 
 	/* branch stack specific sort keys */
 	__SORT_BRANCH_STACK,
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH V2 1/2] tools/perf: Include global and local variants for p_stage_cyc sort key
@ 2021-12-03  2:20 ` Athira Rajeev
  0 siblings, 0 replies; 12+ messages in thread
From: Athira Rajeev @ 2021-12-03  2:20 UTC (permalink / raw)
  To: acme, jolsa
  Cc: maddy, rnsastry, linux-perf-users, kjain, namhyung, linuxppc-dev

Sort key p_stage_cyc is used to present the latency
cycles spend in pipeline stages. perf tool has local
p_stage_cyc sort key to display this info. There is no
global variant available for this sort key. local variant
shows latency in a sinlge sample, whereas, global value
will be useful to present the total latency (sum of
latencies) in the hist entry. It represents latency
number multiplied by the number of samples.

Add global (p_stage_cyc) and local variant
(local_p_stage_cyc) for this sort key. Use the
local_p_stage_cyc as default option for "mem" sort mode.
Also add this to list of dynamic sort keys and made the
"dynamic_headers" and "arch_specific_sort_keys" as static.

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Reported-by: Namhyung Kim <namhyung@kernel.org>
---
Changelog:
v1 -> v2:
 Addressed review comments from Jiri by making the
 "dynamic_headers" and "arch_specific_sort_keys"
 as static.

 tools/perf/util/hist.c |  4 +++-
 tools/perf/util/hist.h |  3 ++-
 tools/perf/util/sort.c | 34 +++++++++++++++++++++++++---------
 tools/perf/util/sort.h |  3 ++-
 4 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b776465e04ef..0a8033b09e28 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -211,7 +211,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 	hists__new_col_len(hists, HISTC_MEM_BLOCKED, 10);
 	hists__new_col_len(hists, HISTC_LOCAL_INS_LAT, 13);
 	hists__new_col_len(hists, HISTC_GLOBAL_INS_LAT, 13);
-	hists__new_col_len(hists, HISTC_P_STAGE_CYC, 13);
+	hists__new_col_len(hists, HISTC_LOCAL_P_STAGE_CYC, 13);
+	hists__new_col_len(hists, HISTC_GLOBAL_P_STAGE_CYC, 13);
+
 	if (symbol_conf.nanosecs)
 		hists__new_col_len(hists, HISTC_TIME, 16);
 	else
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 621f35ae1efa..2a15e22fb89c 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -75,7 +75,8 @@ enum hist_column {
 	HISTC_MEM_BLOCKED,
 	HISTC_LOCAL_INS_LAT,
 	HISTC_GLOBAL_INS_LAT,
-	HISTC_P_STAGE_CYC,
+	HISTC_LOCAL_P_STAGE_CYC,
+	HISTC_GLOBAL_P_STAGE_CYC,
 	HISTC_NR_COLS, /* Last entry */
 };
 
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index a111065b484e..e417e47f51b9 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -37,7 +37,7 @@ const char	default_parent_pattern[] = "^sys_|^do_page_fault";
 const char	*parent_pattern = default_parent_pattern;
 const char	*default_sort_order = "comm,dso,symbol";
 const char	default_branch_sort_order[] = "comm,dso_from,symbol_from,symbol_to,cycles";
-const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,p_stage_cyc";
+const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,local_p_stage_cyc";
 const char	default_top_sort_order[] = "dso,symbol";
 const char	default_diff_sort_order[] = "dso,symbol";
 const char	default_tracepoint_sort_order[] = "trace";
@@ -46,8 +46,8 @@ const char	*field_order;
 regex_t		ignore_callees_regex;
 int		have_ignore_callees = 0;
 enum sort_mode	sort__mode = SORT_MODE__NORMAL;
-const char	*dynamic_headers[] = {"local_ins_lat", "p_stage_cyc"};
-const char	*arch_specific_sort_keys[] = {"p_stage_cyc"};
+static const char *const dynamic_headers[] = {"local_ins_lat", "ins_lat", "local_p_stage_cyc", "p_stage_cyc"};
+static const char *const arch_specific_sort_keys[] = {"local_p_stage_cyc", "p_stage_cyc"};
 
 /*
  * Replaces all occurrences of a char used with the:
@@ -1392,22 +1392,37 @@ struct sort_entry sort_global_ins_lat = {
 };
 
 static int64_t
-sort__global_p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
+sort__p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
 {
 	return left->p_stage_cyc - right->p_stage_cyc;
 }
 
+static int hist_entry__global_p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
+					size_t size, unsigned int width)
+{
+	return repsep_snprintf(bf, size, "%-*u", width,
+			he->p_stage_cyc * he->stat.nr_events);
+}
+
+
 static int hist_entry__p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
 					size_t size, unsigned int width)
 {
 	return repsep_snprintf(bf, size, "%-*u", width, he->p_stage_cyc);
 }
 
-struct sort_entry sort_p_stage_cyc = {
-	.se_header      = "Pipeline Stage Cycle",
-	.se_cmp         = sort__global_p_stage_cyc_cmp,
+struct sort_entry sort_local_p_stage_cyc = {
+	.se_header      = "Local Pipeline Stage Cycle",
+	.se_cmp         = sort__p_stage_cyc_cmp,
 	.se_snprintf	= hist_entry__p_stage_cyc_snprintf,
-	.se_width_idx	= HISTC_P_STAGE_CYC,
+	.se_width_idx	= HISTC_LOCAL_P_STAGE_CYC,
+};
+
+struct sort_entry sort_global_p_stage_cyc = {
+	.se_header      = "Pipeline Stage Cycle",
+	.se_cmp         = sort__p_stage_cyc_cmp,
+	.se_snprintf    = hist_entry__global_p_stage_cyc_snprintf,
+	.se_width_idx   = HISTC_GLOBAL_P_STAGE_CYC,
 };
 
 struct sort_entry sort_mem_daddr_sym = {
@@ -1858,7 +1873,8 @@ static struct sort_dimension common_sort_dimensions[] = {
 	DIM(SORT_CODE_PAGE_SIZE, "code_page_size", sort_code_page_size),
 	DIM(SORT_LOCAL_INS_LAT, "local_ins_lat", sort_local_ins_lat),
 	DIM(SORT_GLOBAL_INS_LAT, "ins_lat", sort_global_ins_lat),
-	DIM(SORT_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_p_stage_cyc),
+	DIM(SORT_LOCAL_PIPELINE_STAGE_CYC, "local_p_stage_cyc", sort_local_p_stage_cyc),
+	DIM(SORT_GLOBAL_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_global_p_stage_cyc),
 };
 
 #undef DIM
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 7b7145501933..f994261888e1 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -235,7 +235,8 @@ enum sort_type {
 	SORT_CODE_PAGE_SIZE,
 	SORT_LOCAL_INS_LAT,
 	SORT_GLOBAL_INS_LAT,
-	SORT_PIPELINE_STAGE_CYC,
+	SORT_LOCAL_PIPELINE_STAGE_CYC,
+	SORT_GLOBAL_PIPELINE_STAGE_CYC,
 
 	/* branch stack specific sort keys */
 	__SORT_BRANCH_STACK,
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH V2 2/2] tools/perf: Update global/local variants for p_stage_cyc in powerpc
  2021-12-03  2:20 ` Athira Rajeev
@ 2021-12-03  2:20   ` Athira Rajeev
  -1 siblings, 0 replies; 12+ messages in thread
From: Athira Rajeev @ 2021-12-03  2:20 UTC (permalink / raw)
  To: acme, jolsa
  Cc: mpe, linux-perf-users, linuxppc-dev, maddy, rnsastry, kjain, namhyung

Update the arch_support_sort_key() function in powerpc
to enable presenting local and global variants of sort
key: p_stage_cyc. Update the "se_header" strings for
these in arch_perf_header_entry() function along with
instruction latency.

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Reported-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/arch/powerpc/util/event.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/arch/powerpc/util/event.c b/tools/perf/arch/powerpc/util/event.c
index 3bf441257466..cf430a4c55b9 100644
--- a/tools/perf/arch/powerpc/util/event.c
+++ b/tools/perf/arch/powerpc/util/event.c
@@ -40,8 +40,12 @@ const char *arch_perf_header_entry(const char *se_header)
 {
 	if (!strcmp(se_header, "Local INSTR Latency"))
 		return "Finish Cyc";
-	else if (!strcmp(se_header, "Pipeline Stage Cycle"))
+	else if (!strcmp(se_header, "INSTR Latency"))
+		return "Global Finish_cyc";
+	else if (!strcmp(se_header, "Local Pipeline Stage Cycle"))
 		return "Dispatch Cyc";
+	else if (!strcmp(se_header, "Pipeline Stage Cycle"))
+		return "Global Dispatch_cyc";
 	return se_header;
 }
 
@@ -49,5 +53,7 @@ int arch_support_sort_key(const char *sort_key)
 {
 	if (!strcmp(sort_key, "p_stage_cyc"))
 		return 1;
+	if (!strcmp(sort_key, "local_p_stage_cyc"))
+		return 1;
 	return 0;
 }
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH V2 2/2] tools/perf: Update global/local variants for p_stage_cyc in powerpc
@ 2021-12-03  2:20   ` Athira Rajeev
  0 siblings, 0 replies; 12+ messages in thread
From: Athira Rajeev @ 2021-12-03  2:20 UTC (permalink / raw)
  To: acme, jolsa
  Cc: maddy, rnsastry, linux-perf-users, kjain, namhyung, linuxppc-dev

Update the arch_support_sort_key() function in powerpc
to enable presenting local and global variants of sort
key: p_stage_cyc. Update the "se_header" strings for
these in arch_perf_header_entry() function along with
instruction latency.

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Reported-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/arch/powerpc/util/event.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/arch/powerpc/util/event.c b/tools/perf/arch/powerpc/util/event.c
index 3bf441257466..cf430a4c55b9 100644
--- a/tools/perf/arch/powerpc/util/event.c
+++ b/tools/perf/arch/powerpc/util/event.c
@@ -40,8 +40,12 @@ const char *arch_perf_header_entry(const char *se_header)
 {
 	if (!strcmp(se_header, "Local INSTR Latency"))
 		return "Finish Cyc";
-	else if (!strcmp(se_header, "Pipeline Stage Cycle"))
+	else if (!strcmp(se_header, "INSTR Latency"))
+		return "Global Finish_cyc";
+	else if (!strcmp(se_header, "Local Pipeline Stage Cycle"))
 		return "Dispatch Cyc";
+	else if (!strcmp(se_header, "Pipeline Stage Cycle"))
+		return "Global Dispatch_cyc";
 	return se_header;
 }
 
@@ -49,5 +53,7 @@ int arch_support_sort_key(const char *sort_key)
 {
 	if (!strcmp(sort_key, "p_stage_cyc"))
 		return 1;
+	if (!strcmp(sort_key, "local_p_stage_cyc"))
+		return 1;
 	return 0;
 }
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/2] tools/perf: Include global and local variants for p_stage_cyc sort key
  2021-12-03  2:20 ` Athira Rajeev
@ 2021-12-07 14:52   ` Arnaldo Carvalho de Melo
  -1 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-12-07 14:52 UTC (permalink / raw)
  To: Athira Rajeev
  Cc: jolsa, mpe, linux-perf-users, linuxppc-dev, maddy, rnsastry,
	kjain, namhyung

Em Fri, Dec 03, 2021 at 07:50:37AM +0530, Athira Rajeev escreveu:
> Sort key p_stage_cyc is used to present the latency
> cycles spend in pipeline stages. perf tool has local
> p_stage_cyc sort key to display this info. There is no
> global variant available for this sort key. local variant
> shows latency in a sinlge sample, whereas, global value
> will be useful to present the total latency (sum of
> latencies) in the hist entry. It represents latency
> number multiplied by the number of samples.
> 
> Add global (p_stage_cyc) and local variant
> (local_p_stage_cyc) for this sort key. Use the
> local_p_stage_cyc as default option for "mem" sort mode.
> Also add this to list of dynamic sort keys and made the
> "dynamic_headers" and "arch_specific_sort_keys" as static.
> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Reported-by: Namhyung Kim <namhyung@kernel.org>

I got this for v1, does it stand for v2?

Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>

> ---
> Changelog:
> v1 -> v2:
>  Addressed review comments from Jiri by making the
>  "dynamic_headers" and "arch_specific_sort_keys"
>  as static.
> 
>  tools/perf/util/hist.c |  4 +++-
>  tools/perf/util/hist.h |  3 ++-
>  tools/perf/util/sort.c | 34 +++++++++++++++++++++++++---------
>  tools/perf/util/sort.h |  3 ++-
>  4 files changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index b776465e04ef..0a8033b09e28 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -211,7 +211,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
>  	hists__new_col_len(hists, HISTC_MEM_BLOCKED, 10);
>  	hists__new_col_len(hists, HISTC_LOCAL_INS_LAT, 13);
>  	hists__new_col_len(hists, HISTC_GLOBAL_INS_LAT, 13);
> -	hists__new_col_len(hists, HISTC_P_STAGE_CYC, 13);
> +	hists__new_col_len(hists, HISTC_LOCAL_P_STAGE_CYC, 13);
> +	hists__new_col_len(hists, HISTC_GLOBAL_P_STAGE_CYC, 13);
> +
>  	if (symbol_conf.nanosecs)
>  		hists__new_col_len(hists, HISTC_TIME, 16);
>  	else
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 621f35ae1efa..2a15e22fb89c 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -75,7 +75,8 @@ enum hist_column {
>  	HISTC_MEM_BLOCKED,
>  	HISTC_LOCAL_INS_LAT,
>  	HISTC_GLOBAL_INS_LAT,
> -	HISTC_P_STAGE_CYC,
> +	HISTC_LOCAL_P_STAGE_CYC,
> +	HISTC_GLOBAL_P_STAGE_CYC,
>  	HISTC_NR_COLS, /* Last entry */
>  };
>  
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index a111065b484e..e417e47f51b9 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -37,7 +37,7 @@ const char	default_parent_pattern[] = "^sys_|^do_page_fault";
>  const char	*parent_pattern = default_parent_pattern;
>  const char	*default_sort_order = "comm,dso,symbol";
>  const char	default_branch_sort_order[] = "comm,dso_from,symbol_from,symbol_to,cycles";
> -const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,p_stage_cyc";
> +const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,local_p_stage_cyc";
>  const char	default_top_sort_order[] = "dso,symbol";
>  const char	default_diff_sort_order[] = "dso,symbol";
>  const char	default_tracepoint_sort_order[] = "trace";
> @@ -46,8 +46,8 @@ const char	*field_order;
>  regex_t		ignore_callees_regex;
>  int		have_ignore_callees = 0;
>  enum sort_mode	sort__mode = SORT_MODE__NORMAL;
> -const char	*dynamic_headers[] = {"local_ins_lat", "p_stage_cyc"};
> -const char	*arch_specific_sort_keys[] = {"p_stage_cyc"};
> +static const char *const dynamic_headers[] = {"local_ins_lat", "ins_lat", "local_p_stage_cyc", "p_stage_cyc"};
> +static const char *const arch_specific_sort_keys[] = {"local_p_stage_cyc", "p_stage_cyc"};
>  
>  /*
>   * Replaces all occurrences of a char used with the:
> @@ -1392,22 +1392,37 @@ struct sort_entry sort_global_ins_lat = {
>  };
>  
>  static int64_t
> -sort__global_p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
> +sort__p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
>  {
>  	return left->p_stage_cyc - right->p_stage_cyc;
>  }
>  
> +static int hist_entry__global_p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
> +					size_t size, unsigned int width)
> +{
> +	return repsep_snprintf(bf, size, "%-*u", width,
> +			he->p_stage_cyc * he->stat.nr_events);
> +}
> +
> +
>  static int hist_entry__p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
>  					size_t size, unsigned int width)
>  {
>  	return repsep_snprintf(bf, size, "%-*u", width, he->p_stage_cyc);
>  }
>  
> -struct sort_entry sort_p_stage_cyc = {
> -	.se_header      = "Pipeline Stage Cycle",
> -	.se_cmp         = sort__global_p_stage_cyc_cmp,
> +struct sort_entry sort_local_p_stage_cyc = {
> +	.se_header      = "Local Pipeline Stage Cycle",
> +	.se_cmp         = sort__p_stage_cyc_cmp,
>  	.se_snprintf	= hist_entry__p_stage_cyc_snprintf,
> -	.se_width_idx	= HISTC_P_STAGE_CYC,
> +	.se_width_idx	= HISTC_LOCAL_P_STAGE_CYC,
> +};
> +
> +struct sort_entry sort_global_p_stage_cyc = {
> +	.se_header      = "Pipeline Stage Cycle",
> +	.se_cmp         = sort__p_stage_cyc_cmp,
> +	.se_snprintf    = hist_entry__global_p_stage_cyc_snprintf,
> +	.se_width_idx   = HISTC_GLOBAL_P_STAGE_CYC,
>  };
>  
>  struct sort_entry sort_mem_daddr_sym = {
> @@ -1858,7 +1873,8 @@ static struct sort_dimension common_sort_dimensions[] = {
>  	DIM(SORT_CODE_PAGE_SIZE, "code_page_size", sort_code_page_size),
>  	DIM(SORT_LOCAL_INS_LAT, "local_ins_lat", sort_local_ins_lat),
>  	DIM(SORT_GLOBAL_INS_LAT, "ins_lat", sort_global_ins_lat),
> -	DIM(SORT_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_p_stage_cyc),
> +	DIM(SORT_LOCAL_PIPELINE_STAGE_CYC, "local_p_stage_cyc", sort_local_p_stage_cyc),
> +	DIM(SORT_GLOBAL_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_global_p_stage_cyc),
>  };
>  
>  #undef DIM
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index 7b7145501933..f994261888e1 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -235,7 +235,8 @@ enum sort_type {
>  	SORT_CODE_PAGE_SIZE,
>  	SORT_LOCAL_INS_LAT,
>  	SORT_GLOBAL_INS_LAT,
> -	SORT_PIPELINE_STAGE_CYC,
> +	SORT_LOCAL_PIPELINE_STAGE_CYC,
> +	SORT_GLOBAL_PIPELINE_STAGE_CYC,
>  
>  	/* branch stack specific sort keys */
>  	__SORT_BRANCH_STACK,
> -- 
> 2.33.0

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/2] tools/perf: Include global and local variants for p_stage_cyc sort key
@ 2021-12-07 14:52   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-12-07 14:52 UTC (permalink / raw)
  To: Athira Rajeev
  Cc: maddy, rnsastry, linux-perf-users, jolsa, kjain, namhyung, linuxppc-dev

Em Fri, Dec 03, 2021 at 07:50:37AM +0530, Athira Rajeev escreveu:
> Sort key p_stage_cyc is used to present the latency
> cycles spend in pipeline stages. perf tool has local
> p_stage_cyc sort key to display this info. There is no
> global variant available for this sort key. local variant
> shows latency in a sinlge sample, whereas, global value
> will be useful to present the total latency (sum of
> latencies) in the hist entry. It represents latency
> number multiplied by the number of samples.
> 
> Add global (p_stage_cyc) and local variant
> (local_p_stage_cyc) for this sort key. Use the
> local_p_stage_cyc as default option for "mem" sort mode.
> Also add this to list of dynamic sort keys and made the
> "dynamic_headers" and "arch_specific_sort_keys" as static.
> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Reported-by: Namhyung Kim <namhyung@kernel.org>

I got this for v1, does it stand for v2?

Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>

> ---
> Changelog:
> v1 -> v2:
>  Addressed review comments from Jiri by making the
>  "dynamic_headers" and "arch_specific_sort_keys"
>  as static.
> 
>  tools/perf/util/hist.c |  4 +++-
>  tools/perf/util/hist.h |  3 ++-
>  tools/perf/util/sort.c | 34 +++++++++++++++++++++++++---------
>  tools/perf/util/sort.h |  3 ++-
>  4 files changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index b776465e04ef..0a8033b09e28 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -211,7 +211,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
>  	hists__new_col_len(hists, HISTC_MEM_BLOCKED, 10);
>  	hists__new_col_len(hists, HISTC_LOCAL_INS_LAT, 13);
>  	hists__new_col_len(hists, HISTC_GLOBAL_INS_LAT, 13);
> -	hists__new_col_len(hists, HISTC_P_STAGE_CYC, 13);
> +	hists__new_col_len(hists, HISTC_LOCAL_P_STAGE_CYC, 13);
> +	hists__new_col_len(hists, HISTC_GLOBAL_P_STAGE_CYC, 13);
> +
>  	if (symbol_conf.nanosecs)
>  		hists__new_col_len(hists, HISTC_TIME, 16);
>  	else
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 621f35ae1efa..2a15e22fb89c 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -75,7 +75,8 @@ enum hist_column {
>  	HISTC_MEM_BLOCKED,
>  	HISTC_LOCAL_INS_LAT,
>  	HISTC_GLOBAL_INS_LAT,
> -	HISTC_P_STAGE_CYC,
> +	HISTC_LOCAL_P_STAGE_CYC,
> +	HISTC_GLOBAL_P_STAGE_CYC,
>  	HISTC_NR_COLS, /* Last entry */
>  };
>  
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index a111065b484e..e417e47f51b9 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -37,7 +37,7 @@ const char	default_parent_pattern[] = "^sys_|^do_page_fault";
>  const char	*parent_pattern = default_parent_pattern;
>  const char	*default_sort_order = "comm,dso,symbol";
>  const char	default_branch_sort_order[] = "comm,dso_from,symbol_from,symbol_to,cycles";
> -const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,p_stage_cyc";
> +const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,local_p_stage_cyc";
>  const char	default_top_sort_order[] = "dso,symbol";
>  const char	default_diff_sort_order[] = "dso,symbol";
>  const char	default_tracepoint_sort_order[] = "trace";
> @@ -46,8 +46,8 @@ const char	*field_order;
>  regex_t		ignore_callees_regex;
>  int		have_ignore_callees = 0;
>  enum sort_mode	sort__mode = SORT_MODE__NORMAL;
> -const char	*dynamic_headers[] = {"local_ins_lat", "p_stage_cyc"};
> -const char	*arch_specific_sort_keys[] = {"p_stage_cyc"};
> +static const char *const dynamic_headers[] = {"local_ins_lat", "ins_lat", "local_p_stage_cyc", "p_stage_cyc"};
> +static const char *const arch_specific_sort_keys[] = {"local_p_stage_cyc", "p_stage_cyc"};
>  
>  /*
>   * Replaces all occurrences of a char used with the:
> @@ -1392,22 +1392,37 @@ struct sort_entry sort_global_ins_lat = {
>  };
>  
>  static int64_t
> -sort__global_p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
> +sort__p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
>  {
>  	return left->p_stage_cyc - right->p_stage_cyc;
>  }
>  
> +static int hist_entry__global_p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
> +					size_t size, unsigned int width)
> +{
> +	return repsep_snprintf(bf, size, "%-*u", width,
> +			he->p_stage_cyc * he->stat.nr_events);
> +}
> +
> +
>  static int hist_entry__p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
>  					size_t size, unsigned int width)
>  {
>  	return repsep_snprintf(bf, size, "%-*u", width, he->p_stage_cyc);
>  }
>  
> -struct sort_entry sort_p_stage_cyc = {
> -	.se_header      = "Pipeline Stage Cycle",
> -	.se_cmp         = sort__global_p_stage_cyc_cmp,
> +struct sort_entry sort_local_p_stage_cyc = {
> +	.se_header      = "Local Pipeline Stage Cycle",
> +	.se_cmp         = sort__p_stage_cyc_cmp,
>  	.se_snprintf	= hist_entry__p_stage_cyc_snprintf,
> -	.se_width_idx	= HISTC_P_STAGE_CYC,
> +	.se_width_idx	= HISTC_LOCAL_P_STAGE_CYC,
> +};
> +
> +struct sort_entry sort_global_p_stage_cyc = {
> +	.se_header      = "Pipeline Stage Cycle",
> +	.se_cmp         = sort__p_stage_cyc_cmp,
> +	.se_snprintf    = hist_entry__global_p_stage_cyc_snprintf,
> +	.se_width_idx   = HISTC_GLOBAL_P_STAGE_CYC,
>  };
>  
>  struct sort_entry sort_mem_daddr_sym = {
> @@ -1858,7 +1873,8 @@ static struct sort_dimension common_sort_dimensions[] = {
>  	DIM(SORT_CODE_PAGE_SIZE, "code_page_size", sort_code_page_size),
>  	DIM(SORT_LOCAL_INS_LAT, "local_ins_lat", sort_local_ins_lat),
>  	DIM(SORT_GLOBAL_INS_LAT, "ins_lat", sort_global_ins_lat),
> -	DIM(SORT_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_p_stage_cyc),
> +	DIM(SORT_LOCAL_PIPELINE_STAGE_CYC, "local_p_stage_cyc", sort_local_p_stage_cyc),
> +	DIM(SORT_GLOBAL_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_global_p_stage_cyc),
>  };
>  
>  #undef DIM
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index 7b7145501933..f994261888e1 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -235,7 +235,8 @@ enum sort_type {
>  	SORT_CODE_PAGE_SIZE,
>  	SORT_LOCAL_INS_LAT,
>  	SORT_GLOBAL_INS_LAT,
> -	SORT_PIPELINE_STAGE_CYC,
> +	SORT_LOCAL_PIPELINE_STAGE_CYC,
> +	SORT_GLOBAL_PIPELINE_STAGE_CYC,
>  
>  	/* branch stack specific sort keys */
>  	__SORT_BRANCH_STACK,
> -- 
> 2.33.0

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/2] tools/perf: Include global and local variants for p_stage_cyc sort key
  2021-12-07 14:52   ` Arnaldo Carvalho de Melo
@ 2021-12-08  3:51     ` Nageswara Sastry
  -1 siblings, 0 replies; 12+ messages in thread
From: Nageswara Sastry @ 2021-12-08  3:51 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Athira Rajeev
  Cc: jolsa, mpe, linux-perf-users, linuxppc-dev, maddy, kjain, namhyung



On 07/12/21 8:22 pm, Arnaldo Carvalho de Melo wrote:
> Em Fri, Dec 03, 2021 at 07:50:37AM +0530, Athira Rajeev escreveu:
>> Sort key p_stage_cyc is used to present the latency
>> cycles spend in pipeline stages. perf tool has local
>> p_stage_cyc sort key to display this info. There is no
>> global variant available for this sort key. local variant
>> shows latency in a sinlge sample, whereas, global value
>> will be useful to present the total latency (sum of
>> latencies) in the hist entry. It represents latency
>> number multiplied by the number of samples.
>>
>> Add global (p_stage_cyc) and local variant
>> (local_p_stage_cyc) for this sort key. Use the
>> local_p_stage_cyc as default option for "mem" sort mode.
>> Also add this to list of dynamic sort keys and made the
>> "dynamic_headers" and "arch_specific_sort_keys" as static.
>>
>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>> Reported-by: Namhyung Kim <namhyung@kernel.org>
> 
> I got this for v1, does it stand for v2?
> 
> Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>


Tested with v2 also.


Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>

> 
>> ---
>> Changelog:
>> v1 -> v2:
>>   Addressed review comments from Jiri by making the
>>   "dynamic_headers" and "arch_specific_sort_keys"
>>   as static.
>>
>>   tools/perf/util/hist.c |  4 +++-
>>   tools/perf/util/hist.h |  3 ++-
>>   tools/perf/util/sort.c | 34 +++++++++++++++++++++++++---------
>>   tools/perf/util/sort.h |  3 ++-
>>   4 files changed, 32 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
>> index b776465e04ef..0a8033b09e28 100644
>> --- a/tools/perf/util/hist.c
>> +++ b/tools/perf/util/hist.c
>> @@ -211,7 +211,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
>>   	hists__new_col_len(hists, HISTC_MEM_BLOCKED, 10);
>>   	hists__new_col_len(hists, HISTC_LOCAL_INS_LAT, 13);
>>   	hists__new_col_len(hists, HISTC_GLOBAL_INS_LAT, 13);
>> -	hists__new_col_len(hists, HISTC_P_STAGE_CYC, 13);
>> +	hists__new_col_len(hists, HISTC_LOCAL_P_STAGE_CYC, 13);
>> +	hists__new_col_len(hists, HISTC_GLOBAL_P_STAGE_CYC, 13);
>> +
>>   	if (symbol_conf.nanosecs)
>>   		hists__new_col_len(hists, HISTC_TIME, 16);
>>   	else
>> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
>> index 621f35ae1efa..2a15e22fb89c 100644
>> --- a/tools/perf/util/hist.h
>> +++ b/tools/perf/util/hist.h
>> @@ -75,7 +75,8 @@ enum hist_column {
>>   	HISTC_MEM_BLOCKED,
>>   	HISTC_LOCAL_INS_LAT,
>>   	HISTC_GLOBAL_INS_LAT,
>> -	HISTC_P_STAGE_CYC,
>> +	HISTC_LOCAL_P_STAGE_CYC,
>> +	HISTC_GLOBAL_P_STAGE_CYC,
>>   	HISTC_NR_COLS, /* Last entry */
>>   };
>>   
>> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
>> index a111065b484e..e417e47f51b9 100644
>> --- a/tools/perf/util/sort.c
>> +++ b/tools/perf/util/sort.c
>> @@ -37,7 +37,7 @@ const char	default_parent_pattern[] = "^sys_|^do_page_fault";
>>   const char	*parent_pattern = default_parent_pattern;
>>   const char	*default_sort_order = "comm,dso,symbol";
>>   const char	default_branch_sort_order[] = "comm,dso_from,symbol_from,symbol_to,cycles";
>> -const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,p_stage_cyc";
>> +const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,local_p_stage_cyc";
>>   const char	default_top_sort_order[] = "dso,symbol";
>>   const char	default_diff_sort_order[] = "dso,symbol";
>>   const char	default_tracepoint_sort_order[] = "trace";
>> @@ -46,8 +46,8 @@ const char	*field_order;
>>   regex_t		ignore_callees_regex;
>>   int		have_ignore_callees = 0;
>>   enum sort_mode	sort__mode = SORT_MODE__NORMAL;
>> -const char	*dynamic_headers[] = {"local_ins_lat", "p_stage_cyc"};
>> -const char	*arch_specific_sort_keys[] = {"p_stage_cyc"};
>> +static const char *const dynamic_headers[] = {"local_ins_lat", "ins_lat", "local_p_stage_cyc", "p_stage_cyc"};
>> +static const char *const arch_specific_sort_keys[] = {"local_p_stage_cyc", "p_stage_cyc"};
>>   
>>   /*
>>    * Replaces all occurrences of a char used with the:
>> @@ -1392,22 +1392,37 @@ struct sort_entry sort_global_ins_lat = {
>>   };
>>   
>>   static int64_t
>> -sort__global_p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
>> +sort__p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
>>   {
>>   	return left->p_stage_cyc - right->p_stage_cyc;
>>   }
>>   
>> +static int hist_entry__global_p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
>> +					size_t size, unsigned int width)
>> +{
>> +	return repsep_snprintf(bf, size, "%-*u", width,
>> +			he->p_stage_cyc * he->stat.nr_events);
>> +}
>> +
>> +
>>   static int hist_entry__p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
>>   					size_t size, unsigned int width)
>>   {
>>   	return repsep_snprintf(bf, size, "%-*u", width, he->p_stage_cyc);
>>   }
>>   
>> -struct sort_entry sort_p_stage_cyc = {
>> -	.se_header      = "Pipeline Stage Cycle",
>> -	.se_cmp         = sort__global_p_stage_cyc_cmp,
>> +struct sort_entry sort_local_p_stage_cyc = {
>> +	.se_header      = "Local Pipeline Stage Cycle",
>> +	.se_cmp         = sort__p_stage_cyc_cmp,
>>   	.se_snprintf	= hist_entry__p_stage_cyc_snprintf,
>> -	.se_width_idx	= HISTC_P_STAGE_CYC,
>> +	.se_width_idx	= HISTC_LOCAL_P_STAGE_CYC,
>> +};
>> +
>> +struct sort_entry sort_global_p_stage_cyc = {
>> +	.se_header      = "Pipeline Stage Cycle",
>> +	.se_cmp         = sort__p_stage_cyc_cmp,
>> +	.se_snprintf    = hist_entry__global_p_stage_cyc_snprintf,
>> +	.se_width_idx   = HISTC_GLOBAL_P_STAGE_CYC,
>>   };
>>   
>>   struct sort_entry sort_mem_daddr_sym = {
>> @@ -1858,7 +1873,8 @@ static struct sort_dimension common_sort_dimensions[] = {
>>   	DIM(SORT_CODE_PAGE_SIZE, "code_page_size", sort_code_page_size),
>>   	DIM(SORT_LOCAL_INS_LAT, "local_ins_lat", sort_local_ins_lat),
>>   	DIM(SORT_GLOBAL_INS_LAT, "ins_lat", sort_global_ins_lat),
>> -	DIM(SORT_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_p_stage_cyc),
>> +	DIM(SORT_LOCAL_PIPELINE_STAGE_CYC, "local_p_stage_cyc", sort_local_p_stage_cyc),
>> +	DIM(SORT_GLOBAL_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_global_p_stage_cyc),
>>   };
>>   
>>   #undef DIM
>> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
>> index 7b7145501933..f994261888e1 100644
>> --- a/tools/perf/util/sort.h
>> +++ b/tools/perf/util/sort.h
>> @@ -235,7 +235,8 @@ enum sort_type {
>>   	SORT_CODE_PAGE_SIZE,
>>   	SORT_LOCAL_INS_LAT,
>>   	SORT_GLOBAL_INS_LAT,
>> -	SORT_PIPELINE_STAGE_CYC,
>> +	SORT_LOCAL_PIPELINE_STAGE_CYC,
>> +	SORT_GLOBAL_PIPELINE_STAGE_CYC,
>>   
>>   	/* branch stack specific sort keys */
>>   	__SORT_BRANCH_STACK,
>> -- 
>> 2.33.0
> 

-- 
Thanks and Regards
R.Nageswara Sastry

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/2] tools/perf: Include global and local variants for p_stage_cyc sort key
@ 2021-12-08  3:51     ` Nageswara Sastry
  0 siblings, 0 replies; 12+ messages in thread
From: Nageswara Sastry @ 2021-12-08  3:51 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Athira Rajeev
  Cc: maddy, linux-perf-users, jolsa, kjain, namhyung, linuxppc-dev



On 07/12/21 8:22 pm, Arnaldo Carvalho de Melo wrote:
> Em Fri, Dec 03, 2021 at 07:50:37AM +0530, Athira Rajeev escreveu:
>> Sort key p_stage_cyc is used to present the latency
>> cycles spend in pipeline stages. perf tool has local
>> p_stage_cyc sort key to display this info. There is no
>> global variant available for this sort key. local variant
>> shows latency in a sinlge sample, whereas, global value
>> will be useful to present the total latency (sum of
>> latencies) in the hist entry. It represents latency
>> number multiplied by the number of samples.
>>
>> Add global (p_stage_cyc) and local variant
>> (local_p_stage_cyc) for this sort key. Use the
>> local_p_stage_cyc as default option for "mem" sort mode.
>> Also add this to list of dynamic sort keys and made the
>> "dynamic_headers" and "arch_specific_sort_keys" as static.
>>
>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>> Reported-by: Namhyung Kim <namhyung@kernel.org>
> 
> I got this for v1, does it stand for v2?
> 
> Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>


Tested with v2 also.


Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>

> 
>> ---
>> Changelog:
>> v1 -> v2:
>>   Addressed review comments from Jiri by making the
>>   "dynamic_headers" and "arch_specific_sort_keys"
>>   as static.
>>
>>   tools/perf/util/hist.c |  4 +++-
>>   tools/perf/util/hist.h |  3 ++-
>>   tools/perf/util/sort.c | 34 +++++++++++++++++++++++++---------
>>   tools/perf/util/sort.h |  3 ++-
>>   4 files changed, 32 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
>> index b776465e04ef..0a8033b09e28 100644
>> --- a/tools/perf/util/hist.c
>> +++ b/tools/perf/util/hist.c
>> @@ -211,7 +211,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
>>   	hists__new_col_len(hists, HISTC_MEM_BLOCKED, 10);
>>   	hists__new_col_len(hists, HISTC_LOCAL_INS_LAT, 13);
>>   	hists__new_col_len(hists, HISTC_GLOBAL_INS_LAT, 13);
>> -	hists__new_col_len(hists, HISTC_P_STAGE_CYC, 13);
>> +	hists__new_col_len(hists, HISTC_LOCAL_P_STAGE_CYC, 13);
>> +	hists__new_col_len(hists, HISTC_GLOBAL_P_STAGE_CYC, 13);
>> +
>>   	if (symbol_conf.nanosecs)
>>   		hists__new_col_len(hists, HISTC_TIME, 16);
>>   	else
>> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
>> index 621f35ae1efa..2a15e22fb89c 100644
>> --- a/tools/perf/util/hist.h
>> +++ b/tools/perf/util/hist.h
>> @@ -75,7 +75,8 @@ enum hist_column {
>>   	HISTC_MEM_BLOCKED,
>>   	HISTC_LOCAL_INS_LAT,
>>   	HISTC_GLOBAL_INS_LAT,
>> -	HISTC_P_STAGE_CYC,
>> +	HISTC_LOCAL_P_STAGE_CYC,
>> +	HISTC_GLOBAL_P_STAGE_CYC,
>>   	HISTC_NR_COLS, /* Last entry */
>>   };
>>   
>> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
>> index a111065b484e..e417e47f51b9 100644
>> --- a/tools/perf/util/sort.c
>> +++ b/tools/perf/util/sort.c
>> @@ -37,7 +37,7 @@ const char	default_parent_pattern[] = "^sys_|^do_page_fault";
>>   const char	*parent_pattern = default_parent_pattern;
>>   const char	*default_sort_order = "comm,dso,symbol";
>>   const char	default_branch_sort_order[] = "comm,dso_from,symbol_from,symbol_to,cycles";
>> -const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,p_stage_cyc";
>> +const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,local_p_stage_cyc";
>>   const char	default_top_sort_order[] = "dso,symbol";
>>   const char	default_diff_sort_order[] = "dso,symbol";
>>   const char	default_tracepoint_sort_order[] = "trace";
>> @@ -46,8 +46,8 @@ const char	*field_order;
>>   regex_t		ignore_callees_regex;
>>   int		have_ignore_callees = 0;
>>   enum sort_mode	sort__mode = SORT_MODE__NORMAL;
>> -const char	*dynamic_headers[] = {"local_ins_lat", "p_stage_cyc"};
>> -const char	*arch_specific_sort_keys[] = {"p_stage_cyc"};
>> +static const char *const dynamic_headers[] = {"local_ins_lat", "ins_lat", "local_p_stage_cyc", "p_stage_cyc"};
>> +static const char *const arch_specific_sort_keys[] = {"local_p_stage_cyc", "p_stage_cyc"};
>>   
>>   /*
>>    * Replaces all occurrences of a char used with the:
>> @@ -1392,22 +1392,37 @@ struct sort_entry sort_global_ins_lat = {
>>   };
>>   
>>   static int64_t
>> -sort__global_p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
>> +sort__p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
>>   {
>>   	return left->p_stage_cyc - right->p_stage_cyc;
>>   }
>>   
>> +static int hist_entry__global_p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
>> +					size_t size, unsigned int width)
>> +{
>> +	return repsep_snprintf(bf, size, "%-*u", width,
>> +			he->p_stage_cyc * he->stat.nr_events);
>> +}
>> +
>> +
>>   static int hist_entry__p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
>>   					size_t size, unsigned int width)
>>   {
>>   	return repsep_snprintf(bf, size, "%-*u", width, he->p_stage_cyc);
>>   }
>>   
>> -struct sort_entry sort_p_stage_cyc = {
>> -	.se_header      = "Pipeline Stage Cycle",
>> -	.se_cmp         = sort__global_p_stage_cyc_cmp,
>> +struct sort_entry sort_local_p_stage_cyc = {
>> +	.se_header      = "Local Pipeline Stage Cycle",
>> +	.se_cmp         = sort__p_stage_cyc_cmp,
>>   	.se_snprintf	= hist_entry__p_stage_cyc_snprintf,
>> -	.se_width_idx	= HISTC_P_STAGE_CYC,
>> +	.se_width_idx	= HISTC_LOCAL_P_STAGE_CYC,
>> +};
>> +
>> +struct sort_entry sort_global_p_stage_cyc = {
>> +	.se_header      = "Pipeline Stage Cycle",
>> +	.se_cmp         = sort__p_stage_cyc_cmp,
>> +	.se_snprintf    = hist_entry__global_p_stage_cyc_snprintf,
>> +	.se_width_idx   = HISTC_GLOBAL_P_STAGE_CYC,
>>   };
>>   
>>   struct sort_entry sort_mem_daddr_sym = {
>> @@ -1858,7 +1873,8 @@ static struct sort_dimension common_sort_dimensions[] = {
>>   	DIM(SORT_CODE_PAGE_SIZE, "code_page_size", sort_code_page_size),
>>   	DIM(SORT_LOCAL_INS_LAT, "local_ins_lat", sort_local_ins_lat),
>>   	DIM(SORT_GLOBAL_INS_LAT, "ins_lat", sort_global_ins_lat),
>> -	DIM(SORT_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_p_stage_cyc),
>> +	DIM(SORT_LOCAL_PIPELINE_STAGE_CYC, "local_p_stage_cyc", sort_local_p_stage_cyc),
>> +	DIM(SORT_GLOBAL_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_global_p_stage_cyc),
>>   };
>>   
>>   #undef DIM
>> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
>> index 7b7145501933..f994261888e1 100644
>> --- a/tools/perf/util/sort.h
>> +++ b/tools/perf/util/sort.h
>> @@ -235,7 +235,8 @@ enum sort_type {
>>   	SORT_CODE_PAGE_SIZE,
>>   	SORT_LOCAL_INS_LAT,
>>   	SORT_GLOBAL_INS_LAT,
>> -	SORT_PIPELINE_STAGE_CYC,
>> +	SORT_LOCAL_PIPELINE_STAGE_CYC,
>> +	SORT_GLOBAL_PIPELINE_STAGE_CYC,
>>   
>>   	/* branch stack specific sort keys */
>>   	__SORT_BRANCH_STACK,
>> -- 
>> 2.33.0
> 

-- 
Thanks and Regards
R.Nageswara Sastry

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/2] tools/perf: Include global and local variants for p_stage_cyc sort key
  2021-12-08  3:51     ` Nageswara Sastry
@ 2022-01-06 10:51       ` Athira Rajeev
  -1 siblings, 0 replies; 12+ messages in thread
From: Athira Rajeev @ 2022-01-06 10:51 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Michael Ellerman, linux-perf-users, linuxppc-dev,
	maddy, kajoljain, Namhyung Kim, Nageswara Sastry



> On 08-Dec-2021, at 9:21 AM, Nageswara Sastry <rnsastry@linux.ibm.com> wrote:
> 
> 
> 
> On 07/12/21 8:22 pm, Arnaldo Carvalho de Melo wrote:
>> Em Fri, Dec 03, 2021 at 07:50:37AM +0530, Athira Rajeev escreveu:
>>> Sort key p_stage_cyc is used to present the latency
>>> cycles spend in pipeline stages. perf tool has local
>>> p_stage_cyc sort key to display this info. There is no
>>> global variant available for this sort key. local variant
>>> shows latency in a sinlge sample, whereas, global value
>>> will be useful to present the total latency (sum of
>>> latencies) in the hist entry. It represents latency
>>> number multiplied by the number of samples.
>>> 
>>> Add global (p_stage_cyc) and local variant
>>> (local_p_stage_cyc) for this sort key. Use the
>>> local_p_stage_cyc as default option for "mem" sort mode.
>>> Also add this to list of dynamic sort keys and made the
>>> "dynamic_headers" and "arch_specific_sort_keys" as static.
>>> 
>>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>>> Reported-by: Namhyung Kim <namhyung@kernel.org>
>> I got this for v1, does it stand for v2?
>> Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> 
> 
> Tested with v2 also.

Hi Arnaldo,

If this patchset looks fine to you, can you please consider pulling it.

Thanks
Athira
> 
> 
> Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> 
>>> ---
>>> Changelog:
>>> v1 -> v2:
>>>  Addressed review comments from Jiri by making the
>>>  "dynamic_headers" and "arch_specific_sort_keys"
>>>  as static.
>>> 
>>>  tools/perf/util/hist.c |  4 +++-
>>>  tools/perf/util/hist.h |  3 ++-
>>>  tools/perf/util/sort.c | 34 +++++++++++++++++++++++++---------
>>>  tools/perf/util/sort.h |  3 ++-
>>>  4 files changed, 32 insertions(+), 12 deletions(-)
>>> 
>>> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
>>> index b776465e04ef..0a8033b09e28 100644
>>> --- a/tools/perf/util/hist.c
>>> +++ b/tools/perf/util/hist.c
>>> @@ -211,7 +211,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
>>>  	hists__new_col_len(hists, HISTC_MEM_BLOCKED, 10);
>>>  	hists__new_col_len(hists, HISTC_LOCAL_INS_LAT, 13);
>>>  	hists__new_col_len(hists, HISTC_GLOBAL_INS_LAT, 13);
>>> -	hists__new_col_len(hists, HISTC_P_STAGE_CYC, 13);
>>> +	hists__new_col_len(hists, HISTC_LOCAL_P_STAGE_CYC, 13);
>>> +	hists__new_col_len(hists, HISTC_GLOBAL_P_STAGE_CYC, 13);
>>> +
>>>  	if (symbol_conf.nanosecs)
>>>  		hists__new_col_len(hists, HISTC_TIME, 16);
>>>  	else
>>> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
>>> index 621f35ae1efa..2a15e22fb89c 100644
>>> --- a/tools/perf/util/hist.h
>>> +++ b/tools/perf/util/hist.h
>>> @@ -75,7 +75,8 @@ enum hist_column {
>>>  	HISTC_MEM_BLOCKED,
>>>  	HISTC_LOCAL_INS_LAT,
>>>  	HISTC_GLOBAL_INS_LAT,
>>> -	HISTC_P_STAGE_CYC,
>>> +	HISTC_LOCAL_P_STAGE_CYC,
>>> +	HISTC_GLOBAL_P_STAGE_CYC,
>>>  	HISTC_NR_COLS, /* Last entry */
>>>  };
>>>  diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
>>> index a111065b484e..e417e47f51b9 100644
>>> --- a/tools/perf/util/sort.c
>>> +++ b/tools/perf/util/sort.c
>>> @@ -37,7 +37,7 @@ const char	default_parent_pattern[] = "^sys_|^do_page_fault";
>>>  const char	*parent_pattern = default_parent_pattern;
>>>  const char	*default_sort_order = "comm,dso,symbol";
>>>  const char	default_branch_sort_order[] = "comm,dso_from,symbol_from,symbol_to,cycles";
>>> -const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,p_stage_cyc";
>>> +const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,local_p_stage_cyc";
>>>  const char	default_top_sort_order[] = "dso,symbol";
>>>  const char	default_diff_sort_order[] = "dso,symbol";
>>>  const char	default_tracepoint_sort_order[] = "trace";
>>> @@ -46,8 +46,8 @@ const char	*field_order;
>>>  regex_t		ignore_callees_regex;
>>>  int		have_ignore_callees = 0;
>>>  enum sort_mode	sort__mode = SORT_MODE__NORMAL;
>>> -const char	*dynamic_headers[] = {"local_ins_lat", "p_stage_cyc"};
>>> -const char	*arch_specific_sort_keys[] = {"p_stage_cyc"};
>>> +static const char *const dynamic_headers[] = {"local_ins_lat", "ins_lat", "local_p_stage_cyc", "p_stage_cyc"};
>>> +static const char *const arch_specific_sort_keys[] = {"local_p_stage_cyc", "p_stage_cyc"};
>>>    /*
>>>   * Replaces all occurrences of a char used with the:
>>> @@ -1392,22 +1392,37 @@ struct sort_entry sort_global_ins_lat = {
>>>  };
>>>    static int64_t
>>> -sort__global_p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
>>> +sort__p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
>>>  {
>>>  	return left->p_stage_cyc - right->p_stage_cyc;
>>>  }
>>>  +static int hist_entry__global_p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
>>> +					size_t size, unsigned int width)
>>> +{
>>> +	return repsep_snprintf(bf, size, "%-*u", width,
>>> +			he->p_stage_cyc * he->stat.nr_events);
>>> +}
>>> +
>>> +
>>>  static int hist_entry__p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
>>>  					size_t size, unsigned int width)
>>>  {
>>>  	return repsep_snprintf(bf, size, "%-*u", width, he->p_stage_cyc);
>>>  }
>>>  -struct sort_entry sort_p_stage_cyc = {
>>> -	.se_header      = "Pipeline Stage Cycle",
>>> -	.se_cmp         = sort__global_p_stage_cyc_cmp,
>>> +struct sort_entry sort_local_p_stage_cyc = {
>>> +	.se_header      = "Local Pipeline Stage Cycle",
>>> +	.se_cmp         = sort__p_stage_cyc_cmp,
>>>  	.se_snprintf	= hist_entry__p_stage_cyc_snprintf,
>>> -	.se_width_idx	= HISTC_P_STAGE_CYC,
>>> +	.se_width_idx	= HISTC_LOCAL_P_STAGE_CYC,
>>> +};
>>> +
>>> +struct sort_entry sort_global_p_stage_cyc = {
>>> +	.se_header      = "Pipeline Stage Cycle",
>>> +	.se_cmp         = sort__p_stage_cyc_cmp,
>>> +	.se_snprintf    = hist_entry__global_p_stage_cyc_snprintf,
>>> +	.se_width_idx   = HISTC_GLOBAL_P_STAGE_CYC,
>>>  };
>>>    struct sort_entry sort_mem_daddr_sym = {
>>> @@ -1858,7 +1873,8 @@ static struct sort_dimension common_sort_dimensions[] = {
>>>  	DIM(SORT_CODE_PAGE_SIZE, "code_page_size", sort_code_page_size),
>>>  	DIM(SORT_LOCAL_INS_LAT, "local_ins_lat", sort_local_ins_lat),
>>>  	DIM(SORT_GLOBAL_INS_LAT, "ins_lat", sort_global_ins_lat),
>>> -	DIM(SORT_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_p_stage_cyc),
>>> +	DIM(SORT_LOCAL_PIPELINE_STAGE_CYC, "local_p_stage_cyc", sort_local_p_stage_cyc),
>>> +	DIM(SORT_GLOBAL_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_global_p_stage_cyc),
>>>  };
>>>    #undef DIM
>>> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
>>> index 7b7145501933..f994261888e1 100644
>>> --- a/tools/perf/util/sort.h
>>> +++ b/tools/perf/util/sort.h
>>> @@ -235,7 +235,8 @@ enum sort_type {
>>>  	SORT_CODE_PAGE_SIZE,
>>>  	SORT_LOCAL_INS_LAT,
>>>  	SORT_GLOBAL_INS_LAT,
>>> -	SORT_PIPELINE_STAGE_CYC,
>>> +	SORT_LOCAL_PIPELINE_STAGE_CYC,
>>> +	SORT_GLOBAL_PIPELINE_STAGE_CYC,
>>>    	/* branch stack specific sort keys */
>>>  	__SORT_BRANCH_STACK,
>>> -- 
>>> 2.33.0
> 
> -- 
> Thanks and Regards
> R.Nageswara Sastry


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/2] tools/perf: Include global and local variants for p_stage_cyc sort key
@ 2022-01-06 10:51       ` Athira Rajeev
  0 siblings, 0 replies; 12+ messages in thread
From: Athira Rajeev @ 2022-01-06 10:51 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: maddy, Nageswara Sastry, linux-perf-users, Jiri Olsa, kajoljain,
	Namhyung Kim, linuxppc-dev



> On 08-Dec-2021, at 9:21 AM, Nageswara Sastry <rnsastry@linux.ibm.com> wrote:
> 
> 
> 
> On 07/12/21 8:22 pm, Arnaldo Carvalho de Melo wrote:
>> Em Fri, Dec 03, 2021 at 07:50:37AM +0530, Athira Rajeev escreveu:
>>> Sort key p_stage_cyc is used to present the latency
>>> cycles spend in pipeline stages. perf tool has local
>>> p_stage_cyc sort key to display this info. There is no
>>> global variant available for this sort key. local variant
>>> shows latency in a sinlge sample, whereas, global value
>>> will be useful to present the total latency (sum of
>>> latencies) in the hist entry. It represents latency
>>> number multiplied by the number of samples.
>>> 
>>> Add global (p_stage_cyc) and local variant
>>> (local_p_stage_cyc) for this sort key. Use the
>>> local_p_stage_cyc as default option for "mem" sort mode.
>>> Also add this to list of dynamic sort keys and made the
>>> "dynamic_headers" and "arch_specific_sort_keys" as static.
>>> 
>>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>>> Reported-by: Namhyung Kim <namhyung@kernel.org>
>> I got this for v1, does it stand for v2?
>> Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> 
> 
> Tested with v2 also.

Hi Arnaldo,

If this patchset looks fine to you, can you please consider pulling it.

Thanks
Athira
> 
> 
> Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> 
>>> ---
>>> Changelog:
>>> v1 -> v2:
>>>  Addressed review comments from Jiri by making the
>>>  "dynamic_headers" and "arch_specific_sort_keys"
>>>  as static.
>>> 
>>>  tools/perf/util/hist.c |  4 +++-
>>>  tools/perf/util/hist.h |  3 ++-
>>>  tools/perf/util/sort.c | 34 +++++++++++++++++++++++++---------
>>>  tools/perf/util/sort.h |  3 ++-
>>>  4 files changed, 32 insertions(+), 12 deletions(-)
>>> 
>>> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
>>> index b776465e04ef..0a8033b09e28 100644
>>> --- a/tools/perf/util/hist.c
>>> +++ b/tools/perf/util/hist.c
>>> @@ -211,7 +211,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
>>>  	hists__new_col_len(hists, HISTC_MEM_BLOCKED, 10);
>>>  	hists__new_col_len(hists, HISTC_LOCAL_INS_LAT, 13);
>>>  	hists__new_col_len(hists, HISTC_GLOBAL_INS_LAT, 13);
>>> -	hists__new_col_len(hists, HISTC_P_STAGE_CYC, 13);
>>> +	hists__new_col_len(hists, HISTC_LOCAL_P_STAGE_CYC, 13);
>>> +	hists__new_col_len(hists, HISTC_GLOBAL_P_STAGE_CYC, 13);
>>> +
>>>  	if (symbol_conf.nanosecs)
>>>  		hists__new_col_len(hists, HISTC_TIME, 16);
>>>  	else
>>> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
>>> index 621f35ae1efa..2a15e22fb89c 100644
>>> --- a/tools/perf/util/hist.h
>>> +++ b/tools/perf/util/hist.h
>>> @@ -75,7 +75,8 @@ enum hist_column {
>>>  	HISTC_MEM_BLOCKED,
>>>  	HISTC_LOCAL_INS_LAT,
>>>  	HISTC_GLOBAL_INS_LAT,
>>> -	HISTC_P_STAGE_CYC,
>>> +	HISTC_LOCAL_P_STAGE_CYC,
>>> +	HISTC_GLOBAL_P_STAGE_CYC,
>>>  	HISTC_NR_COLS, /* Last entry */
>>>  };
>>>  diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
>>> index a111065b484e..e417e47f51b9 100644
>>> --- a/tools/perf/util/sort.c
>>> +++ b/tools/perf/util/sort.c
>>> @@ -37,7 +37,7 @@ const char	default_parent_pattern[] = "^sys_|^do_page_fault";
>>>  const char	*parent_pattern = default_parent_pattern;
>>>  const char	*default_sort_order = "comm,dso,symbol";
>>>  const char	default_branch_sort_order[] = "comm,dso_from,symbol_from,symbol_to,cycles";
>>> -const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,p_stage_cyc";
>>> +const char	default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,local_p_stage_cyc";
>>>  const char	default_top_sort_order[] = "dso,symbol";
>>>  const char	default_diff_sort_order[] = "dso,symbol";
>>>  const char	default_tracepoint_sort_order[] = "trace";
>>> @@ -46,8 +46,8 @@ const char	*field_order;
>>>  regex_t		ignore_callees_regex;
>>>  int		have_ignore_callees = 0;
>>>  enum sort_mode	sort__mode = SORT_MODE__NORMAL;
>>> -const char	*dynamic_headers[] = {"local_ins_lat", "p_stage_cyc"};
>>> -const char	*arch_specific_sort_keys[] = {"p_stage_cyc"};
>>> +static const char *const dynamic_headers[] = {"local_ins_lat", "ins_lat", "local_p_stage_cyc", "p_stage_cyc"};
>>> +static const char *const arch_specific_sort_keys[] = {"local_p_stage_cyc", "p_stage_cyc"};
>>>    /*
>>>   * Replaces all occurrences of a char used with the:
>>> @@ -1392,22 +1392,37 @@ struct sort_entry sort_global_ins_lat = {
>>>  };
>>>    static int64_t
>>> -sort__global_p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
>>> +sort__p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
>>>  {
>>>  	return left->p_stage_cyc - right->p_stage_cyc;
>>>  }
>>>  +static int hist_entry__global_p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
>>> +					size_t size, unsigned int width)
>>> +{
>>> +	return repsep_snprintf(bf, size, "%-*u", width,
>>> +			he->p_stage_cyc * he->stat.nr_events);
>>> +}
>>> +
>>> +
>>>  static int hist_entry__p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
>>>  					size_t size, unsigned int width)
>>>  {
>>>  	return repsep_snprintf(bf, size, "%-*u", width, he->p_stage_cyc);
>>>  }
>>>  -struct sort_entry sort_p_stage_cyc = {
>>> -	.se_header      = "Pipeline Stage Cycle",
>>> -	.se_cmp         = sort__global_p_stage_cyc_cmp,
>>> +struct sort_entry sort_local_p_stage_cyc = {
>>> +	.se_header      = "Local Pipeline Stage Cycle",
>>> +	.se_cmp         = sort__p_stage_cyc_cmp,
>>>  	.se_snprintf	= hist_entry__p_stage_cyc_snprintf,
>>> -	.se_width_idx	= HISTC_P_STAGE_CYC,
>>> +	.se_width_idx	= HISTC_LOCAL_P_STAGE_CYC,
>>> +};
>>> +
>>> +struct sort_entry sort_global_p_stage_cyc = {
>>> +	.se_header      = "Pipeline Stage Cycle",
>>> +	.se_cmp         = sort__p_stage_cyc_cmp,
>>> +	.se_snprintf    = hist_entry__global_p_stage_cyc_snprintf,
>>> +	.se_width_idx   = HISTC_GLOBAL_P_STAGE_CYC,
>>>  };
>>>    struct sort_entry sort_mem_daddr_sym = {
>>> @@ -1858,7 +1873,8 @@ static struct sort_dimension common_sort_dimensions[] = {
>>>  	DIM(SORT_CODE_PAGE_SIZE, "code_page_size", sort_code_page_size),
>>>  	DIM(SORT_LOCAL_INS_LAT, "local_ins_lat", sort_local_ins_lat),
>>>  	DIM(SORT_GLOBAL_INS_LAT, "ins_lat", sort_global_ins_lat),
>>> -	DIM(SORT_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_p_stage_cyc),
>>> +	DIM(SORT_LOCAL_PIPELINE_STAGE_CYC, "local_p_stage_cyc", sort_local_p_stage_cyc),
>>> +	DIM(SORT_GLOBAL_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_global_p_stage_cyc),
>>>  };
>>>    #undef DIM
>>> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
>>> index 7b7145501933..f994261888e1 100644
>>> --- a/tools/perf/util/sort.h
>>> +++ b/tools/perf/util/sort.h
>>> @@ -235,7 +235,8 @@ enum sort_type {
>>>  	SORT_CODE_PAGE_SIZE,
>>>  	SORT_LOCAL_INS_LAT,
>>>  	SORT_GLOBAL_INS_LAT,
>>> -	SORT_PIPELINE_STAGE_CYC,
>>> +	SORT_LOCAL_PIPELINE_STAGE_CYC,
>>> +	SORT_GLOBAL_PIPELINE_STAGE_CYC,
>>>    	/* branch stack specific sort keys */
>>>  	__SORT_BRANCH_STACK,
>>> -- 
>>> 2.33.0
> 
> -- 
> Thanks and Regards
> R.Nageswara Sastry


^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/2] tools/perf: Include global and local variants for p_stage_cyc sort key
  2022-01-06 10:51       ` Athira Rajeev
@ 2022-01-06 20:22         ` Arnaldo Carvalho de Melo
  -1 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-01-06 20:22 UTC (permalink / raw)
  To: Athira Rajeev
  Cc: Jiri Olsa, Michael Ellerman, linux-perf-users, linuxppc-dev,
	maddy, kajoljain, Namhyung Kim, Nageswara Sastry

Em Thu, Jan 06, 2022 at 04:21:05PM +0530, Athira Rajeev escreveu:
> 
> 
> > On 08-Dec-2021, at 9:21 AM, Nageswara Sastry <rnsastry@linux.ibm.com> wrote:
> > 
> > 
> > 
> > On 07/12/21 8:22 pm, Arnaldo Carvalho de Melo wrote:
> >> Em Fri, Dec 03, 2021 at 07:50:37AM +0530, Athira Rajeev escreveu:
> >>> Sort key p_stage_cyc is used to present the latency
> >>> cycles spend in pipeline stages. perf tool has local
> >>> p_stage_cyc sort key to display this info. There is no
> >>> global variant available for this sort key. local variant
> >>> shows latency in a sinlge sample, whereas, global value
> >>> will be useful to present the total latency (sum of
> >>> latencies) in the hist entry. It represents latency
> >>> number multiplied by the number of samples.
> >>> 
> >>> Add global (p_stage_cyc) and local variant
> >>> (local_p_stage_cyc) for this sort key. Use the
> >>> local_p_stage_cyc as default option for "mem" sort mode.
> >>> Also add this to list of dynamic sort keys and made the
> >>> "dynamic_headers" and "arch_specific_sort_keys" as static.
> >>> 
> >>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> >>> Reported-by: Namhyung Kim <namhyung@kernel.org>
> >> I got this for v1, does it stand for v2?
> >> Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> > 
> > 
> > Tested with v2 also.
> 
> Hi Arnaldo,
> 
> If this patchset looks fine to you, can you please consider pulling it.

Thanks, applied to perf/core, for 5.17.

- Arnaldo

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/2] tools/perf: Include global and local variants for p_stage_cyc sort key
@ 2022-01-06 20:22         ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-01-06 20:22 UTC (permalink / raw)
  To: Athira Rajeev
  Cc: maddy, Nageswara Sastry, linux-perf-users, Jiri Olsa, kajoljain,
	Namhyung Kim, linuxppc-dev

Em Thu, Jan 06, 2022 at 04:21:05PM +0530, Athira Rajeev escreveu:
> 
> 
> > On 08-Dec-2021, at 9:21 AM, Nageswara Sastry <rnsastry@linux.ibm.com> wrote:
> > 
> > 
> > 
> > On 07/12/21 8:22 pm, Arnaldo Carvalho de Melo wrote:
> >> Em Fri, Dec 03, 2021 at 07:50:37AM +0530, Athira Rajeev escreveu:
> >>> Sort key p_stage_cyc is used to present the latency
> >>> cycles spend in pipeline stages. perf tool has local
> >>> p_stage_cyc sort key to display this info. There is no
> >>> global variant available for this sort key. local variant
> >>> shows latency in a sinlge sample, whereas, global value
> >>> will be useful to present the total latency (sum of
> >>> latencies) in the hist entry. It represents latency
> >>> number multiplied by the number of samples.
> >>> 
> >>> Add global (p_stage_cyc) and local variant
> >>> (local_p_stage_cyc) for this sort key. Use the
> >>> local_p_stage_cyc as default option for "mem" sort mode.
> >>> Also add this to list of dynamic sort keys and made the
> >>> "dynamic_headers" and "arch_specific_sort_keys" as static.
> >>> 
> >>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> >>> Reported-by: Namhyung Kim <namhyung@kernel.org>
> >> I got this for v1, does it stand for v2?
> >> Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> > 
> > 
> > Tested with v2 also.
> 
> Hi Arnaldo,
> 
> If this patchset looks fine to you, can you please consider pulling it.

Thanks, applied to perf/core, for 5.17.

- Arnaldo

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-01-06 20:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-03  2:20 [PATCH V2 1/2] tools/perf: Include global and local variants for p_stage_cyc sort key Athira Rajeev
2021-12-03  2:20 ` Athira Rajeev
2021-12-03  2:20 ` [PATCH V2 2/2] tools/perf: Update global/local variants for p_stage_cyc in powerpc Athira Rajeev
2021-12-03  2:20   ` Athira Rajeev
2021-12-07 14:52 ` [PATCH V2 1/2] tools/perf: Include global and local variants for p_stage_cyc sort key Arnaldo Carvalho de Melo
2021-12-07 14:52   ` Arnaldo Carvalho de Melo
2021-12-08  3:51   ` Nageswara Sastry
2021-12-08  3:51     ` Nageswara Sastry
2022-01-06 10:51     ` Athira Rajeev
2022-01-06 10:51       ` Athira Rajeev
2022-01-06 20:22       ` Arnaldo Carvalho de Melo
2022-01-06 20:22         ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.