dle-develop — Dependable Linux Effort mailing list for developers

Re: [Dle-develop] [PATCH tracing/kprobes v2 4/5] perf: Add perf probe subcommand for kprobe-event setup helper

[Steven R. <ro...@go...>]

On Mon, 2009-10-05 at 20:57 -0400, Masami Hiramatsu wrote:
> Steven Rostedt wrote:
> > On Fri, 2009-10-02 at 17:49 -0400, Masami Hiramatsu wrote:
> >> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> >> index b5f1953..6dabcf1 100644
> >> --- a/tools/perf/Makefile
> >> +++ b/tools/perf/Makefile
> >> @@ -419,6 +419,16 @@ ifneq ($(shell sh -c "(echo '\#include <libelf.h>'; echo 'int main(void) { Elf *
> >>  	msg := $(error No libelf.h/libelf found, please install libelf-dev/elfutils-libelf-devel);
> >>  endif
> >>  
> >> +ifneq ($(shell sh -c "(echo '\#include <libdwarf/dwarf.h>'; echo '\#include <libdwarf/libdwarf.h>'; echo 'int main(void) { Dwarf_Debug dbg; Dwarf_Error err; dwarf_init(0, DW_DLC_READ, 0, 0, &dbg, &err); return (long)dbg; }') | $(CC) -x c - $(ALL_CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -ldwarf -lelf -o /dev/null $(ALL_LDFLAGS) > /dev/null 2>&1 && echo y"), y)
> >> +	msg := $(warning No libdwarf.h found, disables probe subcommand. Please install libdwarf-dev/libdwarf-devel);
> > 
> > Wow! And I thought my macros were ugly ;-)
> 
> :-)
> Maybe, would I better make a separate c file to check this?
> Like "autoconf-checkdwarf.c".


Nah, I think it is pretty obvious to what it is doing. But then again, I
like ugly hacks like the above.

-- Steve

Re: [Dle-develop] [PATCH tracing/kprobes v2 2/5] tracing/kprobes: Avoid field name confliction

[Masami H. <mhi...@re...>]

Steven Rostedt wrote:
> On Fri, 2009-10-02 at 17:48 -0400, Masami Hiramatsu wrote:
>> Check whether the argument name is conflict with other field names.
>>
>> Changes in v2:
>>  - Add common_lock_depth to reserved name list.
>>
>> Signed-off-by: Masami Hiramatsu <mhi...@re...>
>> Cc: Frederic Weisbecker <fwe...@gm...>
>> Cc: Ingo Molnar <mi...@el...>
>> Cc: Thomas Gleixner <tg...@li...>
>> Cc: Arnaldo Carvalho de Melo <ac...@re...>
>> Cc: Steven Rostedt <ro...@go...>
>> Cc: Mike Galbraith <ef...@gm...>
>> Cc: Paul Mackerras <pa...@sa...>
>> Cc: Peter Zijlstra <a.p...@ch...>
>> Cc: Christoph Hellwig <hc...@in...>
>> Cc: Ananth N Mavinakayanahalli <an...@in...>
>> Cc: Jim Keniston <jke...@us...>
>> Cc: Frank Ch. Eigler <fc...@re...>
>> ---
>>
>>  kernel/trace/trace_kprobe.c |   65 +++++++++++++++++++++++++++++++++++--------
>>  1 files changed, 53 insertions(+), 12 deletions(-)
>>
>> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
>> index f63ead0..eb1fa0f 100644
>> --- a/kernel/trace/trace_kprobe.c
>> +++ b/kernel/trace/trace_kprobe.c
>> @@ -38,6 +38,25 @@
>>  #define MAX_EVENT_NAME_LEN 64
>>  #define KPROBE_EVENT_SYSTEM "kprobes"
>>  
>> +/* Reserved field names */
>> +#define FIELD_STRING_IP "ip"
>> +#define FIELD_STRING_NARGS "nargs"
>> +#define FIELD_STRING_RETIP "ret_ip"
>> +#define FIELD_STRING_FUNC "func"
>> +
>> +const char *reserved_field_names[] = {
>> +	"common_type",
>> +	"common_flags",
>> +	"common_preempt_count",
>> +	"common_pid",
>> +	"common_tgid",
>> +	"common_lock_depth",
>> +	FIELD_STRING_IP,
>> +	FIELD_STRING_NARGS,
>> +	FIELD_STRING_RETIP,
>> +	FIELD_STRING_FUNC,
>> +};
>> +
>>  /* currently, trace_kprobe only supports X86. */
>>  
>>  struct fetch_func {
>> @@ -551,6 +570,20 @@ static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
>>  	return ret;
>>  }
>>  
>> +/* Return 1 if name is reserved or already used by another argument */
>> +static int conflict_field_name(const char *name,
>> +			       struct probe_arg *args, int narg)
>> +{
>> +	int i;
>> +	for (i = 0; i < ARRAY_SIZE(reserved_field_names); i++)
>> +		if (!strcmp(reserved_field_names[i], name))
>> +			return 1;
>> +	for (i = 0; i < narg; i++)
>> +		if (!strcmp(args[i].name, name))
>> +			return 1;
> 
> Just a coding preference, but still, I've seen too many mistakes (made
> them myself too).
> 
> 	if (strcmp(args[i].name, name) == 0)
> 
> Looks better as a match then
> 
> 	if (!strcmp(args[i].name, name))
> 
> That stands out to me as a miss match. But this is still just a
> preference and not something to make me argue the patch.

Agreed, !strcmp() pattern would better be warned by checkpatch.pl :-)
I'll fix that.

Thank you,

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH tracing/kprobes v2 4/5] perf: Add perf probe subcommand for kprobe-event setup helper

[Masami H. <mhi...@re...>]

Steven Rostedt wrote:
> On Fri, 2009-10-02 at 17:49 -0400, Masami Hiramatsu wrote:
>> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
>> index b5f1953..6dabcf1 100644
>> --- a/tools/perf/Makefile
>> +++ b/tools/perf/Makefile
>> @@ -419,6 +419,16 @@ ifneq ($(shell sh -c "(echo '\#include <libelf.h>'; echo 'int main(void) { Elf *
>>  	msg := $(error No libelf.h/libelf found, please install libelf-dev/elfutils-libelf-devel);
>>  endif
>>  
>> +ifneq ($(shell sh -c "(echo '\#include <libdwarf/dwarf.h>'; echo '\#include <libdwarf/libdwarf.h>'; echo 'int main(void) { Dwarf_Debug dbg; Dwarf_Error err; dwarf_init(0, DW_DLC_READ, 0, 0, &dbg, &err); return (long)dbg; }') | $(CC) -x c - $(ALL_CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -ldwarf -lelf -o /dev/null $(ALL_LDFLAGS) > /dev/null 2>&1 && echo y"), y)
>> +	msg := $(warning No libdwarf.h found, disables probe subcommand. Please install libdwarf-dev/libdwarf-devel);
> 
> Wow! And I thought my macros were ugly ;-)

:-)
Maybe, would I better make a separate c file to check this?
Like "autoconf-checkdwarf.c".

>> +
>> +	/* Copy arguments */
>> +	pp->nr_args = argc - 2;
>> +	if (pp->nr_args > 0) {
>> +		pp->args = (char **)malloc(sizeof(char *) * pp->nr_args);
> 
> Hmm, you don't check for failed malloc here?

Oops...

>> +static int synthesize_probepoint(struct probe_point *pp)
>> +{
>> +	char *buf;
>> +	int i, len, ret;
>> +	pp->probes[0] = buf = (char *)calloc(MAX_CMDLEN, sizeof(char));
> 
> Again no check for failed calloc?

Oops again, I'll check it...


>> diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
>> index e11d8d2..ad5f0f4 100644
>> --- a/tools/perf/builtin.h
>> +++ b/tools/perf/builtin.h
> 
> The rest looks fine (for what I can tell, it is all dwarf magic to
> me ;-)

:-)

Thank you!

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH tracing/kprobes v2 4/5] perf: Add perf probe subcommand for kprobe-event setup helper

[Steven R. <ro...@go...>]

On Fri, 2009-10-02 at 17:49 -0400, Masami Hiramatsu wrote:
> Add perf probe subcommand for kprobe-event setup helper to perf command.
> This allows user to define kprobe events by C expressions (C line numbers,
> C function names, and C local variables).
> 
> Usage
> -----
>  perf probe [<options>] -P 'PROBEDEF' [-P 'PROBEDEF' ...]
> 
>     -k, --vmlinux <file>  vmlinux/module pathname
>     -r, --release <rel>   kernel release
>     -P, --probe <p|r:[GRP/]NAME FUNC[+OFFS][@SRC]|@SRC:LINE [ARG ...]>
>                           probe point definition, where
> 		p:	kprobe probe
> 		r:	kretprobe probe
> 		GRP:	Group name (optional)
> 		NAME:	Event name
> 		FUNC:	Function name
> 		OFFS:	Offset from function entry (in byte)
> 		SRC:	Source code path
> 		LINE:	Line number
> 		ARG:	Probe argument (local variable name or
> 			kprobe-tracer argument format is supported.)
> 
> Changes in v2:
>  - Check synthesized string length.
>  - Rename perf kprobe to perf probe.
>  - Use spaces for separator and update usage comment.
>  - Check error paths in parse_probepoint().
>  - Check optimized-out variables.
> 
> Signed-off-by: Masami Hiramatsu <mhi...@re...>
> Cc: Frederic Weisbecker <fwe...@gm...>
> Cc: Ingo Molnar <mi...@el...>
> Cc: Thomas Gleixner <tg...@li...>
> Cc: Arnaldo Carvalho de Melo <ac...@re...>
> Cc: Steven Rostedt <ro...@go...>
> Cc: Mike Galbraith <ef...@gm...>
> Cc: Paul Mackerras <pa...@sa...>
> Cc: Peter Zijlstra <a.p...@ch...>
> Cc: Christoph Hellwig <hc...@in...>
> Cc: Ananth N Mavinakayanahalli <an...@in...>
> Cc: Jim Keniston <jke...@us...>
> Cc: Frank Ch. Eigler <fc...@re...>
> ---
> 
>  tools/perf/Makefile            |   10 +
>  tools/perf/builtin-probe.c     |  356 +++++++++++++++++++++
>  tools/perf/builtin.h           |    1 
>  tools/perf/perf.c              |    3 
>  tools/perf/util/probe-finder.c |  690 ++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/probe-finder.h |   68 ++++
>  6 files changed, 1128 insertions(+), 0 deletions(-)
>  create mode 100644 tools/perf/builtin-probe.c
>  create mode 100644 tools/perf/util/probe-finder.c
>  create mode 100644 tools/perf/util/probe-finder.h
> 
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index b5f1953..6dabcf1 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -419,6 +419,16 @@ ifneq ($(shell sh -c "(echo '\#include <libelf.h>'; echo 'int main(void) { Elf *
>  	msg := $(error No libelf.h/libelf found, please install libelf-dev/elfutils-libelf-devel);
>  endif
>  
> +ifneq ($(shell sh -c "(echo '\#include <libdwarf/dwarf.h>'; echo '\#include <libdwarf/libdwarf.h>'; echo 'int main(void) { Dwarf_Debug dbg; Dwarf_Error err; dwarf_init(0, DW_DLC_READ, 0, 0, &dbg, &err); return (long)dbg; }') | $(CC) -x c - $(ALL_CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -ldwarf -lelf -o /dev/null $(ALL_LDFLAGS) > /dev/null 2>&1 && echo y"), y)
> +	msg := $(warning No libdwarf.h found, disables probe subcommand. Please install libdwarf-dev/libdwarf-devel);

Wow! And I thought my macros were ugly ;-)

> +else
> +	EXTLIBS += -lelf -ldwarf
> +	LIB_H += util/probe-finder.h
> +	LIB_OBJS += util/probe-finder.o
> +	BUILTIN_OBJS += builtin-probe.o
> +	BASIC_CFLAGS += -DSUPPORT_DWARF
> +endif
> +
>  ifdef NO_DEMANGLE
>  	BASIC_CFLAGS += -DNO_DEMANGLE
>  else
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> new file mode 100644
> index 0000000..47c5fb8
> --- /dev/null
> +++ b/tools/perf/builtin-probe.c
> @@ -0,0 +1,356 @@
> +/*
> + * builtin-probe.c
> + *
> + * Builtin probe command: Set up probe events by C expression
> + *
> + * Written by Masami Hiramatsu <mhi...@re...>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> + *
> + */
> +
> +#include <sys/utsname.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include "perf.h"
> +#include "builtin.h"
> +#include "util/util.h"
> +#include "util/parse-options.h"
> +#include "util/parse-events.h"	/* For debugfs_path */
> +#include "util/probe-finder.h"
> +
> +/* Default vmlinux search paths */
> +#define NR_SEARCH_PATH 3
> +const char *default_search_path[NR_SEARCH_PATH] = {
> +"/lib/modules/%s/build/vmlinux",		/* Custom build kernel */
> +"/usr/lib/debug/lib/modules/%s/vmlinux",	/* Red Hat debuginfo */
> +"/boot/vmlinux-debug-%s",			/* Ubuntu */
> +};
> +
> +#define MAX_PATH_LEN 256
> +#define MAX_PROBES 128
> +
> +/* Session management structure */
> +static struct {
> +	char *vmlinux;
> +	char *release;
> +	int nr_probe;
> +	struct probe_point probes[MAX_PROBES];
> +	char *events[MAX_PROBES];
> +} session;
> +
> +static void semantic_error(const char *msg)
> +{
> +	fprintf(stderr, "Semantic error: %s\n", msg);
> +	exit(1);
> +}
> +
> +static void perror_exit(const char *msg)
> +{
> +	perror(msg);
> +	exit(1);
> +}
> +
> +#define MAX_PROBE_ARGS 128
> +
> +static int parse_probepoint(const struct option *opt __used,
> +			    const char *str, int unset __used)
> +{
> +	char *argv[MAX_PROBE_ARGS + 2];	/* Event + probe + args */
> +	int argc, i;
> +	char *arg, *ptr;
> +	struct probe_point *pp = &session.probes[session.nr_probe];
> +	char **event = &session.events[session.nr_probe];
> +	int retp = 0;
> +
> +	if (!str)	/* The end of probe points */
> +		return 0;
> +
> +	debug("Probe-define(%d): %s\n", session.nr_probe, str);
> +	if (++session.nr_probe == MAX_PROBES)
> +		semantic_error("Too many probes");
> +
> +	/* Separate arguments, similar to argv_split */
> +	argc = 0;
> +	do {
> +		/* Skip separators */
> +		while (isspace(*str))
> +			str++;
> +
> +		/* Add an argument */
> +		if (*str != '\0') {
> +			const char *s = str;
> +
> +			/* Skip the argument */
> +			while (!isspace(*str) && *str != '\0')
> +				str++;
> +
> +			/* Duplicate the argument */
> +			argv[argc] = strndup(s, str - s);
> +			if (argv[argc] == NULL)
> +				perror_exit("strndup");
> +			if (++argc == MAX_PROBE_ARGS)
> +				semantic_error("Too many arguments");
> +			debug("argv[%d]=%s\n", argc, argv[argc - 1]);
> +		}
> +	} while (*str != '\0');
> +	if (argc < 2)
> +		semantic_error("Need event-name and probe-point at least.");
> +
> +	/* Parse the event name */
> +	if (argv[0][0] == 'r')
> +		retp = 1;
> +	else if (argv[0][0] != 'p')
> +		semantic_error("You must specify 'p'(kprobe) or"
> +				" 'r'(kretprobe) first.");
> +	/* TODO: check event name */
> +	*event = argv[0];
> +
> +	/* Parse probe point */
> +	arg = argv[1];
> +	if (arg[0] == '@') {
> +		/* Source Line */
> +		arg++;
> +		ptr = strchr(arg, ':');
> +		if (!ptr || !isdigit(ptr[1]))
> +			semantic_error("Line number is required.");
> +		*ptr++ = '\0';
> +		if (strlen(arg) == 0)
> +			semantic_error("No file name.");
> +		pp->file = strdup(arg);
> +		pp->line = atoi(ptr);
> +		if (!pp->file || !pp->line)
> +			semantic_error("Failed to parse line.");
> +		debug("file:%s line:%d\n", pp->file, pp->line);
> +	} else {
> +		/* Function name */
> +		ptr = strchr(arg, '+');
> +		if (ptr) {
> +			if (!isdigit(ptr[1]))
> +				semantic_error("Offset is required.");
> +			*ptr++ = '\0';
> +			pp->offset = atoi(ptr);
> +		} else
> +			ptr = arg;
> +		ptr = strchr(ptr, '@');
> +		if (ptr) {
> +			*ptr++ = '\0';
> +			pp->file = strdup(ptr);
> +		}
> +		pp->function = strdup(arg);
> +		debug("symbol:%s file:%s offset:%d\n",
> +		      pp->function, pp->file, pp->offset);
> +	}
> +	free(argv[1]);
> +
> +	/* Copy arguments */
> +	pp->nr_args = argc - 2;
> +	if (pp->nr_args > 0) {
> +		pp->args = (char **)malloc(sizeof(char *) * pp->nr_args);

Hmm, you don't check for failed malloc here?

> +		memcpy(pp->args, &argv[2], sizeof(char *) * pp->nr_args);
> +	}
> +
> +	/* Ensure return probe has no C argument */
> +	if (retp)
> +		for (i = 0; i < pp->nr_args; i++)
> +			if (is_c_varname(pp->args[i]))
> +				semantic_error("You can't specify local"
> +						" variable for kretprobe");
> +	debug("%d arguments\n", pp->nr_args);
> +	return 0;
> +}
> +
> +static int open_default_vmlinux(void)
> +{
> +	struct utsname uts;
> +	char fname[MAX_PATH_LEN];
> +	int fd, ret, i;
> +
> +	if (!session.release) {
> +		ret = uname(&uts);
> +		if (ret) {
> +			debug("uname() failed.\n");
> +			return -errno;
> +		}
> +		session.release = uts.release;
> +	}
> +	for (i = 0; i < NR_SEARCH_PATH; i++) {
> +		ret = snprintf(fname, MAX_PATH_LEN,
> +			       default_search_path[i], session.release);
> +		if (ret >= MAX_PATH_LEN || ret < 0) {
> +			debug("Filename(%d,%s) is too long.\n", i, uts.release);
> +			errno = E2BIG;
> +			return -E2BIG;
> +		}
> +		debug("try to open %s\n", fname);
> +		fd = open(fname, O_RDONLY);
> +		if (fd >= 0)
> +			break;
> +	}
> +	return fd;
> +}
> +
> +static const char * const probe_usage[] = {
> +	"perf probe [<options>] -P 'PROBEDEF' [-P 'PROBEDEF' ...]",
> +	NULL
> +};
> +
> +static const struct option options[] = {
> +	OPT_STRING('k', "vmlinux", &session.vmlinux, "file",
> +		"vmlinux/module pathname"),
> +	OPT_STRING('r', "release", &session.release, "rel", "kernel release"),
> +	OPT_CALLBACK('P', "probe", NULL,
> +		"p|r:[GRP/]NAME FUNC[+OFFS][@SRC]|@SRC:LINE [ARG ...]",
> +		"probe point definition, where\n"
> +		"\t\tp:\tkprobe probe\n"
> +		"\t\tr:\tkretprobe probe\n"
> +		"\t\tGRP:\tGroup name (optional)\n"
> +		"\t\tNAME:\tEvent name\n"
> +		"\t\tFUNC:\tFunction name\n"
> +		"\t\tOFFS:\tOffset from function entry (in byte)\n"
> +		"\t\tSRC:\tSource code path\n"
> +		"\t\tLINE:\tLine number\n"
> +		"\t\tARG:\tProbe argument (local variable name or\n"
> +		"\t\t\tkprobe-tracer argument format is supported.)\n",
> +		parse_probepoint),
> +	OPT_END()
> +};
> +
> +static int write_new_event(int fd, const char *buf)
> +{
> +	int ret;
> +
> +	printf("Adding new event: %s\n", buf);
> +	ret = write(fd, buf, strlen(buf));
> +	if (ret <= 0)
> +		perror("Error: Failed to create event");
> +
> +	return ret;
> +}
> +
> +#define MAX_CMDLEN 256
> +
> +static int synthesize_probepoint(struct probe_point *pp)
> +{
> +	char *buf;
> +	int i, len, ret;
> +	pp->probes[0] = buf = (char *)calloc(MAX_CMDLEN, sizeof(char));

Again no check for failed calloc?

> +	ret = snprintf(buf, MAX_CMDLEN, "%s+%d", pp->function, pp->offset);
> +	if (ret <= 0 || ret >= MAX_CMDLEN)
> +		goto error;
> +	len = ret;
> +
> +	for (i = 0; i < pp->nr_args; i++) {
> +		ret = snprintf(&buf[len], MAX_CMDLEN - len, " %s",
> +			       pp->args[i]);
> +		if (ret <= 0 || ret >= MAX_CMDLEN - len)
> +			goto error;
> +		len += ret;
> +	}
> +	pp->found = 1;
> +	return pp->found;
> +error:
> +	free(pp->probes[0]);
> +	if (ret > 0)
> +		ret = -E2BIG;
> +	return ret;
> +}
> +
> +int cmd_probe(int argc, const char **argv, const char *prefix __used)
> +{
> +	int i, j, fd, ret, need_dwarf = 0;
> +	struct probe_point *pp;
> +	char buf[MAX_CMDLEN];
> +
> +	argc = parse_options(argc, argv, options, probe_usage,
> +		PARSE_OPT_STOP_AT_NON_OPTION);
> +	if (argc || session.nr_probe == 0)
> +		usage_with_options(probe_usage, options);
> +
> +	/* Synthesize return probes */
> +	for (j = 0; j < session.nr_probe; j++) {
> +		if (session.events[j][0] != 'r') {
> +			need_dwarf = 1;
> +			continue;
> +		}
> +		ret = synthesize_probepoint(&session.probes[j]);
> +		if (ret == -E2BIG)
> +			semantic_error("probe point is too long.");
> +		else if (ret < 0) {
> +			perror("snprintf");
> +			return -1;
> +		}
> +	}
> +
> +	if (!need_dwarf)
> +		goto setup_probes;
> +
> +	if (session.vmlinux)
> +		fd = open(session.vmlinux, O_RDONLY);
> +	else
> +		fd = open_default_vmlinux();
> +	if (fd < 0) {
> +		perror("vmlinux/module file open");
> +		return -1;
> +	}
> +
> +	/* Searching probe points */
> +	for (j = 0; j < session.nr_probe; j++) {
> +		pp = &session.probes[j];
> +		if (pp->found)
> +			continue;
> +
> +		lseek(fd, SEEK_SET, 0);
> +		ret = find_probepoint(fd, pp);
> +		if (ret <= 0) {
> +			fprintf(stderr, "Error: No probe point found.\n");
> +			return -1;
> +		}
> +		debug("probe event %s found\n", session.events[j]);
> +	}
> +	close(fd);
> +
> +setup_probes:
> +	/* Settng up probe points */
> +	snprintf(buf, MAX_CMDLEN, "%s/../kprobe_events", debugfs_path);
> +	fd = open(buf, O_WRONLY, O_APPEND);
> +	if (fd < 0) {
> +		perror("kprobe_events open");
> +		return -1;
> +	}
> +	for (j = 0; j < session.nr_probe; j++) {
> +		pp = &session.probes[j];
> +		if (pp->found == 1) {
> +			snprintf(buf, MAX_CMDLEN, "%s %s\n",
> +				session.events[j], pp->probes[0]);
> +			write_new_event(fd, buf);
> +		} else
> +			for (i = 0; i < pp->found; i++) {
> +				snprintf(buf, MAX_CMDLEN, "%s%d %s\n",
> +					session.events[j], i, pp->probes[i]);
> +				write_new_event(fd, buf);
> +			}
> +	}
> +	close(fd);
> +	return 0;
> +}
> +
> diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
> index e11d8d2..ad5f0f4 100644
> --- a/tools/perf/builtin.h
> +++ b/tools/perf/builtin.h

The rest looks fine (for what I can tell, it is all dwarf magic to
me ;-)

-- Steve

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Steven R. <ro...@go...>]

On Mon, 2009-10-05 at 21:26 +0200, Frederic Weisbecker wrote:
> On Mon, Oct 05, 2009 at 12:59:01PM -0400, Masami Hiramatsu wrote:
> > As far as I can see in arch/*/include/asm/ptrace.h, all registers start with
> > alphabets :-). So, I'd like to suggest renaming sp-vars to '_sp-vars'.
> >
> > Then, we will have;
> > - $local-vars
> 
> 
> There is a risk of bash collision.

I actually prefer the "$" notation. As for bash collision, it is common
for shell script writers to be able to distinguish a variable from bash.
Yes we can backslash it, or quote it. But when I see a $var it sticks
out to me that it is a variable. It's not hard to get around. For
example, type:

$ echo "hello $DISPLAY"' or $DISPLAY'

and see what you get.

Makefiles and Perl use '$' for variables those that need to handle it
with bash can easily cope with it.

So my vote is to keep the '$'. It is the most intuitive to what it
means.

Just my 0.02€

-- Steve

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami H. <mhi...@re...>]

Masami Hiramatsu wrote:
> Then, can we use @@ for prefix of special variables?? :-)

Otherwise, %@vars(means register_of-alias-vars) is another candidate.

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami H. <mhi...@re...>]

Frederic Weisbecker wrote:
> On Mon, Oct 05, 2009 at 05:34:24PM -0400, Masami Hiramatsu wrote:
>> Hmm, one idea hits me, how about this? :)
>> - %register
>> - %%spvars (%%retval, %%arg0)
>
>
> The problem is that such % or %% symbols have a specific
> mean in some other well known areas.
>
> If we borrow the % from the AT&T assembly syntax style
> to use register names, that we can retrieve in gcc inline
> assembly, then one may expect %% to have a meaning inspired
> from the same area. %% has its sense in gcc inline assembly,
> but applied there, it looks confusing.
>
> I mean, I'm trying to think like someone reading a perf probe
> command line without any documentation. The more this person
> can understand this command line without documentation, the better.
> We know that % is used for register names, some people know that %%
> is used for register names too but when we are in gcc inline assembly
> with var to reg resolution and need true registers name.

Hmm, but %%reg syntax is only for the special case of gcc-inline
assembly (e.g. assembler template, see
http://www.ibiblio.org/gferg/ldp/GCC-Inline-Assembly-HOWTO.html#s3).
So, I guess it will not be so confusing.

> Then if I try to mirror this sense from gcc to perf probe use,
> I feel confused, especially in the case of %%arg1.
>
> In this case, we should rather have %%register and %arg0 :)
>
> Hm, %register is a clear pattern.
>
> Somehow, %retval looks clear too, retval is verbose enough and
> % is still logical as return values are most of the time (always?)
> put in a register.
>
> But %%arg0 looks confusing.

Then, can we use @@ for prefix of special variables?? :-)

I'm so anxious about collision between register name and
those vars.

Thank you,

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic W. <fwe...@gm...>]

On Mon, Oct 05, 2009 at 11:55:50PM +0200, Frederic Weisbecker wrote:
> In this case, we should rather have %%register and %arg0 :)

And the above construct would be so much not obvious in its self
meaning, wrt its gcc inline inspiration.

/me should stop quibbling...

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic W. <fwe...@gm...>]

On Mon, Oct 05, 2009 at 05:34:24PM -0400, Masami Hiramatsu wrote:
> Hmm, one idea hits me, how about this? :)
> - %register
> - %%spvars (%%retval, %%arg0)


The problem is that such % or %% symbols have a specific
mean in some other well known areas.

If we borrow the % from the AT&T assembly syntax style
to use register names, that we can retrieve in gcc inline
assembly, then one may expect %% to have a meaning inspired
from the same area. %% has its sense in gcc inline assembly,
but applied there, it looks confusing.

I mean, I'm trying to think like someone reading a perf probe
command line without any documentation. The more this person
can understand this command line without documentation, the better.
We know that % is used for register names, some people know that %%
is used for register names too but when we are in gcc inline assembly
with var to reg resolution and need true registers name.
Then if I try to mirror this sense from gcc to perf probe use,
I feel confused, especially in the case of %%arg1.

In this case, we should rather have %%register and %arg0 :)

Hm, %register is a clear pattern.

Somehow, %retval looks clear too, retval is verbose enough and
% is still logical as return values are most of the time (always?)
put in a register.

But %%arg0 looks confusing.

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami H. <mhi...@re...>]

Frederic Weisbecker wrote:
> On Mon, Oct 05, 2009 at 05:11:05PM -0400, Masami Hiramatsu wrote:
>> Frederic Weisbecker wrote:
>>> Hmm, the problem is that %1, %2, etc. is not very self-explainable.
>>>
>>> May be %arg1, %arg2, etc.. But would that sound confusing since we
>>> have % for registers?
>>
>> As I sent right now, how about %argumentN ? it will not conflict with
>> register names...
>>
>
> There are archs that have %arg0 %arg1, ... as register names?
>
> Well, arg(n) looks shorter but I won't personnally mind if
> we eventually chose %argumentN. It's also clear, self-explainable
> and it won't collide.

Hmm, one idea hits me, how about this? :)
- %register
- %%spvars (%%retval, %%arg0)

Thank you,

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic W. <fwe...@gm...>]

On Mon, Oct 05, 2009 at 05:11:05PM -0400, Masami Hiramatsu wrote:
> Frederic Weisbecker wrote:
>> Hmm, the problem is that %1, %2, etc. is not very self-explainable.
>>
>> May be %arg1, %arg2, etc.. But would that sound confusing since we
>> have % for registers?
>
> As I sent right now, how about %argumentN ? it will not conflict with
> register names...
>

There are archs that have %arg0 %arg1, ... as register names?

Well, arg(n) looks shorter but I won't personnally mind if
we eventually chose %argumentN. It's also clear, self-explainable
and it won't collide.

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic W. <fwe...@gm...>]

On Mon, Oct 05, 2009 at 05:05:32PM -0400, Masami Hiramatsu wrote:
> Frederic Weisbecker wrote:
>>> - @global-symbol
>>
>>
>> We could use global-symbol as is. Shadowing between global
>> and local vars could be dealt with scope resolution:
>>
>> function:var
>> file:var
>> file:line:var
>>
>> And throw errors while submitting a shadowed var name, crying until
>> the user defines the scope, only if needed of course (if there are
>> no shadowing detected, we can submit a naked variable name).
>
> Sure, via perf-probe, global/local symbols will be translated into
> @address by debuginfo.
> But without debuginfo, we still need @symbol syntax for accessing
> global defined symbols.



Ah right.


>>> - +|-Offs(ARG)
>>
>> You mean for arg numbers?
>> So we would have +1 for argument 1?
>
> Sorry for confusing you, it is for deref syntax.


Ok :)

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami H. <mhi...@re...>]

Frederic Weisbecker wrote:
> On Mon, Oct 05, 2009 at 04:18:39PM -0400, Masami Hiramatsu wrote:
>> Frederic Weisbecker wrote:
>>> For the function arguments, I guess we don't need to worry
>>> anymore about r0, r1, etc... but we can deal with the true var
>>> name, without any kind of prefixes.
>>
>> This depends on ABI, function argument from ABI doesn't need
>> debuginfo, but it will be unstable on some arch, e.g. x86-32
>> with/without asmlinkage.
>>
>> Thus, I think that we can just describe where function arguments
>> will be(e.g. arg0 is ax) as a note for each architecture
>> in Documents/trace/kprobetrace.txt.
>
>
> Yeah that may help. Although everyone can look at the calling convention
> ABI for a given arch but that would still help.
>
>
>>> What about @return :-) ?
>>
>> Hmm, it might conflict with global symbol... Maybe, we can remove this
>> because retprobe already shows return address in the head of entry.
>
>
> It won't conflict since "return" is a reserved word and can't then be
> used as a symbol.
>
> But yeah, if it's an embeded field, we should remove it.
>
>
>>> What if we take the following:
>>>
>>> [Ftrace and perf probe side]
>>>
>>> %reg = registers, we can also play with deref and offsets like (%esp), 8(%esp), etc.
>>
>> Hmm, on x86-32, sp at intr context is not pointing the top of stack. actually&sp is
>> the real address of the stack :(
>> Perhaps, on x86-32, we can translate %sp to stack address in kprobe-tracer.
>
>
> Oh? You mean in the saved registers while triggering an int 3?

Yes, interrupt/exception handlers don't save sp on x86-32.

>>> arg(n) = arg number, where n is the number
>>
>> How about %N? or just adds a note in documents.
>>
>
>
> Hmm, the problem is that %1, %2, etc. is not very self-explainable.
>
> May be %arg1, %arg2, etc.. But would that sound confusing since we
> have % for registers?

As I sent right now, how about %argumentN ? it will not conflict with
register names...

Thank you,

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami H. <mhi...@re...>]

Frederic Weisbecker wrote:
>> - @global-symbol
>
>
> We could use global-symbol as is. Shadowing between global
> and local vars could be dealt with scope resolution:
>
> function:var
> file:var
> file:line:var
>
> And throw errors while submitting a shadowed var name, crying until
> the user defines the scope, only if needed of course (if there are
> no shadowing detected, we can submit a naked variable name).

Sure, via perf-probe, global/local symbols will be translated into
@address by debuginfo.
But without debuginfo, we still need @symbol syntax for accessing
global defined symbols.

>> - +|-Offs(ARG)
>
> You mean for arg numbers?
> So we would have +1 for argument 1?

Sorry for confusing you, it is for deref syntax.

> arg(1) looks more easy to remember and to understand, no?

I think arg(N) seems a bit different from other parts.
Perhaps, %argumentN is possible ? :-)

Thank you,

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic W. <fwe...@gm...>]

On Mon, Oct 05, 2009 at 04:18:39PM -0400, Masami Hiramatsu wrote:
> Frederic Weisbecker wrote:
>> For the function arguments, I guess we don't need to worry
>> anymore about r0, r1, etc... but we can deal with the true var
>> name, without any kind of prefixes.
>
> This depends on ABI, function argument from ABI doesn't need
> debuginfo, but it will be unstable on some arch, e.g. x86-32
> with/without asmlinkage.
>
> Thus, I think that we can just describe where function arguments
> will be(e.g. arg0 is ax) as a note for each architecture
> in Documents/trace/kprobetrace.txt.


Yeah that may help. Although everyone can look at the calling convention
ABI for a given arch but that would still help.


>> What about @return :-) ?
>
> Hmm, it might conflict with global symbol... Maybe, we can remove this
> because retprobe already shows return address in the head of entry.


It won't conflict since "return" is a reserved word and can't then be
used as a symbol.

But yeah, if it's an embeded field, we should remove it.


>> What if we take the following:
>>
>> [Ftrace and perf probe side]
>>
>> %reg = registers, we can also play with deref and offsets like (%esp), 8(%esp), etc.
>
> Hmm, on x86-32, sp at intr context is not pointing the top of stack. actually &sp is
> the real address of the stack :(
> Perhaps, on x86-32, we can translate %sp to stack address in kprobe-tracer.



Oh? You mean in the saved registers while triggering an int 3?



> > %return = return value
>
> or %retval? :)


Yeah, better!


>
>> @return = return addr
>
> I'd like to remove it, because it's already shown.


Ok.



>> arg(n) = arg number, where n is the number
>
> How about %N? or just adds a note in documents.
>


Hmm, the problem is that %1, %2, etc. is not very self-explainable.

May be %arg1, %arg2, etc.. But would that sound confusing since we
have % for registers?



>> [Perf probe only]
>>
>> var = variable name, global or local, we can deal with shadow issues later
>>        through variable scope: func_name:var, filename:var, whatever for now
>>        it's not a problem. Local also includes argument names.
>
> That's fine to me. :-)
>


Great :)
Thanks!

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami H. <mhi...@re...>]

Frederic Weisbecker wrote:
> On Sun, Oct 04, 2009 at 01:21:52AM -0400, Masami Hiramatsu wrote:
>> Hmm, # is widely used for comment, including some kernel pseudo
>> file interfaces, kprobe_events too. Comments are useful if a
>> probe list is restored from a file.
>>
>
>
> Right, let's think about something else.
>
>
>> For accessing local variables, kprobe-tracer needs to support *at least*
>> below variables:
>> - Registers
>> - Stack address (if a register points stack address, this isn't needed)
>
>
> Ok.
> Well, thinking more about the % sign, we shouldn't worry about
> format confusion, since it's a commonly used character for registers,
> we can take it for them: %rax, %rbx, etc. (is that what you did
> in this patch? I don't remember exactly...)

Yeah, that's what it does currently.

> And for addresses: @addr
>
>
>> Below special vars are complementary aliases.
>> - Function arguments
>
>
>
> For the function arguments, I guess we don't need to worry
> anymore about r0, r1, etc... but we can deal with the true var
> name, without any kind of prefixes.

This depends on ABI, function argument from ABI doesn't need
debuginfo, but it will be unstable on some arch, e.g. x86-32
with/without asmlinkage.

Thus, I think that we can just describe where function arguments
will be(e.g. arg0 is ax) as a note for each architecture
in Documents/trace/kprobetrace.txt.

>> - Return value
>
> What about %return ?
>
> As return values are usually stored in a register (at least in Arm
> and x86, I don't know about the others), the % prefix fits well for
> that.

Sure, that's what I

>> - Return address
>
>
> What about @return :-) ?

Hmm, it might conflict with global symbol... Maybe, we can remove this
because retprobe already shows return address in the head of entry.

(BTW, ideally, I think 'IP' should be the return address in kretprobe.
  But current implementation isn't.)

> That said we shouldn't worry about that in perf, since we can
> take snapshots of the backtraces, like in some other perf events.

Agreed.

>> and I'd like perf-probe to have a transparent syntax with kprobe-tracer.
>
>
> That's feasible. But think about the fact that perf probe benefits
> from a higher level of code view. Now that we have global and local
> variables resolution, we can't anymore expect using r1, r2, rax,
> a1, rv, ra without risking to collide with variable names.
>
> But this tracer hasn't been merged yet, so it's still time
> to update its interface.

Sure.

>> This means, if we can remove special vars except registers, or rename it
>> non-conflictable name with registers, we just need to separate name spaces
>> of
>> - Regsiters
>> - Local variables
>
>
> Yeah.
>
>
>> Here, local variables will support fields of data structs, and it will
>> use '->' expression. Since'>' means redirection in bash, local variables
>> need to be *escaped* in this case. Thus, I think we can use '$' prefix
>> for it. (I'm OK, because this is similar syntax to systemtap:-).
>>
>> So, if you don't like %regs, $svars and locals, we can use regs and $locals :-)
>
>
> '>' means redirection, but at least inside quotes it's not interpreted.

Ah, indeed.

> I'm fine with %regs actually. But I really don't like the "$" because
> I really worry about shell substitutions.
>
> Most people delimit their strings with double quotes.

Sure, especially C programmers :-)

> What if we take the following:
>
> [Ftrace and perf probe side]
>
> %reg = registers, we can also play with deref and offsets like (%esp), 8(%esp), etc.

Hmm, on x86-32, sp at intr context is not pointing the top of stack. actually &sp is
the real address of the stack :(
Perhaps, on x86-32, we can translate %sp to stack address in kprobe-tracer.

 > %return = return value

or %retval? :)

> @return = return addr

I'd like to remove it, because it's already shown.

> arg(n) = arg number, where n is the number

How about %N? or just adds a note in documents.


> [Perf probe only]
>
> var = variable name, global or local, we can deal with shadow issues later
>        through variable scope: func_name:var, filename:var, whatever for now
>        it's not a problem. Local also includes argument names.

That's fine to me. :-)

Thank you!

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic W. <fwe...@gm...>]

On Mon, Oct 05, 2009 at 09:18:31PM +0200, Frederic Weisbecker wrote:
> On Sun, Oct 04, 2009 at 01:21:52AM -0400, Masami Hiramatsu wrote:
> > Hmm, # is widely used for comment, including some kernel pseudo
> > file interfaces, kprobe_events too. Comments are useful if a
> > probe list is restored from a file.
> >
> 
> 
> Right, let's think about something else.
> 
> 
> > For accessing local variables, kprobe-tracer needs to support *at least*
> > below variables:
> > - Registers
> > - Stack address (if a register points stack address, this isn't needed)
> 
> 
> Ok.
> Well, thinking more about the % sign, we shouldn't worry about
> format confusion, since it's a commonly used character for registers,
> we can take it for them: %rax, %rbx, etc. (is that what you did
> in this patch? I don't remember exactly...)
> 
> And for addresses: @addr
> 
> 
> > Below special vars are complementary aliases.
> > - Function arguments
> 
> 
> 
> For the function arguments, I guess we don't need to worry
> anymore about r0, r1, etc... but we can deal with the true var
> name, without any kind of prefixes.


Sorry, as you pointed out, it's better to keep the same syntax for
both perf probe and ftrace, but having perf probe able to support
variable name.

Hence the following that don't collide: varname, arg(n)


> 
> > - Return value
> 
> 
> 
> What about %return ?
> 
> As return values are usually stored in a register (at least in Arm
> and x86, I don't know about the others), the % prefix fits well for
> that.
> 
> 
> > - Return address
> 
> 
> What about @return :-) ?
> 
> That said we shouldn't worry about that in perf, since we can
> take snapshots of the backtraces, like in some other perf events.
> 



But we need to worry about it because we want to share the ftrace syntax.
So yeah, why not @return ?

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic W. <fwe...@gm...>]

On Mon, Oct 05, 2009 at 12:59:01PM -0400, Masami Hiramatsu wrote:
> As far as I can see in arch/*/include/asm/ptrace.h, all registers start with
> alphabets :-). So, I'd like to suggest renaming sp-vars to '_sp-vars'.
>
> Then, we will have;
> - $local-vars


There is a risk of bash collision.


> - @global-symbol


We could use global-symbol as is. Shadowing between global
and local vars could be dealt with scope resolution:

function:var
file:var
file:line:var

And throw errors while submitting a shadowed var name, crying until
the user defines the scope, only if needed of course (if there are
no shadowing detected, we can submit a naked variable name).


> - regs


That can conflict with variable names


> - _sp-vars


That too.


> - +|-Offs(ARG)

You mean for arg numbers?
So we would have +1 for argument 1?

arg(1) looks more easy to remember and to understand, no?

Thanks.

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic W. <fwe...@gm...>]

On Sun, Oct 04, 2009 at 01:21:52AM -0400, Masami Hiramatsu wrote:
> Hmm, # is widely used for comment, including some kernel pseudo
> file interfaces, kprobe_events too. Comments are useful if a
> probe list is restored from a file.
>


Right, let's think about something else.


> For accessing local variables, kprobe-tracer needs to support *at least*
> below variables:
> - Registers
> - Stack address (if a register points stack address, this isn't needed)


Ok.
Well, thinking more about the % sign, we shouldn't worry about
format confusion, since it's a commonly used character for registers,
we can take it for them: %rax, %rbx, etc. (is that what you did
in this patch? I don't remember exactly...)

And for addresses: @addr


> Below special vars are complementary aliases.
> - Function arguments



For the function arguments, I guess we don't need to worry
anymore about r0, r1, etc... but we can deal with the true var
name, without any kind of prefixes.


> - Return value



What about %return ?

As return values are usually stored in a register (at least in Arm
and x86, I don't know about the others), the % prefix fits well for
that.


> - Return address


What about @return :-) ?

That said we shouldn't worry about that in perf, since we can
take snapshots of the backtraces, like in some other perf events.


> and I'd like perf-probe to have a transparent syntax with kprobe-tracer.


That's feasible. But think about the fact that perf probe benefits
from a higher level of code view. Now that we have global and local
variables resolution, we can't anymore expect using r1, r2, rax, 
a1, rv, ra without risking to collide with variable names.

But this tracer hasn't been merged yet, so it's still time
to update its interface.

 
> This means, if we can remove special vars except registers, or rename it
> non-conflictable name with registers, we just need to separate name spaces
> of
> - Regsiters
> - Local variables


Yeah.


> Here, local variables will support fields of data structs, and it will
> use '->' expression. Since '>' means redirection in bash, local variables
> need to be *escaped* in this case. Thus, I think we can use '$' prefix
> for it. (I'm OK, because this is similar syntax to systemtap:-).
> 
> So, if you don't like %regs, $svars and locals, we can use regs and $locals :-)


'>' means redirection, but at least inside quotes it's not interpreted.

I'm fine with %regs actually. But I really don't like the "$" because
I really worry about shell substitutions.

Most people delimit their strings with double quotes.

What if we take the following:

[Ftrace and perf probe side]

%reg = registers, we can also play with deref and offsets like (%esp), 8(%esp), etc.
%return = return value
@return = return addr
arg(n) = arg number, where n is the number


[Perf probe only]

var = variable name, global or local, we can deal with shadow issues later
      through variable scope: func_name:var, filename:var, whatever for now
      it's not a problem. Local also includes argument names.

Hmm?

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami H. <mhi...@re...>]

Masami Hiramatsu wrote:
> Frederic Weisbecker wrote:
>> On Fri, Oct 02, 2009 at 05:48:42PM -0400, Masami Hiramatsu wrote:
>>>    FETCHARGS	: Arguments. Each probe can have up to 128 args.
>>>     %REG	: Fetch register REG
>>> -  sN	: Fetch Nth entry of stack (N>= 0)
>>> -  sa	: Fetch stack address.
>>>     @ADDR	: Fetch memory at ADDR (ADDR should be in kernel)
>>>     @SYM[+|-offs]	: Fetch memory at SYM +|- offs (SYM should be a data symbol)
>>> -  aN	: Fetch function argument. (N>= 0)(*)
>>> -  rv	: Fetch return value.(**)
>>> -  ra	: Fetch return address.(**)
>>> +  $sN	: Fetch Nth entry of stack (N>= 0)
>>> +  $sa	: Fetch stack address.
>>> +  $aN	: Fetch function argument. (N>= 0)(*)
>>> +  $rv	: Fetch return value.(**)
>>> +  $ra	: Fetch return address.(**)
>>
>>
>>
>> I feel uncomfortable with that, because of bash scripts that
>> may use it and always need to escape it with antislashes or
>> use single quotes for it to not be replaced by a random variable
>> value. If one uses double quotes without antislashes to protect
>> the command line, the result is unpredictable, depending of
>> the current set of variables...
>>
>> May be we can use # instead of $ ? That's a kind of pity because
>> $ suggested a variable whereas # suggests a constant, we are then
>> losing this self-explainable characteristic for kprobes
>> "specific variable fetchs". But that will work without ambiguity.
>
> Hmm, # is widely used for comment, including some kernel pseudo
> file interfaces, kprobe_events too. Comments are useful if a
> probe list is restored from a file.
>
> For accessing local variables, kprobe-tracer needs to support *at least*
> below variables:
> - Registers
> - Stack address (if a register points stack address, this isn't needed)
>
> Below special vars are complementary aliases.
> - Function arguments
> - Return value
> - Return address
>
> and I'd like perf-probe to have a transparent syntax with kprobe-tracer.
>
> This means, if we can remove special vars except registers, or rename it
> non-conflictable name with registers, we just need to separate name spaces
> of
> - Regsiters
> - Local variables
>
> Here, local variables will support fields of data structs, and it will
> use '->' expression. Since'>' means redirection in bash, local variables
> need to be *escaped* in this case. Thus, I think we can use '$' prefix
> for it. (I'm OK, because this is similar syntax to systemtap:-).
>
> So, if you don't like %regs, $svars and locals, we can use regs and $locals :-)

As far as I can see in arch/*/include/asm/ptrace.h, all registers start with
alphabets :-). So, I'd like to suggest renaming sp-vars to '_sp-vars'.

Then, we will have;
- $local-vars
- @global-symbol
- regs
- _sp-vars
- +|-Offs(ARG)

Thank you,

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH tracing/kprobes v2 0/5] tracing/kprobes, perf: perf probe support take 2

[Masami H. <mhi...@re...>]

Frank Ch. Eigler wrote:
> Masami Hiramatsu<mhi...@re...>  writes:
>
>> [...]
>> These patches introduce 'perf probe' command and update kprobe-tracer. [...]
>> Usage
>> -----
>>   perf probe [<options>] -P 'PROBEDEF' [-P 'PROBEDEF' ...]
>> [...]
>>      -k, --vmlinux<file>   vmlinux/module pathname
>>      -r, --release<rel>    kernel release
>> [...]
>
> Can you outline a reason for the -r flag?  We use it in systemtap for
> cross-compiling purposes, but I didn't think perf was interested in that.

Indeed, thanks!
I just forgot to remove -r option from the code of c2kpe.c :(

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH tracing/kprobes v2 0/5] tracing/kprobes, perf: perf probe support take 2

[<fc...@re...>]

Masami Hiramatsu <mhi...@re...> writes:

> [...]
> These patches introduce 'perf probe' command and update kprobe-tracer. [...]
> Usage
> -----
>  perf probe [<options>] -P 'PROBEDEF' [-P 'PROBEDEF' ...]
> [...]
>     -k, --vmlinux <file>  vmlinux/module pathname
>     -r, --release <rel>   kernel release
> [...]

Can you outline a reason for the -r flag?  We use it in systemtap for
cross-compiling purposes, but I didn't think perf was interested in that.

- FChE

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami H. <mhi...@re...>]

Frederic Weisbecker wrote:
> On Fri, Oct 02, 2009 at 05:48:42PM -0400, Masami Hiramatsu wrote:
>> Add $ prefix to the special variables(e.g. sa, rv) of kprobe-tracer.
>> This resolves consistency issue between kprobe_events and perf-kprobe.
>>
>> Signed-off-by: Masami Hiramatsu <mhi...@re...>
>> Cc: Frederic Weisbecker <fwe...@gm...>
>> Cc: Ingo Molnar <mi...@el...>
>> Cc: Thomas Gleixner <tg...@li...>
>> Cc: Arnaldo Carvalho de Melo <ac...@re...>
>> Cc: Steven Rostedt <ro...@go...>
>> Cc: Mike Galbraith <ef...@gm...>
>> Cc: Paul Mackerras <pa...@sa...>
>> Cc: Peter Zijlstra <a.p...@ch...>
>> Cc: Christoph Hellwig <hc...@in...>
>> Cc: Ananth N Mavinakayanahalli <an...@in...>
>> Cc: Jim Keniston <jke...@us...>
>> Cc: Frank Ch. Eigler <fc...@re...>
>> ---
>>
>>  Documentation/trace/kprobetrace.txt |   10 +++---
>>  kernel/trace/trace_kprobe.c         |   60 ++++++++++++++++++++++-------------
>>  2 files changed, 42 insertions(+), 28 deletions(-)
>>
>> diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
>> index 9b8f7c6..40caef0 100644
>> --- a/Documentation/trace/kprobetrace.txt
>> +++ b/Documentation/trace/kprobetrace.txt
>> @@ -36,13 +36,13 @@ Synopsis of kprobe_events
>>  
>>   FETCHARGS	: Arguments. Each probe can have up to 128 args.
>>    %REG	: Fetch register REG
>> -  sN	: Fetch Nth entry of stack (N >= 0)
>> -  sa	: Fetch stack address.
>>    @ADDR	: Fetch memory at ADDR (ADDR should be in kernel)
>>    @SYM[+|-offs]	: Fetch memory at SYM +|- offs (SYM should be a data symbol)
>> -  aN	: Fetch function argument. (N >= 0)(*)
>> -  rv	: Fetch return value.(**)
>> -  ra	: Fetch return address.(**)
>> +  $sN	: Fetch Nth entry of stack (N >= 0)
>> +  $sa	: Fetch stack address.
>> +  $aN	: Fetch function argument. (N >= 0)(*)
>> +  $rv	: Fetch return value.(**)
>> +  $ra	: Fetch return address.(**)
> 
> 
> 
> I feel uncomfortable with that, because of bash scripts that
> may use it and always need to escape it with antislashes or
> use single quotes for it to not be replaced by a random variable
> value. If one uses double quotes without antislashes to protect
> the command line, the result is unpredictable, depending of
> the current set of variables...
> 
> May be we can use # instead of $ ? That's a kind of pity because
> $ suggested a variable whereas # suggests a constant, we are then
> losing this self-explainable characteristic for kprobes
> "specific variable fetchs". But that will work without ambiguity.

Hmm, # is widely used for comment, including some kernel pseudo
file interfaces, kprobe_events too. Comments are useful if a
probe list is restored from a file.

For accessing local variables, kprobe-tracer needs to support *at least*
below variables:
- Registers
- Stack address (if a register points stack address, this isn't needed)

Below special vars are complementary aliases.
- Function arguments
- Return value
- Return address

and I'd like perf-probe to have a transparent syntax with kprobe-tracer.

This means, if we can remove special vars except registers, or rename it
non-conflictable name with registers, we just need to separate name spaces
of
- Regsiters
- Local variables

Here, local variables will support fields of data structs, and it will
use '->' expression. Since '>' means redirection in bash, local variables
need to be *escaped* in this case. Thus, I think we can use '$' prefix
for it. (I'm OK, because this is similar syntax to systemtap:-).

So, if you don't like %regs, $svars and locals, we can use regs and $locals :-)

Thank you,


-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhi...@re...

Re: [Dle-develop] [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic W. <fwe...@gm...>]

On Fri, Oct 02, 2009 at 05:48:42PM -0400, Masami Hiramatsu wrote:
> Add $ prefix to the special variables(e.g. sa, rv) of kprobe-tracer.
> This resolves consistency issue between kprobe_events and perf-kprobe.
> 
> Signed-off-by: Masami Hiramatsu <mhi...@re...>
> Cc: Frederic Weisbecker <fwe...@gm...>
> Cc: Ingo Molnar <mi...@el...>
> Cc: Thomas Gleixner <tg...@li...>
> Cc: Arnaldo Carvalho de Melo <ac...@re...>
> Cc: Steven Rostedt <ro...@go...>
> Cc: Mike Galbraith <ef...@gm...>
> Cc: Paul Mackerras <pa...@sa...>
> Cc: Peter Zijlstra <a.p...@ch...>
> Cc: Christoph Hellwig <hc...@in...>
> Cc: Ananth N Mavinakayanahalli <an...@in...>
> Cc: Jim Keniston <jke...@us...>
> Cc: Frank Ch. Eigler <fc...@re...>
> ---
> 
>  Documentation/trace/kprobetrace.txt |   10 +++---
>  kernel/trace/trace_kprobe.c         |   60 ++++++++++++++++++++++-------------
>  2 files changed, 42 insertions(+), 28 deletions(-)
> 
> diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
> index 9b8f7c6..40caef0 100644
> --- a/Documentation/trace/kprobetrace.txt
> +++ b/Documentation/trace/kprobetrace.txt
> @@ -36,13 +36,13 @@ Synopsis of kprobe_events
>  
>   FETCHARGS	: Arguments. Each probe can have up to 128 args.
>    %REG	: Fetch register REG
> -  sN	: Fetch Nth entry of stack (N >= 0)
> -  sa	: Fetch stack address.
>    @ADDR	: Fetch memory at ADDR (ADDR should be in kernel)
>    @SYM[+|-offs]	: Fetch memory at SYM +|- offs (SYM should be a data symbol)
> -  aN	: Fetch function argument. (N >= 0)(*)
> -  rv	: Fetch return value.(**)
> -  ra	: Fetch return address.(**)
> +  $sN	: Fetch Nth entry of stack (N >= 0)
> +  $sa	: Fetch stack address.
> +  $aN	: Fetch function argument. (N >= 0)(*)
> +  $rv	: Fetch return value.(**)
> +  $ra	: Fetch return address.(**)



I feel uncomfortable with that, because of bash scripts that
may use it and always need to escape it with antislashes or
use single quotes for it to not be replaced by a random variable
value. If one uses double quotes without antislashes to protect
the command line, the result is unpredictable, depending of
the current set of variables...

May be we can use # instead of $ ? That's a kind of pity because
$ suggested a variable whereas # suggests a constant, we are then
losing this self-explainable characteristic for kprobes
"specific variable fetchs". But that will work without ambiguity.

I hope someone else has a better idea...

Re: [Dle-develop] [PATCH tracing/kprobes v2 0/5] tracing/kprobes, perf: perf probe support take 2

[Frederic W. <fwe...@gm...>]

On Fri, Oct 02, 2009 at 05:48:34PM -0400, Masami Hiramatsu wrote:
> Hi,
> 
> These patches introduce 'perf probe' command and update kprobe-tracer.
> perf probe command allows you to add new probe points by C line number
> and local variable names.
> 
> This version fixes some bugs, changes subcommand name from kprobe to
> probe and use spaces for separator instead of ',' for visibility (this
> also make it easy to support probe list from stdin).
> 
> Usage
> -----
>  perf probe [<options>] -P 'PROBEDEF' [-P 'PROBEDEF' ...]
> 
>     -k, --vmlinux <file>  vmlinux/module pathname
>     -r, --release <rel>   kernel release
>     -P, --probe <p|r:[GRP/]NAME FUNC[+OFFS][@SRC]|@SRC:LINE [ARG ...]>
>                           probe point definition, where
> 		p:	kprobe probe
> 		r:	kretprobe probe
> 		GRP:	Group name (optional)
> 		NAME:	Event name
> 		FUNC:	Function name
> 		OFFS:	Offset from function entry (in byte)
> 		SRC:	Source code path
> 		LINE:	Line number
> 		ARG:	Probe argument (local variable name or
> 			kprobe-tracer argument format is supported.)
> 
> Examples
> --------
> 1) Add a new kprobe probe on a line of C source code.
> ./perf probe -P 'p:myprobe @fs/read_write.c:285 file buf count'
> Adding new event: p:myprobe vfs_read+57 file=%bx buf=%si count=%ax


Nice! Great thing.

One neat, at a first glance, file=%bx buf=%si look like a format
definition.

How about using file=bx ? Or does that introduce any ambiguities
with kprobes definitions syntax?



> 
> 2) Add a new kretprobe probe on a function return.
> ./perf probe -P 'r:myretprobe vfs_read $rv'
> Adding new event: r:myretprobe vfs_read+0 $rv


The '$' character may perhaps also confuse bash scripts that create
perf probe.



> 3) Check it in the perf list.
> ./perf list
> ...
>   rNNN                                       [raw hardware event descriptor]
> 
>   kprobes:myprobe                            [Tracepoint event]
>   kprobes:myretprobe                         [Tracepoint event]
>   skb:kfree_skb                              [Tracepoint event]
> ...
> 
> 4) Record the event by perf
> ./perf record -f -e kprobes:myprobe:record  -F 1 -a ls
> ...
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.081 MB perf.data (~3540 samples) ]
> 
> 5) Trace the event
> ./perf trace
>             perf-11445 [000] 95862.048894383: myprobe: (c04bbed5) file=dae15e80 buf=b78b2000 count=400
>             perf-11445 [000] 95862.049066533: myprobe: (c04bbed5) file=dae15d80 buf=b78b2000 count=400
>             perf-11445 [000] 95862.049134394: myprobe: (c04bbed5) file=dae15d80 buf=b78b2000 count=400
>             perf-11445 [000] 95862.049171495: myprobe: (c04bbed5) file=dae15a80 buf=b78b2000 count=400
> 
> NOTE
> ----
>  perf still fails to parse format if arguments have special charactors
> (e.g. $rv, +10($sp) etc.) So, tracing myretprobe will fail with this
> version. This will be solved by naming arguments automatically if it
> doesn't have C-language name.
> 
> TODO
> ----
>  - Support sys_perf_counter_open (non-root)


Hmm, we really want it to be only usable by the root
(except if the policy is tuned to allow that, but that's
managed by perf already).


>  - Input from stdin/output to stdout
>  - Non-auto static variable
>  - Fields of data structures (var->field)
>  - Type support
>    - Bit fields
>  - Array (var[N])
>  - Dynamic array indexing (var[var2])
>  - String/dynamic arrays (var:string, var[N..M])
>  - Force Type casting ((type)var)
>  - Non-inline search
>  - libdw, libdwfl
>  - etc.
> 
> Thank you,
> 


Cool, I'm reviewing/testing it and if no rough problem
arise I'll apply it.

Thanks a lot!