perf list: Also append PMU name in verbose mode

[PATCH] perf list: Also append PMU name in verbose mode

Posted by James Clark 10 months ago

When listing in verbose mode, the long description is used but the PMU
name isn't appended. There doesn't seem to be a reason to exclude it
when asking for more information, so use the same print block for both
long and short descriptions.

Before:
  $ perf list -v
  ...
  inst_retired
       [Instruction architecturally executed]

After:
  $ perf list -v
  ...
   inst_retired
       [Instruction architecturally executed. Unit: armv8_cortex_a57]

Signed-off-by: James Clark <james.clark@linaro.org>
---
 tools/perf/builtin-list.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 9e7fdfcdd7ff..c19826f218a0 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -163,11 +163,10 @@ static void default_print_event(void *ps, const char *topic, const char *pmu_nam
 	} else
 		fputc('\n', fp);
 
-	if (long_desc && print_state->long_desc) {
-		fprintf(fp, "%*s", 8, "[");
-		wordwrap(fp, long_desc, 8, pager_get_columns(), 0);
-		fprintf(fp, "]\n");
-	} else if (desc && print_state->desc) {
+	if (long_desc && print_state->long_desc)
+		desc = long_desc;
+
+	if (desc && (print_state->desc || print_state->long_desc)) {
 		char *desc_with_unit = NULL;
 		int desc_len = -1;
 
-- 
2.34.1

Re: [PATCH] perf list: Also append PMU name in verbose mode

Posted by Namhyung Kim 10 months ago

On Wed, 19 Feb 2025 15:16:21 +0000, James Clark wrote:
> When listing in verbose mode, the long description is used but the PMU
> name isn't appended. There doesn't seem to be a reason to exclude it
> when asking for more information, so use the same print block for both
> long and short descriptions.
> 
> Before:
>   $ perf list -v
>   ...
>   inst_retired
>        [Instruction architecturally executed]
> 
> [...]
Applied to perf-tools-next, thanks!

Best regards,
Namhyung

Re: [PATCH] perf list: Also append PMU name in verbose mode

Posted by Yangyu Chen 10 months ago


> On 19 Feb 2025, at 23:16, James Clark <james.clark@linaro.org> wrote:
> 
> When listing in verbose mode, the long description is used but the PMU
> name isn't appended. There doesn't seem to be a reason to exclude it
> when asking for more information, so use the same print block for both
> long and short descriptions.
> 
> Before:
>  $ perf list -v
>  ...
>  inst_retired
>       [Instruction architecturally executed]
> 
> After:
>  $ perf list -v
>  ...
>   inst_retired
>       [Instruction architecturally executed. Unit: armv8_cortex_a57]
> 
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> tools/perf/builtin-list.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 9e7fdfcdd7ff..c19826f218a0 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -163,11 +163,10 @@ static void default_print_event(void *ps, const char *topic, const char *pmu_nam
> } else
> fputc('\n', fp);
> 
> - if (long_desc && print_state->long_desc) {
> - fprintf(fp, "%*s", 8, "[");
> - wordwrap(fp, long_desc, 8, pager_get_columns(), 0);
> - fprintf(fp, "]\n");
> - } else if (desc && print_state->desc) {
> + if (long_desc && print_state->long_desc)
> + desc = long_desc;
> +
> + if (desc && (print_state->desc || print_state->long_desc)) {
> char *desc_with_unit = NULL;
> int desc_len = -1;
> 
> -- 
> 2.34.1
> 
> When listing in verbose mode, the long description is used but the PMU
> name isn't appended. There doesn't seem to be a reason to exclude it
> when asking for more information, so use the same print block for both
> long and short descriptions.
> 
> Before:
>  $ perf list -v
>  ...
>  inst_retired
>       [Instruction architecturally executed]
> 
> After:
>  $ perf list -v
>  ...
>   inst_retired
>       [Instruction architecturally executed. Unit: armv8_cortex_a57]
> 
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> tools/perf/builtin-list.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 9e7fdfcdd7ff..c19826f218a0 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -163,11 +163,10 @@ static void default_print_event(void *ps, const char *topic, const char *pmu_nam
> } else
> fputc('\n', fp);
> 
> - if (long_desc && print_state->long_desc) {
> - fprintf(fp, "%*s", 8, "[");
> - wordwrap(fp, long_desc, 8, pager_get_columns(), 0);
> - fprintf(fp, "]\n");
> - } else if (desc && print_state->desc) {
> + if (long_desc && print_state->long_desc)
> + desc = long_desc;
> +
> + if (desc && (print_state->desc || print_state->long_desc)) {
> char *desc_with_unit = NULL;
> int desc_len = -1;
> 
> -- 
> 2.34.1
> 
> When listing in verbose mode, the long description is used but the PMU
> name isn't appended. There doesn't seem to be a reason to exclude it
> when asking for more information, so use the same print block for both
> long and short descriptions.
> 
> Before:
>  $ perf list -v
>  ...
>  inst_retired
>       [Instruction architecturally executed]
> 
> After:
>  $ perf list -v
>  ...
>   inst_retired
>       [Instruction architecturally executed. Unit: armv8_cortex_a57]
> 
> Signed-off-by: James Clark <james.clark@linaro.org>

Tested-by: Yangyu Chen <cyy@cyyself.name>

Thanks,
Yangyu Chen

Re: [PATCH] perf list: Also append PMU name in verbose mode

Posted by Ian Rogers 10 months ago

On Wed, Feb 19, 2025 at 7:17 AM James Clark <james.clark@linaro.org> wrote:
>
> When listing in verbose mode, the long description is used but the PMU
> name isn't appended. There doesn't seem to be a reason to exclude it
> when asking for more information, so use the same print block for both
> long and short descriptions.
>
> Before:
>   $ perf list -v
>   ...
>   inst_retired
>        [Instruction architecturally executed]
>
> After:
>   $ perf list -v
>   ...
>    inst_retired
>        [Instruction architecturally executed. Unit: armv8_cortex_a57]
>
> Signed-off-by: James Clark <james.clark@linaro.org>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/builtin-list.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 9e7fdfcdd7ff..c19826f218a0 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -163,11 +163,10 @@ static void default_print_event(void *ps, const char *topic, const char *pmu_nam
>         } else
>                 fputc('\n', fp);
>
> -       if (long_desc && print_state->long_desc) {
> -               fprintf(fp, "%*s", 8, "[");
> -               wordwrap(fp, long_desc, 8, pager_get_columns(), 0);
> -               fprintf(fp, "]\n");
> -       } else if (desc && print_state->desc) {
> +       if (long_desc && print_state->long_desc)
> +               desc = long_desc;
> +
> +       if (desc && (print_state->desc || print_state->long_desc)) {
>                 char *desc_with_unit = NULL;
>                 int desc_len = -1;
>
> --
> 2.34.1
>