(2013/10/23 14:05), 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 <hks...@linux.vnet.ibm.com>
Almost! please check below comments :)
> static int convert_to_probe_trace_events(struct perf_probe_event *pev,
> struct probe_trace_event **tevs,
> int max_tevs, const char *target)
> @@ -1916,11 +1975,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;
These "ret != 0" checkers can be merged.
[...]
> +int search_sdt_note(struct sdt_note *key, const char *target)
> +{
> + Elf *elf;
> + int fd, ret;
> + bool found = false;
> + struct sdt_note *pos = NULL;
> + LIST_HEAD(sdt_notes);
> +
> + 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;
Here, you skipped the closing process. maybe ret = -ENOENT is enough here.
> + }
> +
> +out_end:
> + elf_end(elf);
> +out_close:
> + close(fd);
> + ret = sdt_err(ret, target);
It seems the sdt_err is only for the return value of contruct_sdt_notes_list(),
thus it is better to integrate it.
> + if (!ret && list_empty(&sdt_notes))
> + ret = -ENOENT;
I think this can be removed, because it always be false (caught by previous
!found).
> + cleanup_sdt_note_list(&sdt_notes);
> + return ret;
> +}
Thank you,
--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
