Re: [PATCH v2] perf/sdt: Show proper hint

[Masami Hiramatsu <mhiramat@kernel.org>]

On Fri,  3 Feb 2017 15:56:42 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:

> All events from 'perf list', except SDT events, can be directly recorded
> with 'perf record'. But, the flow is little different for SDT events.
> Probe point for SDT event needs to be created using 'perf probe' before
> recording it using 'perf record'. Perf shows misleading hint when user
> tries to record SDT event without creating a probe point. Show proper
> hint there.
> 
> Before patch:
>   $ perf record -a -e sdt_glib:idle__add
>     event syntax error: 'sdt_glib:idle__add'
>                          \___ unknown tracepoint
> 
>     Error: File /sys/kernel/debug/tracing/events/sdt_glib/idle__add not found.
>     Hint:  Perhaps this kernel misses some CONFIG_ setting to enable this feature?.
>     ...
> 
> After patch:
>   $ perf record -a -e sdt_glib:idle__add
>     event syntax error: 'sdt_glib:idle__add'
>                          \___ unknown tracepoint
> 
>     Error: File /sys/kernel/debug/tracing/events/sdt_glib/idle__add not found.
>     Hint:  SDT event cannot be directly recorded on. Please use 'perf probe sdt_glib:idle__add' before recording it.
>     ...
> 
>   $ perf probe sdt_glib:idle__add
>     Added new event:
>       sdt_glib:idle__add   (on %idle__add in /usr/lib64/libglib-2.0.so.0.5000.2)
> 
>     You can now use it in all perf tools, such as:
> 
>         perf record -e sdt_glib:idle__add -aR sleep 1
> 
>   $ perf record -a -e sdt_glib:idle__add
>     [ perf record: Woken up 1 times to write data ]
>     [ perf record: Captured and wrote 0.175 MB perf.data ]
> 

Looks good to me:)

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks!

> Suggested-by: Ingo Molnar <mingo@redhat.com>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
> Changes in v2:
>   - More precise hint
> 
>  tools/lib/api/fs/tracing_path.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/lib/api/fs/tracing_path.c b/tools/lib/api/fs/tracing_path.c
> index 251b7c3..aaafc99 100644
> --- a/tools/lib/api/fs/tracing_path.c
> +++ b/tools/lib/api/fs/tracing_path.c
> @@ -86,9 +86,13 @@ void put_tracing_file(char *file)
>  	free(file);
>  }
>  
> -static int strerror_open(int err, char *buf, size_t size, const char *filename)
> +int tracing_path__strerror_open_tp(int err, char *buf, size_t size,
> +				   const char *sys, const char *name)
>  {
>  	char sbuf[128];
> +	char filename[PATH_MAX];
> +
> +	snprintf(filename, PATH_MAX, "%s/%s", sys, name ?: "*");
>  
>  	switch (err) {
>  	case ENOENT:
> @@ -99,10 +103,18 @@ static int strerror_open(int err, char *buf, size_t size, const char *filename)
>  		 * - jirka
>  		 */
>  		if (debugfs__configured() || tracefs__configured()) {
> -			snprintf(buf, size,
> -				 "Error:\tFile %s/%s not found.\n"
> -				 "Hint:\tPerhaps this kernel misses some CONFIG_ setting to enable this feature?.\n",
> -				 tracing_events_path, filename);
> +			/* sdt markers */
> +			if (!strncmp(filename, "sdt_", 4)) {
> +				snprintf(buf, size,
> +					"Error:\tFile %s/%s not found.\n"
> +					"Hint:\tSDT event cannot be directly recorded on. Please use 'perf probe %s:%s' before recording it.\n",
> +					tracing_events_path, filename, sys, name);
> +			} else {
> +				snprintf(buf, size,
> +					 "Error:\tFile %s/%s not found.\n"
> +					 "Hint:\tPerhaps this kernel misses some CONFIG_ setting to enable this feature?.\n",
> +					 tracing_events_path, filename);
> +			}
>  			break;
>  		}
>  		snprintf(buf, size, "%s",
> @@ -125,12 +137,3 @@ static int strerror_open(int err, char *buf, size_t size, const char *filename)
>  
>  	return 0;
>  }
> -
> -int tracing_path__strerror_open_tp(int err, char *buf, size_t size, const char *sys, const char *name)
> -{
> -	char path[PATH_MAX];
> -
> -	snprintf(path, PATH_MAX, "%s/%s", sys, name ?: "*");
> -
> -	return strerror_open(err, buf, size, path);
> -}
> -- 
> 2.9.3
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>