[PATCH v4 0/3] perf/sdt : Support for SDT markers

[PATCH v4 0/3] perf/sdt : Support for SDT markers

[Hemant Kumar]

The v3 patchset had some spacing errors because of my editor messing up during
sending the patches. Resending the patchset again.
This patchset helps in listing dtrace style markers(SDT) present in user space
applications through perf.
Notes/markers are placed at important places by the
developers. They have a negligible overhead when not enabled.
We can enable them and probe at these places and find some important information
like the arguments' values, etc.

We have lots of applications which use SDT markers today, like:
Postgresql, MySql, Mozilla, Perl, Python, Java, Ruby, libvirt, QEMU, glib

To add SDT markers into user applications:
We need to have this header sys/sdt.h present.
sys/sdt.h used is version 3.
If not present, install systemtap-sdt-devel package (for fedora-18).

Please refer to the Documentation patch (3rd patch in this series) to see how the
SDT markers are added into a program.

With this patchset,
- Use perf to list the markers in the app:
# perf list sdt ./user_app

./user_app :
%user_app:foo_start
%user_app:fun_start

This link shows an example of marker probing with Systemtap:
https://sourceware.org/systemtap/wiki/AddingUserSpaceProbingToApps

Also, this link provides important info regarding SDT notes:
http://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation

This patchset has undergone a lot of changes since it was first introduced.
Hence, the patchset has now been subdivided for more simplicity and ease of
review (thanks to the suggestion from Namhyung Kim). This contains the first 2
of the 4 patches as suggested here:
https://lkml.org/lkml/2014/7/20/284

- Markers in binaries :
These SDT markers are present in the ELF in the section named
".note.stapsdt".
Here, the name of the marker, its provider, type, location, base
address, semaphore address.
We can retrieve these values using the members name_off and desc_off in
Nhdr structure. If these are not enabled, they are present in the ELF as nop.

Changes since last series :
- Subdivided the previous patchset into 4 patches to make it easier to review
  as suggested by Namhyung Kim. (This set includes first two of the four patches)
- Made the required changes and some optimizations suggested by Masami, Namhyung
  and Andi.

TODO:
- Listing SDT events present in most of the binaries present in a system.
- Maintaining a cache of the SDT events for faster lookup.
- Add support to probe these SDT markers and integrate with a previous patch
  (support to perf to probe SDT markers) posted in lkml.
  https://lkml.org/lkml/2013/10/23/10

- Recognizing arguments and support to probe on them.
- Add semaphore support.

---

Hemant Kumar (3):
      Raw SDT parsing functions
      Support perf-list to print SDT events in a single file
      Adds documentation for perf support to SDT events.


 tools/perf/Documentation/SDT-support.txt |   48 ++++++
 tools/perf/Documentation/perf-list.txt   |    4 
 tools/perf/Makefile.perf                 |    1 
 tools/perf/builtin-list.c                |    2 
 tools/perf/util/parse-events.h           |    2 
 tools/perf/util/sdt.c                    |  113 ++++++++++++++
 tools/perf/util/symbol-elf.c             |  244 ++++++++++++++++++++++++++++++
 tools/perf/util/symbol.h                 |   19 ++
 8 files changed, 432 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/Documentation/SDT-support.txt
 create mode 100644 tools/perf/util/sdt.c

-- 

[PATCH v4 1/3] perf/sdt : Raw SDT parsing functions

[Hemant Kumar]

This patch serves as the initial support to identify and list SDT events in binaries.
When programs containing SDT markers are compiled, gcc with the help of assembler
directives identifies them and places them in the section ".note.stapsdt". To find these
markers from the binaries, one needs to traverse through this section and parse the
relevant details like the name, type and location of the marker. Also, the original
location could be skewed due to the effect of prelinking. If that is the case, the
locations need to be adjusted.

The functions in this patch open a given ELF, find out the SDT section, parse the
relevant details, adjust the location (if necessary) and populate them in a list.

Signed-off-by : Hemant Kumar <hemant@linux.vnet.ibm.com>
---
 tools/perf/util/symbol-elf.c |  244 ++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/symbol.h     |   19 +++
 2 files changed, 263 insertions(+)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 6864661..a875fdc 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1619,6 +1619,250 @@ void kcore_extract__delete(struct kcore_extract *kce)
 	unlink(kce->extract_filename);
 }
 
+/*
+ * populate_sdt_note() : Responsible for parsing the section .note.stapsdt and
+ * after adjusting the note's location, returns that to the calling functions.
+ */
+static int populate_sdt_note(Elf **elf, const char *data, size_t len, int type,
+			     struct sdt_note **note)
+{
+	const char *provider, *name;
+	struct sdt_note *tmp = NULL;
+	GElf_Ehdr ehdr;
+	GElf_Addr base_off = 0;
+	GElf_Shdr shdr;
+	int ret = -1;
+	int i;
+
+	union {
+		Elf64_Addr a64[3];
+		Elf32_Addr a32[3];
+	} buf;
+
+	Elf_Data dst = {
+		.d_buf = &buf, .d_type = ELF_T_ADDR, .d_version = EV_CURRENT,
+		.d_size = gelf_fsize((*elf), ELF_T_ADDR, 3, EV_CURRENT),
+		.d_off = 0, .d_align = 0
+	};
+	Elf_Data src = {
+		.d_buf = (void *) data, .d_type = ELF_T_ADDR,
+		.d_version = EV_CURRENT, .d_size = dst.d_size, .d_off = 0,
+		.d_align = 0
+	};
+
+	/* Check the type of each of the notes */
+	if (type != SDT_NOTE_TYPE)
+		goto out_err;
+
+	tmp = (struct sdt_note *)calloc(1, sizeof(struct sdt_note));
+	if (!tmp) {
+		ret = -ENOMEM;
+		goto out_err;
+	}
+
+	INIT_LIST_HEAD(&tmp->note_list);
+
+	if (len < dst.d_size + 3)
+		goto out_free_note;
+
+	/* Translation from file representation to memory representation */
+	if (gelf_xlatetom(*elf, &dst, &src,
+			  elf_getident(*elf, NULL)[EI_DATA]) == NULL)
+		printf("gelf_xlatetom : %s\n", elf_errmsg(-1));
+
+	/* Populate the fields of sdt_note */
+	provider = data + dst.d_size;
+
+	name = (const char *)memchr(provider, '\0', data + len - provider);
+	if (name++ == NULL)
+		goto out_free_note;
+
+	tmp->provider = strdup(provider);
+	if (!tmp->provider) {
+		ret = -ENOMEM;
+		goto out_free_note;
+	}
+	tmp->name = strdup(name);
+	if (!tmp->name) {
+		ret = -ENOMEM;
+		goto out_free_prov;
+	}
+
+	/* Obtain the addresses */
+	if (gelf_getclass(*elf) == ELFCLASS32) {
+		for (i = 0; i < 3; i++)
+			tmp->addr.a32[i] = buf.a32[i];
+		tmp->bit32 = true;
+	} else {
+		for (i = 0; i < 3; i++)
+			tmp->addr.a64[i] = buf.a64[i];
+		tmp->bit32 = false;
+	}
+
+	/* Now Adjust the prelink effect */
+	if (!gelf_getehdr(*elf, &ehdr)) {
+		pr_debug("%s : cannot get elf header.\n", __func__);
+		ret = -EBADF;
+		goto out_free_name;
+	}
+
+	/*
+	 * Find out the .stapsdt.base section.
+	 * This scn will help us to handle prelinking (if present).
+	 * Compare the retrieved file offset of the base section with the
+	 * base address in the description of the SDT note. If its different,
+	 * then accordingly, adjust the note location.
+	 */
+	if (elf_section_by_name(*elf, &ehdr, &shdr, SDT_BASE_SCN, NULL)) {
+		base_off = shdr.sh_offset;
+		if (base_off) {
+			if (tmp->bit32)
+				tmp->addr.a32[0] = tmp->addr.a32[0] + base_off -
+					tmp->addr.a32[1];
+			else
+				tmp->addr.a64[0] = tmp->addr.a64[0] + base_off -
+					tmp->addr.a64[1];
+		}
+	}
+
+	*note = tmp;
+	return 0;
+
+out_free_name:
+	free(tmp->name);
+out_free_prov:
+	free(tmp->provider);
+out_free_note:
+	free(tmp);
+out_err:
+	return ret;
+}
+
+/*
+ * construct_sdt_notes_list() : Scans the sections in 'elf' for the section
+ * .note.stapsdt. It, then calls populate_sdt_note to find
+ * out the SDT events and populates the 'sdt_notes'.
+ */
+static int construct_sdt_notes_list(Elf *elf, struct list_head *sdt_notes)
+{
+	GElf_Ehdr ehdr;
+	Elf_Scn *scn = NULL;
+	Elf_Data *data;
+	GElf_Shdr shdr;
+	size_t shstrndx, next;
+	GElf_Nhdr nhdr;
+	size_t name_off, desc_off, offset;
+	struct sdt_note *tmp = NULL;
+	int ret = 0, val = 0;
+
+	if (gelf_getehdr(elf, &ehdr) == NULL) {
+		ret = -EBADF;
+		goto out_ret;
+	}
+	if (elf_getshdrstrndx(elf, &shstrndx) != 0) {
+		ret = -EBADF;
+		goto out_ret;
+	}
+
+	/* Look for the required section */
+	scn = elf_section_by_name(elf, &ehdr, &shdr, SDT_NOTE_SCN, NULL);
+	if (!scn) {
+		ret = -ENOENT;
+		goto out_ret;
+	}
+
+	if (!(shdr.sh_type == SHT_NOTE) || (shdr.sh_flags & SHF_ALLOC)) {
+		ret = -ENOENT;
+		goto out_ret;
+	}
+
+	data = elf_getdata(scn, NULL);
+
+	/* Get the SDT notes */
+	for (offset = 0; (next = gelf_getnote(data, offset, &nhdr, &name_off,
+					      &desc_off)) > 0; offset = next) {
+		if (nhdr.n_namesz == sizeof(SDT_NOTE_NAME) &&
+		    !memcmp(data->d_buf + name_off, SDT_NOTE_NAME,
+			    sizeof(SDT_NOTE_NAME))) {
+			val = populate_sdt_note(&elf, ((data->d_buf) + desc_off),
+						nhdr.n_descsz, nhdr.n_type,
+						&tmp);
+			if (!val)
+				list_add_tail(&tmp->note_list, sdt_notes);
+			if (val == -ENOMEM) {
+				ret = val;
+				goto out_ret;
+			}
+		}
+	}
+	if (list_empty(sdt_notes))
+		ret = -ENOENT;
+
+out_ret:
+	return ret;
+}
+
+/*
+ * get_sdt_note_list() : Takes two arguments "head" and "target", where head
+ * is the head of the SDT events' list and "target" is the file name as to
+ * where the SDT events should be looked for. This opens the file, initializes
+ * the ELF and then calls construct_sdt_notes_list.
+ */
+int get_sdt_note_list(struct list_head *head, const char *target)
+{
+	Elf *elf;
+	int fd, ret;
+
+	fd = open(target, O_RDONLY);
+	if (fd < 0)
+		return -EBADF;
+
+	symbol__elf_init();
+	elf = elf_begin(fd, ELF_C_READ, NULL);
+	if (!elf) {
+		ret = -EBADF;
+		goto out_close;
+	}
+	ret = construct_sdt_notes_list(elf, head);
+	elf_end(elf);
+
+out_close:
+	close(fd);
+	return ret;
+}
+
+/*
+ * is_an_elf() : Returns 'true' if the file is an elf and 'false' otherwise
+ */
+bool is_an_elf(char *file)
+{
+	int fd;
+	Elf *elf;
+	bool ret = true;
+
+	fd = open(file, O_RDONLY);
+	if (fd < 0) {
+		ret = false;
+		goto out_ret;
+	}
+
+	symbol__elf_init();
+	elf = elf_begin(fd, ELF_C_READ, NULL);
+	if (!elf) {
+		ret = false;
+		goto out_close;
+	}
+	if (elf_kind(elf) != ELF_K_ELF)
+		ret = false;
+
+	elf_end(elf);
+
+out_close:
+	close(fd);
+out_ret:
+	return ret;
+}
+
 void symbol__elf_init(void)
 {
 	elf_version(EV_CURRENT);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 615c752..83be31a 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -294,4 +294,23 @@ int compare_proc_modules(const char *from, const char *to);
 int setup_list(struct strlist **list, const char *list_str,
 	       const char *list_name);
 
+struct sdt_note {
+	char *name;
+	char *provider;
+	bool bit32;
+	union {
+		Elf64_Addr a64[3];
+		Elf32_Addr a32[3];
+	} addr;
+	struct list_head note_list;
+};
+
+int get_sdt_note_list(struct list_head *head, const char *target);
+bool is_an_elf(char *file);
+
+#define SDT_BASE_SCN ".stapsdt.base"
+#define SDT_NOTE_SCN  ".note.stapsdt"
+#define SDT_NOTE_TYPE 3
+#define SDT_NOTE_NAME "stapsdt"
+
 #endif /* __PERF_SYMBOL */

[PATCH v4 2/3] perf/sdt : Support perf-list to print SDT events in a single file

[Hemant Kumar]

This patch enables perf to look for SDT markers in a single file.
An individual file argument must be given to "perf list" to find out the SDT markers
present in that file.

Usage is as below :
# perf list sdt /home/hemant/tmp

/home/hemant/tmp:
%user : foo
%user : bar

On using this command, perf looks for SDTs in that file using the ELF functions from
the previous patch (in this series) and dumps them on stdout.

Signed-off-by : Hemant Kumar <hemant@linux.vnet.ibm.com>
---
 tools/perf/Makefile.perf       |    1 
 tools/perf/builtin-list.c      |    2 +
 tools/perf/util/parse-events.h |    2 +
 tools/perf/util/sdt.c          |  113 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 118 insertions(+)
 create mode 100644 tools/perf/util/sdt.c

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 9670a16..e098dcd 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -373,6 +373,7 @@ LIB_OBJS += $(OUTPUT)util/stat.o
 LIB_OBJS += $(OUTPUT)util/record.o
 LIB_OBJS += $(OUTPUT)util/srcline.o
 LIB_OBJS += $(OUTPUT)util/data.o
+LIB_OBJS += $(OUTPUT)util/sdt.o
 
 LIB_OBJS += $(OUTPUT)ui/setup.o
 LIB_OBJS += $(OUTPUT)ui/helpline.o
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 011195e..85be3e3 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -53,6 +53,8 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
 			print_hwcache_events(NULL, false);
 		else if (strcmp(argv[i], "pmu") == 0)
 			print_pmu_events(NULL, false);
+		else if (strcmp(argv[i], "sdt") == 0)
+			print_sdt_events(argv[++i]);
 		else if (strcmp(argv[i], "--raw-dump") == 0)
 			print_events(NULL, true);
 		else {
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index df094b4..fadc729 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -109,4 +109,6 @@ extern int is_valid_tracepoint(const char *event_string);
 
 extern int valid_debugfs_mount(const char *debugfs);
 
+void print_sdt_events(const char *arg);
+
 #endif /* __PERF_PARSE_EVENTS_H */
diff --git a/tools/perf/util/sdt.c b/tools/perf/util/sdt.c
new file mode 100644
index 0000000..12c16a0
--- /dev/null
+++ b/tools/perf/util/sdt.c
@@ -0,0 +1,113 @@
+/*
+ * util/sdt.c
+ * This contains the relevant functions needed to find the SDT markers
+ * in a binary.
+ *
+ * TODOS:
+ * - Listing SDT events in most of the binaries present in the system.
+ * - Build a cache for these SDT events.
+ * - Looking into directories provided by the user for binaries with SDTs, etc.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <string.h>
+
+#include "parse-events.h"
+#include "linux/list.h"
+#include "symbol.h"
+
+
+/*
+ * get_sdt_note_info(): flush the SDT notes onto stdout
+ */
+static void get_sdt_note_info(struct list_head *start, const char *target)
+{
+	struct sdt_note *pos;
+
+	if (list_empty(start))
+		return;
+
+	printf("%s :\n", target);
+	list_for_each_entry(pos, start, note_list) {
+		printf("%%%s : %s\n", pos->provider, pos->name);
+	}
+}
+
+/*
+ * Error displayed in case of query of a
+ * single file for SDT markers
+ */
+static int sdt_err(int val, const char *target)
+{
+	switch (-val) {
+	case 0:
+		break;
+	case ENOENT:
+		/* Absence of SDT markers */
+		printf("%s : No SDT events found\n", target);
+		break;
+	case EBADF:
+		printf("%s : Bad file name\n", target);
+		break;
+	default:
+		printf("%s\n", strerror(val));
+	}
+
+	return val;
+}
+
+/*
+ * cleanup_sdt_note_list() : Free the sdt note list
+ */
+static void cleanup_sdt_note_list(struct list_head *sdt_notes)
+{
+	struct sdt_note *tmp, *pos;
+
+	if (list_empty(sdt_notes))
+		return;
+
+	list_for_each_entry_safe(pos, tmp, sdt_notes, note_list) {
+		list_del(&pos->note_list);
+		free(pos->name);
+		free(pos->provider);
+		free(pos);
+	}
+}
+
+/*
+ * filename__find_sdt() : looks for sdt markers and the list is
+ * stored in sdt_notes
+ */
+static int filename__find_sdt(const char *target)
+{
+	int ret;
+
+	LIST_HEAD(sdt_notes);
+
+	ret = get_sdt_note_list(&sdt_notes, target);
+	if (!ret)
+		get_sdt_note_info(&sdt_notes, target);
+	else
+		sdt_err(ret, target);
+
+	cleanup_sdt_note_list(&sdt_notes);
+
+	return ret;
+}
+
+/*
+ * print_sdt_notes() : wrapper function
+ */
+void print_sdt_events(const char *arg)
+{
+	if (arg) {
+		filename__find_sdt(arg);
+		return;
+	}
+	pr_err("Error : File Name must be specified with \"sdt\" option!\n"
+	       "Usage :\n  perf list sdt <file-name>\n");
+
+	return;
+}

[PATCH v4 3/3] perf/sdt : Documentation for SDT events

[Hemant Kumar]

Adds documentation for perf support to SDT events.

Signed-off-by : Hemant Kumar <hemant@linux.vnet.ibm.com>
---
 tools/perf/Documentation/SDT-support.txt |   48 ++++++++++++++++++++++++++++++
 tools/perf/Documentation/perf-list.txt   |    4 ++-
 2 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/Documentation/SDT-support.txt

diff --git a/tools/perf/Documentation/SDT-support.txt b/tools/perf/Documentation/SDT-support.txt
new file mode 100644
index 0000000..273912b
--- /dev/null
+++ b/tools/perf/Documentation/SDT-support.txt
@@ -0,0 +1,48 @@
+Support to perf for listing the SDT markers :
+
+This helps in listing dtrace style markers(SDT) present in user space
+applications through perf. SDT Notes/markers are placed at important places by the
+developers. They have a negligible overhead when not enabled.
+We can enable them and probe at these places and find some important information
+like the arguments' values, etc.
+
+How to add SDT markers into user applications:
+We need to have this header sys/sdt.h present.
+sys/sdt.h used is version 3.
+If not present, install systemtap-sdt-devel package (for fedora-18).
+
+A very simple example:
+
+$ cat user_app.c
+
+#include <sys/sdt.h>
+
+void main () {
+       /* ... */
+       /*
+        * user_app is the provider name
+        * test_probe is the marker name
+        */
+       STAP_PROBE(user_app, test_mark);
+       /* ... */
+}
+
+$ gcc user_app.c
+$ perf list sdt ./a.out
+./a.out:
+%user_app:test_mark
+
+For more information on usage of SDT markers, visit the following link:
+http://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation
+
+This link shows an example of marker probing with Systemtap:
+https://sourceware.org/systemtap/wiki/AddingUserSpaceProbingToApps
+
+- Markers in binaries :
+These SDT markers are present in the ELF in the section named
+".note.stapsdt".
+This section contains the name of the marker, its provider, type, location, base
+address, semaphore address.
+We can retrieve these values using the members name_off and desc_off in
+Nhdr structure. If these markers are not enabled, they are present in the ELF in
+the form of a "nop" instruction.
diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index 6fce6a6..5c72785 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -8,7 +8,7 @@ perf-list - List all symbolic event types
 SYNOPSIS
 --------
 [verse]
-'perf list' [hw|sw|cache|tracepoint|pmu|event_glob]
+'perf list' [hw|sw|cache|tracepoint|pmu|sdt|event_glob]
 
 DESCRIPTION
 -----------
@@ -108,6 +108,8 @@ To limit the list use:
 
 . 'pmu' to print the kernel supplied PMU events.
 
+. 'sdt' to print the SDT events present in a file. Takes a file_name as an argument.
+
 . If none of the above is matched, it will apply the supplied glob to all
   events, printing the ones that match.
 

Re: [PATCH v4 2/3] perf/sdt : Support perf-list to print SDT events in a single file

[Masami Hiramatsu]

(2014/08/28 6:50), Hemant Kumar wrote:
[...]
> +/*
> + * get_sdt_note_info(): flush the SDT notes onto stdout
> + */
> +static void get_sdt_note_info(struct list_head *start, const char *target)
> +{
> +	struct sdt_note *pos;
> +
> +	if (list_empty(start))
> +		return;
> +
> +	printf("%s :\n", target);
> +	list_for_each_entry(pos, start, note_list) {
> +		printf("%%%s : %s\n", pos->provider, pos->name);

Hmm, this will show

 %app : marker

instead of

 %app:marker

(blanks are placed around ":")
I think it should be the latter format.

> +	}
> +}
> +
> +/*
> + * Error displayed in case of query of a
> + * single file for SDT markers
> + */
> +static int sdt_err(int val, const char *target)
> +{
> +	switch (-val) {
> +	case 0:
> +		break;
> +	case ENOENT:
> +		/* Absence of SDT markers */
> +		printf("%s : No SDT events found\n", target);

Please use pr_err or pr_warning for error messages.

> +		break;
> +	case EBADF:
> +		printf("%s : Bad file name\n", target);
> +		break;
> +	default:
> +		printf("%s\n", strerror(val));
> +	}

And strerror_r instead of strerror (see https://lkml.org/lkml/2014/8/13/828)

> +
> +	return val;
> +}
> +
> +/*
> + * cleanup_sdt_note_list() : Free the sdt note list
> + */
> +static void cleanup_sdt_note_list(struct list_head *sdt_notes)
> +{
> +	struct sdt_note *tmp, *pos;
> +
> +	if (list_empty(sdt_notes))
> +		return;

You don't need to check this. If the list is empty list_for_each...
just skips loops.

> +
> +	list_for_each_entry_safe(pos, tmp, sdt_notes, note_list) {
> +		list_del(&pos->note_list);
> +		free(pos->name);
> +		free(pos->provider);
> +		free(pos);
> +	}
> +}

Thank you,

> +
> +/*
> + * filename__find_sdt() : looks for sdt markers and the list is
> + * stored in sdt_notes
> + */
> +static int filename__find_sdt(const char *target)
> +{
> +	int ret;
> +
> +	LIST_HEAD(sdt_notes);
> +
> +	ret = get_sdt_note_list(&sdt_notes, target);
> +	if (!ret)
> +		get_sdt_note_info(&sdt_notes, target);
> +	else
> +		sdt_err(ret, target);
> +
> +	cleanup_sdt_note_list(&sdt_notes);
> +
> +	return ret;
> +}
> +
> +/*
> + * print_sdt_notes() : wrapper function
> + */
> +void print_sdt_events(const char *arg)
> +{
> +	if (arg) {
> +		filename__find_sdt(arg);
> +		return;
> +	}
> +	pr_err("Error : File Name must be specified with \"sdt\" option!\n"
> +	       "Usage :\n  perf list sdt <file-name>\n");
> +
> +	return;
> +}
> 
> 


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH v4 0/3] perf/sdt : Support for SDT markers

[Masami Hiramatsu]

(2014/08/28 6:43), Hemant Kumar wrote:
> The v3 patchset had some spacing errors because of my editor messing up during
> sending the patches. Resending the patchset again.
> This patchset helps in listing dtrace style markers(SDT) present in user space
> applications through perf.
> Notes/markers are placed at important places by the
> developers. They have a negligible overhead when not enabled.
> We can enable them and probe at these places and find some important information
> like the arguments' values, etc.
> 
> We have lots of applications which use SDT markers today, like:
> Postgresql, MySql, Mozilla, Perl, Python, Java, Ruby, libvirt, QEMU, glib
> 
> To add SDT markers into user applications:
> We need to have this header sys/sdt.h present.
> sys/sdt.h used is version 3.
> If not present, install systemtap-sdt-devel package (for fedora-18).
> 
> Please refer to the Documentation patch (3rd patch in this series) to see how the
> SDT markers are added into a program.
> 
> With this patchset,
> - Use perf to list the markers in the app:
> # perf list sdt ./user_app
> 
> ./user_app :
> %user_app:foo_start
> %user_app:fun_start
> 
> This link shows an example of marker probing with Systemtap:
> https://sourceware.org/systemtap/wiki/AddingUserSpaceProbingToApps
> 
> Also, this link provides important info regarding SDT notes:
> http://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation
> 
> This patchset has undergone a lot of changes since it was first introduced.
> Hence, the patchset has now been subdivided for more simplicity and ease of
> review (thanks to the suggestion from Namhyung Kim). This contains the first 2
> of the 4 patches as suggested here:
> https://lkml.org/lkml/2014/7/20/284

BTW, does this series only contain list command support?

> - Markers in binaries :
> These SDT markers are present in the ELF in the section named
> ".note.stapsdt".
> Here, the name of the marker, its provider, type, location, base
> address, semaphore address.
> We can retrieve these values using the members name_off and desc_off in
> Nhdr structure. If these are not enabled, they are present in the ELF as nop.
> 
> Changes since last series :
> - Subdivided the previous patchset into 4 patches to make it easier to review
>   as suggested by Namhyung Kim. (This set includes first two of the four patches)
> - Made the required changes and some optimizations suggested by Masami, Namhyung
>   and Andi.
> 
> TODO:
> - Listing SDT events present in most of the binaries present in a system.
> - Maintaining a cache of the SDT events for faster lookup.
> - Add support to probe these SDT markers and integrate with a previous patch
>   (support to perf to probe SDT markers) posted in lkml.
>   https://lkml.org/lkml/2013/10/23/10

OK, I think we can postpone sdt-cache command, but this should go with
probe-side improvements, because if perf-list shows sdt markers, users
will be misled that the perf already supports sdt events.
I'm OK if we hold this on a non-released branch on acme's tree. But it
shouldn't be pushed to upstream without probe-side improvements.

Arnaldo, would you think that works on your tree?

Thank you,



-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [PATCH v4 2/3] perf/sdt : Support perf-list to print SDT events in a single file

[Hemant Kumar]

Hi Masami,

On 08/28/2014 04:24 PM, Masami Hiramatsu wrote:
> (2014/08/28 6:50), Hemant Kumar wrote:
> [...]
>> +/*
>> + * get_sdt_note_info(): flush the SDT notes onto stdout
>> + */
>> +static void get_sdt_note_info(struct list_head *start, const char *target)
>> +{
>> +	struct sdt_note *pos;
>> +
>> +	if (list_empty(start))
>> +		return;
>> +
>> +	printf("%s :\n", target);
>> +	list_for_each_entry(pos, start, note_list) {
>> +		printf("%%%s : %s\n", pos->provider, pos->name);
> Hmm, this will show
>
>   %app : marker
>
> instead of
>
>   %app:marker
>
> (blanks are placed around ":")
> I think it should be the latter format.

I think so too. Missed it. Thanks for pointing that.

>> +	}
>> +}
>> +
>> +/*
>> + * Error displayed in case of query of a
>> + * single file for SDT markers
>> + */
>> +static int sdt_err(int val, const char *target)
>> +{
>> +	switch (-val) {
>> +	case 0:
>> +		break;
>> +	case ENOENT:
>> +		/* Absence of SDT markers */
>> +		printf("%s : No SDT events found\n", target);
> Please use pr_err or pr_warning for error messages.

Ah! ok.

>> +		break;
>> +	case EBADF:
>> +		printf("%s : Bad file name\n", target);
>> +		break;
>> +	default:
>> +		printf("%s\n", strerror(val));
>> +	}
> And strerror_r instead of strerror (see https://lkml.org/lkml/2014/8/13/828)

Sure, will use strerror_r instead.

>> +
>> +	return val;
>> +}
>> +
>> +/*
>> + * cleanup_sdt_note_list() : Free the sdt note list
>> + */
>> +static void cleanup_sdt_note_list(struct list_head *sdt_notes)
>> +{
>> +	struct sdt_note *tmp, *pos;
>> +
>> +	if (list_empty(sdt_notes))
>> +		return;
> You don't need to check this. If the list is empty list_for_each...
> just skips loops.

Right. Thanks for pointing that out.

>> +
>> +	list_for_each_entry_safe(pos, tmp, sdt_notes, note_list) {
>> +		list_del(&pos->note_list);
>> +		free(pos->name);
>> +		free(pos->provider);
>> +		free(pos);
>> +	}
>> +}
> Thank you,
>
>> +
>> +/*
>> + * filename__find_sdt() : looks for sdt markers and the list is
>> + * stored in sdt_notes
>> + */
>> +static int filename__find_sdt(const char *target)
>> +{
>> +	int ret;
>> +
>> +	LIST_HEAD(sdt_notes);
>> +
>> +	ret = get_sdt_note_list(&sdt_notes, target);
>> +	if (!ret)
>> +		get_sdt_note_info(&sdt_notes, target);
>> +	else
>> +		sdt_err(ret, target);
>> +
>> +	cleanup_sdt_note_list(&sdt_notes);
>> +
>> +	return ret;
>> +}
>> +
>> +/*
>> + * print_sdt_notes() : wrapper function
>> + */
>> +void print_sdt_events(const char *arg)
>> +{
>> +	if (arg) {
>> +		filename__find_sdt(arg);
>> +		return;
>> +	}
>> +	pr_err("Error : File Name must be specified with \"sdt\" option!\n"
>> +	       "Usage :\n  perf list sdt <file-name>\n");
>> +
>> +	return;
>> +}
>>
>>
>

-- 
Thanks,
Hemant Kumar

Re: [PATCH v4 1/3] perf/sdt : Raw SDT parsing functions

[Namhyung Kim]

Hi Hemant,

On Thu, 28 Aug 2014 03:14:20 +0530, Hemant Kumar wrote:
> This patch serves as the initial support to identify and list SDT events in binaries.
> When programs containing SDT markers are compiled, gcc with the help of assembler
> directives identifies them and places them in the section ".note.stapsdt". To find these
> markers from the binaries, one needs to traverse through this section and parse the
> relevant details like the name, type and location of the marker. Also, the original
> location could be skewed due to the effect of prelinking. If that is the case, the
> locations need to be adjusted.
>
> The functions in this patch open a given ELF, find out the SDT section, parse the
> relevant details, adjust the location (if necessary) and populate them in a list.
>

[SNIP]
> +	/* Get the SDT notes */
> +	for (offset = 0; (next = gelf_getnote(data, offset, &nhdr, &name_off,
> +					      &desc_off)) > 0; offset = next) {
> +		if (nhdr.n_namesz == sizeof(SDT_NOTE_NAME) &&
> +		    !memcmp(data->d_buf + name_off, SDT_NOTE_NAME,
> +			    sizeof(SDT_NOTE_NAME))) {
> +			val = populate_sdt_note(&elf, ((data->d_buf) + desc_off),
> +						nhdr.n_descsz, nhdr.n_type,
> +						&tmp);
> +			if (!val)
> +				list_add_tail(&tmp->note_list, sdt_notes);
> +			if (val == -ENOMEM) {
> +				ret = val;
> +				goto out_ret;
> +			}

It seems populate_sdt_note() can failed with other error than ENOMEM.
So I think it'd be better changing it like:

			ret = populate_sdt_note(...);
			if (ret < 0)
				goto out_ret;

			list_add_tail(...);


So no need to use the 'val' variable.


> +		}
> +	}
> +	if (list_empty(sdt_notes))
> +		ret = -ENOENT;
> +
> +out_ret:
> +	return ret;
> +}
> +
> +/*
> + * get_sdt_note_list() : Takes two arguments "head" and "target", where head
> + * is the head of the SDT events' list and "target" is the file name as to
> + * where the SDT events should be looked for. This opens the file, initializes
> + * the ELF and then calls construct_sdt_notes_list.
> + */
> +int get_sdt_note_list(struct list_head *head, const char *target)
> +{
> +	Elf *elf;
> +	int fd, ret;

Just a nitpick.  It'd be better setting ret to -EBADF IMHO.


> +
> +	fd = open(target, O_RDONLY);
> +	if (fd < 0)
> +		return -EBADF;
> +
> +	symbol__elf_init();

This is really need?  I guess it's not harmful but no need to call it
whenever we check sdt note in every file.  A single call to
simbole__init() can be placed in the cmd_list() instead.


> +	elf = elf_begin(fd, ELF_C_READ, NULL);
> +	if (!elf) {
> +		ret = -EBADF;
> +		goto out_close;
> +	}
> +	ret = construct_sdt_notes_list(elf, head);
> +	elf_end(elf);
> +
> +out_close:
> +	close(fd);
> +	return ret;
> +}
> +
> +/*
> + * is_an_elf() : Returns 'true' if the file is an elf and 'false' otherwise
> + */
> +bool is_an_elf(char *file)
> +{
> +	int fd;
> +	Elf *elf;
> +	bool ret = true;
> +
> +	fd = open(file, O_RDONLY);
> +	if (fd < 0) {
> +		ret = false;
> +		goto out_ret;
> +	}
> +
> +	symbol__elf_init();

Ditto.

Thanks,
Namhyung


> +	elf = elf_begin(fd, ELF_C_READ, NULL);
> +	if (!elf) {
> +		ret = false;
> +		goto out_close;
> +	}
> +	if (elf_kind(elf) != ELF_K_ELF)
> +		ret = false;
> +
> +	elf_end(elf);
> +
> +out_close:
> +	close(fd);
> +out_ret:
> +	return ret;
> +}
> +
>  void symbol__elf_init(void)
>  {
>  	elf_version(EV_CURRENT);
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 615c752..83be31a 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -294,4 +294,23 @@ int compare_proc_modules(const char *from, const char *to);
>  int setup_list(struct strlist **list, const char *list_str,
>  	       const char *list_name);
>  
> +struct sdt_note {
> +	char *name;
> +	char *provider;
> +	bool bit32;
> +	union {
> +		Elf64_Addr a64[3];
> +		Elf32_Addr a32[3];
> +	} addr;
> +	struct list_head note_list;
> +};
> +
> +int get_sdt_note_list(struct list_head *head, const char *target);
> +bool is_an_elf(char *file);
> +
> +#define SDT_BASE_SCN ".stapsdt.base"
> +#define SDT_NOTE_SCN  ".note.stapsdt"
> +#define SDT_NOTE_TYPE 3
> +#define SDT_NOTE_NAME "stapsdt"
> +
>  #endif /* __PERF_SYMBOL */

Re: [PATCH v4 3/3] perf/sdt : Documentation for SDT events

[Namhyung Kim]

On Thu, 28 Aug 2014 03:23:16 +0530, Hemant Kumar wrote:
>  SYNOPSIS
>  --------
>  [verse]
> -'perf list' [hw|sw|cache|tracepoint|pmu|event_glob]
> +'perf list' [hw|sw|cache|tracepoint|pmu|sdt|event_glob]

What about adding it to a different line as the 'sdt' subcommand needs
an extra argument?

  [verse]
  'perf list' [hw|sw|cache|tracepoint|pmu|event_glob]
  'perf list' sdt <file>


Thanks,
Namhyung

Re: Re: [PATCH v4 3/3] perf/sdt : Documentation for SDT events

[Masami Hiramatsu]

(2014/08/29 16:26), Namhyung Kim wrote:
> On Thu, 28 Aug 2014 03:23:16 +0530, Hemant Kumar wrote:
>>  SYNOPSIS
>>  --------
>>  [verse]
>> -'perf list' [hw|sw|cache|tracepoint|pmu|event_glob]
>> +'perf list' [hw|sw|cache|tracepoint|pmu|sdt|event_glob]
> 
> What about adding it to a different line as the 'sdt' subcommand needs
> an extra argument?
> 
>   [verse]
>   'perf list' [hw|sw|cache|tracepoint|pmu|event_glob]
>   'perf list' sdt <file>

Agreed, at this point, "perf list sdt" requires a filename.

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com