Re: [PATCH v2 2/3] Support for perf to probe into SDT markers:
From: Hemant
Date: Tue Oct 08 2013 - 09:09:05 EST
On 10/08/2013 02:39 PM, Namhyung Kim wrote:
[...]
+
+ tmp = strdup(ptr);
+ if (!tmp)
+ return -ENOMEM;
These -ENOMEM returning should free all memory region allocated
previously.
Yes, missed that.
+ 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) {
nc = *ptr;
@@ -1270,6 +1299,13 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
offs, pp->retprobe ? "%return" : "", line,
file);
+ else if (pp->note)
+ if (pp->note->bit32)
+ ret = e_snprintf(buf, MAX_CMDLEN, "0x%x",
+ pp->note->addr.a32[0]);
+ else
+ ret = e_snprintf(buf, MAX_CMDLEN, "0x%lx",
+ pp->note->addr.a64[0]);
This kind of code tends to cause 32/64-bit build problem. Did you test
it on a 32-bit system too? Anyway, I think things like below should
work:
unsigned long long addr;
if (pp->note->bit32)
addr = pp->note->addr.a32[0];
else
addr = pp->note->addr.a64[0];
ret = e_snprintf(buf, MAX_CMDLEN, "0x%llx", addr);
Looks better, thanks, will make the required changes.
else
ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
if (ret <= 0)
@@ -1923,6 +1959,19 @@ 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)
+{
+ struct list_head sdt_notes;
+ int ret = -1;
+
+ INIT_LIST_HEAD(&sdt_notes);
You can use just LIST_HEAD(sdt_notes) here.
Ok.
+ ret = search_sdt_note(pev->point.note, &sdt_notes, target);
+ if (!list_empty(&sdt_notes))
+ cleanup_sdt_note_list(&sdt_notes);
+ 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)
@@ -1930,11 +1979,20 @@ 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;
- /* 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 = 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));
@@ -1942,10 +2000,25 @@ 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;
+ }
+ if (pev->point.note->bit32)
+ sprintf(buf, "0x%x",
+ (unsigned)pev->point.note->addr.a32[0]);
+ else
+ sprintf(buf, "0x%lx",
+ (unsigned long)pev->point.note->addr.a64[0]);
Please see my comment above.
Ok. Will change that too.
+ 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 b8a9347..4bd50cc 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;
I guess what you need is a 'struct list_head'.
Yes, struct list_head will be a better choice in struct perf_probe_point.
};
/* 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 6b17260..d0e7a66 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1022,6 +1022,86 @@ out_ret:
return ret;
}
+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, 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 = -1;
+ struct list_head *pos = NULL, *head = NULL;
+ struct sdt_note *tmp = NULL;
+
+ fd = open(target, O_RDONLY);
+ if (fd < 0) {
+ pr_err("Failed to open %s\n", target);
+ goto out_ret;
+ }
+
+ symbol__elf_init();
+ elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
+ if (!elf) {
+ pr_err("%s : can't read %s ELF file.\n", __func__, target);
+ goto out_close;
+ }
+
+ head = construct_sdt_notes_list(elf);
+ if (!head)
+ goto out_end;
+
+ list_add(sdt_notes, head);
This list code looks strange.
Won't need that after making the required changes.
+
+ /* Iterate through the notes and retrieve the required note */
+ list_for_each(pos, sdt_notes) {
+ tmp = list_entry(pos, struct sdt_note, note_list);
+ if (!strcmp(key->name, tmp->name) &&
+ !strcmp(key->provider, tmp->provider)) {
+ adjust_note_addr(tmp, key, elf);
+ ret = 0;
+ break;
+ }
+ }
+ if (ret)
+ printf("%%%s:%s not found!\n", key->provider, key->name);
+
+out_end:
+ 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 037185c..0b0545b 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -260,9 +260,12 @@ void __map_groups__fixup_end(struct map_groups *mg, enum map_type type);
/* 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 NOTE_SCN ".note.stapsdt"
#define SDT_NOTE_NAME "stapsdt"
+#define SDT_BASE ".stapsdt.base"
What about SDT_BASE_SCN ?
Seems better.
--
Thanks
Hemant
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/