[RFC/PATCH 1/3] perf probe: Split add_perf_probe_events()

[RFC/PATCH 1/3] perf probe: Split add_perf_probe_events()

[Namhyung Kim]

The add_perf_probe_events() does 3 things:

 1. convert all perf events to trace events
 2. add all trace events to kernel
 3. cleanup all trace events

But sometimes we need to do something with the trace events.  So split
the funtion into two, so that it can access intermediate trace events
via struct __event_package if needed.

Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/probe-event.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index eb5f18b75402..8eaa03428d72 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2765,9 +2765,10 @@ struct __event_package {
 	int				ntevs;
 };
 
-int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
+static int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
+				   struct __event_package **ppkgs)
 {
-	int i, j, ret;
+	int i, ret;
 	struct __event_package *pkgs;
 
 	ret = 0;
@@ -2792,7 +2793,7 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 		ret  = convert_to_probe_trace_events(pkgs[i].pev,
 						     &pkgs[i].tevs);
 		if (ret < 0)
-			goto end;
+			return ret;
 		pkgs[i].ntevs = ret;
 	}
 	/* This just release blacklist only if allocated */
@@ -2806,7 +2807,19 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 		if (ret < 0)
 			break;
 	}
-end:
+
+	*ppkgs = pkgs;
+
+	return 0;
+}
+
+static void cleanup_perf_probe_events(struct __event_package *pkgs, int npevs)
+{
+	int i, j;
+
+	if (pkgs == NULL)
+		return;
+
 	/* Loop 3: cleanup and free trace events  */
 	for (i = 0; i < npevs; i++) {
 		for (j = 0; j < pkgs[i].ntevs; j++)
@@ -2815,6 +2828,15 @@ end:
 	}
 	free(pkgs);
 	exit_symbol_maps();
+}
+
+int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
+{
+	int ret;
+	struct __event_package *pkgs = NULL;
+
+	ret = __add_perf_probe_events(pevs, npevs, &pkgs);
+	cleanup_perf_probe_events(pkgs, npevs);
 
 	return ret;
 }
-- 
2.5.0

[RFC/PATCH 2/3] perf probe: Rename __event_package to probe_event_package

[Namhyung Kim]

The struct __event_package can be accessed now from other than
probe-event.c code.  So rename it to more specific name.

Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/probe-event.c | 18 ++++++------------
 tools/perf/util/probe-event.h | 10 ++++++++++
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 8eaa03428d72..eef39338bb2a 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2759,20 +2759,14 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
 	return find_probe_trace_events_from_map(pev, tevs);
 }
 
-struct __event_package {
-	struct perf_probe_event		*pev;
-	struct probe_trace_event	*tevs;
-	int				ntevs;
-};
-
-static int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
-				   struct __event_package **ppkgs)
+int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
+			    struct probe_event_package **ppkgs)
 {
 	int i, ret;
-	struct __event_package *pkgs;
+	struct probe_event_package *pkgs;
 
 	ret = 0;
-	pkgs = zalloc(sizeof(struct __event_package) * npevs);
+	pkgs = zalloc(sizeof(struct probe_event_package) * npevs);
 
 	if (pkgs == NULL)
 		return -ENOMEM;
@@ -2813,7 +2807,7 @@ static int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
 	return 0;
 }
 
-static void cleanup_perf_probe_events(struct __event_package *pkgs, int npevs)
+void cleanup_perf_probe_events(struct probe_event_package *pkgs, int npevs)
 {
 	int i, j;
 
@@ -2833,7 +2827,7 @@ static void cleanup_perf_probe_events(struct __event_package *pkgs, int npevs)
 int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 {
 	int ret;
-	struct __event_package *pkgs = NULL;
+	struct probe_event_package *pkgs = NULL;
 
 	ret = __add_perf_probe_events(pevs, npevs, &pkgs);
 	cleanup_perf_probe_events(pkgs, npevs);
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 6e7ec68a4aa8..73f922fa7cac 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -89,6 +89,12 @@ struct perf_probe_event {
 	struct perf_probe_arg	*args;	/* Arguments */
 };
 
+struct probe_event_package {
+	struct perf_probe_event		*pev;
+	struct probe_trace_event	*tevs;
+	int				ntevs;
+};
+
 /* Line range */
 struct line_range {
 	char			*file;		/* File name */
@@ -138,6 +144,10 @@ extern void line_range__clear(struct line_range *lr);
 extern int line_range__init(struct line_range *lr);
 
 extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs);
+extern int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
+				   struct probe_event_package **ppkgs);
+extern void cleanup_perf_probe_events(struct probe_event_package *pkgs,
+				      int npevs);
 extern int del_perf_probe_events(struct strfilter *filter);
 extern int show_perf_probe_events(struct strfilter *filter);
 extern int show_line_range(struct line_range *lr, const char *module,
-- 
2.5.0

RE: [RFC/PATCH 2/3] perf probe: Rename __event_package to probe_event_package

[平松雅巳 / HIRAMATU，MASAMI]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3793 bytes --]

> From: Namhyung Kim [mailto:namhyung@gmail.com] On Behalf Of Namhyung Kim
> 
> The struct __event_package can be accessed now from other than
> probe-event.c code.  So rename it to more specific name.
> 
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/probe-event.c | 18 ++++++------------
>  tools/perf/util/probe-event.h | 10 ++++++++++
>  2 files changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 8eaa03428d72..eef39338bb2a 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2759,20 +2759,14 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
>  	return find_probe_trace_events_from_map(pev, tevs);
>  }
> 
> -struct __event_package {
> -	struct perf_probe_event		*pev;
> -	struct probe_trace_event	*tevs;
> -	int				ntevs;
> -};
> -
> -static int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
> -				   struct __event_package **ppkgs)
> +int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
> +			    struct probe_event_package **ppkgs)

OK, since now we have probe_event_package object, this function should be
refactored as pkgs = probe_event_packages__new(pevs, npevs)(allocate, initialize)
, probe_event_packages__convert(pkgs) and probe_event_packages__apply(pkgs).

>  {
>  	int i, ret;
> -	struct __event_package *pkgs;
> +	struct probe_event_package *pkgs;
> 
>  	ret = 0;
> -	pkgs = zalloc(sizeof(struct __event_package) * npevs);
> +	pkgs = zalloc(sizeof(struct probe_event_package) * npevs);
> 
>  	if (pkgs == NULL)
>  		return -ENOMEM;
> @@ -2813,7 +2807,7 @@ static int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
>  	return 0;
>  }
> 
> -static void cleanup_perf_probe_events(struct __event_package *pkgs, int npevs)
> +void cleanup_perf_probe_events(struct probe_event_package *pkgs, int npevs)

This also should be perf_event_pacakges__delete() :)

Thanks!

>  {
>  	int i, j;
> 
> @@ -2833,7 +2827,7 @@ static void cleanup_perf_probe_events(struct __event_package *pkgs, int npevs)
>  int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
>  {
>  	int ret;
> -	struct __event_package *pkgs = NULL;
> +	struct probe_event_package *pkgs = NULL;
> 
>  	ret = __add_perf_probe_events(pevs, npevs, &pkgs);
>  	cleanup_perf_probe_events(pkgs, npevs);
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index 6e7ec68a4aa8..73f922fa7cac 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -89,6 +89,12 @@ struct perf_probe_event {
>  	struct perf_probe_arg	*args;	/* Arguments */
>  };
> 
> +struct probe_event_package {
> +	struct perf_probe_event		*pev;
> +	struct probe_trace_event	*tevs;
> +	int				ntevs;
> +};
> +
>  /* Line range */
>  struct line_range {
>  	char			*file;		/* File name */
> @@ -138,6 +144,10 @@ extern void line_range__clear(struct line_range *lr);
>  extern int line_range__init(struct line_range *lr);
> 
>  extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> +extern int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
> +				   struct probe_event_package **ppkgs);
> +extern void cleanup_perf_probe_events(struct probe_event_package *pkgs,
> +				      int npevs);
>  extern int del_perf_probe_events(struct strfilter *filter);
>  extern int show_perf_probe_events(struct strfilter *filter);
>  extern int show_line_range(struct line_range *lr, const char *module,
> --
> 2.5.0

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [RFC/PATCH 2/3] perf probe: Rename __event_package to probe_event_package

[Namhyung Kim]

On Fri, Sep 04, 2015 at 02:11:09AM +0000, 平松雅巳 / HIRAMATU，MASAMI wrote:
> > From: Namhyung Kim [mailto:namhyung@gmail.com] On Behalf Of Namhyung Kim
> > 
> > The struct __event_package can be accessed now from other than
> > probe-event.c code.  So rename it to more specific name.
> > 
> > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/util/probe-event.c | 18 ++++++------------
> >  tools/perf/util/probe-event.h | 10 ++++++++++
> >  2 files changed, 16 insertions(+), 12 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 8eaa03428d72..eef39338bb2a 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -2759,20 +2759,14 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> >  	return find_probe_trace_events_from_map(pev, tevs);
> >  }
> > 
> > -struct __event_package {
> > -	struct perf_probe_event		*pev;
> > -	struct probe_trace_event	*tevs;
> > -	int				ntevs;
> > -};
> > -
> > -static int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
> > -				   struct __event_package **ppkgs)
> > +int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
> > +			    struct probe_event_package **ppkgs)
> 
> OK, since now we have probe_event_package object, this function should be
> refactored as pkgs = probe_event_packages__new(pevs, npevs)(allocate, initialize)
> , probe_event_packages__convert(pkgs) and probe_event_packages__apply(pkgs).

I think it'd be better using Wang Nan's patch below.

  https://lkml.org/lkml/2015/8/29/25

I'll send v2 with this change.

Thanks,
Namhyung


> 
> >  {
> >  	int i, ret;
> > -	struct __event_package *pkgs;
> > +	struct probe_event_package *pkgs;
> > 
> >  	ret = 0;
> > -	pkgs = zalloc(sizeof(struct __event_package) * npevs);
> > +	pkgs = zalloc(sizeof(struct probe_event_package) * npevs);
> > 
> >  	if (pkgs == NULL)
> >  		return -ENOMEM;
> > @@ -2813,7 +2807,7 @@ static int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
> >  	return 0;
> >  }
> > 
> > -static void cleanup_perf_probe_events(struct __event_package *pkgs, int npevs)
> > +void cleanup_perf_probe_events(struct probe_event_package *pkgs, int npevs)
> 
> This also should be perf_event_pacakges__delete() :)
> 
> Thanks!
> 
> >  {
> >  	int i, j;
> > 
> > @@ -2833,7 +2827,7 @@ static void cleanup_perf_probe_events(struct __event_package *pkgs, int npevs)
> >  int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
> >  {
> >  	int ret;
> > -	struct __event_package *pkgs = NULL;
> > +	struct probe_event_package *pkgs = NULL;
> > 
> >  	ret = __add_perf_probe_events(pevs, npevs, &pkgs);
> >  	cleanup_perf_probe_events(pkgs, npevs);
> > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> > index 6e7ec68a4aa8..73f922fa7cac 100644
> > --- a/tools/perf/util/probe-event.h
> > +++ b/tools/perf/util/probe-event.h
> > @@ -89,6 +89,12 @@ struct perf_probe_event {
> >  	struct perf_probe_arg	*args;	/* Arguments */
> >  };
> > 
> > +struct probe_event_package {
> > +	struct perf_probe_event		*pev;
> > +	struct probe_trace_event	*tevs;
> > +	int				ntevs;
> > +};
> > +
> >  /* Line range */
> >  struct line_range {
> >  	char			*file;		/* File name */
> > @@ -138,6 +144,10 @@ extern void line_range__clear(struct line_range *lr);
> >  extern int line_range__init(struct line_range *lr);
> > 
> >  extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> > +extern int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
> > +				   struct probe_event_package **ppkgs);
> > +extern void cleanup_perf_probe_events(struct probe_event_package *pkgs,
> > +				      int npevs);
> >  extern int del_perf_probe_events(struct strfilter *filter);
> >  extern int show_perf_probe_events(struct strfilter *filter);
> >  extern int show_line_range(struct line_range *lr, const char *module,
> > --
> > 2.5.0
> 

RE: Re: [RFC/PATCH 2/3] perf probe: Rename __event_package to probe_event_package

[平松雅巳 / HIRAMATU，MASAMI]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4563 bytes --]

> From: Namhyung Kim [mailto:namhyung@gmail.com] On Behalf Of Namhyung Kim
> 
> On Fri, Sep 04, 2015 at 02:11:09AM +0000, 平松雅巳 / HIRAMATU，MASAMI wrote:
> > > From: Namhyung Kim [mailto:namhyung@gmail.com] On Behalf Of Namhyung Kim
> > >
> > > The struct __event_package can be accessed now from other than
> > > probe-event.c code.  So rename it to more specific name.
> > >
> > > Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > >  tools/perf/util/probe-event.c | 18 ++++++------------
> > >  tools/perf/util/probe-event.h | 10 ++++++++++
> > >  2 files changed, 16 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > > index 8eaa03428d72..eef39338bb2a 100644
> > > --- a/tools/perf/util/probe-event.c
> > > +++ b/tools/perf/util/probe-event.c
> > > @@ -2759,20 +2759,14 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> > >  	return find_probe_trace_events_from_map(pev, tevs);
> > >  }
> > >
> > > -struct __event_package {
> > > -	struct perf_probe_event		*pev;
> > > -	struct probe_trace_event	*tevs;
> > > -	int				ntevs;
> > > -};
> > > -
> > > -static int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
> > > -				   struct __event_package **ppkgs)
> > > +int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
> > > +			    struct probe_event_package **ppkgs)
> >
> > OK, since now we have probe_event_package object, this function should be
> > refactored as pkgs = probe_event_packages__new(pevs, npevs)(allocate, initialize)
> > , probe_event_packages__convert(pkgs) and probe_event_packages__apply(pkgs).
> 
> I think it'd be better using Wang Nan's patch below.
> 
>   https://lkml.org/lkml/2015/8/29/25

Ah, I see. It is certainly good way to solve :)

Thanks!

> 
> I'll send v2 with this change.
> 
> Thanks,
> Namhyung
> 
> 
> >
> > >  {
> > >  	int i, ret;
> > > -	struct __event_package *pkgs;
> > > +	struct probe_event_package *pkgs;
> > >
> > >  	ret = 0;
> > > -	pkgs = zalloc(sizeof(struct __event_package) * npevs);
> > > +	pkgs = zalloc(sizeof(struct probe_event_package) * npevs);
> > >
> > >  	if (pkgs == NULL)
> > >  		return -ENOMEM;
> > > @@ -2813,7 +2807,7 @@ static int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
> > >  	return 0;
> > >  }
> > >
> > > -static void cleanup_perf_probe_events(struct __event_package *pkgs, int npevs)
> > > +void cleanup_perf_probe_events(struct probe_event_package *pkgs, int npevs)
> >
> > This also should be perf_event_pacakges__delete() :)
> >
> > Thanks!
> >
> > >  {
> > >  	int i, j;
> > >
> > > @@ -2833,7 +2827,7 @@ static void cleanup_perf_probe_events(struct __event_package *pkgs, int npevs)
> > >  int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
> > >  {
> > >  	int ret;
> > > -	struct __event_package *pkgs = NULL;
> > > +	struct probe_event_package *pkgs = NULL;
> > >
> > >  	ret = __add_perf_probe_events(pevs, npevs, &pkgs);
> > >  	cleanup_perf_probe_events(pkgs, npevs);
> > > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> > > index 6e7ec68a4aa8..73f922fa7cac 100644
> > > --- a/tools/perf/util/probe-event.h
> > > +++ b/tools/perf/util/probe-event.h
> > > @@ -89,6 +89,12 @@ struct perf_probe_event {
> > >  	struct perf_probe_arg	*args;	/* Arguments */
> > >  };
> > >
> > > +struct probe_event_package {
> > > +	struct perf_probe_event		*pev;
> > > +	struct probe_trace_event	*tevs;
> > > +	int				ntevs;
> > > +};
> > > +
> > >  /* Line range */
> > >  struct line_range {
> > >  	char			*file;		/* File name */
> > > @@ -138,6 +144,10 @@ extern void line_range__clear(struct line_range *lr);
> > >  extern int line_range__init(struct line_range *lr);
> > >
> > >  extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> > > +extern int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
> > > +				   struct probe_event_package **ppkgs);
> > > +extern void cleanup_perf_probe_events(struct probe_event_package *pkgs,
> > > +				      int npevs);
> > >  extern int del_perf_probe_events(struct strfilter *filter);
> > >  extern int show_perf_probe_events(struct strfilter *filter);
> > >  extern int show_line_range(struct line_range *lr, const char *module,
> > > --
> > > 2.5.0
> >
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

[RFC/PATCH 3/3] perf probe: Move print logic into cmd_probe()

[Namhyung Kim]

Showing actual trace event when adding perf events is only needed in
perf probe command.  But the add functionality itself can be used by
other places.  So move the printing code into the cmd_probe().

Also it combines the output if more than one event is added.

Before:
  $ sudo perf probe -a do_fork -a do_exit
  Added new event:
  probe:do_fork        (on do_fork)

  You can now use it in all perf tools, such as:

      perf record -e probe:do_fork -aR sleep 1

  Added new events:
  probe:do_exit        (on do_exit)
  probe:do_exit_1      (on do_exit)

  You can now use it in all perf tools, such as:

      perf record -e probe:do_exit_1 -aR sleep 1

After:
  $ sudo perf probe -a do_fork -a do_exit
  Added new events:
  probe:do_fork        (on do_fork)
  probe:do_exit        (on do_exit)
  probe:do_exit_1      (on do_exit)

  You can now use it in all perf tools, such as:

      perf record -e probe:do_exit_1 -aR sleep 1

Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-probe.c    | 39 ++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/probe-event.c | 22 +++-------------------
 tools/perf/util/probe-event.h |  3 +++
 3 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index b81cec33b4b2..827eb7ed92b2 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -403,6 +403,9 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 	OPT_END()
 	};
 	int ret;
+	int i, k;
+	struct probe_event_package *pkgs = NULL;
+	const char *event = NULL, *group = NULL;
 
 	set_option_flag(options, 'a', "add", PARSE_OPT_EXCLUSIVE);
 	set_option_flag(options, 'd', "del", PARSE_OPT_EXCLUSIVE);
@@ -496,7 +499,41 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
 			usage_with_options(probe_usage, options);
 		}
 
-		ret = add_perf_probe_events(params.events, params.nevents);
+		ret = __add_perf_probe_events(params.events, params.nevents,
+					      &pkgs);
+		if (ret < 0)
+			goto out_cleanup;
+
+		for (i = k = 0; i < params.nevents; i++)
+			k += pkgs[i].ntevs;
+
+		pr_info("Added new event%s\n", (k > 1) ? "s:" : ":");
+		for (i = 0; i < params.nevents; i++) {
+			struct perf_probe_event *pev = pkgs[i].pev;
+
+			for (k = 0; k < pkgs[i].ntevs; k++) {
+				struct probe_trace_event *tev = &pkgs[i].tevs[k];
+
+				/* We use tev's name for showing new events */
+				show_perf_probe_event(tev->group, tev->event, pev,
+						      tev->point.module, false);
+
+				/* Save the last valid name */
+				event = tev->event;
+				group = tev->group;
+			}
+		}
+
+		/* Note that it is possible to skip all events because of blacklist */
+		if (event) {
+			/* Show how to use the event. */
+			pr_info("\nYou can now use it in all perf tools, such as:\n\n");
+			pr_info("\tperf record -e %s:%s -aR sleep 1\n\n", group, event);
+		}
+
+out_cleanup:
+		cleanup_perf_probe_events(pkgs, params.nevents);
+
 		if (ret < 0) {
 			pr_err_with_code("  Error: Failed to add events.", ret);
 			return ret;
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index eef39338bb2a..9c68fc551f9e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2180,9 +2180,9 @@ out:
 }
 
 /* Show an event */
-static int show_perf_probe_event(const char *group, const char *event,
-				 struct perf_probe_event *pev,
-				 const char *module, bool use_stdout)
+int show_perf_probe_event(const char *group, const char *event,
+			  struct perf_probe_event *pev,
+			  const char *module, bool use_stdout)
 {
 	struct strbuf buf = STRBUF_INIT;
 	int ret;
@@ -2399,7 +2399,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 {
 	int i, fd, ret;
 	struct probe_trace_event *tev = NULL;
-	const char *event = NULL, *group = NULL;
 	struct strlist *namelist;
 
 	fd = probe_file__open(PF_FL_RW | (pev->uprobes ? PF_FL_UPROBE : 0));
@@ -2415,7 +2414,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 	}
 
 	ret = 0;
-	pr_info("Added new event%s\n", (ntevs > 1) ? "s:" : ":");
 	for (i = 0; i < ntevs; i++) {
 		tev = &tevs[i];
 		/* Skip if the symbol is out of .text or blacklisted */
@@ -2432,13 +2430,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 		if (ret < 0)
 			break;
 
-		/* We use tev's name for showing new events */
-		show_perf_probe_event(tev->group, tev->event, pev,
-				      tev->point.module, false);
-		/* Save the last valid name */
-		event = tev->event;
-		group = tev->group;
-
 		/*
 		 * Probes after the first probe which comes from same
 		 * user input are always allowed to add suffix, because
@@ -2450,13 +2441,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 	if (ret == -EINVAL && pev->uprobes)
 		warn_uprobe_event_compat(tev);
 
-	/* Note that it is possible to skip all events because of blacklist */
-	if (ret >= 0 && event) {
-		/* Show how to use the event. */
-		pr_info("\nYou can now use it in all perf tools, such as:\n\n");
-		pr_info("\tperf record -e %s:%s -aR sleep 1\n\n", group, event);
-	}
-
 	strlist__delete(namelist);
 close_out:
 	close(fd);
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 73f922fa7cac..843d1feffc97 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -149,6 +149,9 @@ extern int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
 extern void cleanup_perf_probe_events(struct probe_event_package *pkgs,
 				      int npevs);
 extern int del_perf_probe_events(struct strfilter *filter);
+extern int show_perf_probe_event(const char *group, const char *event,
+				 struct perf_probe_event *pev,
+				 const char *module, bool use_stdout);
 extern int show_perf_probe_events(struct strfilter *filter);
 extern int show_line_range(struct line_range *lr, const char *module,
 			   bool user);
-- 
2.5.0

RE: [RFC/PATCH 3/3] perf probe: Move print logic into cmd_probe()

[平松雅巳 / HIRAMATU，MASAMI]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2729 bytes --]

> From: Namhyung Kim [mailto:namhyung@gmail.com] On Behalf Of Namhyung Kim
> 
> Showing actual trace event when adding perf events is only needed in
> perf probe command.  But the add functionality itself can be used by
> other places.  So move the printing code into the cmd_probe().
> 
> Also it combines the output if more than one event is added.
> 
> Before:
>   $ sudo perf probe -a do_fork -a do_exit
>   Added new event:
>   probe:do_fork        (on do_fork)
> 
>   You can now use it in all perf tools, such as:
> 
>       perf record -e probe:do_fork -aR sleep 1
> 
>   Added new events:
>   probe:do_exit        (on do_exit)
>   probe:do_exit_1      (on do_exit)
> 
>   You can now use it in all perf tools, such as:
> 
>       perf record -e probe:do_exit_1 -aR sleep 1
> 
> After:
>   $ sudo perf probe -a do_fork -a do_exit
>   Added new events:
>   probe:do_fork        (on do_fork)
>   probe:do_exit        (on do_exit)
>   probe:do_exit_1      (on do_exit)
> 
>   You can now use it in all perf tools, such as:
> 
>       perf record -e probe:do_exit_1 -aR sleep 1
> 

This change is good for me :)
And have a comment below,

[...]
> @@ -496,7 +499,41 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
>  			usage_with_options(probe_usage, options);
>  		}
> 
> -		ret = add_perf_probe_events(params.events, params.nevents);
> +		ret = __add_perf_probe_events(params.events, params.nevents,
> +					      &pkgs);
> +		if (ret < 0)
> +			goto out_cleanup;
> +
> +		for (i = k = 0; i < params.nevents; i++)
> +			k += pkgs[i].ntevs;
> +
> +		pr_info("Added new event%s\n", (k > 1) ? "s:" : ":");
> +		for (i = 0; i < params.nevents; i++) {
> +			struct perf_probe_event *pev = pkgs[i].pev;
> +
> +			for (k = 0; k < pkgs[i].ntevs; k++) {
> +				struct probe_trace_event *tev = &pkgs[i].tevs[k];
> +
> +				/* We use tev's name for showing new events */
> +				show_perf_probe_event(tev->group, tev->event, pev,
> +						      tev->point.module, false);
> +
> +				/* Save the last valid name */
> +				event = tev->event;
> +				group = tev->group;
> +			}
> +		}
> +
> +		/* Note that it is possible to skip all events because of blacklist */
> +		if (event) {
> +			/* Show how to use the event. */
> +			pr_info("\nYou can now use it in all perf tools, such as:\n\n");
> +			pr_info("\tperf record -e %s:%s -aR sleep 1\n\n", group, event);
> +		}
> +
> +out_cleanup:
> +		cleanup_perf_probe_events(pkgs, params.nevents);

I think this should be a separated function to avoid increasing the size of __cmd_probe()

Thanks,

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [RFC/PATCH 1/3] perf probe: Split add_perf_probe_events()

[平松雅巳 / HIRAMATU，MASAMI]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2622 bytes --]

> From: Namhyung Kim [mailto:namhyung@gmail.com] On Behalf Of Namhyung Kim
> 
> The add_perf_probe_events() does 3 things:
> 
>  1. convert all perf events to trace events
>  2. add all trace events to kernel
>  3. cleanup all trace events
> 
> But sometimes we need to do something with the trace events.  So split
> the funtion into two, so that it can access intermediate trace events
> via struct __event_package if needed.

Yeah, this is good to me.

Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>

Thanks Namhyung!

> 
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/probe-event.c | 30 ++++++++++++++++++++++++++----
>  1 file changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index eb5f18b75402..8eaa03428d72 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2765,9 +2765,10 @@ struct __event_package {
>  	int				ntevs;
>  };
> 
> -int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
> +static int __add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
> +				   struct __event_package **ppkgs)
>  {
> -	int i, j, ret;
> +	int i, ret;
>  	struct __event_package *pkgs;
> 
>  	ret = 0;
> @@ -2792,7 +2793,7 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
>  		ret  = convert_to_probe_trace_events(pkgs[i].pev,
>  						     &pkgs[i].tevs);
>  		if (ret < 0)
> -			goto end;
> +			return ret;
>  		pkgs[i].ntevs = ret;
>  	}
>  	/* This just release blacklist only if allocated */
> @@ -2806,7 +2807,19 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
>  		if (ret < 0)
>  			break;
>  	}
> -end:
> +
> +	*ppkgs = pkgs;
> +
> +	return 0;
> +}
> +
> +static void cleanup_perf_probe_events(struct __event_package *pkgs, int npevs)
> +{
> +	int i, j;
> +
> +	if (pkgs == NULL)
> +		return;
> +
>  	/* Loop 3: cleanup and free trace events  */
>  	for (i = 0; i < npevs; i++) {
>  		for (j = 0; j < pkgs[i].ntevs; j++)
> @@ -2815,6 +2828,15 @@ end:
>  	}
>  	free(pkgs);
>  	exit_symbol_maps();
> +}
> +
> +int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
> +{
> +	int ret;
> +	struct __event_package *pkgs = NULL;
> +
> +	ret = __add_perf_probe_events(pevs, npevs, &pkgs);
> +	cleanup_perf_probe_events(pkgs, npevs);
> 
>  	return ret;
>  }
> --
> 2.5.0

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥