perf list: Also append PMU name in verbose mode

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

Posted by James Clark 11 months, 3 weeks 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 11 months, 3 weeks 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 11 months, 3 weeks 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 11 months, 3 weeks 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
>