[PATCH] perf probe: Introduce --no-advice option when a new event is created

[PATCH] perf probe: Introduce --no-advice option when a new event is created

[Aaron Tomlin]

This patch introduces a new option namely "--no-advice" to be used with
the add command. Now one has the ability to avoid the generation of any
advice when a new event is created. Thus making the output reasonable.
For example:

  # perf probe --no-advice --add 'user_path_at%return return=$retval:s32'
  Added new event:
    probe:user_path_at__return (on user_path_at%return with return=$retval:s32)
  #

Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 tools/perf/builtin-probe.c    | 4 +++-
 tools/perf/util/probe-event.h | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 69800e4d9530..36b8e08f3725 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -391,7 +391,7 @@ static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs)
 	}
 
 	/* Note that it is possible to skip all events because of blacklist */
-	if (event) {
+	if (event && !probe_conf.no_advice) {
 #ifndef HAVE_LIBTRACEEVENT
 		pr_info("\nperf is not linked with libtraceevent, to use the new probe you can use tracefs:\n\n");
 		pr_info("\tcd /sys/kernel/tracing/\n");
@@ -577,6 +577,8 @@ __cmd_probe(int argc, const char **argv)
 		   "directory", "path to kernel source"),
 	OPT_BOOLEAN('\0', "no-inlines", &probe_conf.no_inlines,
 		"Don't search inlined functions"),
+	OPT_BOOLEAN('\0', "no-advice", &probe_conf.no_advice,
+		"Don't provide any advice after a new event is created."),
 	OPT__DRY_RUN(&probe_event_dry_run),
 	OPT_INTEGER('\0', "max-probes", &probe_conf.max_probes,
 		 "Set how many probe points can be found for a probe."),
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 61a5f4ff4e9c..be0dce26e133 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -14,6 +14,7 @@ struct probe_conf {
 	bool	show_location_range;
 	bool	force_add;
 	bool	no_inlines;
+	bool	no_advice;
 	bool	cache;
 	bool	bootconfig;
 	int	max_probes;
-- 
2.47.1

Re: [PATCH] perf probe: Introduce --no-advice option when a new event is created

[Namhyung Kim]

Hello,

Sorry for the late reply.

On Sat, Dec 14, 2024 at 07:34:52PM +0000, Aaron Tomlin wrote:
> This patch introduces a new option namely "--no-advice" to be used with
> the add command. Now one has the ability to avoid the generation of any
> advice when a new event is created. Thus making the output reasonable.
> For example:
> 
>   # perf probe --no-advice --add 'user_path_at%return return=$retval:s32'
>   Added new event:
>     probe:user_path_at__return (on user_path_at%return with return=$retval:s32)
>   #

Thanks for sending the patch but why do you care?  You can just ignore
the message.

Thanks,
Namhyung

> 
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
> ---
>  tools/perf/builtin-probe.c    | 4 +++-
>  tools/perf/util/probe-event.h | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 69800e4d9530..36b8e08f3725 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -391,7 +391,7 @@ static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs)
>  	}
>  
>  	/* Note that it is possible to skip all events because of blacklist */
> -	if (event) {
> +	if (event && !probe_conf.no_advice) {
>  #ifndef HAVE_LIBTRACEEVENT
>  		pr_info("\nperf is not linked with libtraceevent, to use the new probe you can use tracefs:\n\n");
>  		pr_info("\tcd /sys/kernel/tracing/\n");
> @@ -577,6 +577,8 @@ __cmd_probe(int argc, const char **argv)
>  		   "directory", "path to kernel source"),
>  	OPT_BOOLEAN('\0', "no-inlines", &probe_conf.no_inlines,
>  		"Don't search inlined functions"),
> +	OPT_BOOLEAN('\0', "no-advice", &probe_conf.no_advice,
> +		"Don't provide any advice after a new event is created."),
>  	OPT__DRY_RUN(&probe_event_dry_run),
>  	OPT_INTEGER('\0', "max-probes", &probe_conf.max_probes,
>  		 "Set how many probe points can be found for a probe."),
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index 61a5f4ff4e9c..be0dce26e133 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -14,6 +14,7 @@ struct probe_conf {
>  	bool	show_location_range;
>  	bool	force_add;
>  	bool	no_inlines;
> +	bool	no_advice;
>  	bool	cache;
>  	bool	bootconfig;
>  	int	max_probes;
> -- 
> 2.47.1
> 

Re: [PATCH] perf probe: Introduce --no-advice option when a new event is created

[Aaron Tomlin]

On Thu, Jan 23, 2025 at 04:25:10PM -0800, Namhyung Kim wrote:
> Hello,

Hi Namhyung,

> Sorry for the late reply.

No problem at all.

> On Sat, Dec 14, 2024 at 07:34:52PM +0000, Aaron Tomlin wrote:
> > This patch introduces a new option namely "--no-advice" to be used with
> > the add command. Now one has the ability to avoid the generation of any
> > advice when a new event is created. Thus making the output reasonable.
> > For example:
> > 
> >   # perf probe --no-advice --add 'user_path_at%return return=$retval:s32'
> >   Added new event:
> >     probe:user_path_at__return (on user_path_at%return with return=$retval:s32)
> >   #
> 
> Thanks for sending the patch but why do you care?  You can just ignore
> the message.

I understand.

As a frequent and avid perf-probe user, I often only need to confirm the
successful creation of a new event, ensuring its presence under
/sys/kernel/debug/tracing/events/probes for subsequent use with Ftrace.
The ability to suppress verbose output would be a valuable enhancement.

Please let me know your thoughts.


Kind regards,

-- 
Aaron Tomlin

Re: [PATCH] perf probe: Introduce --no-advice option when a new event is created

[Namhyung Kim]

On Fri, Jan 24, 2025 at 10:54:04AM +0000, Aaron Tomlin wrote:
> On Thu, Jan 23, 2025 at 04:25:10PM -0800, Namhyung Kim wrote:
> > Hello,
> 
> Hi Namhyung,
> 
> > Sorry for the late reply.
> 
> No problem at all.
> 
> > On Sat, Dec 14, 2024 at 07:34:52PM +0000, Aaron Tomlin wrote:
> > > This patch introduces a new option namely "--no-advice" to be used with
> > > the add command. Now one has the ability to avoid the generation of any
> > > advice when a new event is created. Thus making the output reasonable.
> > > For example:
> > > 
> > >   # perf probe --no-advice --add 'user_path_at%return return=$retval:s32'
> > >   Added new event:
> > >     probe:user_path_at__return (on user_path_at%return with return=$retval:s32)
> > >   #
> > 
> > Thanks for sending the patch but why do you care?  You can just ignore
> > the message.
> 
> I understand.
> 
> As a frequent and avid perf-probe user, I often only need to confirm the
> successful creation of a new event, ensuring its presence under
> /sys/kernel/debug/tracing/events/probes for subsequent use with Ftrace.
> The ability to suppress verbose output would be a valuable enhancement.
> 
> Please let me know your thoughts.

Sounds like you may want -q option to suppress all output and check the
exit code of the command.

Thanks,
Namhyung

Re: [PATCH] perf probe: Introduce --no-advice option when a new event is created

[Aaron Tomlin]

On Fri, Jan 24, 2025 at 09:37:34AM -0800, Namhyung Kim wrote:
> Sounds like you may want -q option to suppress all output and check the
> exit code of the command.

Hi Namhyung,

I will prepare another patch.

-- 
Aaron Tomlin

Re: [PATCH] perf probe: Introduce --no-advice option when a new event is created

[Aaron Tomlin]

On Fri, Jan 24, 2025 at 12:42:26PM +0100, Arnaldo Carvalho de Melo wrote:
> Consider setting it via 'perf config', see a recent patch I sent for the
> wall clock profiling feature, it would be something like:
> 
> perf config probe.quiet=true
> 
> https://lore.kernel.org/lkml/Z4akewi7UPXpagce@x1/
> 
> - Arnaldo

Thank you Arnaldo.

-- 
Aaron Tomlin

[PATCH 0/1] perf probe: Extend the quiet option

[Aaron Tomlin]

Hi Namhyung,

Apologies for the delay. I suspect this is the idea you had in mind?

Aaron Tomlin (1):
  perf probe: Extend the quiet option

 tools/perf/builtin-probe.c    | 16 +++++++++++-----
 tools/perf/util/probe-event.h |  1 +
 2 files changed, 12 insertions(+), 5 deletions(-)

-- 
2.47.1

[PATCH 1/1] perf probe: Extend the quiet option

[Aaron Tomlin]

To provide a consistent and complete quiet mode, this patch expands the
scope of the [-q|--quiet] option to suppress all informational messages,
including those indicating successful event creation, in addition to the
currently suppressed warnings and messages. For example:

    ❯ sudo ./perf probe --quiet --add proc_sys_open
    ❯ echo $?
    0

Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 tools/perf/builtin-probe.c    | 16 +++++++++++-----
 tools/perf/util/probe-event.h |  1 +
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 69800e4d9530..aee756aad19d 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -391,7 +391,7 @@ static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs)
 	}
 
 	/* Note that it is possible to skip all events because of blacklist */
-	if (event) {
+	if (event && !probe_conf.quiet) {
 #ifndef HAVE_LIBTRACEEVENT
 		pr_info("\nperf is not linked with libtraceevent, to use the new probe you can use tracefs:\n\n");
 		pr_info("\tcd /sys/kernel/tracing/\n");
@@ -467,8 +467,11 @@ static int perf_del_probe_events(struct strfilter *filter)
 
 	ret = probe_file__get_events(kfd, filter, klist);
 	if (ret == 0) {
-		strlist__for_each_entry(ent, klist)
+		strlist__for_each_entry(ent, klist) {
+			if (probe_conf.quiet)
+				continue;
 			pr_info("Removed event: %s\n", ent->s);
+		}
 
 		ret = probe_file__del_strlist(kfd, klist);
 		if (ret < 0)
@@ -478,8 +481,11 @@ static int perf_del_probe_events(struct strfilter *filter)
 
 	ret2 = probe_file__get_events(ufd, filter, ulist);
 	if (ret2 == 0) {
-		strlist__for_each_entry(ent, ulist)
+		strlist__for_each_entry(ent, ulist) {
+			if (probe_conf.quiet)
+				continue;
 			pr_info("Removed event: %s\n", ent->s);
+		}
 
 		ret2 = probe_file__del_strlist(ufd, ulist);
 		if (ret2 < 0)
@@ -531,7 +537,7 @@ __cmd_probe(int argc, const char **argv)
 	struct option options[] = {
 	OPT_INCR('v', "verbose", &verbose,
 		    "be more verbose (show parsed arguments, etc)"),
-	OPT_BOOLEAN('q', "quiet", &quiet,
+	OPT_BOOLEAN('q', "quiet", &probe_conf.quiet,
 		    "be quiet (do not show any warnings or messages)"),
 	OPT_CALLBACK_DEFAULT('l', "list", NULL, "[GROUP:]EVENT",
 			     "list up probe events",
@@ -631,7 +637,7 @@ __cmd_probe(int argc, const char **argv)
 	argc = parse_options(argc, argv, options, probe_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 
-	if (quiet) {
+	if (probe_conf.quiet) {
 		if (verbose != 0) {
 			pr_err("  Error: -v and -q are exclusive.\n");
 			return -EINVAL;
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 71905ede0207..55771113791f 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -10,6 +10,7 @@ struct nsinfo;
 
 /* Probe related configurations */
 struct probe_conf {
+	bool	quiet;
 	bool	show_ext_vars;
 	bool	show_location_range;
 	bool	force_add;
-- 
2.47.1