2009-10-27 00:33:21

by Marti Raudsepp

[permalink] [raw]
Subject: [PATCH 0 of 2] Two small UI tweaks to perf

Hi!

Posting two small user interface tweaks to perf.

[PATCH 1 of 2] perf tools: notify user when unrecognized event is specified

When you specify an unknown --event, perf now outputs:

invalid or unsupported event: 'foobar'
Run 'perf list' for a list of valid events

[PATCH 2 of 2] perf tools: output 'perf list' to stdout not stderr

I certainly expected 'perf list' output to be sent to stdout. It's not errors
or notices: the user asked for a list.

Marti


2009-10-27 00:33:23

by Marti Raudsepp

[permalink] [raw]
Subject: [PATCH 1 of 2] perf tools: notify user when unrecognized event is specified

Previously no indication was given about what went wrong.

Signed-off-by: Marti Raudsepp <[email protected]>

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -677,6 +677,8 @@
if (ret != EVT_FAILED)
goto modifier;

+ fprintf(stderr, "invalid or unsupported event: '%s'\n", *str);
+ fprintf(stderr, "Run 'perf list' for a list of valid events\n");
return EVT_FAILED;

modifier:

2009-10-27 00:33:23

by Marti Raudsepp

[permalink] [raw]
Subject: [PATCH 2 of 2] perf tools: output 'perf list' to stdout not stderr

Writing to stdout is probably the expected behavior because the user
explicitly asked for a list.

Signed-off-by: Marti Raudsepp <[email protected]>

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -784,7 +784,7 @@
for_each_event(sys_dirent, evt_dir, evt_dirent, evt_next) {
snprintf(evt_path, MAXPATHLEN, "%s:%s",
sys_dirent.d_name, evt_dirent.d_name);
- fprintf(stderr, " %-42s [%s]\n", evt_path,
+ printf(" %-42s [%s]\n", evt_path,
event_type_descriptors[PERF_TYPE_TRACEPOINT+1]);
}
closedir(evt_dir);
@@ -801,8 +801,8 @@
unsigned int i, type, op, prev_type = -1;
char name[40];

- fprintf(stderr, "\n");
- fprintf(stderr, "List of pre-defined events (to be used in -e):\n");
+ printf("\n");
+ printf("List of pre-defined events (to be used in -e):\n");

for (i = 0; i < ARRAY_SIZE(event_symbols); i++, syms++) {
type = syms->type + 1;
@@ -810,19 +810,19 @@
type = 0;

if (type != prev_type)
- fprintf(stderr, "\n");
+ printf("\n");

if (strlen(syms->alias))
sprintf(name, "%s OR %s", syms->symbol, syms->alias);
else
strcpy(name, syms->symbol);
- fprintf(stderr, " %-42s [%s]\n", name,
+ printf(" %-42s [%s]\n", name,
event_type_descriptors[type]);

prev_type = type;
}

- fprintf(stderr, "\n");
+ printf("\n");
for (type = 0; type < PERF_COUNT_HW_CACHE_MAX; type++) {
for (op = 0; op < PERF_COUNT_HW_CACHE_OP_MAX; op++) {
/* skip invalid cache type */
@@ -830,17 +830,17 @@
continue;

for (i = 0; i < PERF_COUNT_HW_CACHE_RESULT_MAX; i++) {
- fprintf(stderr, " %-42s [%s]\n",
+ printf(" %-42s [%s]\n",
event_cache_name(type, op, i),
event_type_descriptors[4]);
}
}
}

- fprintf(stderr, "\n");
- fprintf(stderr, " %-42s [raw hardware event descriptor]\n",
+ printf("\n");
+ printf(" %-42s [raw hardware event descriptor]\n",
"rNNN");
- fprintf(stderr, "\n");
+ printf("\n");

print_tracepoint_events();

2009-10-27 13:52:18

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 0 of 2] Two small UI tweaks to perf


* Marti Raudsepp <[email protected]> wrote:

> Hi!
>
> Posting two small user interface tweaks to perf.
>
> [PATCH 1 of 2] perf tools: notify user when unrecognized event is specified
>
> When you specify an unknown --event, perf now outputs:
>
> invalid or unsupported event: 'foobar'
> Run 'perf list' for a list of valid events

ok, agreed!

> [PATCH 2 of 2] perf tools: output 'perf list' to stdout not stderr
>
> I certainly expected 'perf list' output to be sent to stdout. It's not errors
> or notices: the user asked for a list.

Certainly - i meant to fix this for some time - thanks for taking care
of it.

Ingo

2009-10-27 13:55:10

by Marti Raudsepp

[permalink] [raw]
Subject: [tip:perf/core] perf tools: Notify user when unrecognized event is specified

Commit-ID: 85df6f683efa457440eb922272fd5a71aa022ad4
Gitweb: http://git.kernel.org/tip/85df6f683efa457440eb922272fd5a71aa022ad4
Author: Marti Raudsepp <[email protected]>
AuthorDate: Tue, 27 Oct 2009 00:33:04 +0000
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 27 Oct 2009 14:52:31 +0100

perf tools: Notify user when unrecognized event is specified

Previously no indication was given about what went wrong.

Signed-off-by: Marti Raudsepp <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
LKML-Reference: <03ec9ee96f17cef05424.1256603584@localhost>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/util/parse-events.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index b097570..e9e6d5c 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -678,6 +678,8 @@ parse_event_symbols(const char **str, struct perf_event_attr *attr)
if (ret != EVT_FAILED)
goto modifier;

+ fprintf(stderr, "invalid or unsupported event: '%s'\n", *str);
+ fprintf(stderr, "Run 'perf list' for a list of valid events\n");
return EVT_FAILED;

modifier:

2009-10-27 13:55:28

by Marti Raudsepp

[permalink] [raw]
Subject: [tip:perf/core] perf tools: Output 'perf list' to stdout not stderr

Commit-ID: 689d30187828afe1faedf050b2f7593515b90c76
Gitweb: http://git.kernel.org/tip/689d30187828afe1faedf050b2f7593515b90c76
Author: Marti Raudsepp <[email protected]>
AuthorDate: Tue, 27 Oct 2009 00:33:05 +0000
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 27 Oct 2009 14:52:32 +0100

perf tools: Output 'perf list' to stdout not stderr

Writing to stdout is probably the expected behavior because the
user explicitly asked for a list.

Signed-off-by: Marti Raudsepp <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
LKML-Reference: <4ebb59420ef057972167.1256603585@localhost>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/util/parse-events.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index e9e6d5c..31baa5a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -806,7 +806,7 @@ static void print_tracepoint_events(void)
for_each_event(sys_dirent, evt_dir, evt_dirent, evt_next) {
snprintf(evt_path, MAXPATHLEN, "%s:%s",
sys_dirent.d_name, evt_dirent.d_name);
- fprintf(stderr, " %-42s [%s]\n", evt_path,
+ printf(" %-42s [%s]\n", evt_path,
event_type_descriptors[PERF_TYPE_TRACEPOINT+1]);
}
closedir(evt_dir);
@@ -823,8 +823,8 @@ void print_events(void)
unsigned int i, type, op, prev_type = -1;
char name[40];

- fprintf(stderr, "\n");
- fprintf(stderr, "List of pre-defined events (to be used in -e):\n");
+ printf("\n");
+ printf("List of pre-defined events (to be used in -e):\n");

for (i = 0; i < ARRAY_SIZE(event_symbols); i++, syms++) {
type = syms->type + 1;
@@ -832,19 +832,19 @@ void print_events(void)
type = 0;

if (type != prev_type)
- fprintf(stderr, "\n");
+ printf("\n");

if (strlen(syms->alias))
sprintf(name, "%s OR %s", syms->symbol, syms->alias);
else
strcpy(name, syms->symbol);
- fprintf(stderr, " %-42s [%s]\n", name,
+ printf(" %-42s [%s]\n", name,
event_type_descriptors[type]);

prev_type = type;
}

- fprintf(stderr, "\n");
+ printf("\n");
for (type = 0; type < PERF_COUNT_HW_CACHE_MAX; type++) {
for (op = 0; op < PERF_COUNT_HW_CACHE_OP_MAX; op++) {
/* skip invalid cache type */
@@ -852,17 +852,17 @@ void print_events(void)
continue;

for (i = 0; i < PERF_COUNT_HW_CACHE_RESULT_MAX; i++) {
- fprintf(stderr, " %-42s [%s]\n",
+ printf(" %-42s [%s]\n",
event_cache_name(type, op, i),
event_type_descriptors[4]);
}
}
}

- fprintf(stderr, "\n");
- fprintf(stderr, " %-42s [raw hardware event descriptor]\n",
+ printf("\n");
+ printf(" %-42s [raw hardware event descriptor]\n",
"rNNN");
- fprintf(stderr, "\n");
+ printf("\n");

print_tracepoint_events();