Re: [PATCH v14 15/19] perf, tools: Support long descriptions with perf list

2015-06-06 Thread Jiri Olsa 
On Fri, Jun 05, 2015 at 11:16:45AM -0700, Sukadev Bhattiprolu wrote:

SNIP

> | > hum, it should be 'v' , right?
> 
> Good catch. Fixed.
> | 
> | Yes that's right.
> | 
> | Also BTW need to add the new option to the usage line a few lines below.
> 
> How about we do this for consistency with 'perf stat'(and shows the
> long options exactly once with 'perf list -h')?
> 
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index d0f7a18..f800927 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -27,12 +27,12 @@ int cmd_list(int argc, const char **argv, const char 
> *prefi
> OPT_BOOLEAN(0, "raw-dump", _dump, "Dump raw events"),
> OPT_BOOLEAN('d', "desc", _flag,
> "Print extra event descriptions. --no-desc to not 
> p
> -   OPT_BOOLEAN('d', "long-desc", _desc_flag,
> +   OPT_BOOLEAN('v', "long-desc", _desc_flag,
> "Print longer event descriptions."),
> OPT_END()
> };
> const char * const list_usage[] = {
> -   "perf list [--no-desc] 
> [hw|sw|cache|tracepoint|pmu|event_glob]"
> +   "perf list [] 
> [hw|sw|cache|tracepoint|pmu|event_glob]"
> NULL

sounds good to me, and plus the doc update ;-)

thanks,
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v14 15/19] perf, tools: Support long descriptions with perf list

2015-06-06 Thread Jiri Olsa 
On Fri, Jun 05, 2015 at 11:16:45AM -0700, Sukadev Bhattiprolu wrote:

SNIP

 |  hum, it should be 'v' , right?
 
 Good catch. Fixed.
 | 
 | Yes that's right.
 | 
 | Also BTW need to add the new option to the usage line a few lines below.
 
 How about we do this for consistency with 'perf stat'(and shows the
 long options exactly once with 'perf list -h')?
 
 diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
 index d0f7a18..f800927 100644
 --- a/tools/perf/builtin-list.c
 +++ b/tools/perf/builtin-list.c
 @@ -27,12 +27,12 @@ int cmd_list(int argc, const char **argv, const char 
 *prefi
 OPT_BOOLEAN(0, raw-dump, raw_dump, Dump raw events),
 OPT_BOOLEAN('d', desc, desc_flag,
 Print extra event descriptions. --no-desc to not 
 p
 -   OPT_BOOLEAN('d', long-desc, long_desc_flag,
 +   OPT_BOOLEAN('v', long-desc, long_desc_flag,
 Print longer event descriptions.),
 OPT_END()
 };
 const char * const list_usage[] = {
 -   perf list [--no-desc] 
 [hw|sw|cache|tracepoint|pmu|event_glob]
 +   perf list [options] 
 [hw|sw|cache|tracepoint|pmu|event_glob]
 NULL

sounds good to me, and plus the doc update ;-)

thanks,
jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v14 15/19] perf, tools: Support long descriptions with perf list

2015-06-05 Thread Sukadev Bhattiprolu 
Andi Kleen [a...@linux.intel.com] wrote:
| On Fri, Jun 05, 2015 at 12:21:38PM +0200, Jiri Olsa wrote:
| > On Thu, Jun 04, 2015 at 11:27:23PM -0700, Sukadev Bhattiprolu wrote:
| > 
| > SNIP
| > 
| > > ---
| > >  tools/perf/builtin-list.c |   11 ---
| > >  1 file changed, 8 insertions(+), 3 deletions(-)
| > > 
| > > diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
| > > index 3f058f7..d0f7a18 100644
| > > --- a/tools/perf/builtin-list.c
| > > +++ b/tools/perf/builtin-list.c
| > > @@ -22,10 +22,13 @@ int cmd_list(int argc, const char **argv, const char 
*prefix __maybe_unused)
| > >  {
| > >   int i;
| > >   bool raw_dump = false;
| > > + bool long_desc_flag = false;
| > >   struct option list_options[] = {
| > >   OPT_BOOLEAN(0, "raw-dump", _dump, "Dump raw events"),
| > >   OPT_BOOLEAN('d', "desc", _flag,
| > >   "Print extra event descriptions. --no-desc to not 
print."),
| > > + OPT_BOOLEAN('d', "long-desc", _desc_flag,
| > > + "Print longer event descriptions."),
| > 
| > hum, it should be 'v' , right?

Good catch. Fixed.
| 
| Yes that's right.
| 
| Also BTW need to add the new option to the usage line a few lines below.

How about we do this for consistency with 'perf stat'(and shows the
long options exactly once with 'perf list -h')?

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index d0f7a18..f800927 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -27,12 +27,12 @@ int cmd_list(int argc, const char **argv, const char *prefi
OPT_BOOLEAN(0, "raw-dump", _dump, "Dump raw events"),
OPT_BOOLEAN('d', "desc", _flag,
"Print extra event descriptions. --no-desc to not p
-   OPT_BOOLEAN('d', "long-desc", _desc_flag,
+   OPT_BOOLEAN('v', "long-desc", _desc_flag,
"Print longer event descriptions."),
OPT_END()
};
const char * const list_usage[] = {
-   "perf list [--no-desc] [hw|sw|cache|tracepoint|pmu|event_glob]"
+   "perf list [] [hw|sw|cache|tracepoint|pmu|event_glob]"
NULL
};
| 
| -Andi
| 
| -- 
| a...@linux.intel.com -- Speaking for myself only

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v14 15/19] perf, tools: Support long descriptions with perf list

2015-06-05 Thread Andi Kleen 
On Fri, Jun 05, 2015 at 12:21:38PM +0200, Jiri Olsa wrote:
> On Thu, Jun 04, 2015 at 11:27:23PM -0700, Sukadev Bhattiprolu wrote:
> 
> SNIP
> 
> > ---
> >  tools/perf/builtin-list.c |   11 ---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> > index 3f058f7..d0f7a18 100644
> > --- a/tools/perf/builtin-list.c
> > +++ b/tools/perf/builtin-list.c
> > @@ -22,10 +22,13 @@ int cmd_list(int argc, const char **argv, const char 
> > *prefix __maybe_unused)
> >  {
> > int i;
> > bool raw_dump = false;
> > +   bool long_desc_flag = false;
> > struct option list_options[] = {
> > OPT_BOOLEAN(0, "raw-dump", _dump, "Dump raw events"),
> > OPT_BOOLEAN('d', "desc", _flag,
> > "Print extra event descriptions. --no-desc to not 
> > print."),
> > +   OPT_BOOLEAN('d', "long-desc", _desc_flag,
> > +   "Print longer event descriptions."),
> 
> hum, it should be 'v' , right?

Yes that's right.

Also BTW need to add the new option to the usage line a few lines below.

-Andi

-- 
a...@linux.intel.com -- Speaking for myself only
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v14 15/19] perf, tools: Support long descriptions with perf list

2015-06-05 Thread Jiri Olsa 
On Thu, Jun 04, 2015 at 11:27:23PM -0700, Sukadev Bhattiprolu wrote:

SNIP

> ---
>  tools/perf/builtin-list.c |   11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 3f058f7..d0f7a18 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -22,10 +22,13 @@ int cmd_list(int argc, const char **argv, const char 
> *prefix __maybe_unused)
>  {
>   int i;
>   bool raw_dump = false;
> + bool long_desc_flag = false;
>   struct option list_options[] = {
>   OPT_BOOLEAN(0, "raw-dump", _dump, "Dump raw events"),
>   OPT_BOOLEAN('d', "desc", _flag,
>   "Print extra event descriptions. --no-desc to not 
> print."),
> + OPT_BOOLEAN('d', "long-desc", _desc_flag,
> + "Print longer event descriptions."),

hum, it should be 'v' , right?

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v14 15/19] perf, tools: Support long descriptions with perf list

2015-06-05 Thread Sukadev Bhattiprolu 
Previously we were dropping the useful longer descriptions that some
events have in the event list completely. This patch makes them appear with
perf list.

Old perf list:

baclears:
  baclears.all
   [Counts the number of baclears]

vs new:

perf list -v:
...
baclears:
  baclears.all
   [The BACLEARS event counts the number of times the front end is
resteered, mainly when the Branch Prediction Unit cannot provide
a correct prediction and this is corrected by the Branch Address
Calculator at the front end. The BACLEARS.ANY event counts the
number of baclears for any type of branch]

Signed-off-by: Andi Kleen 
Signed-off-by: Sukadev Bhattiprolu 

Changelog[v14]
- [Jiri Olsa] Break up independent parts of the patch into
  separate patches.
---
 tools/perf/builtin-list.c |   11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 3f058f7..d0f7a18 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -22,10 +22,13 @@ int cmd_list(int argc, const char **argv, const char 
*prefix __maybe_unused)
 {
int i;
bool raw_dump = false;
+   bool long_desc_flag = false;
struct option list_options[] = {
OPT_BOOLEAN(0, "raw-dump", _dump, "Dump raw events"),
OPT_BOOLEAN('d', "desc", _flag,
"Print extra event descriptions. --no-desc to not 
print."),
+   OPT_BOOLEAN('d', "long-desc", _desc_flag,
+   "Print longer event descriptions."),
OPT_END()
};
const char * const list_usage[] = {
@@ -44,7 +47,7 @@ int cmd_list(int argc, const char **argv, const char *prefix 
__maybe_unused)
printf("\nList of pre-defined events (to be used in -e):\n\n");
 
if (argc == 0) {
-   print_events(NULL, raw_dump, !desc_flag);
+   print_events(NULL, raw_dump, !desc_flag, long_desc_flag);
return 0;
}
 
@@ -63,13 +66,15 @@ int cmd_list(int argc, const char **argv, const char 
*prefix __maybe_unused)
 strcmp(argv[i], "hwcache") == 0)
print_hwcache_events(NULL, raw_dump);
else if (strcmp(argv[i], "pmu") == 0)
-   print_pmu_events(NULL, raw_dump, !desc_flag);
+   print_pmu_events(NULL, raw_dump, !desc_flag,
+   long_desc_flag);
else {
char *sep = strchr(argv[i], ':'), *s;
int sep_idx;
 
if (sep == NULL) {
-   print_events(argv[i], raw_dump, !desc_flag);
+   print_events(argv[i], raw_dump, !desc_flag,
+   long_desc_flag);
continue;
}
sep_idx = sep - argv[i];
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v14 15/19] perf, tools: Support long descriptions with perf list

2015-06-05 Thread Sukadev Bhattiprolu 
Previously we were dropping the useful longer descriptions that some
events have in the event list completely. This patch makes them appear with
perf list.

Old perf list:

baclears:
  baclears.all
   [Counts the number of baclears]

vs new:

perf list -v:
...
baclears:
  baclears.all
   [The BACLEARS event counts the number of times the front end is
resteered, mainly when the Branch Prediction Unit cannot provide
a correct prediction and this is corrected by the Branch Address
Calculator at the front end. The BACLEARS.ANY event counts the
number of baclears for any type of branch]

Signed-off-by: Andi Kleen a...@linux.intel.com
Signed-off-by: Sukadev Bhattiprolu suka...@linux.vnet.ibm.com

Changelog[v14]
- [Jiri Olsa] Break up independent parts of the patch into
  separate patches.
---
 tools/perf/builtin-list.c |   11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 3f058f7..d0f7a18 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -22,10 +22,13 @@ int cmd_list(int argc, const char **argv, const char 
*prefix __maybe_unused)
 {
int i;
bool raw_dump = false;
+   bool long_desc_flag = false;
struct option list_options[] = {
OPT_BOOLEAN(0, raw-dump, raw_dump, Dump raw events),
OPT_BOOLEAN('d', desc, desc_flag,
Print extra event descriptions. --no-desc to not 
print.),
+   OPT_BOOLEAN('d', long-desc, long_desc_flag,
+   Print longer event descriptions.),
OPT_END()
};
const char * const list_usage[] = {
@@ -44,7 +47,7 @@ int cmd_list(int argc, const char **argv, const char *prefix 
__maybe_unused)
printf(\nList of pre-defined events (to be used in -e):\n\n);
 
if (argc == 0) {
-   print_events(NULL, raw_dump, !desc_flag);
+   print_events(NULL, raw_dump, !desc_flag, long_desc_flag);
return 0;
}
 
@@ -63,13 +66,15 @@ int cmd_list(int argc, const char **argv, const char 
*prefix __maybe_unused)
 strcmp(argv[i], hwcache) == 0)
print_hwcache_events(NULL, raw_dump);
else if (strcmp(argv[i], pmu) == 0)
-   print_pmu_events(NULL, raw_dump, !desc_flag);
+   print_pmu_events(NULL, raw_dump, !desc_flag,
+   long_desc_flag);
else {
char *sep = strchr(argv[i], ':'), *s;
int sep_idx;
 
if (sep == NULL) {
-   print_events(argv[i], raw_dump, !desc_flag);
+   print_events(argv[i], raw_dump, !desc_flag,
+   long_desc_flag);
continue;
}
sep_idx = sep - argv[i];
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v14 15/19] perf, tools: Support long descriptions with perf list

2015-06-05 Thread Sukadev Bhattiprolu 
Andi Kleen [a...@linux.intel.com] wrote:
| On Fri, Jun 05, 2015 at 12:21:38PM +0200, Jiri Olsa wrote:
|  On Thu, Jun 04, 2015 at 11:27:23PM -0700, Sukadev Bhattiprolu wrote:
|  
|  SNIP
|  
|   ---
|tools/perf/builtin-list.c |   11 ---
|1 file changed, 8 insertions(+), 3 deletions(-)
|   
|   diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
|   index 3f058f7..d0f7a18 100644
|   --- a/tools/perf/builtin-list.c
|   +++ b/tools/perf/builtin-list.c
|   @@ -22,10 +22,13 @@ int cmd_list(int argc, const char **argv, const char 
*prefix __maybe_unused)
|{
| int i;
| bool raw_dump = false;
|   + bool long_desc_flag = false;
| struct option list_options[] = {
| OPT_BOOLEAN(0, raw-dump, raw_dump, Dump raw events),
| OPT_BOOLEAN('d', desc, desc_flag,
| Print extra event descriptions. --no-desc to not 
print.),
|   + OPT_BOOLEAN('d', long-desc, long_desc_flag,
|   + Print longer event descriptions.),
|  
|  hum, it should be 'v' , right?

Good catch. Fixed.
| 
| Yes that's right.
| 
| Also BTW need to add the new option to the usage line a few lines below.

How about we do this for consistency with 'perf stat'(and shows the
long options exactly once with 'perf list -h')?

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index d0f7a18..f800927 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -27,12 +27,12 @@ int cmd_list(int argc, const char **argv, const char *prefi
OPT_BOOLEAN(0, raw-dump, raw_dump, Dump raw events),
OPT_BOOLEAN('d', desc, desc_flag,
Print extra event descriptions. --no-desc to not p
-   OPT_BOOLEAN('d', long-desc, long_desc_flag,
+   OPT_BOOLEAN('v', long-desc, long_desc_flag,
Print longer event descriptions.),
OPT_END()
};
const char * const list_usage[] = {
-   perf list [--no-desc] [hw|sw|cache|tracepoint|pmu|event_glob]
+   perf list [options] [hw|sw|cache|tracepoint|pmu|event_glob]
NULL
};
| 
| -Andi
| 
| -- 
| a...@linux.intel.com -- Speaking for myself only

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v14 15/19] perf, tools: Support long descriptions with perf list

2015-06-05 Thread Andi Kleen 
On Fri, Jun 05, 2015 at 12:21:38PM +0200, Jiri Olsa wrote:
 On Thu, Jun 04, 2015 at 11:27:23PM -0700, Sukadev Bhattiprolu wrote:
 
 SNIP
 
  ---
   tools/perf/builtin-list.c |   11 ---
   1 file changed, 8 insertions(+), 3 deletions(-)
  
  diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
  index 3f058f7..d0f7a18 100644
  --- a/tools/perf/builtin-list.c
  +++ b/tools/perf/builtin-list.c
  @@ -22,10 +22,13 @@ int cmd_list(int argc, const char **argv, const char 
  *prefix __maybe_unused)
   {
  int i;
  bool raw_dump = false;
  +   bool long_desc_flag = false;
  struct option list_options[] = {
  OPT_BOOLEAN(0, raw-dump, raw_dump, Dump raw events),
  OPT_BOOLEAN('d', desc, desc_flag,
  Print extra event descriptions. --no-desc to not 
  print.),
  +   OPT_BOOLEAN('d', long-desc, long_desc_flag,
  +   Print longer event descriptions.),
 
 hum, it should be 'v' , right?

Yes that's right.

Also BTW need to add the new option to the usage line a few lines below.

-Andi

-- 
a...@linux.intel.com -- Speaking for myself only
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v14 15/19] perf, tools: Support long descriptions with perf list

2015-06-05 Thread Jiri Olsa 
On Thu, Jun 04, 2015 at 11:27:23PM -0700, Sukadev Bhattiprolu wrote:

SNIP

 ---
  tools/perf/builtin-list.c |   11 ---
  1 file changed, 8 insertions(+), 3 deletions(-)
 
 diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
 index 3f058f7..d0f7a18 100644
 --- a/tools/perf/builtin-list.c
 +++ b/tools/perf/builtin-list.c
 @@ -22,10 +22,13 @@ int cmd_list(int argc, const char **argv, const char 
 *prefix __maybe_unused)
  {
   int i;
   bool raw_dump = false;
 + bool long_desc_flag = false;
   struct option list_options[] = {
   OPT_BOOLEAN(0, raw-dump, raw_dump, Dump raw events),
   OPT_BOOLEAN('d', desc, desc_flag,
   Print extra event descriptions. --no-desc to not 
 print.),
 + OPT_BOOLEAN('d', long-desc, long_desc_flag,
 + Print longer event descriptions.),

hum, it should be 'v' , right?

jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/