Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1387248 > unrolled thread
| Started by | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| First post | 2016-04-26 11:10 +0200 |
| Last post | 2016-04-27 17:40 +0200 |
| Articles | 20 on this page of 40 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH perf/core v4 00/19] perf-probe --cache and SDT support Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:10 +0200
[PATCH perf/core v4 11/19] perf/sdt: ELF support for SDT Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:10 +0200
[PATCH perf/core v4 03/19] perf buildid-cache: Fall back to the old style build-id cache Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:10 +0200
Re: [PATCH perf/core v4 03/19] perf buildid-cache: Fall back to the old style build-id cache Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-04-26 15:50 +0200
Re: [PATCH perf/core v4 03/19] perf buildid-cache: Fall back to the old style build-id cache Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 16:50 +0200
[PATCH perf/core v4 16/19] perf-list: Show SDT and pre-cached events Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:10 +0200
[PATCH perf/core v4 01/19] perf probe: Use strbuf for making strings Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:10 +0200
Re: [PATCH perf/core v4 01/19] perf probe: Use strbuf for making strings Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-04-26 15:40 +0200
Re: [PATCH perf/core v4 01/19] perf probe: Use strbuf for making strings Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 16:50 +0200
Re: [PATCH perf/core v4 01/19] perf probe: Use strbuf for making strings Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-04-26 17:10 +0200
Re: [PATCH perf/core v4 01/19] perf probe: Use strbuf for making strings Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-27 20:50 +0200
[PATCH perf/core v4 04/19] perf: Add lsdir to read a directory Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:10 +0200
Re: [PATCH perf/core v4 04/19] perf: Add lsdir to read a directory Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-04-26 15:50 +0200
Re: [PATCH perf/core v4 04/19] perf: Add lsdir to read a directory Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-04-26 16:10 +0200
Re: [PATCH perf/core v4 04/19] perf: Add lsdir to read a directory Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 17:00 +0200
Re: [PATCH perf/core v4 04/19] perf: Add lsdir to read a directory Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-04-26 17:10 +0200
[tip:perf/core] perf tools: Add lsdir() helper to read a directory tip-bot for Masami Hiramatsu <tipbot@zytor.com> - 2016-04-27 17:40 +0200
[PATCH perf/core v4 15/19] perf probe: Accept %sdt and %cached event name Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:10 +0200
[PATCH perf/core v4 19/19] perf probe: Support @BUILDID or @FILE suffix for SDT events Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:10 +0200
[PATCH perf/core v4 18/19] perf probe: Allow wildcard for cached events Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:10 +0200
Re: [PATCH perf/core v4 18/19] perf probe: Allow wildcard for cached events Hemant Kumar <hemant@linux.vnet.ibm.com> - 2016-04-27 17:40 +0200
Re: [PATCH perf/core v4 18/19] perf probe: Allow wildcard for cached events Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-27 21:00 +0200
[PATCH perf/core v4 05/19] perf-buildid-cache: Use lsdir for looking up buildid caches Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:10 +0200
[PATCH perf/core v4 07/19] perf probe: Add --cache option to cache the probe definitions Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:10 +0200
[PATCH perf/core v4 10/19] perf probe: Remove caches when --cache is given Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:10 +0200
[PATCH perf/core v4 14/19] perf buildid-cache: Scan and import user SDT events to probe cache Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:10 +0200
Re: [PATCH perf/core v4 14/19] perf buildid-cache: Scan and import user SDT events to probe cache Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-04-27 17:30 +0200
Re: [PATCH perf/core v4 14/19] perf buildid-cache: Scan and import user SDT events to probe cache Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-27 21:40 +0200
Re: [PATCH perf/core v4 14/19] perf buildid-cache: Scan and import user SDT events to probe cache Hemant Kumar <hemant@linux.vnet.ibm.com> - 2016-04-27 22:30 +0200
Re: [PATCH perf/core v4 14/19] perf buildid-cache: Scan and import user SDT events to probe cache Hemant Kumar <hemant@linux.vnet.ibm.com> - 2016-04-27 22:20 +0200
Re: [PATCH perf/core v4 14/19] perf buildid-cache: Scan and import user SDT events to probe cache Hemant Kumar <hemant@linux.vnet.ibm.com> - 2016-04-27 17:30 +0200
[PATCH perf/core v4 02/19] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:10 +0200
Re: [PATCH perf/core v4 02/19] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-04-26 15:50 +0200
Re: [PATCH perf/core v4 02/19] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 16:50 +0200
[PATCH perf/core v4 06/19] perf-probe: Let probe_file__add_event return 0 if succeeded Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:10 +0200
Re: [PATCH perf/core v4 06/19] perf-probe: Let probe_file__add_event return 0 if succeeded Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-04-26 15:50 +0200
[tip:perf/core] perf probe: Let probe_file__add_event return 0 if succeeded tip-bot for Masami Hiramatsu <tipbot@zytor.com> - 2016-04-27 17:40 +0200
[PATCH perf/core v4 09/19] perf probe: Show all cached probes Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:20 +0200
[PATCH perf/core v4 08/19] perf probe: Use cache entry if possible Masami Hiramatsu <mhiramat@kernel.org> - 2016-04-26 11:30 +0200
Re: [PATCH perf/core v4 00/19] perf-probe --cache and SDT support Hemant Kumar <hemant@linux.vnet.ibm.com> - 2016-04-27 17:40 +0200
Page 1 of 2 [1] 2 Next page →
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2016-04-26 11:10 +0200 |
| Subject | [PATCH perf/core v4 00/19] perf-probe --cache and SDT support |
| Message-ID | <rs7ai-4SP-5@gated-at.bofh.it> |
Hi,
Here is the 4th version of the patchset for probe-cache and
initial SDT support which are going to be perf-cache finally.
The previous version is here; https://lkml.org/lkml/2015/8/15/52
This version fixes some bugs and improve wildcard support (
including automatic target binary selection, we don't need
-x option for SDT/pre-cached events any more :) ),
and drops perf-record support currently because it seems more
work on cleanup side.
BTW, since I couldn't find list_dir function in Arnaldo's mail,
so this series still have a patch to add lsdir().
Note that all SDT events have "sdt_" prefix on the provider name,
because of easier to distinguish between other named caches.
TODOs:
- (perf record) Support SDT event recording directly
- (perf record) Support @FILE/@BUILDID suffix to record specific SDTs.
- (perf record) Try to unregister SDT events after record.
- (ftrace) Support multiple SDTs on single event.
Done:
- (perf probe) Allow glob matching for pre-cached events.
- (perf probe) Removes -x option when using pre-cached events.
- (perf probe) Support @FILE/@BUILDID suffix for pre-cached events.
- (perf buildid-cache) Fallback to old style buildid-cache.
Thank you,
---
Hemant Kumar (1):
perf/sdt: ELF support for SDT
Masami Hiramatsu (18):
perf probe: Use strbuf for making strings
perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid
perf buildid-cache: Fall back to the old style build-id cache
perf: Add lsdir to read a directory
perf-buildid-cache: Use lsdir for looking up buildid caches
perf-probe: Let probe_file__add_event return 0 if succeeded
perf probe: Add --cache option to cache the probe definitions
perf probe: Use cache entry if possible
perf probe: Show all cached probes
perf probe: Remove caches when --cache is given
perf probe: Add group name support
perf-probe: Set default kprobe group name if it is not given
perf buildid-cache: Scan and import user SDT events to probe cache
perf probe: Accept %sdt and %cached event name
perf-list: Show SDT and pre-cached events
perf-list: Skip SDTs placed in invalid binaries
perf probe: Allow wildcard for cached events
perf probe: Support @BUILDID or @FILE suffix for SDT events
tools/perf/Documentation/perf-probe.txt | 24 +
tools/perf/builtin-list.c | 4
tools/perf/builtin-probe.c | 30 +
tools/perf/util/build-id.c | 278 ++++++++++--
tools/perf/util/build-id.h | 6
tools/perf/util/dso.h | 5
tools/perf/util/parse-events.c | 83 ++++
tools/perf/util/parse-events.h | 2
tools/perf/util/probe-event.c | 692 ++++++++++++++++++++++---------
tools/perf/util/probe-event.h | 9
tools/perf/util/probe-file.c | 495 ++++++++++++++++++++++
tools/perf/util/probe-file.h | 32 +
tools/perf/util/probe-finder.c | 14 -
tools/perf/util/symbol-elf.c | 252 +++++++++++
tools/perf/util/symbol.c | 15 -
tools/perf/util/symbol.h | 22 +
tools/perf/util/util.c | 34 ++
tools/perf/util/util.h | 4
18 files changed, 1725 insertions(+), 276 deletions(-)
--
Masami Hiramatsu
[toc] | [next] | [standalone]
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2016-04-26 11:10 +0200 |
| Subject | [PATCH perf/core v4 11/19] perf/sdt: ELF support for SDT |
| Message-ID | <rs7aj-4SP-23@gated-at.bofh.it> |
| In reply to | #1387248 |
From: Hemant Kumar <hemant@linux.vnet.ibm.com>
This patch serves 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.
A typical note entry in ".note.stapsdt" section is as follows :
|--nhdr.n_namesz--|
------------------------------------
| nhdr | "stapsdt" |
----- |----------------------------------|
| | <location> <base_address> |
| | <semaphore> |
nhdr.n_descsize | "provider_name" "note_name" |
| | <args> |
----- |----------------------------------|
| nhdr | "stapsdt" |
|...
The above shows an excerpt from the section ".note.stapsdt".
'nhdr' is a structure which has the note name size (n_namesz), note
description size (n_desc_sz) and note type (n_type). So, in order to
parse the note note info, we need nhdr to tell us where to start from.
As can be seen from <sys/sdt.h>, the name of the SDT notes given is "stapsdt".
But this is not the identifier of the note.
After that, we go to description of the note to find out its location, the
address of the ".stapsdt.base" section and the semaphore address.
Then, we find the provider name and the SDT marker name and then follow the
arguments.
Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/symbol-elf.c | 252 ++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/symbol.h | 22 ++++
2 files changed, 274 insertions(+)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 3f9d679..11aff8e 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1780,6 +1780,258 @@ void kcore_extract__delete(struct kcore_extract *kce)
unlink(kce->extract_filename);
}
+/**
+ * populate_sdt_note : Parse raw data and identify SDT note
+ * @elf: elf of the opened file
+ * @data: raw data of a section with description offset applied
+ * @len: note description size
+ * @type: type of the note
+ * @sdt_notes: List to add the SDT note
+ *
+ * Responsible for parsing the @data in section .note.stapsdt in @elf and
+ * if its an SDT note, it appends to @sdt_notes list.
+ */
+static int populate_sdt_note(Elf **elf, const char *data, size_t len,
+ struct list_head *sdt_notes)
+{
+ const char *provider, *name;
+ struct sdt_note *tmp = NULL;
+ GElf_Ehdr ehdr;
+ GElf_Addr base_off = 0;
+ GElf_Shdr shdr;
+ int ret = -EINVAL;
+
+ union {
+ Elf64_Addr a64[NR_ADDR];
+ Elf32_Addr a32[NR_ADDR];
+ } buf;
+
+ Elf_Data dst = {
+ .d_buf = &buf, .d_type = ELF_T_ADDR, .d_version = EV_CURRENT,
+ .d_size = gelf_fsize((*elf), ELF_T_ADDR, NR_ADDR, 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
+ };
+
+ 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) {
+ pr_err("gelf_xlatetom : %s\n", elf_errmsg(-1));
+ goto out_free_note;
+ }
+
+ /* 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;
+ }
+
+ if (gelf_getclass(*elf) == ELFCLASS32) {
+ memcpy(&tmp->addr, &buf, 3 * sizeof(Elf32_Addr));
+ tmp->bit32 = true;
+ } else {
+ memcpy(&tmp->addr, &buf, 3 * sizeof(Elf64_Addr));
+ tmp->bit32 = false;
+ }
+
+ if (!gelf_getehdr(*elf, &ehdr)) {
+ pr_debug("%s : cannot get elf header.\n", __func__);
+ ret = -EBADF;
+ goto out_free_name;
+ }
+
+ /* Adjust the prelink effect :
+ * 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];
+ }
+ }
+
+ list_add_tail(&tmp->note_list, sdt_notes);
+ 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 : constructs a list of SDT notes
+ * @elf : elf to look into
+ * @sdt_notes : empty list_head
+ *
+ * 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;
+ int ret = 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))) {
+ /* Check the type of the note */
+ if (nhdr.n_type != SDT_NOTE_TYPE)
+ goto out_ret;
+
+ ret = populate_sdt_note(&elf, ((data->d_buf) + desc_off),
+ nhdr.n_descsz, sdt_notes);
+ if (ret < 0)
+ goto out_ret;
+ }
+ }
+ if (list_empty(sdt_notes))
+ ret = -ENOENT;
+
+out_ret:
+ return ret;
+}
+
+/**
+ * get_sdt_note_list : Wrapper to construct a list of sdt notes
+ * @head : empty list_head
+ * @target : file to find SDT notes from
+ *
+ * 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;
+
+ elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
+ if (!elf) {
+ ret = -EBADF;
+ goto out_close;
+ }
+ ret = construct_sdt_notes_list(elf, head);
+ elf_end(elf);
+out_close:
+ close(fd);
+ return ret;
+}
+
+/**
+ * cleanup_sdt_note_list : free the sdt notes' list
+ * @sdt_notes: sdt notes' list
+ *
+ * Free up the SDT notes in @sdt_notes.
+ * Returns the number of SDT notes free'd.
+ */
+int cleanup_sdt_note_list(struct list_head *sdt_notes)
+{
+ struct sdt_note *tmp, *pos;
+ int nr_free = 0;
+
+ list_for_each_entry_safe(pos, tmp, sdt_notes, note_list) {
+ list_del(&pos->note_list);
+ free(pos->name);
+ free(pos->provider);
+ free(pos);
+ nr_free++;
+ }
+ return nr_free;
+}
+
+/**
+ * sdt_notes__get_count: Counts the number of sdt events
+ * @start: list_head to sdt_notes list
+ *
+ * Returns the number of SDT notes in a list
+ */
+int sdt_notes__get_count(struct list_head *start)
+{
+ struct sdt_note *sdt_ptr;
+ int count = 0;
+
+ list_for_each_entry(sdt_ptr, start, note_list)
+ count++;
+ return count;
+}
+
void symbol__elf_init(void)
{
elf_version(EV_CURRENT);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index c8e4397..6fae31c 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -331,4 +331,26 @@ void arch__elf_sym_adjust(GElf_Sym *sym);
int arch__choose_best_symbol(struct symbol *syma, struct symbol *symb);
+/* structure containing an SDT note's info */
+struct sdt_note {
+ char *name; /* name of the note*/
+ char *provider; /* provider name */
+ bool bit32; /* whether the location is 32 bits? */
+ union { /* location, base and semaphore addrs */
+ Elf64_Addr a64[3];
+ Elf32_Addr a32[3];
+ } addr;
+ struct list_head note_list; /* SDT notes' list */
+};
+
+int get_sdt_note_list(struct list_head *head, const char *target);
+int cleanup_sdt_note_list(struct list_head *sdt_notes);
+int sdt_notes__get_count(struct list_head *start);
+
+#define SDT_BASE_SCN ".stapsdt.base"
+#define SDT_NOTE_SCN ".note.stapsdt"
+#define SDT_NOTE_TYPE 3
+#define SDT_NOTE_NAME "stapsdt"
+#define NR_ADDR 3
+
#endif /* __PERF_SYMBOL */
[toc] | [prev] | [next] | [standalone]
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2016-04-26 11:10 +0200 |
| Subject | [PATCH perf/core v4 03/19] perf buildid-cache: Fall back to the old style build-id cache |
| Message-ID | <rs7aj-4SP-27@gated-at.bofh.it> |
| In reply to | #1387248 |
Fall back to the old style build-id cache
(~/.debug/<path-to-bin>/<build-id>) if possible.
Because there is old build-id cache under ~/.debug, perf
has to check if the old entry is there instead of returning
an error. Or, user have to remove ~/.debug completely.
This patch introduces fallback code to try using old-style
buildid caches. To update the cached entry, you just need to
add a cache. It automatically removes old cache entry and
make a new one in new style
(~/.debug/<path-to-bin>/<build-id>/[elf|vdso|kallsyms]).
IOW, without this patch, "perf buildid-cache --add" always
gets errors when adding existing binaries.
----
# perf buildid-cache -a /usr/lib64/libc-2.20.so,/usr/bin/gcc
Couldn't add /usr/bin/gcc: Not a directory
Couldn't add /usr/lib64/libc-2.20.so: Not a directory
----
Moreover, --update option removes only symlink, thus the
build-id cache is corrupted.
This fixes above issues.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
tools/perf/util/build-id.c | 17 ++++++++++++++++-
tools/perf/util/symbol.c | 13 +++++++++----
2 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 46a8bcc..b035483 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -155,6 +155,12 @@ static char *build_id_cache__linkname(const char *sbuild_id, char *bf,
return bf;
}
+static bool __is_regular_file(const char *pathname)
+{
+ struct stat sb;
+ return stat(pathname, &sb) == 0 && S_ISREG(sb.st_mode);
+}
+
static const char *build_id_cache__basename(bool is_kallsyms, bool is_vdso)
{
return is_kallsyms ? "kallsyms" : (is_vdso ? "vdso" : "elf");
@@ -177,7 +183,11 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size)
if (!linkname)
return NULL;
- ret = asnprintf(&bf, size, "%s/%s", linkname,
+ /* Check if old style build_id cache */
+ if (__is_regular_file(linkname))
+ ret = asnprintf(&bf, size, "%s", linkname);
+ else
+ ret = asnprintf(&bf, size, "%s/%s", linkname,
build_id_cache__basename(is_kallsyms, is_vdso));
if (ret < 0 || (!alloc && size < (unsigned int)ret))
bf = NULL;
@@ -445,6 +455,11 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
if (!dir_name)
goto out_free;
+ /* Remove old style build-id cache */
+ if (__is_regular_file(dir_name))
+ if (unlink(dir_name))
+ goto out_free;
+
if (mkdir_p(dir_name, 0755))
goto out_free;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index c57cb47..9463c7d 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1687,11 +1687,16 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
scnprintf(path, sizeof(path), "%s/[kernel.kallsyms]/%s/kallsyms",
buildid_dir, sbuild_id);
-
+ /* Try old style kallsyms cache */
if (access(path, F_OK)) {
- pr_err("No kallsyms or vmlinux with build-id %s was found\n",
- sbuild_id);
- return NULL;
+ scnprintf(path, sizeof(path), "%s/[kernel.kallsyms]/%s",
+ buildid_dir, sbuild_id);
+
+ if (access(path, F_OK)) {
+ pr_err("No kallsyms or vmlinux with build-id %s was found\n",
+ sbuild_id);
+ return NULL;
+ }
}
return strdup(path);
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-04-26 15:50 +0200 |
| Subject | Re: [PATCH perf/core v4 03/19] perf buildid-cache: Fall back to the old style build-id cache |
| Message-ID | <rsbxg-eJ-9@gated-at.bofh.it> |
| In reply to | #1387251 |
Em Tue, Apr 26, 2016 at 06:02:33PM +0900, Masami Hiramatsu escreveu:
> Fall back to the old style build-id cache
> (~/.debug/<path-to-bin>/<build-id>) if possible.
> Because there is old build-id cache under ~/.debug, perf
> has to check if the old entry is there instead of returning
> an error. Or, user have to remove ~/.debug completely.
So, from the description this seems to address the issues I mentioned in
the previous patch.
Probably it is a good idea not to introduce the problem in the first
place? I.e. combine this and the previous patch, so that we don't have
this bisection break?
- Arnaldo
> This patch introduces fallback code to try using old-style
> buildid caches. To update the cached entry, you just need to
> add a cache. It automatically removes old cache entry and
> make a new one in new style
> (~/.debug/<path-to-bin>/<build-id>/[elf|vdso|kallsyms]).
>
> IOW, without this patch, "perf buildid-cache --add" always
> gets errors when adding existing binaries.
> ----
> # perf buildid-cache -a /usr/lib64/libc-2.20.so,/usr/bin/gcc
> Couldn't add /usr/bin/gcc: Not a directory
> Couldn't add /usr/lib64/libc-2.20.so: Not a directory
> ----
> Moreover, --update option removes only symlink, thus the
> build-id cache is corrupted.
>
> This fixes above issues.
>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
> tools/perf/util/build-id.c | 17 ++++++++++++++++-
> tools/perf/util/symbol.c | 13 +++++++++----
> 2 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 46a8bcc..b035483 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -155,6 +155,12 @@ static char *build_id_cache__linkname(const char *sbuild_id, char *bf,
> return bf;
> }
>
> +static bool __is_regular_file(const char *pathname)
> +{
> + struct stat sb;
> + return stat(pathname, &sb) == 0 && S_ISREG(sb.st_mode);
> +}
> +
> static const char *build_id_cache__basename(bool is_kallsyms, bool is_vdso)
> {
> return is_kallsyms ? "kallsyms" : (is_vdso ? "vdso" : "elf");
> @@ -177,7 +183,11 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size)
> if (!linkname)
> return NULL;
>
> - ret = asnprintf(&bf, size, "%s/%s", linkname,
> + /* Check if old style build_id cache */
> + if (__is_regular_file(linkname))
> + ret = asnprintf(&bf, size, "%s", linkname);
> + else
> + ret = asnprintf(&bf, size, "%s/%s", linkname,
> build_id_cache__basename(is_kallsyms, is_vdso));
> if (ret < 0 || (!alloc && size < (unsigned int)ret))
> bf = NULL;
> @@ -445,6 +455,11 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
> if (!dir_name)
> goto out_free;
>
> + /* Remove old style build-id cache */
> + if (__is_regular_file(dir_name))
> + if (unlink(dir_name))
> + goto out_free;
> +
> if (mkdir_p(dir_name, 0755))
> goto out_free;
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index c57cb47..9463c7d 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1687,11 +1687,16 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
>
> scnprintf(path, sizeof(path), "%s/[kernel.kallsyms]/%s/kallsyms",
> buildid_dir, sbuild_id);
> -
> + /* Try old style kallsyms cache */
> if (access(path, F_OK)) {
> - pr_err("No kallsyms or vmlinux with build-id %s was found\n",
> - sbuild_id);
> - return NULL;
> + scnprintf(path, sizeof(path), "%s/[kernel.kallsyms]/%s",
> + buildid_dir, sbuild_id);
> +
> + if (access(path, F_OK)) {
> + pr_err("No kallsyms or vmlinux with build-id %s was found\n",
> + sbuild_id);
> + return NULL;
> + }
> }
>
> return strdup(path);
[toc] | [prev] | [next] | [standalone]
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2016-04-26 16:50 +0200 |
| Subject | Re: [PATCH perf/core v4 03/19] perf buildid-cache: Fall back to the old style build-id cache |
| Message-ID | <rsctl-Xx-41@gated-at.bofh.it> |
| In reply to | #1387487 |
On Tue, 26 Apr 2016 10:47:49 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> Em Tue, Apr 26, 2016 at 06:02:33PM +0900, Masami Hiramatsu escreveu:
> > Fall back to the old style build-id cache
> > (~/.debug/<path-to-bin>/<build-id>) if possible.
> > Because there is old build-id cache under ~/.debug, perf
> > has to check if the old entry is there instead of returning
> > an error. Or, user have to remove ~/.debug completely.
>
> So, from the description this seems to address the issues I mentioned in
> the previous patch.
yes :)
>
> Probably it is a good idea not to introduce the problem in the first
> place? I.e. combine this and the previous patch, so that we don't have
> this bisection break?
Yeah, I just try to keep original clean for review.
I'll combine this with 02/19.
Thanks!
>
> - Arnaldo
>
> > This patch introduces fallback code to try using old-style
> > buildid caches. To update the cached entry, you just need to
> > add a cache. It automatically removes old cache entry and
> > make a new one in new style
> > (~/.debug/<path-to-bin>/<build-id>/[elf|vdso|kallsyms]).
> >
> > IOW, without this patch, "perf buildid-cache --add" always
> > gets errors when adding existing binaries.
> > ----
> > # perf buildid-cache -a /usr/lib64/libc-2.20.so,/usr/bin/gcc
> > Couldn't add /usr/bin/gcc: Not a directory
> > Couldn't add /usr/lib64/libc-2.20.so: Not a directory
> > ----
> > Moreover, --update option removes only symlink, thus the
> > build-id cache is corrupted.
> >
> > This fixes above issues.
> >
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> > tools/perf/util/build-id.c | 17 ++++++++++++++++-
> > tools/perf/util/symbol.c | 13 +++++++++----
> > 2 files changed, 25 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> > index 46a8bcc..b035483 100644
> > --- a/tools/perf/util/build-id.c
> > +++ b/tools/perf/util/build-id.c
> > @@ -155,6 +155,12 @@ static char *build_id_cache__linkname(const char *sbuild_id, char *bf,
> > return bf;
> > }
> >
> > +static bool __is_regular_file(const char *pathname)
> > +{
> > + struct stat sb;
> > + return stat(pathname, &sb) == 0 && S_ISREG(sb.st_mode);
> > +}
> > +
> > static const char *build_id_cache__basename(bool is_kallsyms, bool is_vdso)
> > {
> > return is_kallsyms ? "kallsyms" : (is_vdso ? "vdso" : "elf");
> > @@ -177,7 +183,11 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size)
> > if (!linkname)
> > return NULL;
> >
> > - ret = asnprintf(&bf, size, "%s/%s", linkname,
> > + /* Check if old style build_id cache */
> > + if (__is_regular_file(linkname))
> > + ret = asnprintf(&bf, size, "%s", linkname);
> > + else
> > + ret = asnprintf(&bf, size, "%s/%s", linkname,
> > build_id_cache__basename(is_kallsyms, is_vdso));
> > if (ret < 0 || (!alloc && size < (unsigned int)ret))
> > bf = NULL;
> > @@ -445,6 +455,11 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
> > if (!dir_name)
> > goto out_free;
> >
> > + /* Remove old style build-id cache */
> > + if (__is_regular_file(dir_name))
> > + if (unlink(dir_name))
> > + goto out_free;
> > +
> > if (mkdir_p(dir_name, 0755))
> > goto out_free;
> >
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index c57cb47..9463c7d 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -1687,11 +1687,16 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
> >
> > scnprintf(path, sizeof(path), "%s/[kernel.kallsyms]/%s/kallsyms",
> > buildid_dir, sbuild_id);
> > -
> > + /* Try old style kallsyms cache */
> > if (access(path, F_OK)) {
> > - pr_err("No kallsyms or vmlinux with build-id %s was found\n",
> > - sbuild_id);
> > - return NULL;
> > + scnprintf(path, sizeof(path), "%s/[kernel.kallsyms]/%s",
> > + buildid_dir, sbuild_id);
> > +
> > + if (access(path, F_OK)) {
> > + pr_err("No kallsyms or vmlinux with build-id %s was found\n",
> > + sbuild_id);
> > + return NULL;
> > + }
> > }
> >
> > return strdup(path);
--
Masami Hiramatsu <mhiramat@kernel.org>
[toc] | [prev] | [next] | [standalone]
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2016-04-26 11:10 +0200 |
| Subject | [PATCH perf/core v4 16/19] perf-list: Show SDT and pre-cached events |
| Message-ID | <rs7aj-4SP-29@gated-at.bofh.it> |
| In reply to | #1387248 |
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Show SDT and pre-cached events by perf-list with "sdt". This also
shows the binary and build-id where the events are placed only
when there are same name events on different binaries.
e.g.
----
# perf list sdt
List of pre-defined events (to be used in -e):
sdt_libc:lll_futex_wake [SDT event]
sdt_libc:lll_lock_wait_private [SDT event]
sdt_libc:longjmp [SDT event]
sdt_libc:longjmp_target [SDT event]
...
sdt_libstdcxx:rethrow@/usr/bin/gcc(0cc207fc4b27) [SDT event]
sdt_libstdcxx:rethrow@/usr/lib64/libstdc++.so.6.0.20(91c7a88fdf49)
sdt_libstdcxx:throw@/usr/bin/gcc(0cc207fc4b27) [SDT event]
sdt_libstdcxx:throw@/usr/lib64/libstdc++.so.6.0.20(91c7a88fdf49)
----
The binary path and build-id are shown in below format;
<GROUP>:<EVENT>@<PATH>(<BUILD-ID>)
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
Changes in v4:
- Update patch description.
- Change event list format.
---
tools/perf/builtin-list.c | 4 ++
tools/perf/util/parse-events.c | 83 ++++++++++++++++++++++++++++++++++++++++
tools/perf/util/parse-events.h | 2 +
3 files changed, 89 insertions(+)
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 5e22db4..3cba865 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -62,6 +62,8 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
print_hwcache_events(NULL, raw_dump);
else if (strcmp(argv[i], "pmu") == 0)
print_pmu_events(NULL, raw_dump);
+ else if (strcmp(argv[i], "sdt") == 0)
+ print_sdt_events(NULL, NULL, raw_dump);
else if ((sep = strchr(argv[i], ':')) != NULL) {
int sep_idx;
@@ -76,6 +78,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
s[sep_idx] = '\0';
print_tracepoint_events(s, s + sep_idx + 1, raw_dump);
+ print_sdt_events(s, s + sep_idx + 1, raw_dump);
free(s);
} else {
if (asprintf(&s, "*%s*", argv[i]) < 0) {
@@ -89,6 +92,7 @@ int cmd_list(int argc, const char **argv, const char *prefix __maybe_unused)
print_hwcache_events(s, raw_dump);
print_pmu_events(s, raw_dump);
print_tracepoint_events(NULL, s, raw_dump);
+ print_sdt_events(NULL, s, raw_dump);
free(s);
}
}
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index bcbc983..f9c8b7b 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -20,6 +20,7 @@
#include "pmu.h"
#include "thread_map.h"
#include "cpumap.h"
+#include "probe-file.h"
#include "asm/bug.h"
#define MAX_NAME_LEN 100
@@ -1976,6 +1977,86 @@ static bool is_event_supported(u8 type, unsigned config)
return ret;
}
+void print_sdt_events(const char *subsys_glob, const char *event_glob,
+ bool name_only)
+{
+ struct probe_cache *pcache;
+ struct probe_cache_entry *ent;
+ struct strlist *bidlist, *sdtlist;
+ struct strlist_config cfg = {.dont_dupstr = true};
+ struct str_node *nd, *nd2;
+ char *buf, *path, *ptr = NULL;
+ bool show_detail = false;
+ int ret;
+
+ sdtlist = strlist__new(NULL, &cfg);
+ if (!sdtlist) {
+ pr_debug("Failed to allocate new strlist for SDT\n");
+ return;
+ }
+ ret = build_id_cache__list_all(&bidlist);
+ if (ret < 0) {
+ pr_debug("Failed to get buildids: %d\n", ret);
+ return;
+ }
+ strlist__for_each(nd, bidlist) {
+ pcache = probe_cache__new(nd->s);
+ if (!pcache)
+ continue;
+ if (!list_empty(&pcache->list))
+ list_for_each_entry(ent, &pcache->list, list) {
+ if (!ent->sdt)
+ continue;
+ if (subsys_glob &&
+ !strglobmatch(ent->pev.group, subsys_glob))
+ continue;
+ if (event_glob &&
+ !strglobmatch(ent->pev.event, event_glob))
+ continue;
+ ret = asprintf(&buf, "%s:%s@%s", ent->pev.group,
+ ent->pev.event, nd->s);
+ if (ret > 0)
+ strlist__add(sdtlist, buf);
+ }
+ probe_cache__delete(pcache);
+ }
+ strlist__delete(bidlist);
+
+ strlist__for_each(nd, sdtlist) {
+ buf = strchr(nd->s, '@');
+ if (buf)
+ *(buf++) = '\0';
+ if (name_only) {
+ printf("%s ", nd->s);
+ continue;
+ }
+ nd2 = strlist__next(nd);
+ if (nd2) {
+ ptr = strchr(nd2->s, '@');
+ if (ptr)
+ *ptr = '\0';
+ if (strcmp(nd->s, nd2->s) == 0)
+ show_detail = true;
+ }
+ if (show_detail) {
+ path = build_id_cache__origname(buf);
+ ret = asprintf(&buf, "%s@%s(%.12s)", nd->s, path, buf);
+ if (ret > 0) {
+ printf(" %-50s [%s]\n", buf, "SDT event");
+ free(buf);
+ }
+ } else
+ printf(" %-50s [%s]\n", nd->s, "SDT event");
+ if (nd2) {
+ if (strcmp(nd->s, nd2->s) != 0)
+ show_detail = false;
+ if (ptr)
+ *ptr = '@';
+ }
+ }
+ strlist__delete(sdtlist);
+}
+
int print_hwcache_events(const char *event_glob, bool name_only)
{
unsigned int type, op, i, evt_i = 0, evt_num = 0;
@@ -2158,6 +2239,8 @@ void print_events(const char *event_glob, bool name_only)
}
print_tracepoint_events(NULL, NULL, name_only);
+
+ print_sdt_events(NULL, NULL, name_only);
}
int parse_events__is_hardcoded_term(struct parse_events_term *term)
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index d740c3c..c08daa9 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -182,6 +182,8 @@ void print_symbol_events(const char *event_glob, unsigned type,
void print_tracepoint_events(const char *subsys_glob, const char *event_glob,
bool name_only);
int print_hwcache_events(const char *event_glob, bool name_only);
+void print_sdt_events(const char *subsys_glob, const char *event_glob,
+ bool name_only);
int is_valid_tracepoint(const char *event_string);
int valid_event_mount(const char *eventfs);
[toc] | [prev] | [next] | [standalone]
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2016-04-26 11:10 +0200 |
| Subject | [PATCH perf/core v4 01/19] perf probe: Use strbuf for making strings |
| Message-ID | <rs7aj-4SP-33@gated-at.bofh.it> |
| In reply to | #1387248 |
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Replace many fixed-length char array with strbuf to
stringify perf_probe_event and probe_trace_event etc.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
Changes in v4:
- Update for the latest kernel.
- Remove "probe-event.c" from title and body, because
this also changes probe-finder.c too :)
Changes in v3:
- Remove unneeded strbuf_release(). (Thanks Namhyung!)
Changes in v2:
- Make perf_probe_event__sprintf() simpler.
---
tools/perf/util/probe-event.c | 246 ++++++++++++++--------------------------
tools/perf/util/probe-event.h | 2
tools/perf/util/probe-finder.c | 14 +-
3 files changed, 93 insertions(+), 169 deletions(-)
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 8319fbb..148af81 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1618,69 +1618,51 @@ out:
}
/* Compose only probe arg */
-int synthesize_perf_probe_arg(struct perf_probe_arg *pa, char *buf, size_t len)
+char *synthesize_perf_probe_arg(struct perf_probe_arg *pa)
{
struct perf_probe_arg_field *field = pa->field;
- int ret;
- char *tmp = buf;
+ struct strbuf buf;
+ char *ret;
+ strbuf_init(&buf, 64);
if (pa->name && pa->var)
- ret = e_snprintf(tmp, len, "%s=%s", pa->name, pa->var);
+ strbuf_addf(&buf, "%s=%s", pa->name, pa->var);
else
- ret = e_snprintf(tmp, len, "%s", pa->name ? pa->name : pa->var);
- if (ret <= 0)
- goto error;
- tmp += ret;
- len -= ret;
+ strbuf_addstr(&buf, pa->name ?: pa->var);
while (field) {
if (field->name[0] == '[')
- ret = e_snprintf(tmp, len, "%s", field->name);
+ strbuf_addstr(&buf, field->name);
else
- ret = e_snprintf(tmp, len, "%s%s",
- field->ref ? "->" : ".", field->name);
- if (ret <= 0)
- goto error;
- tmp += ret;
- len -= ret;
+ strbuf_addf(&buf, "%s%s", field->ref ? "->" : ".",
+ field->name);
field = field->next;
}
- if (pa->type) {
- ret = e_snprintf(tmp, len, ":%s", pa->type);
- if (ret <= 0)
- goto error;
- tmp += ret;
- len -= ret;
- }
+ if (pa->type)
+ strbuf_addf(&buf, ":%s", pa->type);
+
+ ret = strbuf_detach(&buf, NULL);
- return tmp - buf;
-error:
- pr_debug("Failed to synthesize perf probe argument: %d\n", ret);
return ret;
}
/* Compose only probe point (not argument) */
static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
{
- char *buf, *tmp;
- char offs[32] = "", line[32] = "", file[32] = "";
- int ret, len;
-
- buf = zalloc(MAX_CMDLEN);
- if (buf == NULL) {
- ret = -ENOMEM;
- goto error;
- }
- if (pp->offset) {
- ret = e_snprintf(offs, 32, "+%lu", pp->offset);
- if (ret <= 0)
- goto error;
- }
- if (pp->line) {
- ret = e_snprintf(line, 32, ":%d", pp->line);
- if (ret <= 0)
- goto error;
+ struct strbuf buf;
+ char *tmp;
+ int len;
+
+ strbuf_init(&buf, 64);
+ if (pp->function) {
+ strbuf_addstr(&buf, pp->function);
+ if (pp->offset)
+ strbuf_addf(&buf, "+%lu", pp->offset);
+ else if (pp->line)
+ strbuf_addf(&buf, ":%d", pp->line);
+ else if (pp->retprobe)
+ strbuf_addstr(&buf, "%return");
}
if (pp->file) {
tmp = pp->file;
@@ -1689,25 +1671,12 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
tmp = strchr(pp->file + len - 30, '/');
tmp = tmp ? tmp + 1 : pp->file + len - 30;
}
- ret = e_snprintf(file, 32, "@%s", tmp);
- if (ret <= 0)
- goto error;
+ strbuf_addf(&buf, "@%s", tmp);
+ if (!pp->function && pp->line)
+ strbuf_addf(&buf, ":%d", pp->line);
}
- if (pp->function)
- ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
- offs, pp->retprobe ? "%return" : "", line,
- file);
- else
- ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
- if (ret <= 0)
- goto error;
-
- return buf;
-error:
- pr_debug("Failed to synthesize perf probe point: %d\n", ret);
- free(buf);
- return NULL;
+ return strbuf_detach(&buf, NULL);
}
#if 0
@@ -1736,45 +1705,30 @@ char *synthesize_perf_probe_command(struct perf_probe_event *pev)
#endif
static int __synthesize_probe_trace_arg_ref(struct probe_trace_arg_ref *ref,
- char **buf, size_t *buflen,
- int depth)
+ struct strbuf *buf, int depth)
{
- int ret;
if (ref->next) {
depth = __synthesize_probe_trace_arg_ref(ref->next, buf,
- buflen, depth + 1);
+ depth + 1);
if (depth < 0)
goto out;
}
-
- ret = e_snprintf(*buf, *buflen, "%+ld(", ref->offset);
- if (ret < 0)
- depth = ret;
- else {
- *buf += ret;
- *buflen -= ret;
- }
+ strbuf_addf(buf, "%+ld(", ref->offset);
out:
return depth;
-
}
static int synthesize_probe_trace_arg(struct probe_trace_arg *arg,
- char *buf, size_t buflen)
+ struct strbuf *buf)
{
struct probe_trace_arg_ref *ref = arg->ref;
- int ret, depth = 0;
- char *tmp = buf;
+ int depth = 0;
/* Argument name or separator */
if (arg->name)
- ret = e_snprintf(buf, buflen, " %s=", arg->name);
+ strbuf_addf(buf, " %s=", arg->name);
else
- ret = e_snprintf(buf, buflen, " ");
- if (ret < 0)
- return ret;
- buf += ret;
- buflen -= ret;
+ strbuf_addch(buf, ' ');
/* Special case: @XXX */
if (arg->value[0] == '@' && arg->ref)
@@ -1782,60 +1736,41 @@ static int synthesize_probe_trace_arg(struct probe_trace_arg *arg,
/* Dereferencing arguments */
if (ref) {
- depth = __synthesize_probe_trace_arg_ref(ref, &buf,
- &buflen, 1);
+ depth = __synthesize_probe_trace_arg_ref(ref, buf, 1);
if (depth < 0)
return depth;
}
/* Print argument value */
if (arg->value[0] == '@' && arg->ref)
- ret = e_snprintf(buf, buflen, "%s%+ld", arg->value,
- arg->ref->offset);
+ strbuf_addf(buf, "%s%+ld", arg->value, arg->ref->offset);
else
- ret = e_snprintf(buf, buflen, "%s", arg->value);
- if (ret < 0)
- return ret;
- buf += ret;
- buflen -= ret;
+ strbuf_addstr(buf, arg->value);
/* Closing */
- while (depth--) {
- ret = e_snprintf(buf, buflen, ")");
- if (ret < 0)
- return ret;
- buf += ret;
- buflen -= ret;
- }
+ while (depth--)
+ strbuf_addch(buf, ')');
/* Print argument type */
- if (arg->type) {
- ret = e_snprintf(buf, buflen, ":%s", arg->type);
- if (ret <= 0)
- return ret;
- buf += ret;
- }
+ if (arg->type)
+ strbuf_addf(buf, ":%s", arg->type);
- return buf - tmp;
+ return 0;
}
char *synthesize_probe_trace_command(struct probe_trace_event *tev)
{
struct probe_trace_point *tp = &tev->point;
- char *buf;
- int i, len, ret;
-
- buf = zalloc(MAX_CMDLEN);
- if (buf == NULL)
- return NULL;
-
- len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s ", tp->retprobe ? 'r' : 'p',
- tev->group, tev->event);
- if (len <= 0)
- goto error;
+ struct strbuf buf;
+ char *ret = NULL;
+ int i;
/* Uprobes must have tp->module */
if (tev->uprobes && !tp->module)
- goto error;
+ return NULL;
+
+ strbuf_init(&buf, 32);
+ strbuf_addf(&buf, "%c:%s/%s ", tp->retprobe ? 'r' : 'p',
+ tev->group, tev->event);
/*
* If tp->address == 0, then this point must be a
* absolute address uprobe.
@@ -1849,34 +1784,23 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
/* Use the tp->address for uprobes */
if (tev->uprobes)
- ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s:0x%lx",
- tp->module, tp->address);
+ strbuf_addf(&buf, "%s:0x%lx", tp->module, tp->address);
else if (!strncmp(tp->symbol, "0x", 2))
/* Absolute address. See try_to_find_absolute_address() */
- ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s0x%lx",
- tp->module ?: "", tp->module ? ":" : "",
- tp->address);
+ strbuf_addf(&buf, "%s%s0x%lx", tp->module ?: "",
+ tp->module ? ":" : "", tp->address);
else
- ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s%s+%lu",
- tp->module ?: "", tp->module ? ":" : "",
- tp->symbol, tp->offset);
-
- if (ret <= 0)
- goto error;
- len += ret;
+ strbuf_addf(&buf, "%s%s%s+%lu", tp->module ?: "",
+ tp->module ? ":" : "", tp->symbol, tp->offset);
- for (i = 0; i < tev->nargs; i++) {
- ret = synthesize_probe_trace_arg(&tev->args[i], buf + len,
- MAX_CMDLEN - len);
- if (ret <= 0)
+ for (i = 0; i < tev->nargs; i++)
+ if (synthesize_probe_trace_arg(&tev->args[i], &buf) < 0)
goto error;
- len += ret;
- }
- return buf;
+ ret = strbuf_detach(&buf, NULL);
error:
- free(buf);
- return NULL;
+ strbuf_release(&buf);
+ return ret;
}
static int find_perf_probe_point_from_map(struct probe_trace_point *tp,
@@ -1958,7 +1882,7 @@ static int convert_to_perf_probe_point(struct probe_trace_point *tp,
static int convert_to_perf_probe_event(struct probe_trace_event *tev,
struct perf_probe_event *pev, bool is_kprobe)
{
- char buf[64] = "";
+ struct strbuf buf = STRBUF_INIT;
int i, ret;
/* Convert event/group name */
@@ -1981,9 +1905,9 @@ static int convert_to_perf_probe_event(struct probe_trace_event *tev,
if (tev->args[i].name)
pev->args[i].name = strdup(tev->args[i].name);
else {
- ret = synthesize_probe_trace_arg(&tev->args[i],
- buf, 64);
- pev->args[i].name = strdup(buf);
+ strbuf_init(&buf, 32);
+ ret = synthesize_probe_trace_arg(&tev->args[i], &buf);
+ pev->args[i].name = strbuf_detach(&buf, NULL);
}
if (pev->args[i].name == NULL && ret >= 0)
ret = -ENOMEM;
@@ -2161,37 +2085,37 @@ static int perf_probe_event__sprintf(const char *group, const char *event,
const char *module,
struct strbuf *result)
{
- int i, ret;
- char buf[128];
- char *place;
+ int i;
+ char *buf;
- /* Synthesize only event probe point */
- place = synthesize_perf_probe_point(&pev->point);
- if (!place)
- return -EINVAL;
+ if (asprintf(&buf, "%s:%s", group, event) < 0)
+ return -errno;
+ strbuf_addf(result, " %-20s (on ", buf);
+ free(buf);
- ret = e_snprintf(buf, 128, "%s:%s", group, event);
- if (ret < 0)
- goto out;
+ /* Synthesize only event probe point */
+ buf = synthesize_perf_probe_point(&pev->point);
+ if (!buf)
+ return -ENOMEM;
+ strbuf_addstr(result, buf);
+ free(buf);
- strbuf_addf(result, " %-20s (on %s", buf, place);
if (module)
strbuf_addf(result, " in %s", module);
if (pev->nargs > 0) {
strbuf_add(result, " with", 5);
for (i = 0; i < pev->nargs; i++) {
- ret = synthesize_perf_probe_arg(&pev->args[i],
- buf, 128);
- if (ret < 0)
- goto out;
+ buf = synthesize_perf_probe_arg(&pev->args[i]);
+ if (!buf)
+ return -ENOMEM;
strbuf_addf(result, " %s", buf);
+ free(buf);
}
}
strbuf_addch(result, ')');
-out:
- free(place);
- return ret;
+
+ return 0;
}
/* Show an event */
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index e54e7b0..e220962 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -120,7 +120,7 @@ int parse_probe_trace_command(const char *cmd, struct probe_trace_event *tev);
/* Events to command string */
char *synthesize_perf_probe_command(struct perf_probe_event *pev);
char *synthesize_probe_trace_command(struct probe_trace_event *tev);
-int synthesize_perf_probe_arg(struct perf_probe_arg *pa, char *buf, size_t len);
+char *synthesize_perf_probe_arg(struct perf_probe_arg *pa);
/* Check the perf_probe_event needs debuginfo */
bool perf_probe_event_need_dwarf(struct perf_probe_event *pev);
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index b3bd0fb..9f68875 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -553,7 +553,7 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
{
Dwarf_Die vr_die;
- char buf[32], *ptr;
+ char *buf, *ptr;
int ret = 0;
/* Copy raw parameters */
@@ -563,13 +563,13 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
if (pf->pvar->name)
pf->tvar->name = strdup(pf->pvar->name);
else {
- ret = synthesize_perf_probe_arg(pf->pvar, buf, 32);
- if (ret < 0)
- return ret;
+ buf = synthesize_perf_probe_arg(pf->pvar);
+ if (!buf)
+ return -ENOMEM;
ptr = strchr(buf, ':'); /* Change type separator to _ */
if (ptr)
*ptr = '_';
- pf->tvar->name = strdup(buf);
+ pf->tvar->name = buf;
}
if (pf->tvar->name == NULL)
return -ENOMEM;
@@ -1334,8 +1334,8 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
if (ret2 == 0) {
strlist__add(vl->vars,
strbuf_detach(&buf, NULL));
- }
- strbuf_release(&buf);
+ } else
+ strbuf_release(&buf);
}
}
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-04-26 15:40 +0200 |
| Subject | Re: [PATCH perf/core v4 01/19] perf probe: Use strbuf for making strings |
| Message-ID | <rsbnB-9L-31@gated-at.bofh.it> |
| In reply to | #1387253 |
Em Tue, Apr 26, 2016 at 06:02:11PM +0900, Masami Hiramatsu escreveu:
> From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>
> Replace many fixed-length char array with strbuf to
> stringify perf_probe_event and probe_trace_event etc.
Sure you want to do that? From time to time I try to reduce the strbuf
usage, not grow it :-\
That is one of the last users of xrealloc(), via that ALLOC_GROW()
thing.
- Arnaldo
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>
> ---
> Changes in v4:
> - Update for the latest kernel.
> - Remove "probe-event.c" from title and body, because
> this also changes probe-finder.c too :)
>
> Changes in v3:
> - Remove unneeded strbuf_release(). (Thanks Namhyung!)
>
> Changes in v2:
> - Make perf_probe_event__sprintf() simpler.
> ---
> tools/perf/util/probe-event.c | 246 ++++++++++++++--------------------------
> tools/perf/util/probe-event.h | 2
> tools/perf/util/probe-finder.c | 14 +-
> 3 files changed, 93 insertions(+), 169 deletions(-)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 8319fbb..148af81 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -1618,69 +1618,51 @@ out:
> }
>
> /* Compose only probe arg */
> -int synthesize_perf_probe_arg(struct perf_probe_arg *pa, char *buf, size_t len)
> +char *synthesize_perf_probe_arg(struct perf_probe_arg *pa)
> {
> struct perf_probe_arg_field *field = pa->field;
> - int ret;
> - char *tmp = buf;
> + struct strbuf buf;
> + char *ret;
>
> + strbuf_init(&buf, 64);
> if (pa->name && pa->var)
> - ret = e_snprintf(tmp, len, "%s=%s", pa->name, pa->var);
> + strbuf_addf(&buf, "%s=%s", pa->name, pa->var);
> else
> - ret = e_snprintf(tmp, len, "%s", pa->name ? pa->name : pa->var);
> - if (ret <= 0)
> - goto error;
> - tmp += ret;
> - len -= ret;
> + strbuf_addstr(&buf, pa->name ?: pa->var);
>
> while (field) {
> if (field->name[0] == '[')
> - ret = e_snprintf(tmp, len, "%s", field->name);
> + strbuf_addstr(&buf, field->name);
> else
> - ret = e_snprintf(tmp, len, "%s%s",
> - field->ref ? "->" : ".", field->name);
> - if (ret <= 0)
> - goto error;
> - tmp += ret;
> - len -= ret;
> + strbuf_addf(&buf, "%s%s", field->ref ? "->" : ".",
> + field->name);
> field = field->next;
> }
>
> - if (pa->type) {
> - ret = e_snprintf(tmp, len, ":%s", pa->type);
> - if (ret <= 0)
> - goto error;
> - tmp += ret;
> - len -= ret;
> - }
> + if (pa->type)
> + strbuf_addf(&buf, ":%s", pa->type);
> +
> + ret = strbuf_detach(&buf, NULL);
>
> - return tmp - buf;
> -error:
> - pr_debug("Failed to synthesize perf probe argument: %d\n", ret);
> return ret;
> }
>
> /* Compose only probe point (not argument) */
> static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> {
> - char *buf, *tmp;
> - char offs[32] = "", line[32] = "", file[32] = "";
> - int ret, len;
> -
> - buf = zalloc(MAX_CMDLEN);
> - if (buf == NULL) {
> - ret = -ENOMEM;
> - goto error;
> - }
> - if (pp->offset) {
> - ret = e_snprintf(offs, 32, "+%lu", pp->offset);
> - if (ret <= 0)
> - goto error;
> - }
> - if (pp->line) {
> - ret = e_snprintf(line, 32, ":%d", pp->line);
> - if (ret <= 0)
> - goto error;
> + struct strbuf buf;
> + char *tmp;
> + int len;
> +
> + strbuf_init(&buf, 64);
> + if (pp->function) {
> + strbuf_addstr(&buf, pp->function);
> + if (pp->offset)
> + strbuf_addf(&buf, "+%lu", pp->offset);
> + else if (pp->line)
> + strbuf_addf(&buf, ":%d", pp->line);
> + else if (pp->retprobe)
> + strbuf_addstr(&buf, "%return");
> }
> if (pp->file) {
> tmp = pp->file;
> @@ -1689,25 +1671,12 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> tmp = strchr(pp->file + len - 30, '/');
> tmp = tmp ? tmp + 1 : pp->file + len - 30;
> }
> - ret = e_snprintf(file, 32, "@%s", tmp);
> - if (ret <= 0)
> - goto error;
> + strbuf_addf(&buf, "@%s", tmp);
> + if (!pp->function && pp->line)
> + strbuf_addf(&buf, ":%d", pp->line);
> }
>
> - if (pp->function)
> - ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
> - offs, pp->retprobe ? "%return" : "", line,
> - file);
> - else
> - ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
> - if (ret <= 0)
> - goto error;
> -
> - return buf;
> -error:
> - pr_debug("Failed to synthesize perf probe point: %d\n", ret);
> - free(buf);
> - return NULL;
> + return strbuf_detach(&buf, NULL);
> }
>
> #if 0
> @@ -1736,45 +1705,30 @@ char *synthesize_perf_probe_command(struct perf_probe_event *pev)
> #endif
>
> static int __synthesize_probe_trace_arg_ref(struct probe_trace_arg_ref *ref,
> - char **buf, size_t *buflen,
> - int depth)
> + struct strbuf *buf, int depth)
> {
> - int ret;
> if (ref->next) {
> depth = __synthesize_probe_trace_arg_ref(ref->next, buf,
> - buflen, depth + 1);
> + depth + 1);
> if (depth < 0)
> goto out;
> }
> -
> - ret = e_snprintf(*buf, *buflen, "%+ld(", ref->offset);
> - if (ret < 0)
> - depth = ret;
> - else {
> - *buf += ret;
> - *buflen -= ret;
> - }
> + strbuf_addf(buf, "%+ld(", ref->offset);
> out:
> return depth;
> -
> }
>
> static int synthesize_probe_trace_arg(struct probe_trace_arg *arg,
> - char *buf, size_t buflen)
> + struct strbuf *buf)
> {
> struct probe_trace_arg_ref *ref = arg->ref;
> - int ret, depth = 0;
> - char *tmp = buf;
> + int depth = 0;
>
> /* Argument name or separator */
> if (arg->name)
> - ret = e_snprintf(buf, buflen, " %s=", arg->name);
> + strbuf_addf(buf, " %s=", arg->name);
> else
> - ret = e_snprintf(buf, buflen, " ");
> - if (ret < 0)
> - return ret;
> - buf += ret;
> - buflen -= ret;
> + strbuf_addch(buf, ' ');
>
> /* Special case: @XXX */
> if (arg->value[0] == '@' && arg->ref)
> @@ -1782,60 +1736,41 @@ static int synthesize_probe_trace_arg(struct probe_trace_arg *arg,
>
> /* Dereferencing arguments */
> if (ref) {
> - depth = __synthesize_probe_trace_arg_ref(ref, &buf,
> - &buflen, 1);
> + depth = __synthesize_probe_trace_arg_ref(ref, buf, 1);
> if (depth < 0)
> return depth;
> }
>
> /* Print argument value */
> if (arg->value[0] == '@' && arg->ref)
> - ret = e_snprintf(buf, buflen, "%s%+ld", arg->value,
> - arg->ref->offset);
> + strbuf_addf(buf, "%s%+ld", arg->value, arg->ref->offset);
> else
> - ret = e_snprintf(buf, buflen, "%s", arg->value);
> - if (ret < 0)
> - return ret;
> - buf += ret;
> - buflen -= ret;
> + strbuf_addstr(buf, arg->value);
>
> /* Closing */
> - while (depth--) {
> - ret = e_snprintf(buf, buflen, ")");
> - if (ret < 0)
> - return ret;
> - buf += ret;
> - buflen -= ret;
> - }
> + while (depth--)
> + strbuf_addch(buf, ')');
> /* Print argument type */
> - if (arg->type) {
> - ret = e_snprintf(buf, buflen, ":%s", arg->type);
> - if (ret <= 0)
> - return ret;
> - buf += ret;
> - }
> + if (arg->type)
> + strbuf_addf(buf, ":%s", arg->type);
>
> - return buf - tmp;
> + return 0;
> }
>
> char *synthesize_probe_trace_command(struct probe_trace_event *tev)
> {
> struct probe_trace_point *tp = &tev->point;
> - char *buf;
> - int i, len, ret;
> -
> - buf = zalloc(MAX_CMDLEN);
> - if (buf == NULL)
> - return NULL;
> -
> - len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s ", tp->retprobe ? 'r' : 'p',
> - tev->group, tev->event);
> - if (len <= 0)
> - goto error;
> + struct strbuf buf;
> + char *ret = NULL;
> + int i;
>
> /* Uprobes must have tp->module */
> if (tev->uprobes && !tp->module)
> - goto error;
> + return NULL;
> +
> + strbuf_init(&buf, 32);
> + strbuf_addf(&buf, "%c:%s/%s ", tp->retprobe ? 'r' : 'p',
> + tev->group, tev->event);
> /*
> * If tp->address == 0, then this point must be a
> * absolute address uprobe.
> @@ -1849,34 +1784,23 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
>
> /* Use the tp->address for uprobes */
> if (tev->uprobes)
> - ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s:0x%lx",
> - tp->module, tp->address);
> + strbuf_addf(&buf, "%s:0x%lx", tp->module, tp->address);
> else if (!strncmp(tp->symbol, "0x", 2))
> /* Absolute address. See try_to_find_absolute_address() */
> - ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s0x%lx",
> - tp->module ?: "", tp->module ? ":" : "",
> - tp->address);
> + strbuf_addf(&buf, "%s%s0x%lx", tp->module ?: "",
> + tp->module ? ":" : "", tp->address);
> else
> - ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s%s+%lu",
> - tp->module ?: "", tp->module ? ":" : "",
> - tp->symbol, tp->offset);
> -
> - if (ret <= 0)
> - goto error;
> - len += ret;
> + strbuf_addf(&buf, "%s%s%s+%lu", tp->module ?: "",
> + tp->module ? ":" : "", tp->symbol, tp->offset);
>
> - for (i = 0; i < tev->nargs; i++) {
> - ret = synthesize_probe_trace_arg(&tev->args[i], buf + len,
> - MAX_CMDLEN - len);
> - if (ret <= 0)
> + for (i = 0; i < tev->nargs; i++)
> + if (synthesize_probe_trace_arg(&tev->args[i], &buf) < 0)
> goto error;
> - len += ret;
> - }
>
> - return buf;
> + ret = strbuf_detach(&buf, NULL);
> error:
> - free(buf);
> - return NULL;
> + strbuf_release(&buf);
> + return ret;
> }
>
> static int find_perf_probe_point_from_map(struct probe_trace_point *tp,
> @@ -1958,7 +1882,7 @@ static int convert_to_perf_probe_point(struct probe_trace_point *tp,
> static int convert_to_perf_probe_event(struct probe_trace_event *tev,
> struct perf_probe_event *pev, bool is_kprobe)
> {
> - char buf[64] = "";
> + struct strbuf buf = STRBUF_INIT;
> int i, ret;
>
> /* Convert event/group name */
> @@ -1981,9 +1905,9 @@ static int convert_to_perf_probe_event(struct probe_trace_event *tev,
> if (tev->args[i].name)
> pev->args[i].name = strdup(tev->args[i].name);
> else {
> - ret = synthesize_probe_trace_arg(&tev->args[i],
> - buf, 64);
> - pev->args[i].name = strdup(buf);
> + strbuf_init(&buf, 32);
> + ret = synthesize_probe_trace_arg(&tev->args[i], &buf);
> + pev->args[i].name = strbuf_detach(&buf, NULL);
> }
> if (pev->args[i].name == NULL && ret >= 0)
> ret = -ENOMEM;
> @@ -2161,37 +2085,37 @@ static int perf_probe_event__sprintf(const char *group, const char *event,
> const char *module,
> struct strbuf *result)
> {
> - int i, ret;
> - char buf[128];
> - char *place;
> + int i;
> + char *buf;
>
> - /* Synthesize only event probe point */
> - place = synthesize_perf_probe_point(&pev->point);
> - if (!place)
> - return -EINVAL;
> + if (asprintf(&buf, "%s:%s", group, event) < 0)
> + return -errno;
> + strbuf_addf(result, " %-20s (on ", buf);
> + free(buf);
>
> - ret = e_snprintf(buf, 128, "%s:%s", group, event);
> - if (ret < 0)
> - goto out;
> + /* Synthesize only event probe point */
> + buf = synthesize_perf_probe_point(&pev->point);
> + if (!buf)
> + return -ENOMEM;
> + strbuf_addstr(result, buf);
> + free(buf);
>
> - strbuf_addf(result, " %-20s (on %s", buf, place);
> if (module)
> strbuf_addf(result, " in %s", module);
>
> if (pev->nargs > 0) {
> strbuf_add(result, " with", 5);
> for (i = 0; i < pev->nargs; i++) {
> - ret = synthesize_perf_probe_arg(&pev->args[i],
> - buf, 128);
> - if (ret < 0)
> - goto out;
> + buf = synthesize_perf_probe_arg(&pev->args[i]);
> + if (!buf)
> + return -ENOMEM;
> strbuf_addf(result, " %s", buf);
> + free(buf);
> }
> }
> strbuf_addch(result, ')');
> -out:
> - free(place);
> - return ret;
> +
> + return 0;
> }
>
> /* Show an event */
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index e54e7b0..e220962 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -120,7 +120,7 @@ int parse_probe_trace_command(const char *cmd, struct probe_trace_event *tev);
> /* Events to command string */
> char *synthesize_perf_probe_command(struct perf_probe_event *pev);
> char *synthesize_probe_trace_command(struct probe_trace_event *tev);
> -int synthesize_perf_probe_arg(struct perf_probe_arg *pa, char *buf, size_t len);
> +char *synthesize_perf_probe_arg(struct perf_probe_arg *pa);
>
> /* Check the perf_probe_event needs debuginfo */
> bool perf_probe_event_need_dwarf(struct perf_probe_event *pev);
> diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> index b3bd0fb..9f68875 100644
> --- a/tools/perf/util/probe-finder.c
> +++ b/tools/perf/util/probe-finder.c
> @@ -553,7 +553,7 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
> static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
> {
> Dwarf_Die vr_die;
> - char buf[32], *ptr;
> + char *buf, *ptr;
> int ret = 0;
>
> /* Copy raw parameters */
> @@ -563,13 +563,13 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
> if (pf->pvar->name)
> pf->tvar->name = strdup(pf->pvar->name);
> else {
> - ret = synthesize_perf_probe_arg(pf->pvar, buf, 32);
> - if (ret < 0)
> - return ret;
> + buf = synthesize_perf_probe_arg(pf->pvar);
> + if (!buf)
> + return -ENOMEM;
> ptr = strchr(buf, ':'); /* Change type separator to _ */
> if (ptr)
> *ptr = '_';
> - pf->tvar->name = strdup(buf);
> + pf->tvar->name = buf;
> }
> if (pf->tvar->name == NULL)
> return -ENOMEM;
> @@ -1334,8 +1334,8 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
> if (ret2 == 0) {
> strlist__add(vl->vars,
> strbuf_detach(&buf, NULL));
> - }
> - strbuf_release(&buf);
> + } else
> + strbuf_release(&buf);
> }
> }
>
[toc] | [prev] | [next] | [standalone]
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2016-04-26 16:50 +0200 |
| Subject | Re: [PATCH perf/core v4 01/19] perf probe: Use strbuf for making strings |
| Message-ID | <rsctj-Xx-3@gated-at.bofh.it> |
| In reply to | #1387484 |
On Tue, 26 Apr 2016 10:36:57 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> Em Tue, Apr 26, 2016 at 06:02:11PM +0900, Masami Hiramatsu escreveu:
> > From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >
> > Replace many fixed-length char array with strbuf to
> > stringify perf_probe_event and probe_trace_event etc.
>
> Sure you want to do that? From time to time I try to reduce the strbuf
> usage, not grow it :-\
Aah, I got what you want.
>
> That is one of the last users of xrealloc(), via that ALLOC_GROW()
> thing.
Hmm I'm not sure what you considering, memory usage? or code reducing?
I also would like to remove e_snprintf() use in perf probe by replacing
with strbuf functions. I'd like to find better way.
Thank you,
>
> - Arnaldo
>
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >
> > ---
> > Changes in v4:
> > - Update for the latest kernel.
> > - Remove "probe-event.c" from title and body, because
> > this also changes probe-finder.c too :)
> >
> > Changes in v3:
> > - Remove unneeded strbuf_release(). (Thanks Namhyung!)
> >
> > Changes in v2:
> > - Make perf_probe_event__sprintf() simpler.
> > ---
> > tools/perf/util/probe-event.c | 246 ++++++++++++++--------------------------
> > tools/perf/util/probe-event.h | 2
> > tools/perf/util/probe-finder.c | 14 +-
> > 3 files changed, 93 insertions(+), 169 deletions(-)
> >
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 8319fbb..148af81 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -1618,69 +1618,51 @@ out:
> > }
> >
> > /* Compose only probe arg */
> > -int synthesize_perf_probe_arg(struct perf_probe_arg *pa, char *buf, size_t len)
> > +char *synthesize_perf_probe_arg(struct perf_probe_arg *pa)
> > {
> > struct perf_probe_arg_field *field = pa->field;
> > - int ret;
> > - char *tmp = buf;
> > + struct strbuf buf;
> > + char *ret;
> >
> > + strbuf_init(&buf, 64);
> > if (pa->name && pa->var)
> > - ret = e_snprintf(tmp, len, "%s=%s", pa->name, pa->var);
> > + strbuf_addf(&buf, "%s=%s", pa->name, pa->var);
> > else
> > - ret = e_snprintf(tmp, len, "%s", pa->name ? pa->name : pa->var);
> > - if (ret <= 0)
> > - goto error;
> > - tmp += ret;
> > - len -= ret;
> > + strbuf_addstr(&buf, pa->name ?: pa->var);
> >
> > while (field) {
> > if (field->name[0] == '[')
> > - ret = e_snprintf(tmp, len, "%s", field->name);
> > + strbuf_addstr(&buf, field->name);
> > else
> > - ret = e_snprintf(tmp, len, "%s%s",
> > - field->ref ? "->" : ".", field->name);
> > - if (ret <= 0)
> > - goto error;
> > - tmp += ret;
> > - len -= ret;
> > + strbuf_addf(&buf, "%s%s", field->ref ? "->" : ".",
> > + field->name);
> > field = field->next;
> > }
> >
> > - if (pa->type) {
> > - ret = e_snprintf(tmp, len, ":%s", pa->type);
> > - if (ret <= 0)
> > - goto error;
> > - tmp += ret;
> > - len -= ret;
> > - }
> > + if (pa->type)
> > + strbuf_addf(&buf, ":%s", pa->type);
> > +
> > + ret = strbuf_detach(&buf, NULL);
> >
> > - return tmp - buf;
> > -error:
> > - pr_debug("Failed to synthesize perf probe argument: %d\n", ret);
> > return ret;
> > }
> >
> > /* Compose only probe point (not argument) */
> > static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> > {
> > - char *buf, *tmp;
> > - char offs[32] = "", line[32] = "", file[32] = "";
> > - int ret, len;
> > -
> > - buf = zalloc(MAX_CMDLEN);
> > - if (buf == NULL) {
> > - ret = -ENOMEM;
> > - goto error;
> > - }
> > - if (pp->offset) {
> > - ret = e_snprintf(offs, 32, "+%lu", pp->offset);
> > - if (ret <= 0)
> > - goto error;
> > - }
> > - if (pp->line) {
> > - ret = e_snprintf(line, 32, ":%d", pp->line);
> > - if (ret <= 0)
> > - goto error;
> > + struct strbuf buf;
> > + char *tmp;
> > + int len;
> > +
> > + strbuf_init(&buf, 64);
> > + if (pp->function) {
> > + strbuf_addstr(&buf, pp->function);
> > + if (pp->offset)
> > + strbuf_addf(&buf, "+%lu", pp->offset);
> > + else if (pp->line)
> > + strbuf_addf(&buf, ":%d", pp->line);
> > + else if (pp->retprobe)
> > + strbuf_addstr(&buf, "%return");
> > }
> > if (pp->file) {
> > tmp = pp->file;
> > @@ -1689,25 +1671,12 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> > tmp = strchr(pp->file + len - 30, '/');
> > tmp = tmp ? tmp + 1 : pp->file + len - 30;
> > }
> > - ret = e_snprintf(file, 32, "@%s", tmp);
> > - if (ret <= 0)
> > - goto error;
> > + strbuf_addf(&buf, "@%s", tmp);
> > + if (!pp->function && pp->line)
> > + strbuf_addf(&buf, ":%d", pp->line);
> > }
> >
> > - if (pp->function)
> > - ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
> > - offs, pp->retprobe ? "%return" : "", line,
> > - file);
> > - else
> > - ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
> > - if (ret <= 0)
> > - goto error;
> > -
> > - return buf;
> > -error:
> > - pr_debug("Failed to synthesize perf probe point: %d\n", ret);
> > - free(buf);
> > - return NULL;
> > + return strbuf_detach(&buf, NULL);
> > }
> >
> > #if 0
> > @@ -1736,45 +1705,30 @@ char *synthesize_perf_probe_command(struct perf_probe_event *pev)
> > #endif
> >
> > static int __synthesize_probe_trace_arg_ref(struct probe_trace_arg_ref *ref,
> > - char **buf, size_t *buflen,
> > - int depth)
> > + struct strbuf *buf, int depth)
> > {
> > - int ret;
> > if (ref->next) {
> > depth = __synthesize_probe_trace_arg_ref(ref->next, buf,
> > - buflen, depth + 1);
> > + depth + 1);
> > if (depth < 0)
> > goto out;
> > }
> > -
> > - ret = e_snprintf(*buf, *buflen, "%+ld(", ref->offset);
> > - if (ret < 0)
> > - depth = ret;
> > - else {
> > - *buf += ret;
> > - *buflen -= ret;
> > - }
> > + strbuf_addf(buf, "%+ld(", ref->offset);
> > out:
> > return depth;
> > -
> > }
> >
> > static int synthesize_probe_trace_arg(struct probe_trace_arg *arg,
> > - char *buf, size_t buflen)
> > + struct strbuf *buf)
> > {
> > struct probe_trace_arg_ref *ref = arg->ref;
> > - int ret, depth = 0;
> > - char *tmp = buf;
> > + int depth = 0;
> >
> > /* Argument name or separator */
> > if (arg->name)
> > - ret = e_snprintf(buf, buflen, " %s=", arg->name);
> > + strbuf_addf(buf, " %s=", arg->name);
> > else
> > - ret = e_snprintf(buf, buflen, " ");
> > - if (ret < 0)
> > - return ret;
> > - buf += ret;
> > - buflen -= ret;
> > + strbuf_addch(buf, ' ');
> >
> > /* Special case: @XXX */
> > if (arg->value[0] == '@' && arg->ref)
> > @@ -1782,60 +1736,41 @@ static int synthesize_probe_trace_arg(struct probe_trace_arg *arg,
> >
> > /* Dereferencing arguments */
> > if (ref) {
> > - depth = __synthesize_probe_trace_arg_ref(ref, &buf,
> > - &buflen, 1);
> > + depth = __synthesize_probe_trace_arg_ref(ref, buf, 1);
> > if (depth < 0)
> > return depth;
> > }
> >
> > /* Print argument value */
> > if (arg->value[0] == '@' && arg->ref)
> > - ret = e_snprintf(buf, buflen, "%s%+ld", arg->value,
> > - arg->ref->offset);
> > + strbuf_addf(buf, "%s%+ld", arg->value, arg->ref->offset);
> > else
> > - ret = e_snprintf(buf, buflen, "%s", arg->value);
> > - if (ret < 0)
> > - return ret;
> > - buf += ret;
> > - buflen -= ret;
> > + strbuf_addstr(buf, arg->value);
> >
> > /* Closing */
> > - while (depth--) {
> > - ret = e_snprintf(buf, buflen, ")");
> > - if (ret < 0)
> > - return ret;
> > - buf += ret;
> > - buflen -= ret;
> > - }
> > + while (depth--)
> > + strbuf_addch(buf, ')');
> > /* Print argument type */
> > - if (arg->type) {
> > - ret = e_snprintf(buf, buflen, ":%s", arg->type);
> > - if (ret <= 0)
> > - return ret;
> > - buf += ret;
> > - }
> > + if (arg->type)
> > + strbuf_addf(buf, ":%s", arg->type);
> >
> > - return buf - tmp;
> > + return 0;
> > }
> >
> > char *synthesize_probe_trace_command(struct probe_trace_event *tev)
> > {
> > struct probe_trace_point *tp = &tev->point;
> > - char *buf;
> > - int i, len, ret;
> > -
> > - buf = zalloc(MAX_CMDLEN);
> > - if (buf == NULL)
> > - return NULL;
> > -
> > - len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s ", tp->retprobe ? 'r' : 'p',
> > - tev->group, tev->event);
> > - if (len <= 0)
> > - goto error;
> > + struct strbuf buf;
> > + char *ret = NULL;
> > + int i;
> >
> > /* Uprobes must have tp->module */
> > if (tev->uprobes && !tp->module)
> > - goto error;
> > + return NULL;
> > +
> > + strbuf_init(&buf, 32);
> > + strbuf_addf(&buf, "%c:%s/%s ", tp->retprobe ? 'r' : 'p',
> > + tev->group, tev->event);
> > /*
> > * If tp->address == 0, then this point must be a
> > * absolute address uprobe.
> > @@ -1849,34 +1784,23 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
> >
> > /* Use the tp->address for uprobes */
> > if (tev->uprobes)
> > - ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s:0x%lx",
> > - tp->module, tp->address);
> > + strbuf_addf(&buf, "%s:0x%lx", tp->module, tp->address);
> > else if (!strncmp(tp->symbol, "0x", 2))
> > /* Absolute address. See try_to_find_absolute_address() */
> > - ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s0x%lx",
> > - tp->module ?: "", tp->module ? ":" : "",
> > - tp->address);
> > + strbuf_addf(&buf, "%s%s0x%lx", tp->module ?: "",
> > + tp->module ? ":" : "", tp->address);
> > else
> > - ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s%s+%lu",
> > - tp->module ?: "", tp->module ? ":" : "",
> > - tp->symbol, tp->offset);
> > -
> > - if (ret <= 0)
> > - goto error;
> > - len += ret;
> > + strbuf_addf(&buf, "%s%s%s+%lu", tp->module ?: "",
> > + tp->module ? ":" : "", tp->symbol, tp->offset);
> >
> > - for (i = 0; i < tev->nargs; i++) {
> > - ret = synthesize_probe_trace_arg(&tev->args[i], buf + len,
> > - MAX_CMDLEN - len);
> > - if (ret <= 0)
> > + for (i = 0; i < tev->nargs; i++)
> > + if (synthesize_probe_trace_arg(&tev->args[i], &buf) < 0)
> > goto error;
> > - len += ret;
> > - }
> >
> > - return buf;
> > + ret = strbuf_detach(&buf, NULL);
> > error:
> > - free(buf);
> > - return NULL;
> > + strbuf_release(&buf);
> > + return ret;
> > }
> >
> > static int find_perf_probe_point_from_map(struct probe_trace_point *tp,
> > @@ -1958,7 +1882,7 @@ static int convert_to_perf_probe_point(struct probe_trace_point *tp,
> > static int convert_to_perf_probe_event(struct probe_trace_event *tev,
> > struct perf_probe_event *pev, bool is_kprobe)
> > {
> > - char buf[64] = "";
> > + struct strbuf buf = STRBUF_INIT;
> > int i, ret;
> >
> > /* Convert event/group name */
> > @@ -1981,9 +1905,9 @@ static int convert_to_perf_probe_event(struct probe_trace_event *tev,
> > if (tev->args[i].name)
> > pev->args[i].name = strdup(tev->args[i].name);
> > else {
> > - ret = synthesize_probe_trace_arg(&tev->args[i],
> > - buf, 64);
> > - pev->args[i].name = strdup(buf);
> > + strbuf_init(&buf, 32);
> > + ret = synthesize_probe_trace_arg(&tev->args[i], &buf);
> > + pev->args[i].name = strbuf_detach(&buf, NULL);
> > }
> > if (pev->args[i].name == NULL && ret >= 0)
> > ret = -ENOMEM;
> > @@ -2161,37 +2085,37 @@ static int perf_probe_event__sprintf(const char *group, const char *event,
> > const char *module,
> > struct strbuf *result)
> > {
> > - int i, ret;
> > - char buf[128];
> > - char *place;
> > + int i;
> > + char *buf;
> >
> > - /* Synthesize only event probe point */
> > - place = synthesize_perf_probe_point(&pev->point);
> > - if (!place)
> > - return -EINVAL;
> > + if (asprintf(&buf, "%s:%s", group, event) < 0)
> > + return -errno;
> > + strbuf_addf(result, " %-20s (on ", buf);
> > + free(buf);
> >
> > - ret = e_snprintf(buf, 128, "%s:%s", group, event);
> > - if (ret < 0)
> > - goto out;
> > + /* Synthesize only event probe point */
> > + buf = synthesize_perf_probe_point(&pev->point);
> > + if (!buf)
> > + return -ENOMEM;
> > + strbuf_addstr(result, buf);
> > + free(buf);
> >
> > - strbuf_addf(result, " %-20s (on %s", buf, place);
> > if (module)
> > strbuf_addf(result, " in %s", module);
> >
> > if (pev->nargs > 0) {
> > strbuf_add(result, " with", 5);
> > for (i = 0; i < pev->nargs; i++) {
> > - ret = synthesize_perf_probe_arg(&pev->args[i],
> > - buf, 128);
> > - if (ret < 0)
> > - goto out;
> > + buf = synthesize_perf_probe_arg(&pev->args[i]);
> > + if (!buf)
> > + return -ENOMEM;
> > strbuf_addf(result, " %s", buf);
> > + free(buf);
> > }
> > }
> > strbuf_addch(result, ')');
> > -out:
> > - free(place);
> > - return ret;
> > +
> > + return 0;
> > }
> >
> > /* Show an event */
> > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> > index e54e7b0..e220962 100644
> > --- a/tools/perf/util/probe-event.h
> > +++ b/tools/perf/util/probe-event.h
> > @@ -120,7 +120,7 @@ int parse_probe_trace_command(const char *cmd, struct probe_trace_event *tev);
> > /* Events to command string */
> > char *synthesize_perf_probe_command(struct perf_probe_event *pev);
> > char *synthesize_probe_trace_command(struct probe_trace_event *tev);
> > -int synthesize_perf_probe_arg(struct perf_probe_arg *pa, char *buf, size_t len);
> > +char *synthesize_perf_probe_arg(struct perf_probe_arg *pa);
> >
> > /* Check the perf_probe_event needs debuginfo */
> > bool perf_probe_event_need_dwarf(struct perf_probe_event *pev);
> > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > index b3bd0fb..9f68875 100644
> > --- a/tools/perf/util/probe-finder.c
> > +++ b/tools/perf/util/probe-finder.c
> > @@ -553,7 +553,7 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
> > static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
> > {
> > Dwarf_Die vr_die;
> > - char buf[32], *ptr;
> > + char *buf, *ptr;
> > int ret = 0;
> >
> > /* Copy raw parameters */
> > @@ -563,13 +563,13 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
> > if (pf->pvar->name)
> > pf->tvar->name = strdup(pf->pvar->name);
> > else {
> > - ret = synthesize_perf_probe_arg(pf->pvar, buf, 32);
> > - if (ret < 0)
> > - return ret;
> > + buf = synthesize_perf_probe_arg(pf->pvar);
> > + if (!buf)
> > + return -ENOMEM;
> > ptr = strchr(buf, ':'); /* Change type separator to _ */
> > if (ptr)
> > *ptr = '_';
> > - pf->tvar->name = strdup(buf);
> > + pf->tvar->name = buf;
> > }
> > if (pf->tvar->name == NULL)
> > return -ENOMEM;
> > @@ -1334,8 +1334,8 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
> > if (ret2 == 0) {
> > strlist__add(vl->vars,
> > strbuf_detach(&buf, NULL));
> > - }
> > - strbuf_release(&buf);
> > + } else
> > + strbuf_release(&buf);
> > }
> > }
> >
--
Masami Hiramatsu <mhiramat@kernel.org>
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-04-26 17:10 +0200 |
| Subject | Re: [PATCH perf/core v4 01/19] perf probe: Use strbuf for making strings |
| Message-ID | <rscMG-1n6-3@gated-at.bofh.it> |
| In reply to | #1387554 |
Em Tue, Apr 26, 2016 at 11:40:47PM +0900, Masami Hiramatsu escreveu:
> On Tue, 26 Apr 2016 10:36:57 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> > Em Tue, Apr 26, 2016 at 06:02:11PM +0900, Masami Hiramatsu escreveu:
> > > From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> > >
> > > Replace many fixed-length char array with strbuf to
> > > stringify perf_probe_event and probe_trace_event etc.
> >
> > Sure you want to do that? From time to time I try to reduce the strbuf
> > usage, not grow it :-\
>
> Aah, I got what you want.
>
> >
> > That is one of the last users of xrealloc(), via that ALLOC_GROW()
> > thing.
>
> Hmm I'm not sure what you considering, memory usage? or code reducing?
> I also would like to remove e_snprintf() use in perf probe by replacing
> with strbuf functions. I'd like to find better way.
What I don't like is that xrealloc ends up calling die() :-\ It should
instead propagate back the error.
In lots of cases I managed to use what is in glibc, asprintf(), that
will allocate space and format in one go.
But strbuf has this addf() one that indeed is handy. So I think I'll
take this patch as it is not for the simpler cases where asprintf() can
be used.
- Arnaldo
> Thank you,
>
> >
> > - Arnaldo
> >
> > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> > >
> > > ---
> > > Changes in v4:
> > > - Update for the latest kernel.
> > > - Remove "probe-event.c" from title and body, because
> > > this also changes probe-finder.c too :)
> > >
> > > Changes in v3:
> > > - Remove unneeded strbuf_release(). (Thanks Namhyung!)
> > >
> > > Changes in v2:
> > > - Make perf_probe_event__sprintf() simpler.
> > > ---
> > > tools/perf/util/probe-event.c | 246 ++++++++++++++--------------------------
> > > tools/perf/util/probe-event.h | 2
> > > tools/perf/util/probe-finder.c | 14 +-
> > > 3 files changed, 93 insertions(+), 169 deletions(-)
> > >
> > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > > index 8319fbb..148af81 100644
> > > --- a/tools/perf/util/probe-event.c
> > > +++ b/tools/perf/util/probe-event.c
> > > @@ -1618,69 +1618,51 @@ out:
> > > }
> > >
> > > /* Compose only probe arg */
> > > -int synthesize_perf_probe_arg(struct perf_probe_arg *pa, char *buf, size_t len)
> > > +char *synthesize_perf_probe_arg(struct perf_probe_arg *pa)
> > > {
> > > struct perf_probe_arg_field *field = pa->field;
> > > - int ret;
> > > - char *tmp = buf;
> > > + struct strbuf buf;
> > > + char *ret;
> > >
> > > + strbuf_init(&buf, 64);
> > > if (pa->name && pa->var)
> > > - ret = e_snprintf(tmp, len, "%s=%s", pa->name, pa->var);
> > > + strbuf_addf(&buf, "%s=%s", pa->name, pa->var);
> > > else
> > > - ret = e_snprintf(tmp, len, "%s", pa->name ? pa->name : pa->var);
> > > - if (ret <= 0)
> > > - goto error;
> > > - tmp += ret;
> > > - len -= ret;
> > > + strbuf_addstr(&buf, pa->name ?: pa->var);
> > >
> > > while (field) {
> > > if (field->name[0] == '[')
> > > - ret = e_snprintf(tmp, len, "%s", field->name);
> > > + strbuf_addstr(&buf, field->name);
> > > else
> > > - ret = e_snprintf(tmp, len, "%s%s",
> > > - field->ref ? "->" : ".", field->name);
> > > - if (ret <= 0)
> > > - goto error;
> > > - tmp += ret;
> > > - len -= ret;
> > > + strbuf_addf(&buf, "%s%s", field->ref ? "->" : ".",
> > > + field->name);
> > > field = field->next;
> > > }
> > >
> > > - if (pa->type) {
> > > - ret = e_snprintf(tmp, len, ":%s", pa->type);
> > > - if (ret <= 0)
> > > - goto error;
> > > - tmp += ret;
> > > - len -= ret;
> > > - }
> > > + if (pa->type)
> > > + strbuf_addf(&buf, ":%s", pa->type);
> > > +
> > > + ret = strbuf_detach(&buf, NULL);
> > >
> > > - return tmp - buf;
> > > -error:
> > > - pr_debug("Failed to synthesize perf probe argument: %d\n", ret);
> > > return ret;
> > > }
> > >
> > > /* Compose only probe point (not argument) */
> > > static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> > > {
> > > - char *buf, *tmp;
> > > - char offs[32] = "", line[32] = "", file[32] = "";
> > > - int ret, len;
> > > -
> > > - buf = zalloc(MAX_CMDLEN);
> > > - if (buf == NULL) {
> > > - ret = -ENOMEM;
> > > - goto error;
> > > - }
> > > - if (pp->offset) {
> > > - ret = e_snprintf(offs, 32, "+%lu", pp->offset);
> > > - if (ret <= 0)
> > > - goto error;
> > > - }
> > > - if (pp->line) {
> > > - ret = e_snprintf(line, 32, ":%d", pp->line);
> > > - if (ret <= 0)
> > > - goto error;
> > > + struct strbuf buf;
> > > + char *tmp;
> > > + int len;
> > > +
> > > + strbuf_init(&buf, 64);
> > > + if (pp->function) {
> > > + strbuf_addstr(&buf, pp->function);
> > > + if (pp->offset)
> > > + strbuf_addf(&buf, "+%lu", pp->offset);
> > > + else if (pp->line)
> > > + strbuf_addf(&buf, ":%d", pp->line);
> > > + else if (pp->retprobe)
> > > + strbuf_addstr(&buf, "%return");
> > > }
> > > if (pp->file) {
> > > tmp = pp->file;
> > > @@ -1689,25 +1671,12 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp)
> > > tmp = strchr(pp->file + len - 30, '/');
> > > tmp = tmp ? tmp + 1 : pp->file + len - 30;
> > > }
> > > - ret = e_snprintf(file, 32, "@%s", tmp);
> > > - if (ret <= 0)
> > > - goto error;
> > > + strbuf_addf(&buf, "@%s", tmp);
> > > + if (!pp->function && pp->line)
> > > + strbuf_addf(&buf, ":%d", pp->line);
> > > }
> > >
> > > - if (pp->function)
> > > - ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s%s", pp->function,
> > > - offs, pp->retprobe ? "%return" : "", line,
> > > - file);
> > > - else
> > > - ret = e_snprintf(buf, MAX_CMDLEN, "%s%s", file, line);
> > > - if (ret <= 0)
> > > - goto error;
> > > -
> > > - return buf;
> > > -error:
> > > - pr_debug("Failed to synthesize perf probe point: %d\n", ret);
> > > - free(buf);
> > > - return NULL;
> > > + return strbuf_detach(&buf, NULL);
> > > }
> > >
> > > #if 0
> > > @@ -1736,45 +1705,30 @@ char *synthesize_perf_probe_command(struct perf_probe_event *pev)
> > > #endif
> > >
> > > static int __synthesize_probe_trace_arg_ref(struct probe_trace_arg_ref *ref,
> > > - char **buf, size_t *buflen,
> > > - int depth)
> > > + struct strbuf *buf, int depth)
> > > {
> > > - int ret;
> > > if (ref->next) {
> > > depth = __synthesize_probe_trace_arg_ref(ref->next, buf,
> > > - buflen, depth + 1);
> > > + depth + 1);
> > > if (depth < 0)
> > > goto out;
> > > }
> > > -
> > > - ret = e_snprintf(*buf, *buflen, "%+ld(", ref->offset);
> > > - if (ret < 0)
> > > - depth = ret;
> > > - else {
> > > - *buf += ret;
> > > - *buflen -= ret;
> > > - }
> > > + strbuf_addf(buf, "%+ld(", ref->offset);
> > > out:
> > > return depth;
> > > -
> > > }
> > >
> > > static int synthesize_probe_trace_arg(struct probe_trace_arg *arg,
> > > - char *buf, size_t buflen)
> > > + struct strbuf *buf)
> > > {
> > > struct probe_trace_arg_ref *ref = arg->ref;
> > > - int ret, depth = 0;
> > > - char *tmp = buf;
> > > + int depth = 0;
> > >
> > > /* Argument name or separator */
> > > if (arg->name)
> > > - ret = e_snprintf(buf, buflen, " %s=", arg->name);
> > > + strbuf_addf(buf, " %s=", arg->name);
> > > else
> > > - ret = e_snprintf(buf, buflen, " ");
> > > - if (ret < 0)
> > > - return ret;
> > > - buf += ret;
> > > - buflen -= ret;
> > > + strbuf_addch(buf, ' ');
> > >
> > > /* Special case: @XXX */
> > > if (arg->value[0] == '@' && arg->ref)
> > > @@ -1782,60 +1736,41 @@ static int synthesize_probe_trace_arg(struct probe_trace_arg *arg,
> > >
> > > /* Dereferencing arguments */
> > > if (ref) {
> > > - depth = __synthesize_probe_trace_arg_ref(ref, &buf,
> > > - &buflen, 1);
> > > + depth = __synthesize_probe_trace_arg_ref(ref, buf, 1);
> > > if (depth < 0)
> > > return depth;
> > > }
> > >
> > > /* Print argument value */
> > > if (arg->value[0] == '@' && arg->ref)
> > > - ret = e_snprintf(buf, buflen, "%s%+ld", arg->value,
> > > - arg->ref->offset);
> > > + strbuf_addf(buf, "%s%+ld", arg->value, arg->ref->offset);
> > > else
> > > - ret = e_snprintf(buf, buflen, "%s", arg->value);
> > > - if (ret < 0)
> > > - return ret;
> > > - buf += ret;
> > > - buflen -= ret;
> > > + strbuf_addstr(buf, arg->value);
> > >
> > > /* Closing */
> > > - while (depth--) {
> > > - ret = e_snprintf(buf, buflen, ")");
> > > - if (ret < 0)
> > > - return ret;
> > > - buf += ret;
> > > - buflen -= ret;
> > > - }
> > > + while (depth--)
> > > + strbuf_addch(buf, ')');
> > > /* Print argument type */
> > > - if (arg->type) {
> > > - ret = e_snprintf(buf, buflen, ":%s", arg->type);
> > > - if (ret <= 0)
> > > - return ret;
> > > - buf += ret;
> > > - }
> > > + if (arg->type)
> > > + strbuf_addf(buf, ":%s", arg->type);
> > >
> > > - return buf - tmp;
> > > + return 0;
> > > }
> > >
> > > char *synthesize_probe_trace_command(struct probe_trace_event *tev)
> > > {
> > > struct probe_trace_point *tp = &tev->point;
> > > - char *buf;
> > > - int i, len, ret;
> > > -
> > > - buf = zalloc(MAX_CMDLEN);
> > > - if (buf == NULL)
> > > - return NULL;
> > > -
> > > - len = e_snprintf(buf, MAX_CMDLEN, "%c:%s/%s ", tp->retprobe ? 'r' : 'p',
> > > - tev->group, tev->event);
> > > - if (len <= 0)
> > > - goto error;
> > > + struct strbuf buf;
> > > + char *ret = NULL;
> > > + int i;
> > >
> > > /* Uprobes must have tp->module */
> > > if (tev->uprobes && !tp->module)
> > > - goto error;
> > > + return NULL;
> > > +
> > > + strbuf_init(&buf, 32);
> > > + strbuf_addf(&buf, "%c:%s/%s ", tp->retprobe ? 'r' : 'p',
> > > + tev->group, tev->event);
> > > /*
> > > * If tp->address == 0, then this point must be a
> > > * absolute address uprobe.
> > > @@ -1849,34 +1784,23 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
> > >
> > > /* Use the tp->address for uprobes */
> > > if (tev->uprobes)
> > > - ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s:0x%lx",
> > > - tp->module, tp->address);
> > > + strbuf_addf(&buf, "%s:0x%lx", tp->module, tp->address);
> > > else if (!strncmp(tp->symbol, "0x", 2))
> > > /* Absolute address. See try_to_find_absolute_address() */
> > > - ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s0x%lx",
> > > - tp->module ?: "", tp->module ? ":" : "",
> > > - tp->address);
> > > + strbuf_addf(&buf, "%s%s0x%lx", tp->module ?: "",
> > > + tp->module ? ":" : "", tp->address);
> > > else
> > > - ret = e_snprintf(buf + len, MAX_CMDLEN - len, "%s%s%s+%lu",
> > > - tp->module ?: "", tp->module ? ":" : "",
> > > - tp->symbol, tp->offset);
> > > -
> > > - if (ret <= 0)
> > > - goto error;
> > > - len += ret;
> > > + strbuf_addf(&buf, "%s%s%s+%lu", tp->module ?: "",
> > > + tp->module ? ":" : "", tp->symbol, tp->offset);
> > >
> > > - for (i = 0; i < tev->nargs; i++) {
> > > - ret = synthesize_probe_trace_arg(&tev->args[i], buf + len,
> > > - MAX_CMDLEN - len);
> > > - if (ret <= 0)
> > > + for (i = 0; i < tev->nargs; i++)
> > > + if (synthesize_probe_trace_arg(&tev->args[i], &buf) < 0)
> > > goto error;
> > > - len += ret;
> > > - }
> > >
> > > - return buf;
> > > + ret = strbuf_detach(&buf, NULL);
> > > error:
> > > - free(buf);
> > > - return NULL;
> > > + strbuf_release(&buf);
> > > + return ret;
> > > }
> > >
> > > static int find_perf_probe_point_from_map(struct probe_trace_point *tp,
> > > @@ -1958,7 +1882,7 @@ static int convert_to_perf_probe_point(struct probe_trace_point *tp,
> > > static int convert_to_perf_probe_event(struct probe_trace_event *tev,
> > > struct perf_probe_event *pev, bool is_kprobe)
> > > {
> > > - char buf[64] = "";
> > > + struct strbuf buf = STRBUF_INIT;
> > > int i, ret;
> > >
> > > /* Convert event/group name */
> > > @@ -1981,9 +1905,9 @@ static int convert_to_perf_probe_event(struct probe_trace_event *tev,
> > > if (tev->args[i].name)
> > > pev->args[i].name = strdup(tev->args[i].name);
> > > else {
> > > - ret = synthesize_probe_trace_arg(&tev->args[i],
> > > - buf, 64);
> > > - pev->args[i].name = strdup(buf);
> > > + strbuf_init(&buf, 32);
> > > + ret = synthesize_probe_trace_arg(&tev->args[i], &buf);
> > > + pev->args[i].name = strbuf_detach(&buf, NULL);
> > > }
> > > if (pev->args[i].name == NULL && ret >= 0)
> > > ret = -ENOMEM;
> > > @@ -2161,37 +2085,37 @@ static int perf_probe_event__sprintf(const char *group, const char *event,
> > > const char *module,
> > > struct strbuf *result)
> > > {
> > > - int i, ret;
> > > - char buf[128];
> > > - char *place;
> > > + int i;
> > > + char *buf;
> > >
> > > - /* Synthesize only event probe point */
> > > - place = synthesize_perf_probe_point(&pev->point);
> > > - if (!place)
> > > - return -EINVAL;
> > > + if (asprintf(&buf, "%s:%s", group, event) < 0)
> > > + return -errno;
> > > + strbuf_addf(result, " %-20s (on ", buf);
> > > + free(buf);
> > >
> > > - ret = e_snprintf(buf, 128, "%s:%s", group, event);
> > > - if (ret < 0)
> > > - goto out;
> > > + /* Synthesize only event probe point */
> > > + buf = synthesize_perf_probe_point(&pev->point);
> > > + if (!buf)
> > > + return -ENOMEM;
> > > + strbuf_addstr(result, buf);
> > > + free(buf);
> > >
> > > - strbuf_addf(result, " %-20s (on %s", buf, place);
> > > if (module)
> > > strbuf_addf(result, " in %s", module);
> > >
> > > if (pev->nargs > 0) {
> > > strbuf_add(result, " with", 5);
> > > for (i = 0; i < pev->nargs; i++) {
> > > - ret = synthesize_perf_probe_arg(&pev->args[i],
> > > - buf, 128);
> > > - if (ret < 0)
> > > - goto out;
> > > + buf = synthesize_perf_probe_arg(&pev->args[i]);
> > > + if (!buf)
> > > + return -ENOMEM;
> > > strbuf_addf(result, " %s", buf);
> > > + free(buf);
> > > }
> > > }
> > > strbuf_addch(result, ')');
> > > -out:
> > > - free(place);
> > > - return ret;
> > > +
> > > + return 0;
> > > }
> > >
> > > /* Show an event */
> > > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> > > index e54e7b0..e220962 100644
> > > --- a/tools/perf/util/probe-event.h
> > > +++ b/tools/perf/util/probe-event.h
> > > @@ -120,7 +120,7 @@ int parse_probe_trace_command(const char *cmd, struct probe_trace_event *tev);
> > > /* Events to command string */
> > > char *synthesize_perf_probe_command(struct perf_probe_event *pev);
> > > char *synthesize_probe_trace_command(struct probe_trace_event *tev);
> > > -int synthesize_perf_probe_arg(struct perf_probe_arg *pa, char *buf, size_t len);
> > > +char *synthesize_perf_probe_arg(struct perf_probe_arg *pa);
> > >
> > > /* Check the perf_probe_event needs debuginfo */
> > > bool perf_probe_event_need_dwarf(struct perf_probe_event *pev);
> > > diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
> > > index b3bd0fb..9f68875 100644
> > > --- a/tools/perf/util/probe-finder.c
> > > +++ b/tools/perf/util/probe-finder.c
> > > @@ -553,7 +553,7 @@ static int convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
> > > static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
> > > {
> > > Dwarf_Die vr_die;
> > > - char buf[32], *ptr;
> > > + char *buf, *ptr;
> > > int ret = 0;
> > >
> > > /* Copy raw parameters */
> > > @@ -563,13 +563,13 @@ static int find_variable(Dwarf_Die *sc_die, struct probe_finder *pf)
> > > if (pf->pvar->name)
> > > pf->tvar->name = strdup(pf->pvar->name);
> > > else {
> > > - ret = synthesize_perf_probe_arg(pf->pvar, buf, 32);
> > > - if (ret < 0)
> > > - return ret;
> > > + buf = synthesize_perf_probe_arg(pf->pvar);
> > > + if (!buf)
> > > + return -ENOMEM;
> > > ptr = strchr(buf, ':'); /* Change type separator to _ */
> > > if (ptr)
> > > *ptr = '_';
> > > - pf->tvar->name = strdup(buf);
> > > + pf->tvar->name = buf;
> > > }
> > > if (pf->tvar->name == NULL)
> > > return -ENOMEM;
> > > @@ -1334,8 +1334,8 @@ static int collect_variables_cb(Dwarf_Die *die_mem, void *data)
> > > if (ret2 == 0) {
> > > strlist__add(vl->vars,
> > > strbuf_detach(&buf, NULL));
> > > - }
> > > - strbuf_release(&buf);
> > > + } else
> > > + strbuf_release(&buf);
> > > }
> > > }
> > >
>
>
> --
> Masami Hiramatsu <mhiramat@kernel.org>
[toc] | [prev] | [next] | [standalone]
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2016-04-27 20:50 +0200 |
| Subject | Re: [PATCH perf/core v4 01/19] perf probe: Use strbuf for making strings |
| Message-ID | <rsCH8-61V-13@gated-at.bofh.it> |
| In reply to | #1387588 |
On Tue, 26 Apr 2016 11:59:34 -0300 Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > Em Tue, Apr 26, 2016 at 11:40:47PM +0900, Masami Hiramatsu escreveu: > > On Tue, 26 Apr 2016 10:36:57 -0300 > > Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > > > > Em Tue, Apr 26, 2016 at 06:02:11PM +0900, Masami Hiramatsu escreveu: > > > > From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > > > > > > > > Replace many fixed-length char array with strbuf to > > > > stringify perf_probe_event and probe_trace_event etc. > > > > > > Sure you want to do that? From time to time I try to reduce the strbuf > > > usage, not grow it :-\ > > > > Aah, I got what you want. > > > > > > > > That is one of the last users of xrealloc(), via that ALLOC_GROW() > > > thing. > > > > Hmm I'm not sure what you considering, memory usage? or code reducing? > > I also would like to remove e_snprintf() use in perf probe by replacing > > with strbuf functions. I'd like to find better way. > > What I don't like is that xrealloc ends up calling die() :-\ It should > instead propagate back the error. > > In lots of cases I managed to use what is in glibc, asprintf(), that > will allocate space and format in one go. > > But strbuf has this addf() one that indeed is handy. So I think I'll > take this patch as it is not for the simpler cases where asprintf() can > be used. OK, I'll try to remove die() and xrealloc() from the strbuf, since it seems most of users of strbuf() is perf-probe. Thanks, -- Masami Hiramatsu <mhiramat@kernel.org>
[toc] | [prev] | [next] | [standalone]
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2016-04-26 11:10 +0200 |
| Subject | [PATCH perf/core v4 04/19] perf: Add lsdir to read a directory |
| Message-ID | <rs7aj-4SP-31@gated-at.bofh.it> |
| In reply to | #1387248 |
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
As a utility function, add lsdir() which reads given
directory and store entry name into a strlist.
lsdir accepts a filter function so that user can
filter out unneeded entries.
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
tools/perf/util/util.c | 34 ++++++++++++++++++++++++++++++++++
tools/perf/util/util.h | 4 ++++
2 files changed, 38 insertions(+)
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index b7766c5..44642b6 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -117,6 +117,40 @@ int rm_rf(char *path)
return rmdir(path);
}
+/* A filter which removes dot files */
+bool lsdir_no_dot_filter(const char *dirname __maybe_unused, struct dirent *d)
+{
+ return d->d_name[0] != '.';
+}
+
+/* lsdir reads a directory and store it in strlist */
+struct strlist *lsdir(const char *dirname,
+ bool (*filter)(const char *, struct dirent *))
+{
+ struct strlist *list = NULL;
+ DIR *dir;
+ struct dirent *d;
+
+ dir = opendir(dirname);
+ if (!dir)
+ return NULL;
+
+ list = strlist__new(NULL, NULL);
+ if (!list) {
+ errno = -ENOMEM;
+ goto out;
+ }
+
+ while ((d = readdir(dir)) != NULL) {
+ if (!filter || filter(dirname, d))
+ strlist__add(list, d->d_name);
+ }
+
+out:
+ closedir(dir);
+ return list;
+}
+
static int slow_copyfile(const char *from, const char *to)
{
int err = -1;
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 3bf3de8..3f2fbf9 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -79,6 +79,7 @@
#include <termios.h>
#include <linux/bitops.h>
#include <termios.h>
+#include "strlist.h"
extern const char *graph_line;
extern const char *graph_dotted_line;
@@ -222,6 +223,9 @@ static inline int sane_case(int x, int high)
int mkdir_p(char *path, mode_t mode);
int rm_rf(char *path);
+struct strlist *lsdir(const char *dirname,
+ bool (*filter)(const char *, struct dirent *));
+bool lsdir_no_dot_filter(const char *dirname __maybe_unused, struct dirent *d);
int copyfile(const char *from, const char *to);
int copyfile_mode(const char *from, const char *to, mode_t mode);
int copyfile_offset(int fromfd, loff_t from_ofs, int tofd, loff_t to_ofs, u64 size);
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-04-26 15:50 +0200 |
| Subject | Re: [PATCH perf/core v4 04/19] perf: Add lsdir to read a directory |
| Message-ID | <rsbxh-eJ-31@gated-at.bofh.it> |
| In reply to | #1387254 |
Em Tue, Apr 26, 2016 at 06:02:42PM +0900, Masami Hiramatsu escreveu: > From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > > As a utility function, add lsdir() which reads given > directory and store entry name into a strlist. > lsdir accepts a filter function so that user can > filter out unneeded entries. > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> Thanks, applied. - Arnaldo
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-04-26 16:10 +0200 |
| Subject | Re: [PATCH perf/core v4 04/19] perf: Add lsdir to read a directory |
| Message-ID | <rsbQD-Gm-39@gated-at.bofh.it> |
| In reply to | #1387495 |
Em Tue, Apr 26, 2016 at 10:40:20AM -0300, Arnaldo Carvalho de Melo escreveu: > Em Tue, Apr 26, 2016 at 06:02:42PM +0900, Masami Hiramatsu escreveu: > > From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > > > > As a utility function, add lsdir() which reads given > > directory and store entry name into a strlist. > > lsdir accepts a filter function so that user can > > filter out unneeded entries. > > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > > Thanks, applied. Had to fix it to build on some distros, like ubuntu 12.04: alldeps-ubuntu-12.04: FAIL util/util.h:228:38: error: declaration of 'dirname' shadows a global declaration [-Werror=shadow] cc1: all warnings being treated as errors mv: cannot stat `/tmp/build/perf/util/.db-export.o.tmp': No such file or directory make[3]: *** [/tmp/build/perf/util/db-export.o] Error 1 make[3]: *** Waiting for unfinished jobs.... make[2]: *** [util] Error 2 make[2]: *** [ui] Error 2 make[1]: *** [/tmp/build/perf/libperf-in.o] Error 2 make: *** [install-bin] Error 2
[toc] | [prev] | [next] | [standalone]
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2016-04-26 17:00 +0200 |
| Subject | Re: [PATCH perf/core v4 04/19] perf: Add lsdir to read a directory |
| Message-ID | <rscD0-11Z-21@gated-at.bofh.it> |
| In reply to | #1387519 |
On Tue, 26 Apr 2016 11:07:42 -0300 Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > Em Tue, Apr 26, 2016 at 10:40:20AM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Tue, Apr 26, 2016 at 06:02:42PM +0900, Masami Hiramatsu escreveu: > > > From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > > > > > > As a utility function, add lsdir() which reads given > > > directory and store entry name into a strlist. > > > lsdir accepts a filter function so that user can > > > filter out unneeded entries. > > > > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > > > > Thanks, applied. > > Had to fix it to build on some distros, like ubuntu 12.04: > > alldeps-ubuntu-12.04: FAIL > util/util.h:228:38: error: declaration of 'dirname' shadows a global declaration [-Werror=shadow] > cc1: all warnings being treated as errors > mv: cannot stat `/tmp/build/perf/util/.db-export.o.tmp': No such file or directory > make[3]: *** [/tmp/build/perf/util/db-export.o] Error 1 > make[3]: *** Waiting for unfinished jobs.... > make[2]: *** [util] Error 2 > make[2]: *** [ui] Error 2 > make[1]: *** [/tmp/build/perf/libperf-in.o] Error > make: *** [install-bin] Error 2 > Hmm, I didn't know that the dummy argument can shadow a global declaration. I might be better to rename "dirname" to "dir_name". Thank you, -- Masami Hiramatsu <mhiramat@kernel.org>
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-04-26 17:10 +0200 |
| Subject | Re: [PATCH perf/core v4 04/19] perf: Add lsdir to read a directory |
| Message-ID | <rscMH-1n6-35@gated-at.bofh.it> |
| In reply to | #1387576 |
Em Tue, Apr 26, 2016 at 11:52:29PM +0900, Masami Hiramatsu escreveu: > On Tue, 26 Apr 2016 11:07:42 -0300 > Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > > Em Tue, Apr 26, 2016 at 10:40:20AM -0300, Arnaldo Carvalho de Melo escreveu: > > > Em Tue, Apr 26, 2016 at 06:02:42PM +0900, Masami Hiramatsu escreveu: > > > > From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > > > > > > > > As a utility function, add lsdir() which reads given > > > > directory and store entry name into a strlist. > > > > lsdir accepts a filter function so that user can > > > > filter out unneeded entries. > > > > > > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > > > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > > > > > > Thanks, applied. > > > > Had to fix it to build on some distros, like ubuntu 12.04: > > > > alldeps-ubuntu-12.04: FAIL > > util/util.h:228:38: error: declaration of 'dirname' shadows a global declaration [-Werror=shadow] > > cc1: all warnings being treated as errors > > mv: cannot stat `/tmp/build/perf/util/.db-export.o.tmp': No such file or directory > > make[3]: *** [/tmp/build/perf/util/db-export.o] Error 1 > > make[3]: *** Waiting for unfinished jobs.... > > make[2]: *** [util] Error 2 > > make[2]: *** [ui] Error 2 > > make[1]: *** [/tmp/build/perf/libperf-in.o] Error > > make: *** [install-bin] Error 2 > > > > Hmm, I didn't know that the dummy argument can shadow a global declaration. > I might be better to rename "dirname" to "dir_name". I used 'name', its a short function and the name (lsdir) should indicate that name is indeed the directory name. - Arnaldo
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Masami Hiramatsu <tipbot@zytor.com> |
|---|---|
| Date | 2016-04-27 17:40 +0200 |
| Subject | [tip:perf/core] perf tools: Add lsdir() helper to read a directory |
| Message-ID | <rszJf-3tA-3@gated-at.bofh.it> |
| In reply to | #1387254 |
Commit-ID: e1ce726e1db2522b4848b3acffb7ece12439517c
Gitweb: http://git.kernel.org/tip/e1ce726e1db2522b4848b3acffb7ece12439517c
Author: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Tue, 26 Apr 2016 18:02:42 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 26 Apr 2016 13:14:55 -0300
perf tools: Add lsdir() helper to read a directory
As a utility function, add lsdir() which reads given directory and store
entry name into a strlist. lsdir accepts a filter function so that user
can filter out unneeded entries.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20160426090242.11891.79014.stgit@devbox
[ Do not use the 'dirname' it is used in some distros ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/util.c | 34 ++++++++++++++++++++++++++++++++++
tools/perf/util/util.h | 3 +++
2 files changed, 37 insertions(+)
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index b7766c5..9473d46 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -117,6 +117,40 @@ int rm_rf(char *path)
return rmdir(path);
}
+/* A filter which removes dot files */
+bool lsdir_no_dot_filter(const char *name __maybe_unused, struct dirent *d)
+{
+ return d->d_name[0] != '.';
+}
+
+/* lsdir reads a directory and store it in strlist */
+struct strlist *lsdir(const char *name,
+ bool (*filter)(const char *, struct dirent *))
+{
+ struct strlist *list = NULL;
+ DIR *dir;
+ struct dirent *d;
+
+ dir = opendir(name);
+ if (!dir)
+ return NULL;
+
+ list = strlist__new(NULL, NULL);
+ if (!list) {
+ errno = -ENOMEM;
+ goto out;
+ }
+
+ while ((d = readdir(dir)) != NULL) {
+ if (!filter || filter(name, d))
+ strlist__add(list, d->d_name);
+ }
+
+out:
+ closedir(dir);
+ return list;
+}
+
static int slow_copyfile(const char *from, const char *to)
{
int err = -1;
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 3bf3de8..26a9246 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -79,6 +79,7 @@
#include <termios.h>
#include <linux/bitops.h>
#include <termios.h>
+#include "strlist.h"
extern const char *graph_line;
extern const char *graph_dotted_line;
@@ -222,6 +223,8 @@ static inline int sane_case(int x, int high)
int mkdir_p(char *path, mode_t mode);
int rm_rf(char *path);
+struct strlist *lsdir(const char *name, bool (*filter)(const char *, struct dirent *));
+bool lsdir_no_dot_filter(const char *name, struct dirent *d);
int copyfile(const char *from, const char *to);
int copyfile_mode(const char *from, const char *to, mode_t mode);
int copyfile_offset(int fromfd, loff_t from_ofs, int tofd, loff_t to_ofs, u64 size);
[toc] | [prev] | [next] | [standalone]
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2016-04-26 11:10 +0200 |
| Subject | [PATCH perf/core v4 15/19] perf probe: Accept %sdt and %cached event name |
| Message-ID | <rs7ak-4SP-39@gated-at.bofh.it> |
| In reply to | #1387248 |
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
To improbe usability, support %[PROVIDER:]SDTEVENT format to
add new probes on SDT and cached events.
e.g.
----
# perf probe -x /lib/libc-2.17.so %lll_lock_wait_private
Added new event:
sdt_libc:lll_lock_wait_private (on %lll_lock_wait_private in
/usr/lib/libc-2.17.so)
You can now use it in all perf tools, such as:
perf record -e sdt_libc:lll_lock_wait_private -aR sleep 1
# perf probe -l | more
sdt_libc:lll_lock_wait_private (on __lll_lock_wait_private+21
in /usr/lib/libc-2.17.so)
----
Note that this is not only for SDT events, but also normal
events with event-name.
e.g. define "myevent" on cache (-n doesn't add the real probe)
----
# perf probe -x ./perf --cache -n --add 'myevent=dso__load $params'
----
Reuse the "myevent" from cache as below.
----
# perf probe -x ./perf %myevent
----
TODO:
Wildcard is not supported yet.
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
tools/perf/Documentation/perf-probe.txt | 3 +
tools/perf/util/probe-event.c | 78 ++++++++++++++++++++++---------
tools/perf/util/probe-event.h | 1
tools/perf/util/probe-file.c | 9 ++++
4 files changed, 69 insertions(+), 22 deletions(-)
diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index 7a258e9..43523be 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -151,6 +151,8 @@ Probe points are defined by following syntax.
3) Define event based on source file with lazy pattern
[[GROUP:]EVENT=]SRC;PTN [ARG ...]
+ 4) Pre-defined SDT events
+ %[PROVIDER:]SDTEVENT
'EVENT' specifies the name of new event, if omitted, it will be set the name of the probed function. You can also specify a group name by 'GROUP', if omitted, set 'probe' is used for kprobe and 'probe_<bin>' is used for uprobe.
Note that using existing group name can conflict with other events. Especially, using the group name reserved for kernel modules can hide embedded events in the
@@ -158,6 +160,7 @@ modules.
'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).
+'SDTEVENT' and 'PROVIDER' is the pre-defined event name which is defined by user SDT (Statically Defined Tracing) or the pre-cached probes with event name.
PROBE ARGUMENT
--------------
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 934797e..1128631 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1144,6 +1144,34 @@ err:
return err;
}
+static int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev)
+{
+ char *ptr;
+
+ ptr = strchr(*arg, ':');
+ if (ptr) {
+ *ptr = '\0';
+ if (!is_c_func_name(*arg))
+ goto ng_name;
+ pev->group = strdup(*arg);
+ if (!pev->group)
+ return -ENOMEM;
+ *arg = ptr + 1;
+ } else
+ pev->group = NULL;
+ if (!is_c_func_name(*arg)) {
+ng_name:
+ semantic_error("%s is bad for event name -it must "
+ "follow C symbol-naming rule.\n", *arg);
+ return -EINVAL;
+ }
+ pev->event = strdup(*arg);
+ if (pev->event == NULL)
+ return -ENOMEM;
+
+ return 0;
+}
+
/* Parse probepoint definition. */
static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
{
@@ -1151,38 +1179,43 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
char *ptr, *tmp;
char c, nc = 0;
bool file_spec = false;
+ int ret;
+
/*
* <Syntax>
* perf probe [GRP:][EVENT=]SRC[:LN|;PTN]
* perf probe [GRP:][EVENT=]FUNC[@SRC][+OFFS|%return|:LN|;PAT]
+ * perf probe %[GRP:]SDT_EVENT
*/
if (!arg)
return -EINVAL;
+ if (arg[0] == '%') {
+ pev->sdt = true;
+ arg++;
+ }
+
ptr = strpbrk(arg, ";=@+%");
- if (ptr && *ptr == '=') { /* Event name */
- *ptr = '\0';
- tmp = ptr + 1;
- ptr = strchr(arg, ':');
+ if (pev->sdt) {
if (ptr) {
- *ptr = '\0';
- if (!is_c_func_name(arg))
- goto not_fname;
- pev->group = strdup(arg);
- if (!pev->group)
- return -ENOMEM;
- arg = ptr + 1;
- } else
- pev->group = NULL;
- if (!is_c_func_name(arg)) {
-not_fname:
- semantic_error("%s is bad for event name -it must "
- "follow C symbol-naming rule.\n", arg);
+ semantic_error("%s must contain only an SDT event name.\n", arg);
return -EINVAL;
}
- pev->event = strdup(arg);
- if (pev->event == NULL)
- return -ENOMEM;
+ ret = parse_perf_probe_event_name(&arg, pev);
+ if (ret == 0) {
+ if (asprintf(&pev->point.function, "%%%s", pev->event) < 0)
+ ret = -errno;
+ }
+ return ret;
+ }
+
+ if (ptr && *ptr == '=') { /* Event name */
+ *ptr = '\0';
+ tmp = ptr + 1;
+ ret = parse_perf_probe_event_name(&arg, pev);
+ if (ret < 0)
+ return ret;
+
arg = tmp;
}
@@ -2783,7 +2816,8 @@ static int find_probe_trace_events_from_cache(struct perf_probe_event *pev,
pev->point.function);
}
if (!entry) {
- ret = 0;
+ /* SDT must be in the cache */
+ ret = pev->sdt ? -ENOENT : 0;
goto out;
}
@@ -2822,7 +2856,7 @@ static int convert_to_probe_trace_events(struct perf_probe_event *pev,
{
int ret;
- if (!pev->group) {
+ if (!pev->group && !pev->sdt) {
/* Set group name if not given */
if (!pev->uprobes) {
pev->group = strdup(PERFPROBE_GROUP);
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index c451223..2a23efe 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -85,6 +85,7 @@ struct perf_probe_event {
char *group; /* Group name */
struct perf_probe_point point; /* Probe point */
int nargs; /* Number of arguments */
+ bool sdt; /* SDT/cached event flag */
bool uprobes; /* Uprobe event flag */
char *target; /* Target binary */
struct perf_probe_arg *args; /* Arguments */
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index c10a647..ee560dd 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -529,6 +529,15 @@ probe_cache__find(struct probe_cache *pcache, struct perf_probe_event *pev)
return NULL;
list_for_each_entry(entry, &pcache->list, list) {
+ if (pev->sdt) {
+ if (entry->pev.event &&
+ streql(entry->pev.event, pev->event) &&
+ (!pev->group ||
+ streql(entry->pev.group, pev->group)))
+ goto found;
+
+ continue;
+ }
/* Hit if same event name or same command-string */
if ((pev->event &&
(streql(entry->pev.group, pev->group) &&
[toc] | [prev] | [next] | [standalone]
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2016-04-26 11:10 +0200 |
| Subject | [PATCH perf/core v4 19/19] perf probe: Support @BUILDID or @FILE suffix for SDT events |
| Message-ID | <rs7ak-4SP-41@gated-at.bofh.it> |
| In reply to | #1387248 |
Support @BUILDID or @FILE suffix for SDT events. This allows
perf to add probes on SDTs/pre-cached events on given FILE
or the file which has given BUILDID (also, this complements
BUILDID.)
For example, both gcc and libstdc++ has same SDTs as below.
If you would like to add a probe on sdt_libstdcxx:catch on gcc,
you can do as below.
----
# perf list sdt | tail -n 6
sdt_libstdcxx:catch@/usr/bin/gcc(0cc207fc4b27) [SDT event]
sdt_libstdcxx:catch@/usr/lib64/libstdc++.so.6.0.20(91c7a88fdf49)
sdt_libstdcxx:rethrow@/usr/bin/gcc(0cc207fc4b27) [SDT event]
sdt_libstdcxx:rethrow@/usr/lib64/libstdc++.so.6.0.20(91c7a88fdf49)
sdt_libstdcxx:throw@/usr/bin/gcc(0cc207fc4b27) [SDT event]
sdt_libstdcxx:throw@/usr/lib64/libstdc++.so.6.0.20(91c7a88fdf49)
# perf probe -a sdt_libstdcxx:catch@0cc
Added new event:
sdt_libstdcxx:catch (on %catch in /usr/bin/gcc)
You can now use it in all perf tools, such as:
perf record -e sdt_libstdcxx:catch -aR sleep 1
----
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
tools/perf/util/build-id.c | 40 ++++++++++++++++++++++++++++++++++++++++
tools/perf/util/build-id.h | 1 +
tools/perf/util/probe-event.c | 17 +++++++++++++++--
3 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index c849253..4fec09e 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -472,6 +472,46 @@ out:
return ret;
}
+static bool str_is_build_id(const char *maybe_sbuild_id, size_t len)
+{
+ size_t i;
+
+ for (i = 0; i < len; i++) {
+ if (!isxdigit(maybe_sbuild_id[i]))
+ return false;
+ }
+ return true;
+}
+
+/* Return the valid complete build-id */
+char *build_id_cache__complement(const char *incomplete_sbuild_id)
+{
+ struct strlist *list;
+ struct str_node *nd, *cand = NULL;
+ char *sbuild_id = NULL;
+ size_t len = strlen(incomplete_sbuild_id);
+
+ if (len >= SBUILD_ID_SIZE ||
+ !str_is_build_id(incomplete_sbuild_id, len) ||
+ build_id_cache__list_all(&list, true) < 0)
+ return NULL;
+
+ strlist__for_each(nd, list) {
+ if (strncmp(nd->s, incomplete_sbuild_id, len) != 0)
+ continue;
+ if (cand) { /* Error: There are more than 2 candidates. */
+ cand = NULL;
+ break;
+ }
+ cand = nd;
+ }
+ if (cand)
+ sbuild_id = strdup(cand->s);
+ strlist__delete(list);
+
+ return sbuild_id;
+}
+
char *build_id_cache__dirname_from_path(const char *sbuild_id, const char *name,
bool is_kallsyms, bool is_vdso)
{
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index 5e29cfc..cafab5e 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -33,6 +33,7 @@ char *build_id_cache__linkname(const char *sbuild_id, char *bf, size_t size);
char *build_id_cache__dirname_from_path(const char *sbuild_id, const char *name,
bool is_kallsyms, bool is_vdso);
int build_id_cache__list_all(struct strlist **result, bool validonly);
+char *build_id_cache__complement(const char *incomplete_sbuild_id);
int build_id_cache__list_build_ids(const char *pathname,
struct strlist **result);
bool build_id_cache__cached(const char *sbuild_id);
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 3bd81f0..bb9fc34 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1198,8 +1198,21 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
ptr = strpbrk(arg, ";=@+%");
if (pev->sdt) {
if (ptr) {
- semantic_error("%s must contain only an SDT event name.\n", arg);
- return -EINVAL;
+ if (*ptr != '@') {
+ semantic_error("%s must contain only an SDT event name.\n", arg);
+ return -EINVAL;
+ }
+ /* This must be a target file name or build id */
+ tmp = build_id_cache__complement(ptr + 1);
+ if (tmp) {
+ pev->target = build_id_cache__origname(tmp);
+ free(tmp);
+ } else
+ pev->target = strdup(ptr + 1);
+ if (!pev->target)
+ return -ENOMEM;
+ pev->uprobes = 1;
+ *ptr = '\0';
}
ret = parse_perf_probe_event_name(&arg, pev);
if (ret == 0) {
[toc] | [prev] | [next] | [standalone]
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2016-04-26 11:10 +0200 |
| Subject | [PATCH perf/core v4 18/19] perf probe: Allow wildcard for cached events |
| Message-ID | <rs7ak-4SP-37@gated-at.bofh.it> |
| In reply to | #1387248 |
Allo glob wildcard for reusing cached/SDT events. This also
automatically find the target binaries, e.g.
# perf probe -a %sdt_libc:\*
This example adds probes for all SDT in libc.
Note that the SDTs must have been scanned by perf buildid-cache.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
tools/perf/util/probe-event.c | 153 ++++++++++++++++++++++++++++++++++++++++-
tools/perf/util/probe-event.h | 1
tools/perf/util/probe-file.c | 33 ++++++++-
tools/perf/util/probe-file.h | 5 +
4 files changed, 183 insertions(+), 9 deletions(-)
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 1128631..3bd81f0 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -236,7 +236,7 @@ static void clear_perf_probe_point(struct perf_probe_point *pp)
free(pp->lazy_line);
}
-static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
+void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
{
int i;
@@ -1151,7 +1151,7 @@ static int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev)
ptr = strchr(*arg, ':');
if (ptr) {
*ptr = '\0';
- if (!is_c_func_name(*arg))
+ if (!pev->sdt && !is_c_func_name(*arg))
goto ng_name;
pev->group = strdup(*arg);
if (!pev->group)
@@ -1159,7 +1159,7 @@ static int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev)
*arg = ptr + 1;
} else
pev->group = NULL;
- if (!is_c_func_name(*arg)) {
+ if (!pev->sdt && !is_c_func_name(*arg)) {
ng_name:
semantic_error("%s is bad for event name -it must "
"follow C symbol-naming rule.\n", *arg);
@@ -1585,6 +1585,11 @@ int parse_probe_trace_command(const char *cmd, struct probe_trace_event *tev)
p = strchr(argv[1], ':');
if (p) {
tp->module = strndup(argv[1], p - argv[1]);
+ if (!tp->module) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ tev->uprobes = (tp->module[0] == '/');
p++;
} else
p = argv[1];
@@ -2430,7 +2435,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
int ret;
/* If probe_event or trace_event already have the name, reuse it */
- if (pev->event)
+ if (pev->event && !pev->sdt)
event = pev->event;
else if (tev->event)
event = tev->event;
@@ -2443,7 +2448,7 @@ static int probe_trace_event__set_name(struct probe_trace_event *tev,
else
event = tev->point.realname;
}
- if (pev->group)
+ if (pev->group && !pev->sdt)
group = pev->group;
else if (tev->group)
group = tev->group;
@@ -2794,6 +2799,137 @@ errout:
bool __weak arch__prefers_symtab(void) { return false; }
+/* Concatinate two arrays */
+static void *memcat(void *a, size_t sz_a, void *b, size_t sz_b)
+{
+ void *ret;
+
+ ret = malloc(sz_a + sz_b);
+ if (ret) {
+ memcpy(ret, a, sz_a);
+ memcpy(ret + sz_a, b, sz_b);
+ }
+ return ret;
+}
+
+static int
+concat_probe_trace_events(struct probe_trace_event **tevs, int *ntevs,
+ struct probe_trace_event **tevs2, int ntevs2)
+{
+ struct probe_trace_event *new_tevs;
+ int ret = 0;
+
+ if (ntevs == 0) {
+ *tevs = *tevs2;
+ *ntevs = ntevs2;
+ *tevs2 = NULL;
+ return 0;
+ }
+
+ if (*ntevs + ntevs2 > probe_conf.max_probes)
+ ret = -E2BIG;
+ else {
+ /* Concatinate the array of probe_trace_event */
+ new_tevs = memcat(*tevs, (*ntevs) * sizeof(**tevs),
+ *tevs2, ntevs2 * sizeof(**tevs2));
+ if (!new_tevs)
+ ret = -ENOMEM;
+ else {
+ free(*tevs);
+ *tevs = new_tevs;
+ *ntevs += ntevs2;
+ }
+ }
+ if (ret < 0)
+ clear_probe_trace_events(*tevs2, ntevs2);
+ zfree(tevs2);
+
+ return ret;
+}
+
+/* Try to find probe_trace_event from given probe caches */
+static int find_cached_events(struct perf_probe_event *pev,
+ struct probe_trace_event **tevs,
+ const char *target)
+{
+ struct probe_cache *cache;
+ struct probe_cache_entry *entry;
+ struct probe_trace_event *tmp_tevs = NULL;
+ int ntevs = 0;
+ int ret = 0;
+
+ cache = probe_cache__new(target);
+ if (!cache)
+ return -ENOENT;
+
+ for_each_probe_cache_entry(entry, cache) {
+ /* Skip the cache entry which has no name */
+ if (!entry->pev.event || !entry->pev.group)
+ continue;
+ if (strglobmatch(entry->pev.group, pev->group) &&
+ strglobmatch(entry->pev.event, pev->event)) {
+ ret = probe_cache_entry__get_event(entry, &tmp_tevs);
+ if (ret > 0)
+ ret = concat_probe_trace_events(tevs, &ntevs,
+ &tmp_tevs, ret);
+ if (ret < 0)
+ break;
+ }
+ }
+ probe_cache__delete(cache);
+ if (ret < 0) {
+ clear_probe_trace_events(*tevs, ntevs);
+ zfree(tevs);
+ } else {
+ ret = ntevs;
+ if (target[0] == '/')
+ pev->uprobes = true;
+ }
+
+ return ret;
+}
+
+/* Try to find probe_trace_event from all probe caches */
+static int find_cached_events_all(struct perf_probe_event *pev,
+ struct probe_trace_event **tevs)
+{
+ struct probe_trace_event *tmp_tevs = NULL;
+ struct strlist *bidlist;
+ struct str_node *nd;
+ char *pathname;
+ int ntevs = 0;
+ int ret;
+
+ /* Get the buildid list of all valid caches */
+ ret = build_id_cache__list_all(&bidlist, true);
+ if (ret < 0) {
+ pr_debug("Failed to get buildids: %d\n", ret);
+ return ret;
+ }
+
+ ret = 0;
+ strlist__for_each(nd, bidlist) {
+ pathname = build_id_cache__origname(nd->s);
+ ret = find_cached_events(pev, &tmp_tevs, pathname);
+ if (ret > 0)
+ ret = concat_probe_trace_events(tevs, &ntevs,
+ &tmp_tevs, ret);
+ /* In the case of cnt == 0, we just skip it */
+ free(pathname);
+ if (ret < 0)
+ break;
+ }
+ strlist__delete(bidlist);
+
+ if (ret < 0) {
+ clear_probe_trace_events(*tevs, ntevs);
+ zfree(tevs);
+ } else
+ ret = ntevs;
+
+ return ret;
+}
+
static int find_probe_trace_events_from_cache(struct perf_probe_event *pev,
struct probe_trace_event **tevs)
{
@@ -2803,6 +2939,13 @@ static int find_probe_trace_events_from_cache(struct perf_probe_event *pev,
struct str_node *node;
int ret, i;
+ if (pev->sdt) {
+ /* For SDT/cached events, we use special search functions */
+ if (!pev->target)
+ return find_cached_events_all(pev, tevs);
+ else
+ return find_cached_events(pev, tevs, pev->target);
+ }
cache = probe_cache__new(pev->target);
if (!cache)
return 0;
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 2a23efe..39b5a35 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -134,6 +134,7 @@ bool perf_probe_event_need_dwarf(struct perf_probe_event *pev);
/* Release event contents */
void clear_perf_probe_event(struct perf_probe_event *pev);
void clear_probe_trace_event(struct probe_trace_event *tev);
+void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs);
/* Command string to line-range */
int parse_line_range_desc(const char *cmd, struct line_range *lr);
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 2437b48..896d645 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -360,6 +360,31 @@ probe_cache_entry__new(struct perf_probe_event *pev)
return ret;
}
+int probe_cache_entry__get_event(struct probe_cache_entry *entry,
+ struct probe_trace_event **tevs)
+{
+ struct probe_trace_event *tev;
+ struct str_node *node;
+ int ret, i;
+
+ ret = strlist__nr_entries(entry->tevlist);
+ if (ret > probe_conf.max_probes)
+ return -E2BIG;
+
+ *tevs = zalloc(ret * sizeof(*tev));
+ if (!*tevs)
+ return -ENOMEM;
+
+ i = 0;
+ strlist__for_each(node, entry->tevlist) {
+ tev = &(*tevs)[i++];
+ ret = parse_probe_trace_command(node->s, tev);
+ if (ret < 0)
+ break;
+ }
+ return i;
+}
+
/* For the kernel probe caches, pass target = NULL */
static int probe_cache__open(struct probe_cache *pcache, const char *target)
{
@@ -528,7 +553,7 @@ probe_cache__find(struct probe_cache *pcache, struct perf_probe_event *pev)
if (!cmd)
return NULL;
- list_for_each_entry(entry, &pcache->list, list) {
+ for_each_probe_cache_entry(entry, pcache) {
if (pev->sdt) {
if (entry->pev.event &&
streql(entry->pev.event, pev->event) &&
@@ -558,7 +583,7 @@ probe_cache__find_by_name(struct probe_cache *pcache,
{
struct probe_cache_entry *entry = NULL;
- list_for_each_entry(entry, &pcache->list, list) {
+ for_each_probe_cache_entry(entry, pcache) {
/* Hit if same event name or same command-string */
if (streql(entry->pev.group, group) &&
streql(entry->pev.event, event))
@@ -711,7 +736,7 @@ int probe_cache__commit(struct probe_cache *pcache)
if (ret < 0)
goto out;
- list_for_each_entry(entry, &pcache->list, list) {
+ for_each_probe_cache_entry(entry, pcache) {
ret = probe_cache_entry__write(entry, pcache->fd);
pr_debug("Cache committed: %d\n", ret);
if (ret < 0)
@@ -750,7 +775,7 @@ static int probe_cache__show_entries(struct probe_cache *pcache,
{
struct probe_cache_entry *entry;
- list_for_each_entry(entry, &pcache->list, list) {
+ for_each_probe_cache_entry(entry, pcache) {
if (probe_cache_entry__compare(entry, filter))
printf("%s\n", entry->spev);
}
diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index 55322f1..a02bbbd 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -32,6 +32,11 @@ struct probe_cache {
struct list_head list;
};
+int probe_cache_entry__get_event(struct probe_cache_entry *entry,
+ struct probe_trace_event **tevs);
+#define for_each_probe_cache_entry(entry, pcache) \
+ list_for_each_entry(entry, &pcache->list, list)
+
struct probe_cache *probe_cache__new(const char *target);
int probe_cache__add_entry(struct probe_cache *pcache,
struct perf_probe_event *pev,
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web