On 16-Mar-2021, at 4:48 AM, Jiri Olsa <jolsa@redhat.com> wrote:
=
On Mon, Mar 15, 2021 at 01:22:09PM +0530, Athira Rajeev wrote:

S= NIP

+
+static char *setup_dynamic_sort_= keys(char *str)
+{
+ unsigned int j;
+
+ if (sort__mode =3D=3D SORT_MODE__MEMORY)
= + for (j =3D 0; j <= ; ARRAY_SIZE(dynamic_sort_keys_mem); j++)
+ if (arch_support_dynamic_key(dynamic_sort_keys_mem[j])) {+ str =3D suffix_if_not_in(dynamic_sort= _keys_mem[j], str);
+ if (str =3D=3D N= ULL)
+ <= span class=3D"Apple-tab-span" style=3D"white-space:pre"> return str;
+ }
+
+ return str;
+}
+
static int __setup_sorting(s= truct evlist *evlist)
{
char *str;
@@ -3050,6 +3085,12 @@ static int __setup= _sorting(struct evlist *evlist)
}
}

+ str =3D setup_dynamic_sort_keys(str);
+ if (str =3D=3D NULL) {
+ pr_err("Not enough memory = to setup dynamic sort keys");
+ return -ENOMEM;
+ }

hum, so this is basicaly over= loading 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_dim= ension__add or somehow add arch's specific stuff to
memory_sort_dimensio= ns

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_orde= r 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 appro= ach
which is easier. The new sort dimension "p_stage_cyc" is presently o= nly supported
on powerpc. For unsupported platforms, we don't want to di= splay it
in the perf report output columns. Hence added check in sort_di= mension__add()
and skip the sort key incase its not applicable for parti= cular arch.

Please help to check if below approach looks fine.

diff --git a/tools/perf/arch/powerpc/util/event.c b/tools/perf/arch/p= owerpc/util/event.c
index b80fbee83b6e..7205767d75eb 100644
--- a/too= ls/perf/arch/powerpc/util/event.c
+++ b/tools/perf/arch/powerpc/util/eve= nt.c
@@ -44,3 +44,10 @@ const char *arch_perf_header_entry__add(const ch= ar *se_header)
r= eturn "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 --g= it a/tools/perf/util/event.h b/tools/perf/util/event.h
index 65f89e80916= f..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, cha= r *str);
void arch_perf_parse_sample_weight(struct perf_sample *data, c= onst __u64 *array, u64 type);
void arch_perf_synthesize_sample_weight(c= onst struct perf_sample *data, __u64 *array, u64 type);
const char *arc= h_perf_header_entry__add(const char *se_header);
+int arch_support_sort_= key(const char *sort_key);

#endif /* __PERF_RECORD_H */
diff --g= it a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index cbb3899e7eca.= =2Ed8b0b0b43a81 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 =3D 0;
enum sort_mode<= span class=3D"Apple-tab-span" style=3D"white-space:pre"> sort__mode = =3D SORT_MODE__NORMAL;
const char *dynamic_headers[] =3D {"local_ins_lat", "p_st= age_cyc"};
+const char *arch_specific_sort_keys[] =3D {"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 +2= 779,18 @@ int sort_dimension__add(struct perf_hpp_list *list, const char *t= ok,
{
unsigned int i, j;

+ /* Check to see if there are any arch specific
+ * sort dimen= sions not applicable for the current
+ * architecture. If so, Skip that sort key s= ince
+ = * we don't want to display it in the output fields.
+ */
+ for (j =3D 0; j < ARRAY_SIZE= (arch_specific_sort_keys); j++) {
+ if (!strcmp(arch_specific_sort_keys[j], tok) &&<= br>+ !arch_support_sort_key(tok)) {
+<= span class=3D"Apple-tab-span" style=3D"white-space:pre"> return 0;
+ }
+ }
+
for (i =3D 0; i < ARRAY_SIZE(common_s= ort_dimensions); i++) {
= struct sort_dimension *sd =3D &common_sort_dimensions[i];
=E2=80=94
2.26.2

Thanks
Athira

jirka