This patchset helps in probing 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.
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 to show this :
$ 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 probe -M -x ./a.out
%user_app:test_mark
A different example to show the same :
- Create a file with .d extension and mention the probe names in it with
provider name and marker name.
$ cat probes.d
provider user_app {
probe foo_start();
probe fun_start();
};
- Now create the probes.h and probes.o file :
$ dtrace -C -h -s probes.d -o probes.h
$ dtrace -C -G -s probes.d -o probes.o
- A program using the markers:
$ cat user_app.c
#include <stdio.h>
#include "probes.h"
void foo(void)
{
USER_APP_FOO_START();
printf("This is foo\n");
}
void fun(void)
{
USER_APP_FUN_START();
printf("Inside fun\n");
}
int main(void)
{
printf("In main\n");
foo();
fun();
return 0;
}
- Compile it and also provide probes.o file to linker:
$ gcc user_app.c probes.o -o user_app
- Now use perf to list the markers in the app:
# perf probe --markers -x ./user_app
%user_app:foo_start
%user_app:fun_start
- And then use perf probe to add a probe point :
# perf probe -x ./user_app -a '%user_app:foo_start'
Added new event :
event = foo_start (on 0x530)
You can now use it on all perf tools such as :
perf record -e probe_user:foo_start -aR sleep 1
# perf record -e probe_user:foo_start -aR ./user_app
In main
This is foo
Inside fun
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.235 MB perf.data (~10279 samples) ]
- Then use perf tools to analyze it.
# perf report --stdio
# ========
# captured on: Tue Sep 3 16:19:55 2013
# hostname : hemant-fedora
# os release : 3.11.0-rc3+
# perf version : 3.9.4-200.fc18.x86_64
# arch : x86_64
# nrcpus online : 2
# nrcpus avail : 2
# cpudesc : QEMU Virtual CPU version 1.2.2
# cpuid : GenuineIntel,6,2,3
# total memory : 2051912 kBIf these are not enabled, they are present in the \
ELF as nop.
# cmdline : /usr/bin/perf record -e probe_user:foo_start -aR ./user_app
# event : name = probe_user:foo_start, type = 2, config = 0x38e, config1
= 0x0, config2 = 0x0, excl_usr = 0, excl_kern = 0, excl_host = 0,
excl_guest = 1, precise_ip = 0
# HEADER_CPU_TOPOLOGY info available, use -I to display
# HEADER_NUMA_TOPOLOGY info available, use -I to display
# pmu mappings: software = 1, tracepoint = 2, breakpoint = 5
# ========
#
# Samples: 1 of event 'probe_user:foo_start'
# Event count (approx.): 1
#
# Overhead Command Shared Object Symbol
# ........ ........ ............. .......
#
100.00% user_app user_app [.] foo
#
# (For a higher level overview, try: perf report --sort comm,dso)
#
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
- 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.
All the above info is moved in sdt-probes.txt file present in the Documentation
directory in tools/perf/.
Changes since last version:
- Added suggested changes by Masami and Namhyung.
- Fixed some bugs.
- Removed some redundancies.
- Updated with error codes.
- Rebased it to latest kernel tip.
TODO:
- Recognizing arguments and support to probe on them.
---
Hemant Kumar (3):
SDT markers listing by perf:
Support for perf to probe into SDT markers:
Documentation regarding perf/sdt
tools/perf/Documentation/perf-probe.txt | 17 ++
tools/perf/Documentation/sdt-probes.txt | 184 ++++++++++++++++++++
tools/perf/builtin-probe.c | 45 +++++
tools/perf/util/probe-event.c | 134 +++++++++++++-
tools/perf/util/probe-event.h | 3
tools/perf/util/symbol-elf.c | 291 +++++++++++++++++++++++++++++++
tools/perf/util/symbol.h | 21 ++
7 files changed, 681 insertions(+), 14 deletions(-)
create mode 100644 tools/perf/Documentation/sdt-probes.txt
--
This patch will enable perf to list all the sdt markers present
in an elf file. The markers are present in the .note.stapsdt section
of the elf. We can traverse through this section and collect the
required info about the markers.
We can use '-M/--markers' with perf to view the SDT notes.
Currently, the sdt notes which have their semaphores enabled, are being
ignored silently. But, they will be supported soon.
Wrapping this inside #ifdef LIBELF_SUPPORT pair is not required,
because, if NO_LIBELF = 1, then 'probe' command of perf is itself disabled.
Usage:
perf probe --markers -x /lib64/libc.so.6
Output :
%libc:setjmp
%libc:longjmp
%libc:longjmp_target
%libc:lll_futex_wake
%libc:lll_lock_wait_private
%libc:longjmp
%libc:longjmp_target
%libc:lll_futex_wake
Signed-off-by: Hemant Kumar Shaw <[email protected]>
---
tools/perf/builtin-probe.c | 41 ++++++++
tools/perf/util/probe-event.c | 35 +++++++
tools/perf/util/probe-event.h | 1
tools/perf/util/symbol-elf.c | 211 +++++++++++++++++++++++++++++++++++++++++
tools/perf/util/symbol.h | 18 +++
5 files changed, 304 insertions(+), 2 deletions(-)
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 89acc17..2450613 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -55,6 +55,8 @@ static struct {
bool show_funcs;
bool mod_events;
bool uprobes;
+ bool exec;
+ bool sdt;
int nevents;
struct perf_probe_event events[MAX_PROBES];
struct strlist *dellist;
@@ -171,8 +173,10 @@ static int opt_set_target(const struct option *opt, const char *str,
int ret = -ENOENT;
if (str && !params.target) {
- if (!strcmp(opt->long_name, "exec"))
+ if (!strcmp(opt->long_name, "exec")) {
params.uprobes = true;
+ params.exec = true;
+ }
#ifdef HAVE_DWARF_SUPPORT
else if (!strcmp(opt->long_name, "module"))
params.uprobes = false;
@@ -325,6 +329,7 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
opt_set_filter),
OPT_CALLBACK('x', "exec", NULL, "executable|path",
"target executable name or path", opt_set_target),
+ OPT_BOOLEAN('M', "markers", ¶ms.sdt, "Show proba-able sdt notes"),
OPT_END()
};
int ret;
@@ -347,7 +352,7 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
params.max_probe_points = MAX_PROBES;
if ((!params.nevents && !params.dellist && !params.list_events &&
- !params.show_lines && !params.show_funcs))
+ !params.show_lines && !params.show_funcs && !params.sdt))
usage_with_options(probe_usage, options);
/*
@@ -355,6 +360,38 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
*/
symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
+ if (params.sdt) {
+ if (params.show_lines) {
+ pr_err("Error: Don't use --markers with --lines.\n");
+ usage_with_options(probe_usage, options);
+ }
+ if (params.show_vars) {
+ pr_err("Error: Don't use --markers with --vars.\n");
+ usage_with_options(probe_usage, options);
+ }
+ if (params.show_funcs) {
+ pr_err("Error: Don't use --markers with --funcs.\n");
+ usage_with_options(probe_usage, options);
+ }
+ if (params.mod_events) {
+ pr_err("Error: Don't use --markers with --add/--del.\n");
+ usage_with_options(probe_usage, options);
+ }
+ if (!params.exec) {
+ pr_err("Error: Always use --exec with --markers.\n");
+ usage_with_options(probe_usage, options);
+ }
+ if (!params.target) {
+ pr_err("Error: Please specify a target binary!\n");
+ usage_with_options(probe_usage, options);
+ }
+ ret = show_sdt_notes(params.target);
+ if (ret < 0) {
+ pr_err(" Error : Failed to find SDT markers in %s !"
+ " (%d)\n", params.target, ret);
+ }
+ return ret;
+ }
if (params.list_events) {
if (params.mod_events) {
pr_err(" Error: Don't use --list with --add/--del.\n");
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 779b2da..c228fe6 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1909,6 +1909,18 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
return ret;
}
+static void cleanup_sdt_note_list(struct list_head *sdt_notes)
+{
+ struct sdt_note *tmp, *pos;
+
+ list_for_each_entry_safe(pos, tmp, sdt_notes, note_list) {
+ list_del(&pos->note_list);
+ free(pos->name);
+ free(pos->provider);
+ free(pos);
+ }
+}
+
static int convert_to_probe_trace_events(struct perf_probe_event *pev,
struct probe_trace_event **tevs,
int max_tevs, const char *target)
@@ -2372,3 +2384,26 @@ out:
free(name);
return ret;
}
+
+static void display_sdt_note_info(struct list_head *start)
+{
+ struct sdt_note *pos;
+
+ list_for_each_entry(pos, start, note_list) {
+ printf("%%%s:%s\n", pos->provider, pos->name);
+ }
+}
+
+int show_sdt_notes(const char *target)
+{
+ int ret;
+ LIST_HEAD(sdt_notes);
+
+ ret = get_sdt_note_list(&sdt_notes, target);
+ if (!list_empty(&sdt_notes)) {
+ if (!ret)
+ display_sdt_note_info(&sdt_notes);
+ cleanup_sdt_note_list(&sdt_notes);
+ }
+ return ret;
+}
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index f9f3de8..32de5a3 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -133,6 +133,7 @@ extern int show_available_vars(struct perf_probe_event *pevs, int npevs,
struct strfilter *filter, bool externs);
extern int show_available_funcs(const char *module, struct strfilter *filter,
bool user);
+int show_sdt_notes(const char *target);
/* Maximum index number of event-name postfix */
#define MAX_EVENT_INDEX 1024
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index eed0b96..828ecad 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1613,6 +1613,217 @@ void kcore_extract__delete(struct kcore_extract *kce)
unlink(kce->extract_filename);
}
+/*
+ * Populate the name, type, offset in the SDT note structure and
+ * ignore the argument fields (for now)
+ */
+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;
+ int ret = -1;
+
+ /*
+ * Three addresses need to be obtained :
+ * Marker location, address of base section and semaphore location
+ */
+ union {
+ Elf64_Addr a64[3];
+ Elf32_Addr a32[3];
+ } buf;
+
+ /*
+ * dst and src are required for translation from file to memory
+ * representation
+ */
+ 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 *)zalloc(sizeof(struct sdt_note));
+ if (tmp == NULL) {
+ 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)
+ pr_debug("gelf_xlatetom : %s", 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 and ignore notes with semaphores set*/
+ if (gelf_getclass(*elf) == ELFCLASS32) {
+ if (buf.a32[2] != 0)
+ goto out_free_name;
+ tmp->addr.a32[0] = buf.a32[0];
+ tmp->addr.a32[1] = buf.a32[1];
+ tmp->addr.a32[2] = buf.a32[2];
+ tmp->bit32 = true;
+ } else {
+ if (buf.a64[2] != 0)
+ goto out_free_name;
+ tmp->addr.a64[0] = buf.a64[0];
+ tmp->addr.a64[1] = buf.a64[1];
+ tmp->addr.a64[2] = buf.a64[2];
+ tmp->bit32 = false;
+ }
+ *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;
+}
+
+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;
+ size_t 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;
+ pr_debug("Can't get elf header!");
+ goto out_ret;
+ }
+ if (elf_getshdrstrndx(elf, &shstrndx) != 0) {
+ ret = -EBADF;
+ pr_debug("getshdrstrndx failed\n");
+ goto out_ret;
+ }
+
+ /*
+ * Look for section type = SHT_NOTE, flags = no SHF_ALLOC
+ * and name = .note.stapsdt
+ */
+ scn = elf_section_by_name(elf, &ehdr, &shdr, SDT_NOTE_SCN, NULL);
+ if (!scn) {
+ ret = -ENOENT;
+ pr_debug("Can't get section .note.stapsdt\n");
+ 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 = -ENOMEM;
+ goto out_ret;
+ }
+ }
+ }
+ if (!sdt_notes)
+ ret = -ENOENT;
+
+out_ret:
+ return ret;
+}
+
+static int sdt_err(int val, const char *target)
+{
+ switch (-val) {
+ case 0:
+ break;
+ case ENOENT:
+ /* Absence of SDT markers isn't an error */
+ val = 0;
+ printf("%s : No SDT markers found!\n", target);
+ break;
+ case EBADF:
+ pr_err("%s : Bad file name\n", target);
+ break;
+ default:
+ pr_err("%s\n", strerror(val));
+ }
+ return val;
+}
+
+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) {
+ pr_err("%s : %s\n", target, strerror(errno));
+ return -errno;
+ }
+
+ symbol__elf_init();
+ elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
+ if (!elf) {
+ ret = -EBADF;
+ pr_debug("%s : %s\n", target, elf_errmsg(elf_errno()));
+ goto out_close;
+ }
+ ret = construct_sdt_notes_list(elf, head);
+ elf_end(elf);
+
+out_close:
+ close(fd);
+ return sdt_err(ret, target);
+}
+
void symbol__elf_init(void)
{
elf_version(EV_CURRENT);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 07de8fe..b78397b 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -198,6 +198,17 @@ struct symsrc {
#endif
};
+struct sdt_note {
+ char *name;
+ char *provider;
+ bool bit32; /* 32 or 64 bit flag */
+ union {
+ Elf64_Addr a64[3];
+ Elf32_Addr a32[3];
+ } addr;
+ struct list_head note_list;
+};
+
void symsrc__destroy(struct symsrc *ss);
int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
enum dso_binary_type type);
@@ -273,4 +284,11 @@ void kcore_extract__delete(struct kcore_extract *kce);
int kcore_copy(const char *from_dir, const char *to_dir);
int compare_proc_modules(const char *from, const char *to);
+/* Specific to SDT notes */
+int get_sdt_note_list(struct list_head *head, const char *target);
+
+#define SDT_NOTE_TYPE 3
+#define SDT_NOTE_SCN ".note.stapsdt"
+#define SDT_NOTE_NAME "stapsdt"
+
#endif /* __PERF_SYMBOL */
This allows perf to probe into the sdt markers/notes present in
the libraries and executables. We try to find the associated location
and handle prelinking (since, stapsdt notes section is not allocated
during runtime). Prelinking is handled with the help of base
section which is allocated during runtime. This address can be compared
with the address retrieved from the notes' description. If its different,
we can take this difference and then add to the note's location.
We can use existing '-a/--add' option to add events for sdt markers.
Also, we can add multiple events at once using the same '-a' option.
Usage:
perf probe -x /lib64/libc.so.6 -a 'my_event=%libc:setjmp'
Output:
Added new event:
libc:my_event (on 0x35981)
You can now use it in all perf tools, such as:
perf record -e libc:my_event -aR sleep 1
Signed-off-by: Hemant Kumar Shaw <[email protected]>
---
tools/perf/builtin-probe.c | 4 ++
tools/perf/util/probe-event.c | 99 +++++++++++++++++++++++++++++++++++++----
tools/perf/util/probe-event.h | 2 +
tools/perf/util/symbol-elf.c | 80 +++++++++++++++++++++++++++++++++
tools/perf/util/symbol.h | 3 +
5 files changed, 178 insertions(+), 10 deletions(-)
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 2450613..8f0dd48 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -494,6 +494,10 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
}
if (params.nevents) {
+ if (params.events->sdt && !params.target && !params.exec) {
+ pr_err("SDT markers can be probed only with --exec.\n");
+ usage_with_options(probe_usage, options);
+ }
ret = add_perf_probe_events(params.events, params.nevents,
params.max_probe_points,
params.target,
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index c228fe6..06665a8 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -816,6 +816,40 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
pev->group = NULL;
arg = tmp;
}
+ /* Check for SDT marker */
+ if (*arg == '%') {
+ ptr = strchr(++arg, ':');
+ if (!ptr) {
+ semantic_error("Provider name must follow an event "
+ "name\n");
+ return -EINVAL;
+ }
+ *ptr++ = '\0';
+ tmp = strdup(arg);
+ if (!tmp)
+ return -ENOMEM;
+
+ pev->point.note = (struct sdt_note *)
+ zalloc(sizeof(struct sdt_note));
+ if (!pev->point.note) {
+ free(tmp);
+ return -ENOMEM;
+ }
+ pev->point.note->provider = tmp;
+
+ tmp = strdup(ptr);
+ if (!tmp) {
+ free(pev->point.note->provider);
+ free(pev->point.note);
+ return -ENOMEM;
+ }
+ pev->point.note->name = tmp;
+ pev->group = pev->point.note->provider;
+ if (!pev->event)
+ pev->event = pev->point.note->name;
+ pev->sdt = true;
+ return 0;
+ }
ptr = strpbrk(arg, ";:+@%");
if (ptr) {
@@ -1238,6 +1272,7 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
char *buf, *tmp;
char offs[32] = "", line[32] = "", file[32] = "";
int ret, len;
+ unsigned long long addr;
buf = zalloc(MAX_CMDLEN);
if (buf == NULL) {
@@ -1266,12 +1301,17 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
goto error;
}
- if (pp->function)
+ if (pp->function) {
ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
offs, pp->retprobe ? "%return" : "", line,
file);
- else
+ } else if (pp->note) {
+ addr = pp->note->bit32 ? pp->note->addr.a32[0] :
+ pp->note->addr.a64[0];
+ ret = e_snprintf(buf, MAX_CMDLEN, "0x%llx", addr);
+ } else {
ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
+ }
if (ret <= 0)
goto error;
@@ -1921,6 +1961,21 @@ static void cleanup_sdt_note_list(struct list_head *sdt_notes)
}
}
+static int try_to_find_sdt_notes(struct perf_probe_event *pev,
+ const char *target)
+{
+ int ret;
+ LIST_HEAD(sdt_notes);
+
+ ret = search_sdt_note(pev->point.note, &sdt_notes, target);
+ if (!list_empty(&sdt_notes))
+ cleanup_sdt_note_list(&sdt_notes);
+ /* Abssence of SDT markers is an error in this case */
+ else if (list_empty(&sdt_notes) && !ret)
+ ret = -ENOENT;
+ return ret;
+}
+
static int convert_to_probe_trace_events(struct perf_probe_event *pev,
struct probe_trace_event **tevs,
int max_tevs, const char *target)
@@ -1928,11 +1983,23 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
struct symbol *sym;
int ret = 0, i;
struct probe_trace_event *tev;
+ char *buf;
+ unsigned long long addr;
- /* Convert perf_probe_event with debuginfo */
- ret = try_to_find_probe_trace_events(pev, tevs, max_tevs, target);
- if (ret != 0)
- return ret; /* Found in debuginfo or got an error */
+ if (pev->sdt) {
+ ret = -EBADF;
+ if (pev->uprobes)
+ ret = try_to_find_sdt_notes(pev, target);
+ if (ret)
+ return ret;
+ } else {
+ /* Convert perf_probe_event with debuginfo */
+ ret = try_to_find_probe_trace_events(pev, tevs, max_tevs,
+ target);
+ /* Found in debuginfo or got an error */
+ if (ret != 0)
+ return ret;
+ }
/* Allocate trace event buffer */
tev = *tevs = zalloc(sizeof(struct probe_trace_event));
@@ -1940,10 +2007,22 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
return -ENOMEM;
/* Copy parameters */
- tev->point.symbol = strdup(pev->point.function);
- if (tev->point.symbol == NULL) {
- ret = -ENOMEM;
- goto error;
+ if (pev->sdt) {
+ buf = (char *)zalloc(sizeof(char) * MAX_CMDLEN);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto error;
+ }
+ addr = pev->point.note->bit32 ? pev->point.note->addr.a32[0] :
+ pev->point.note->addr.a64[0];
+ sprintf(buf, "0x%llx", addr);
+ tev->point.symbol = buf;
+ } else {
+ tev->point.symbol = strdup(pev->point.function);
+ if (tev->point.symbol == NULL) {
+ ret = -ENOMEM;
+ goto error;
+ }
}
if (target) {
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 32de5a3..beb1b4a 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -47,6 +47,7 @@ struct perf_probe_point {
bool retprobe; /* Return probe flag */
char *lazy_line; /* Lazy matching pattern */
unsigned long offset; /* Offset from function entry */
+ struct sdt_note *note;
};
/* Perf probe probing argument field chain */
@@ -72,6 +73,7 @@ struct perf_probe_event {
struct perf_probe_point point; /* Probe point */
int nargs; /* Number of arguments */
bool uprobes;
+ bool sdt;
struct perf_probe_arg *args; /* Arguments */
};
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 828ecad..73b368c 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1824,6 +1824,86 @@ out_close:
return sdt_err(ret, target);
}
+static void adjust_note_addr(struct sdt_note *tmp, struct sdt_note *key,
+ Elf *elf)
+{
+ GElf_Ehdr ehdr;
+ GElf_Addr base_off = 0;
+ GElf_Shdr shdr;
+
+ if (!gelf_getehdr(elf, &ehdr)) {
+ pr_debug("%s : cannot get elf header.\n", __func__);
+ return;
+ }
+
+ /*
+ * 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)
+ key->addr.a32[0] = tmp->addr.a32[0] + base_off -
+ tmp->addr.a32[1];
+ else
+ key->addr.a64[0] = tmp->addr.a64[0] + base_off -
+ tmp->addr.a64[1];
+ }
+ key->bit32 = tmp->bit32;
+}
+
+int search_sdt_note(struct sdt_note *key, struct list_head *sdt_notes,
+ const char *target)
+{
+ Elf *elf;
+ int fd, ret;
+ bool found = false;
+ struct sdt_note *pos = NULL;
+
+ fd = open(target, O_RDONLY);
+ if (fd < 0) {
+ pr_err("%s : %s\n", target, strerror(errno));
+ return -errno;
+ }
+
+ symbol__elf_init();
+ elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
+ if (!elf) {
+ ret = -EBADF;
+ pr_debug("Can't read the elf of %s\n", target);
+ goto out_close;
+ }
+
+ ret = construct_sdt_notes_list(elf, sdt_notes);
+ if (ret)
+ goto out_end;
+
+ /* Iterate through the notes and retrieve the required note */
+ list_for_each_entry(pos, sdt_notes, note_list) {
+ if (!strcmp(key->name, pos->name) &&
+ !strcmp(key->provider, pos->provider)) {
+ adjust_note_addr(pos, key, elf);
+ found = true;
+ break;
+ }
+ }
+ if (!found) {
+ printf("%%%s:%s not found in %s!\n", key->provider, key->name,
+ target);
+ return -ENOENT;
+ }
+
+out_end:
+ elf_end(elf);
+out_close:
+ close(fd);
+ return sdt_err(ret, target);
+}
+
void symbol__elf_init(void)
{
elf_version(EV_CURRENT);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index b78397b..52b04e2 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -286,9 +286,12 @@ int compare_proc_modules(const char *from, const char *to);
/* Specific to SDT notes */
int get_sdt_note_list(struct list_head *head, const char *target);
+int search_sdt_note(struct sdt_note *key, struct list_head *head,
+ const char *target);
#define SDT_NOTE_TYPE 3
#define SDT_NOTE_SCN ".note.stapsdt"
#define SDT_NOTE_NAME "stapsdt"
+#define SDT_BASE_SCN ".stapsdt.base"
#endif /* __PERF_SYMBOL */
This patch adds documentation regarding perf support
to SDT notes/markers.
Signed-off-by: Hemant Kumar Shaw <[email protected]>
---
tools/perf/Documentation/perf-probe.txt | 17 +++
tools/perf/Documentation/sdt-probes.txt | 184 +++++++++++++++++++++++++++++++
2 files changed, 199 insertions(+), 2 deletions(-)
create mode 100644 tools/perf/Documentation/sdt-probes.txt
diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index b715cb7..f0169d9 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -99,10 +99,15 @@ OPTIONS
--max-probes::
Set the maximum number of probe points for an event. Default is 128.
+-M::
+--markers::
+ View the SDT markers present in a user space application/library.
+
-x::
--exec=PATH::
Specify path to the executable or shared library file for user
- space tracing. Can also be used with --funcs option.
+ space tracing. Can also be used with --funcs option and must be used
+ with --markers/-M option.
In absence of -m/-x options, perf probe checks if the first argument after
the options is an absolute path name. If its an absolute path, perf probe
@@ -121,11 +126,15 @@ Probe points are defined by following syntax.
3) Define event based on source file with lazy pattern
[EVENT=]SRC;PTN [ARG ...]
+ 4) Define event based on SDT marker
+ [[EVENT=]%PROVIDER:MARKER
+
-'EVENT' specifies the name of new event, if omitted, it will be set the name of the probed function. Currently, event group name is set as 'probe'.
+'EVENT' specifies the name of new event, if omitted, it will be set the name of the probed function. Currently, event group name is set as 'probe' except in case of SDT markers where it is set to provider name.
'FUNC' specifies a probed function name, and it may have one of the following options; '+OFFS' is the offset from function entry address in bytes, ':RLN' is the relative-line number from function entry line, and '%return' means that it probes function return. And ';PTN' means lazy matching pattern (see LAZY MATCHING). Note that ';PTN' must be the end of the probe point definition. In addition, '@SRC' specifies a source file which has that function.
It is also possible to specify a probe point by the source line number or lazy matching by using 'SRC:ALN' or 'SRC;PTN' syntax, where 'SRC' is the source file path, ':ALN' is the line number and ';PTN' is the lazy matching pattern.
'ARG' specifies the arguments of this probe point, (see PROBE ARGUMENT).
+'%PROVIDER:MARKER' is the syntax of SDT markers present in an ELF.
PROBE ARGUMENT
--------------
@@ -200,6 +209,10 @@ Add probes at malloc() function on libc
./perf probe -x /lib/libc.so.6 malloc or ./perf probe /lib/libc.so.6 malloc
+Add probes at longjmp SDT marker on libc
+
+ ./perf probe -x /lib64/libc.so.6 %libc:longjmp
+
SEE ALSO
--------
linkperf:perf-trace[1], linkperf:perf-record[1]
diff --git a/tools/perf/Documentation/sdt-probes.txt b/tools/perf/Documentation/sdt-probes.txt
new file mode 100644
index 0000000..d5556b7
--- /dev/null
+++ b/tools/perf/Documentation/sdt-probes.txt
@@ -0,0 +1,184 @@
+Perf probing on SDT markers:
+
+Goal:
+Probe dtrace style markers(SDT) present in user space applications.
+
+Scope:
+Put probe points at SDT markers in user space applications and libraries
+and also probe them using perf.
+
+Why supprt SDT markers? :
+We have lots of applications which use SDT markers today like:
+Postgresql, MySql, Mozilla, Perl, Python, Java, Ruby, libvirt, QEMU, glib
+
+These markers are placed at important places by the developers. Now, these
+markers 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.
+
+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 probe -M -x ./a.out
+%user_app:test_mark
+
+A different example to show the same:
+- Create a file with .d extension and mention the probe names in it with
+provider name and marker name.
+
+$ cat probes.d
+provider user_app {
+ probe foo_start();
+ probe fun_start();
+};
+
+- Now create the probes.h and probes.o file :
+$ dtrace -C -h -s probes.d -o probes.h
+$ dtrace -C -G -s probes.d -o probes.o
+
+- A program using the markers:
+
+$ cat user_app.c
+
+#include <stdio.h>
+#include "probes.h"
+
+void foo(void)
+{
+ USER_APP_FOO_START();
+ printf("This is foo\n");
+}
+
+void fun(void)
+{
+ USER_APP_FUN_START();
+ printf("Inside fun\n");
+}
+int main(void)
+{
+ printf("In main\n");
+ foo();
+ fun();
+ return 0;
+}
+
+- Compile it and also provide probes.o file to linker:
+$ gcc user_app.c probes.o -o user_app
+
+- Now use perf to list the markers in the app:
+# perf probe --markers -x ./user_app
+
+%user_app:foo_start
+%user_app:fun_start
+
+- And then use perf probe to add a probe point :
+
+# perf probe -x ./user_app 'my_event=%user_app:foo_start'
+
+Added new event :
+user_app:my_event (on 0x530)
+
+You can now use it on all perf tools such as :
+
+ perf record -e user_app:my_event -aR sleep 1
+
+# perf record -e probe_user:my_event -aR ./user_app
+In main
+This is foo
+Inside fun
+[ perf record: Woken up 1 times to write data ]
+[ perf record: Captured and wrote 0.235 MB perf.data (~10279 samples) ]
+
+- Then use perf tools to analyze it.
+# perf report --stdio
+
+# ========
+# captured on: Tue Sep 3 16:19:55 2013
+# hostname : hemant-fedora
+# os release : 3.11.0-rc3+
+# perf version : 3.9.4-200.fc18.x86_64
+# arch : x86_64
+# nrcpus online : 2
+# nrcpus avail : 2
+# cpudesc : QEMU Virtual CPU version 1.2.2
+# cpuid : GenuineIntel,6,2,3
+# total memory : 2051912 kBIf these are not enabled, they are present in the ELF as nop.
+
+# cmdline : /usr/bin/perf record -e probe_user:foo_start -aR ./user_app
+# event : name = probe_user:foo_start, type = 2, config = 0x38e, config1
+= 0x0, config2 = 0x0, excl_usr = 0, excl_kern = 0, excl_host = 0,
+excl_guest = 1, precise_ip = 0
+# HEADER_CPU_TOPOLOGY info available, use -I to display
+# HEADER_NUMA_TOPOLOGY info available, use -I to display
+# pmu mappings: software = 1, tracepoint = 2, breakpoint = 5
+# ========
+#
+# Samples: 1 of event 'probe_user:foo_start'
+# Event count (approx.): 1
+#
+# Overhead Command Shared Object Symbol
+# ........ ........ ............. .......
+#
+ 100.00% user_app user_app [.] foo
+
+
+#
+# (For a higher level overview, try: perf report --sort comm,dso)
+#
+
+
+We can see the existing markers in libc (if present) :
+$ perf probe --markers -x /lib64/libc.so.6
+
+%libc:setjmp
+%libc:longjmp
+%libc:longjmp_target
+%libc:lll_futex_wake
+%libc:lll_lock_wait_private
+%libc:longjmp
+%libc:longjmp_target
+%libc:lll_futex_wake
+
+- And then use perf to probe into any marker:
+
+# perf probe -x /lib64/libc.so.6 %libc:setjmp
+Added new event:
+ libc:setjmp (on 0x35981)
+
+You can now use it in all perf tools, such as:
+
+ perf record -e libc:setjmp -aR sleep 1
+
+
+This link shows an example of marker probing with Systemtap:
+https://sourceware.org/systemtap/wiki/AddingUserSpaceProbingToApps
+
+And, this link shows more info on SDT markers:
+http://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation
+
+- 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, arguments are present.
+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.
On 10/18/2013 08:12 PM, Hemant Kumar wrote:
> This patchset helps in probing 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.
>
> 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 to show this :
> $ 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 probe -M -x ./a.out
> %user_app:test_mark
>
> A different example to show the same :
> - Create a file with .d extension and mention the probe names in it with
> provider name and marker name.
>
> $ cat probes.d
> provider user_app {
> probe foo_start();
> probe fun_start();
> };
>
> - Now create the probes.h and probes.o file :
> $ dtrace -C -h -s probes.d -o probes.h
> $ dtrace -C -G -s probes.d -o probes.o
>
> - A program using the markers:
>
> $ cat user_app.c
>
> #include <stdio.h>
> #include "probes.h"
>
> void foo(void)
> {
> USER_APP_FOO_START();
> printf("This is foo\n");
> }
>
> void fun(void)
> {
> USER_APP_FUN_START();
> printf("Inside fun\n");
> }
> int main(void)
> {
> printf("In main\n");
> foo();
> fun();
> return 0;
> }
>
> - Compile it and also provide probes.o file to linker:
> $ gcc user_app.c probes.o -o user_app
>
> - Now use perf to list the markers in the app:
> # perf probe --markers -x ./user_app
>
> %user_app:foo_start
> %user_app:fun_start
>
> - And then use perf probe to add a probe point :
>
> # perf probe -x ./user_app -a '%user_app:foo_start'
>
> Added new event :
> event = foo_start (on 0x530)
>
> You can now use it on all perf tools such as :
>
> perf record -e probe_user:foo_start -aR sleep 1
>
> # perf record -e probe_user:foo_start -aR ./user_app
> In main
> This is foo
> Inside fun
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.235 MB perf.data (~10279 samples) ]
>
> - Then use perf tools to analyze it.
> # perf report --stdio
>
> # ========
> # captured on: Tue Sep 3 16:19:55 2013
> # hostname : hemant-fedora
> # os release : 3.11.0-rc3+
> # perf version : 3.9.4-200.fc18.x86_64
> # arch : x86_64
> # nrcpus online : 2
> # nrcpus avail : 2
> # cpudesc : QEMU Virtual CPU version 1.2.2
> # cpuid : GenuineIntel,6,2,3
> # total memory : 2051912 kBIf these are not enabled, they are present in the \
> ELF as nop.
>
> # cmdline : /usr/bin/perf record -e probe_user:foo_start -aR ./user_app
> # event : name = probe_user:foo_start, type = 2, config = 0x38e, config1
> = 0x0, config2 = 0x0, excl_usr = 0, excl_kern = 0, excl_host = 0,
> excl_guest = 1, precise_ip = 0
> # HEADER_CPU_TOPOLOGY info available, use -I to display
> # HEADER_NUMA_TOPOLOGY info available, use -I to display
> # pmu mappings: software = 1, tracepoint = 2, breakpoint = 5
> # ========
> #
> # Samples: 1 of event 'probe_user:foo_start'
> # Event count (approx.): 1
> #
> # Overhead Command Shared Object Symbol
> # ........ ........ ............. .......
> #
> 100.00% user_app user_app [.] foo
>
>
> #
> # (For a higher level overview, try: perf report --sort comm,dso)
> #
>
> 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
>
> - 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.
>
> All the above info is moved in sdt-probes.txt file present in the Documentation
> directory in tools/perf/.
>
> Changes since last version:
> - Added suggested changes by Masami and Namhyung.
> - Fixed some bugs.
> - Removed some redundancies.
> - Updated with error codes.
> - Rebased it to latest kernel tip.
>
> TODO:
> - Recognizing arguments and support to probe on them.
> ---
>
> Hemant Kumar (3):
> SDT markers listing by perf:
> Support for perf to probe into SDT markers:
> Documentation regarding perf/sdt
>
>
> tools/perf/Documentation/perf-probe.txt | 17 ++
> tools/perf/Documentation/sdt-probes.txt | 184 ++++++++++++++++++++
> tools/perf/builtin-probe.c | 45 +++++
> tools/perf/util/probe-event.c | 134 +++++++++++++-
> tools/perf/util/probe-event.h | 3
> tools/perf/util/symbol-elf.c | 291 +++++++++++++++++++++++++++++++
> tools/perf/util/symbol.h | 21 ++
> 7 files changed, 681 insertions(+), 14 deletions(-)
> create mode 100644 tools/perf/Documentation/sdt-probes.txt
>
Please ignore the previous patchset. Due to some git config issues, the
from email id was wrong. It has been fixed in this patchset.
--
Thanks
Hemant
(2013/10/18 23:44), Hemant Kumar wrote:
> This patch will enable perf to list all the sdt markers present
> in an elf file. The markers are present in the .note.stapsdt section
> of the elf. We can traverse through this section and collect the
> required info about the markers.
> We can use '-M/--markers' with perf to view the SDT notes.
>
> Currently, the sdt notes which have their semaphores enabled, are being
> ignored silently. But, they will be supported soon.
>
> Wrapping this inside #ifdef LIBELF_SUPPORT pair is not required,
> because, if NO_LIBELF = 1, then 'probe' command of perf is itself disabled.
>
> Usage:
> perf probe --markers -x /lib64/libc.so.6
>
> Output :
> %libc:setjmp
> %libc:longjmp
> %libc:longjmp_target
> %libc:lll_futex_wake
> %libc:lll_lock_wait_private
> %libc:longjmp
> %libc:longjmp_target
> %libc:lll_futex_wake
>
> Signed-off-by: Hemant Kumar Shaw <[email protected]>
> ---
> tools/perf/builtin-probe.c | 41 ++++++++
> tools/perf/util/probe-event.c | 35 +++++++
> tools/perf/util/probe-event.h | 1
> tools/perf/util/symbol-elf.c | 211 +++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/symbol.h | 18 +++
> 5 files changed, 304 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 89acc17..2450613 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -55,6 +55,8 @@ static struct {
> bool show_funcs;
> bool mod_events;
> bool uprobes;
> + bool exec;
> + bool sdt;
> int nevents;
> struct perf_probe_event events[MAX_PROBES];
> struct strlist *dellist;
> @@ -171,8 +173,10 @@ static int opt_set_target(const struct option *opt, const char *str,
> int ret = -ENOENT;
>
> if (str && !params.target) {
> - if (!strcmp(opt->long_name, "exec"))
> + if (!strcmp(opt->long_name, "exec")) {
> params.uprobes = true;
> + params.exec = true;
> + }
> #ifdef HAVE_DWARF_SUPPORT
> else if (!strcmp(opt->long_name, "module"))
> params.uprobes = false;
> @@ -325,6 +329,7 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> opt_set_filter),
> OPT_CALLBACK('x', "exec", NULL, "executable|path",
> "target executable name or path", opt_set_target),
> + OPT_BOOLEAN('M', "markers", ¶ms.sdt, "Show proba-able sdt notes"),
> OPT_END()
> };
> int ret;
> @@ -347,7 +352,7 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> params.max_probe_points = MAX_PROBES;
>
> if ((!params.nevents && !params.dellist && !params.list_events &&
> - !params.show_lines && !params.show_funcs))
> + !params.show_lines && !params.show_funcs && !params.sdt))
> usage_with_options(probe_usage, options);
>
> /*
> @@ -355,6 +360,38 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> */
> symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL);
>
> + if (params.sdt) {
> + if (params.show_lines) {
> + pr_err("Error: Don't use --markers with --lines.\n");
> + usage_with_options(probe_usage, options);
> + }
> + if (params.show_vars) {
> + pr_err("Error: Don't use --markers with --vars.\n");
> + usage_with_options(probe_usage, options);
> + }
> + if (params.show_funcs) {
> + pr_err("Error: Don't use --markers with --funcs.\n");
> + usage_with_options(probe_usage, options);
> + }
> + if (params.mod_events) {
> + pr_err("Error: Don't use --markers with --add/--del.\n");
> + usage_with_options(probe_usage, options);
> + }
> + if (!params.exec) {
> + pr_err("Error: Always use --exec with --markers.\n");
> + usage_with_options(probe_usage, options);
> + }
> + if (!params.target) {
> + pr_err("Error: Please specify a target binary!\n");
> + usage_with_options(probe_usage, options);
> + }
> + ret = show_sdt_notes(params.target);
> + if (ret < 0) {
> + pr_err(" Error : Failed to find SDT markers in %s !"
> + " (%d)\n", params.target, ret);
> + }
> + return ret;
> + }
> if (params.list_events) {
> if (params.mod_events) {
> pr_err(" Error: Don't use --list with --add/--del.\n");
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 779b2da..c228fe6 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -1909,6 +1909,18 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
> return ret;
> }
>
> +static void cleanup_sdt_note_list(struct list_head *sdt_notes)
> +{
> + struct sdt_note *tmp, *pos;
> +
> + list_for_each_entry_safe(pos, tmp, sdt_notes, note_list) {
> + list_del(&pos->note_list);
> + free(pos->name);
> + free(pos->provider);
> + free(pos);
> + }
> +}
> +
> static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> struct probe_trace_event **tevs,
> int max_tevs, const char *target)
> @@ -2372,3 +2384,26 @@ out:
> free(name);
> return ret;
> }
> +
> +static void display_sdt_note_info(struct list_head *start)
> +{
> + struct sdt_note *pos;
> +
> + list_for_each_entry(pos, start, note_list) {
> + printf("%%%s:%s\n", pos->provider, pos->name);
> + }
> +}
> +
> +int show_sdt_notes(const char *target)
> +{
> + int ret;
> + LIST_HEAD(sdt_notes);
> +
> + ret = get_sdt_note_list(&sdt_notes, target);
> + if (!list_empty(&sdt_notes)) {
> + if (!ret)
Hmm, why don't you check the ret first? And I think the
empty check should be done in display_sdt_note_info() and
cleanup_sdt_note_list() (anyway, since both uses list_for_each*()
it is already done).
> + display_sdt_note_info(&sdt_notes);
> + cleanup_sdt_note_list(&sdt_notes);
> + }
> + return ret;
> +}
Others are good for me. :)
Thank you!
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index f9f3de8..32de5a3 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -133,6 +133,7 @@ extern int show_available_vars(struct perf_probe_event *pevs, int npevs,
> struct strfilter *filter, bool externs);
> extern int show_available_funcs(const char *module, struct strfilter *filter,
> bool user);
> +int show_sdt_notes(const char *target);
>
> /* Maximum index number of event-name postfix */
> #define MAX_EVENT_INDEX 1024
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index eed0b96..828ecad 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -1613,6 +1613,217 @@ void kcore_extract__delete(struct kcore_extract *kce)
> unlink(kce->extract_filename);
> }
>
> +/*
> + * Populate the name, type, offset in the SDT note structure and
> + * ignore the argument fields (for now)
> + */
> +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;
> + int ret = -1;
> +
> + /*
> + * Three addresses need to be obtained :
> + * Marker location, address of base section and semaphore location
> + */
> + union {
> + Elf64_Addr a64[3];
> + Elf32_Addr a32[3];
> + } buf;
> +
> + /*
> + * dst and src are required for translation from file to memory
> + * representation
> + */
> + 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 *)zalloc(sizeof(struct sdt_note));
> + if (tmp == NULL) {
> + 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)
> + pr_debug("gelf_xlatetom : %s", 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 and ignore notes with semaphores set*/
> + if (gelf_getclass(*elf) == ELFCLASS32) {
> + if (buf.a32[2] != 0)
> + goto out_free_name;
> + tmp->addr.a32[0] = buf.a32[0];
> + tmp->addr.a32[1] = buf.a32[1];
> + tmp->addr.a32[2] = buf.a32[2];
> + tmp->bit32 = true;
> + } else {
> + if (buf.a64[2] != 0)
> + goto out_free_name;
> + tmp->addr.a64[0] = buf.a64[0];
> + tmp->addr.a64[1] = buf.a64[1];
> + tmp->addr.a64[2] = buf.a64[2];
> + tmp->bit32 = false;
> + }
> + *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;
> +}
> +
> +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;
> + size_t 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;
> + pr_debug("Can't get elf header!");
> + goto out_ret;
> + }
> + if (elf_getshdrstrndx(elf, &shstrndx) != 0) {
> + ret = -EBADF;
> + pr_debug("getshdrstrndx failed\n");
> + goto out_ret;
> + }
> +
> + /*
> + * Look for section type = SHT_NOTE, flags = no SHF_ALLOC
> + * and name = .note.stapsdt
> + */
> + scn = elf_section_by_name(elf, &ehdr, &shdr, SDT_NOTE_SCN, NULL);
> + if (!scn) {
> + ret = -ENOENT;
> + pr_debug("Can't get section .note.stapsdt\n");
> + 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 = -ENOMEM;
> + goto out_ret;
> + }
> + }
> + }
> + if (!sdt_notes)
> + ret = -ENOENT;
> +
> +out_ret:
> + return ret;
> +}
> +
> +static int sdt_err(int val, const char *target)
> +{
> + switch (-val) {
> + case 0:
> + break;
> + case ENOENT:
> + /* Absence of SDT markers isn't an error */
> + val = 0;
> + printf("%s : No SDT markers found!\n", target);
> + break;
> + case EBADF:
> + pr_err("%s : Bad file name\n", target);
> + break;
> + default:
> + pr_err("%s\n", strerror(val));
> + }
> + return val;
> +}
> +
> +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) {
> + pr_err("%s : %s\n", target, strerror(errno));
> + return -errno;
> + }
> +
> + symbol__elf_init();
> + elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
> + if (!elf) {
> + ret = -EBADF;
> + pr_debug("%s : %s\n", target, elf_errmsg(elf_errno()));
> + goto out_close;
> + }
> + ret = construct_sdt_notes_list(elf, head);
> + elf_end(elf);
> +
> +out_close:
> + close(fd);
> + return sdt_err(ret, target);
> +}
> +
> void symbol__elf_init(void)
> {
> elf_version(EV_CURRENT);
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 07de8fe..b78397b 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -198,6 +198,17 @@ struct symsrc {
> #endif
> };
>
> +struct sdt_note {
> + char *name;
> + char *provider;
> + bool bit32; /* 32 or 64 bit flag */
> + union {
> + Elf64_Addr a64[3];
> + Elf32_Addr a32[3];
> + } addr;
> + struct list_head note_list;
> +};
> +
> void symsrc__destroy(struct symsrc *ss);
> int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
> enum dso_binary_type type);
> @@ -273,4 +284,11 @@ void kcore_extract__delete(struct kcore_extract *kce);
> int kcore_copy(const char *from_dir, const char *to_dir);
> int compare_proc_modules(const char *from, const char *to);
>
> +/* Specific to SDT notes */
> +int get_sdt_note_list(struct list_head *head, const char *target);
> +
> +#define SDT_NOTE_TYPE 3
> +#define SDT_NOTE_SCN ".note.stapsdt"
> +#define SDT_NOTE_NAME "stapsdt"
> +
> #endif /* __PERF_SYMBOL */
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
(2013/10/18 23:44), Hemant Kumar wrote:
> This allows perf to probe into the sdt markers/notes present in
> the libraries and executables. We try to find the associated location
> and handle prelinking (since, stapsdt notes section is not allocated
> during runtime). Prelinking is handled with the help of base
> section which is allocated during runtime. This address can be compared
> with the address retrieved from the notes' description. If its different,
> we can take this difference and then add to the note's location.
>
> We can use existing '-a/--add' option to add events for sdt markers.
> Also, we can add multiple events at once using the same '-a' option.
>
> Usage:
> perf probe -x /lib64/libc.so.6 -a 'my_event=%libc:setjmp'
>
> Output:
> Added new event:
> libc:my_event (on 0x35981)
>
> You can now use it in all perf tools, such as:
>
> perf record -e libc:my_event -aR sleep 1
>
>
> Signed-off-by: Hemant Kumar Shaw <[email protected]>
> ---
> tools/perf/builtin-probe.c | 4 ++
> tools/perf/util/probe-event.c | 99 +++++++++++++++++++++++++++++++++++++----
> tools/perf/util/probe-event.h | 2 +
> tools/perf/util/symbol-elf.c | 80 +++++++++++++++++++++++++++++++++
> tools/perf/util/symbol.h | 3 +
> 5 files changed, 178 insertions(+), 10 deletions(-)
>
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 2450613..8f0dd48 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -494,6 +494,10 @@ int cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> }
>
> if (params.nevents) {
> + if (params.events->sdt && !params.target && !params.exec) {
> + pr_err("SDT markers can be probed only with --exec.\n");
> + usage_with_options(probe_usage, options);
> + }
> ret = add_perf_probe_events(params.events, params.nevents,
> params.max_probe_points,
> params.target,
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index c228fe6..06665a8 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -816,6 +816,40 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> pev->group = NULL;
> arg = tmp;
> }
> + /* Check for SDT marker */
> + if (*arg == '%') {
> + ptr = strchr(++arg, ':');
> + if (!ptr) {
> + semantic_error("Provider name must follow an event "
> + "name\n");
> + return -EINVAL;
> + }
> + *ptr++ = '\0';
> + tmp = strdup(arg);
> + if (!tmp)
> + return -ENOMEM;
> +
> + pev->point.note = (struct sdt_note *)
> + zalloc(sizeof(struct sdt_note));
> + if (!pev->point.note) {
> + free(tmp);
> + return -ENOMEM;
> + }
> + pev->point.note->provider = tmp;
> +
> + tmp = strdup(ptr);
> + if (!tmp) {
> + free(pev->point.note->provider);
> + free(pev->point.note);
> + return -ENOMEM;
> + }
> + pev->point.note->name = tmp;
> + pev->group = pev->point.note->provider;
> + if (!pev->event)
> + pev->event = pev->point.note->name;
> + pev->sdt = true;
> + return 0;
> + }
>
> ptr = strpbrk(arg, ";:+@%");
> if (ptr) {
> @@ -1238,6 +1272,7 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> char *buf, *tmp;
> char offs[32] = "", line[32] = "", file[32] = "";
> int ret, len;
> + unsigned long long addr;
>
> buf = zalloc(MAX_CMDLEN);
> if (buf == NULL) {
> @@ -1266,12 +1301,17 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> goto error;
> }
>
> - if (pp->function)
> + if (pp->function) {
> ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
> offs, pp->retprobe ? "%return" : "", line,
> file);
> - else
> + } else if (pp->note) {
> + addr = pp->note->bit32 ? pp->note->addr.a32[0] :
> + pp->note->addr.a64[0];
> + ret = e_snprintf(buf, MAX_CMDLEN, "0x%llx", addr);
> + } else {
> ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
> + }
> if (ret <= 0)
> goto error;
>
> @@ -1921,6 +1961,21 @@ static void cleanup_sdt_note_list(struct list_head *sdt_notes)
> }
> }
>
> +static int try_to_find_sdt_notes(struct perf_probe_event *pev,
> + const char *target)
> +{
> + int ret;
> + LIST_HEAD(sdt_notes);
> +
> + ret = search_sdt_note(pev->point.note, &sdt_notes, target);
> + if (!list_empty(&sdt_notes))
> + cleanup_sdt_note_list(&sdt_notes);
> + /* Abssence of SDT markers is an error in this case */
> + else if (list_empty(&sdt_notes) && !ret)
> + ret = -ENOENT;
> + return ret;
> +}
> +
> static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> struct probe_trace_event **tevs,
> int max_tevs, const char *target)
> @@ -1928,11 +1983,23 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> struct symbol *sym;
> int ret = 0, i;
> struct probe_trace_event *tev;
> + char *buf;
> + unsigned long long addr;
>
> - /* Convert perf_probe_event with debuginfo */
> - ret = try_to_find_probe_trace_events(pev, tevs, max_tevs, target);
> - if (ret != 0)
> - return ret; /* Found in debuginfo or got an error */
> + if (pev->sdt) {
> + ret = -EBADF;
> + if (pev->uprobes)
> + ret = try_to_find_sdt_notes(pev, target);
> + if (ret)
> + return ret;
> + } else {
> + /* Convert perf_probe_event with debuginfo */
> + ret = try_to_find_probe_trace_events(pev, tevs, max_tevs,
> + target);
> + /* Found in debuginfo or got an error */
> + if (ret != 0)
> + return ret;
> + }
>
> /* Allocate trace event buffer */
> tev = *tevs = zalloc(sizeof(struct probe_trace_event));
> @@ -1940,10 +2007,22 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> return -ENOMEM;
>
> /* Copy parameters */
> - tev->point.symbol = strdup(pev->point.function);
> - if (tev->point.symbol == NULL) {
> - ret = -ENOMEM;
> - goto error;
> + if (pev->sdt) {
> + buf = (char *)zalloc(sizeof(char) * MAX_CMDLEN);
> + if (!buf) {
> + ret = -ENOMEM;
> + goto error;
> + }
> + addr = pev->point.note->bit32 ? pev->point.note->addr.a32[0] :
> + pev->point.note->addr.a64[0];
> + sprintf(buf, "0x%llx", addr);
> + tev->point.symbol = buf;
> + } else {
> + tev->point.symbol = strdup(pev->point.function);
> + if (tev->point.symbol == NULL) {
> + ret = -ENOMEM;
> + goto error;
> + }
> }
>
> if (target) {
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index 32de5a3..beb1b4a 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -47,6 +47,7 @@ struct perf_probe_point {
> bool retprobe; /* Return probe flag */
> char *lazy_line; /* Lazy matching pattern */
> unsigned long offset; /* Offset from function entry */
> + struct sdt_note *note;
> };
>
> /* Perf probe probing argument field chain */
> @@ -72,6 +73,7 @@ struct perf_probe_event {
> struct perf_probe_point point; /* Probe point */
> int nargs; /* Number of arguments */
> bool uprobes;
> + bool sdt;
> struct perf_probe_arg *args; /* Arguments */
> };
>
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 828ecad..73b368c 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -1824,6 +1824,86 @@ out_close:
> return sdt_err(ret, target);
> }
>
> +static void adjust_note_addr(struct sdt_note *tmp, struct sdt_note *key,
> + Elf *elf)
> +{
> + GElf_Ehdr ehdr;
> + GElf_Addr base_off = 0;
> + GElf_Shdr shdr;
> +
> + if (!gelf_getehdr(elf, &ehdr)) {
> + pr_debug("%s : cannot get elf header.\n", __func__);
> + return;
> + }
> +
> + /*
> + * 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)
> + key->addr.a32[0] = tmp->addr.a32[0] + base_off -
> + tmp->addr.a32[1];
> + else
> + key->addr.a64[0] = tmp->addr.a64[0] + base_off -
> + tmp->addr.a64[1];
> + }
> + key->bit32 = tmp->bit32;
> +}
> +
> +int search_sdt_note(struct sdt_note *key, struct list_head *sdt_notes,
> + const char *target)
Hmm, this interface looks very odd.
- This just finds the first matched sdt_note entry instead of the list.
In that case, we don't need to pass the list_head.
- If the purpose is just counting the number of matched entries (or just
checking existence), we also don't need the list. Just returning the
number is enough.
I recommend you to remove the "sdt_notes", and make the caller check
key->addr is set.
Thank you,
> +{
> + Elf *elf;
> + int fd, ret;
> + bool found = false;
> + struct sdt_note *pos = NULL;
> +
> + fd = open(target, O_RDONLY);
> + if (fd < 0) {
> + pr_err("%s : %s\n", target, strerror(errno));
> + return -errno;
> + }
> +
> + symbol__elf_init();
> + elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
> + if (!elf) {
> + ret = -EBADF;
> + pr_debug("Can't read the elf of %s\n", target);
> + goto out_close;
> + }
> +
> + ret = construct_sdt_notes_list(elf, sdt_notes);
> + if (ret)
> + goto out_end;
> +
> + /* Iterate through the notes and retrieve the required note */
> + list_for_each_entry(pos, sdt_notes, note_list) {
> + if (!strcmp(key->name, pos->name) &&
> + !strcmp(key->provider, pos->provider)) {
> + adjust_note_addr(pos, key, elf);
> + found = true;
> + break;
> + }
> + }
> + if (!found) {
> + printf("%%%s:%s not found in %s!\n", key->provider, key->name,
> + target);
> + return -ENOENT;
> + }
> +
> +out_end:
> + elf_end(elf);
> +out_close:
> + close(fd);
> + return sdt_err(ret, target);
> +}
> +
> void symbol__elf_init(void)
> {
> elf_version(EV_CURRENT);
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index b78397b..52b04e2 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -286,9 +286,12 @@ int compare_proc_modules(const char *from, const char *to);
>
> /* Specific to SDT notes */
> int get_sdt_note_list(struct list_head *head, const char *target);
> +int search_sdt_note(struct sdt_note *key, struct list_head *head,
> + const char *target);
>
> #define SDT_NOTE_TYPE 3
> #define SDT_NOTE_SCN ".note.stapsdt"
> #define SDT_NOTE_NAME "stapsdt"
> +#define SDT_BASE_SCN ".stapsdt.base"
>
> #endif /* __PERF_SYMBOL */
>
>
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
Hi Masami,
On 10/19/2013 08:57 PM, Masami Hiramatsu wrote:
> (2013/10/18 23:44), Hemant Kumar wrote:
[...]
>> +int show_sdt_notes(const char *target)
>> +{
>> + int ret;
>> + LIST_HEAD(sdt_notes);
>> +
>> + ret = get_sdt_note_list(&sdt_notes, target);
>> + if (!list_empty(&sdt_notes)) {
>> + if (!ret)
> Hmm, why don't you check the ret first? And I think the
> empty check should be done in display_sdt_note_info() and
> cleanup_sdt_note_list() (anyway, since both uses list_for_each*()
> it is already done).
Okay, will do that.
>
>> + display_sdt_note_info(&sdt_notes);
>> + cleanup_sdt_note_list(&sdt_notes);
>> + }
>> + return ret;
>> +}
> Others are good for me. :)
Great! thanks for the review. :)
--
Thanks
Hemant Kumar
On 10/19/2013 09:18 PM, Masami Hiramatsu wrote:
> [...]
> +
> + /*
> + * 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)
> + key->addr.a32[0] = tmp->addr.a32[0] + base_off -
> + tmp->addr.a32[1];
> + else
> + key->addr.a64[0] = tmp->addr.a64[0] + base_off -
> + tmp->addr.a64[1];
> + }
> + key->bit32 = tmp->bit32;
> +}
> +
> +int search_sdt_note(struct sdt_note *key, struct list_head *sdt_notes,
> + const char *target)
> Hmm, this interface looks very odd.
> - This just finds the first matched sdt_note entry instead of the list.
> In that case, we don't need to pass the list_head.
> - If the purpose is just counting the number of matched entries (or just
> checking existence), we also don't need the list. Just returning the
> number is enough.
>
> I recommend you to remove the "sdt_notes", and make the caller check
> key->addr is set.
Yeah, list isn't required in this case. Thanks for pointing that out.
Will change that.
> [...]
>
--
Thanks
Hemant Kumar