Re: [PATCH 4/4] tools/perf: Support pipeline stage cycles for powerpc

[Athira Rajeev]

On 16-Mar-2021, at 4:48 AM, Jiri Olsa <jo...@redhat.com> wrote: On Mon, Mar 15, 2021 at 01:22:09PM +0530, Athira Rajeev wrote: SNIP + +static char *setup_dynamic_sort_keys(char *str) +{ + unsigned int j; + + if (sort__mode == SORT_MODE__MEMORY) + for (j = 0; j < ARRAY_SIZE(dynamic_sort_keys_mem); j++) + if (arch_support_dynamic_key(dynamic_sort_keys_mem[j])) { + str = suffix_if_not_in(dynamic_sort_keys_mem[j], str); + if (str == NULL) + return str; + } + + return str; +} + static int __setup_sorting(struct evlist *evlist) { char *str; @@ -3050,6 +3085,12 @@ static int __setup_sorting(struct evlist *evlist) } } + str = setup_dynamic_sort_keys(str); + if (str == NULL) { + pr_err("Not enough memory to setup dynamic sort keys"); + return -ENOMEM; + } hum, so this is basicaly overloading the default_mem_sort_order for architecture, right? then I think it'd be easier just overload default_mem_sort_order directly I was thinking more about adding extra (arch specific) loop to sort_dimension__add or somehow add arch's specific stuff to memory_sort_dimensions Hi Jiri, Above patch was to append additional sort keys to sort order based on sort mode and architecture support. I had initially thought of defining two orders ( say default_mem_sort_order plus mem_sort_order_pstage ). But if new sort keys gets added for mem mode in future, we will need to keep updating both orders. So preferred the approach of "appending" supported sort keys to default order. Following your thought on using "sort_dimension__add", I tried below approach which is easier. The new sort dimension "p_stage_cyc" is presently only supported on powerpc. For unsupported platforms, we don't want to display it in the perf report output columns. Hence added check in sort_dimension__add() and skip the sort key incase its not applicable for particular arch. Please help to check if below approach looks fine. diff --git a/tools/perf/arch/powerpc/util/event.c b/tools/perf/arch/powerpc/util/event.c index b80fbee83b6e..7205767d75eb 100644 --- a/tools/perf/arch/powerpc/util/event.c +++ b/tools/perf/arch/powerpc/util/event.c @@ -44,3 +44,10 @@ const char *arch_perf_header_entry__add(const char *se_header) return "Dispatch Cyc"; return se_header; } + +int arch_support_sort_key(const char *sort_key) +{ + if (!strcmp(sort_key, "p_stage_cyc")) + return 1; + return 0; +} diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h index 65f89e80916f..612a92aaaefb 100644 --- a/tools/perf/util/event.h +++ b/tools/perf/util/event.h @@ -429,5 +429,6 @@ char *get_page_size_name(u64 size, char *str); void arch_perf_parse_sample_weight(struct perf_sample *data, const __u64 *array, u64 type); void arch_perf_synthesize_sample_weight(const struct perf_sample *data, __u64 *array, u64 type); const char *arch_perf_header_entry__add(const char *se_header); +int arch_support_sort_key(const char *sort_key); #endif /* __PERF_RECORD_H */ diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c index cbb3899e7eca..d8b0b0b43a81 100644 --- a/tools/perf/util/sort.c +++ b/tools/perf/util/sort.c @@ -47,6 +47,7 @@ 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"}; /*  * Replaces all occurrences of a char used with the: @@ -1837,6 +1838,11 @@ struct sort_dimension { int taken; }; +int __weak arch_support_sort_key(const char *sort_key __maybe_unused) +{ + return 0; +} + const char * __weak arch_perf_header_entry__add(const char *se_header) { return se_header; @@ -2773,6 +2779,18 @@ int sort_dimension__add(struct perf_hpp_list *list, const char *tok, { unsigned int i, j; + /* Check to see if there are any arch specific + * sort dimensions not applicable for the current + * architecture. If so, Skip that sort key since + * we don't want to display it in the output fields. + */ + for (j = 0; j < ARRAY_SIZE(arch_specific_sort_keys); j++) { + if (!strcmp(arch_specific_sort_keys[j], tok) && + !arch_support_sort_key(tok)) { + return 0; + } + } + for (i = 0; i < ARRAY_SIZE(common_sort_dimensions); i++) { struct sort_dimension *sd = &common_sort_dimensions[i]; — 2.26.2 Thanks Athira jirka