* [PATCH v4 1/5] perf/sdt: ELF support for SDT
2014-11-02 12:40 [PATCH v4 0/5] perf/sdt: SDT events listing/probing Hemant Kumar
@ 2014-11-02 12:41 ` Hemant Kumar
2014-11-02 12:42 ` [PATCH v4 3/5] perf/sdt: Show SDT cache contents Hemant Kumar
` (3 subsequent siblings)
4 siblings, 0 replies; 49+ messages in thread
From: Hemant Kumar @ 2014-11-02 12:41 UTC (permalink / raw)
To: linux-kernel
Cc: srikar, peterz, oleg, hegdevasant, mingo, anton, systemtap,
namhyung, masami.hiramatsu.pt, aravinda, penberg
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>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/symbol-elf.c | 252 ++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/symbol.h | 21 ++++
2 files changed, 273 insertions(+)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 1e23a5b..800bc97 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1677,6 +1677,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 eb2c19b..ed4ecfa 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -313,4 +313,25 @@ int compare_proc_modules(const char *from, const char *to);
int setup_list(struct strlist **list, const char *list_str,
const char *list_name);
+/* 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 */
^ permalink raw reply [flat|nested] 49+ messages in thread* [PATCH v4 3/5] perf/sdt: Show SDT cache contents
2014-11-02 12:40 [PATCH v4 0/5] perf/sdt: SDT events listing/probing Hemant Kumar
2014-11-02 12:41 ` [PATCH v4 1/5] perf/sdt: ELF support for SDT Hemant Kumar
@ 2014-11-02 12:42 ` Hemant Kumar
2014-11-02 12:42 ` [PATCH v4 4/5] perf/sdt: Delete SDT events from cache Hemant Kumar
` (2 subsequent siblings)
4 siblings, 0 replies; 49+ messages in thread
From: Hemant Kumar @ 2014-11-02 12:42 UTC (permalink / raw)
To: linux-kernel
Cc: srikar, peterz, oleg, hegdevasant, mingo, anton, systemtap,
namhyung, masami.hiramatsu.pt, aravinda, penberg
This patch adds support to dump the SDT cache onto sdtout.
The cache data is read into a hash_list and then, it iterates through
the hash_list to dump the data onto stdout.
# ./perf sdt-cache --dump
/usr/lib64/libc-2.16.so:
%libc:lll_futex_wake
%libc:longjmp_target
%libc:longjmp
%libc:lll_lock_wait_private
%libc:lll_futex_wake
%libc:longjmp_target
%libc:longjmp
%libc:setjmp
Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
---
tools/perf/Documentation/perf-sdt-cache.txt | 7 +++-
tools/perf/builtin-sdt-cache.c | 28 +++++++++++++++-
tools/perf/util/parse-events.h | 1 +
tools/perf/util/sdt.c | 48 +++++++++++++++++++++++++++
4 files changed, 82 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Documentation/perf-sdt-cache.txt b/tools/perf/Documentation/perf-sdt-cache.txt
index 08b9985..e57a867 100644
--- a/tools/perf/Documentation/perf-sdt-cache.txt
+++ b/tools/perf/Documentation/perf-sdt-cache.txt
@@ -9,11 +9,13 @@ SYNOPSIS
--------
[verse]
'perf sdt-cache --add <file_name>'
+'perf sdt-cache --dump'
DESCRIPTION
-----------
This command manages the SDT events cache. It can add/remove SDT events
-associated with an ELF to the cache.
+associated with an ELF to the cache. It can also dump the SDT events' cache
+onto stdout.
OPTIONS
-------
@@ -21,6 +23,9 @@ OPTIONS
--add::
Add SDT events in the specified file to the cache. Takes file name
as an argument.
+-D::
+--dump::
+ Dump the contents of SDT events cache onto stdout.
SEE ALSO
--------
diff --git a/tools/perf/builtin-sdt-cache.c b/tools/perf/builtin-sdt-cache.c
index 0e012f6..11f41a2 100644
--- a/tools/perf/builtin-sdt-cache.c
+++ b/tools/perf/builtin-sdt-cache.c
@@ -16,6 +16,7 @@
/* Session management structure */
static struct {
bool add;
+ bool dump;
const char *target;
} params;
@@ -28,20 +29,37 @@ static int opt_add_sdt_events(const struct option *opt __maybe_unused,
return 0;
}
+static int opt_show_sdt_events(const struct option *opt __maybe_unused,
+ const char *str, int unset __maybe_unused)
+{
+ if (str)
+ pr_err("Unknown option %s\n", str);
+ params.dump = true;
+ return 0;
+}
+
int cmd_sdt_cache(int argc, const char **argv, const char *prefix __maybe_unused)
{
int ret;
- const struct option sdt_cache_options[] = {
+ struct option sdt_cache_options[] = {
OPT_CALLBACK('a', "add", NULL, "filename",
"add SDT events from a file.",
opt_add_sdt_events),
+ OPT_CALLBACK_NOOPT('D', "dump", NULL, "show SDT events",
+ "Read SDT events from cache and display.",
+ opt_show_sdt_events),
OPT_END()
};
+
const char * const sdt_cache_usage[] = {
"perf sdt-cache --add filename",
+ "perf sdt-cache --dump",
NULL
};
+ set_option_flag(sdt_cache_options, 'a', "add", PARSE_OPT_EXCLUSIVE);
+ set_option_flag(sdt_cache_options, 'D', "dump", PARSE_OPT_EXCLUSIVE);
+
argc = parse_options(argc, argv, sdt_cache_options,
sdt_cache_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
@@ -53,6 +71,14 @@ int cmd_sdt_cache(int argc, const char **argv, const char *prefix __maybe_unused
ret = add_sdt_events(params.target);
if (ret < 0)
pr_err("Cannot add SDT events to cache\n");
+ } else if (params.dump) {
+ if (argc == 0) {
+ ret = dump_sdt_events();
+ if (ret < 0)
+ pr_err("Cannot dump SDT event cache\n");
+ } else
+ usage_with_options(sdt_cache_usage, sdt_cache_options);
+
} else
usage_with_options(sdt_cache_usage, sdt_cache_options);
return 0;
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 8bde8ea..cca6859 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -124,5 +124,6 @@ extern int is_valid_tracepoint(const char *event_string);
extern int valid_debugfs_mount(const char *debugfs);
int add_sdt_events(const char *file);
+int dump_sdt_events(void);
#endif /* __PERF_PARSE_EVENTS_H */
diff --git a/tools/perf/util/sdt.c b/tools/perf/util/sdt.c
index 3b84355..5f70dd6 100644
--- a/tools/perf/util/sdt.c
+++ b/tools/perf/util/sdt.c
@@ -613,3 +613,51 @@ out:
file_hash_list__cleanup(&file_hash);
return ret;
}
+
+/**
+ * file_hash_list__display: Dump the entries of file_hash list
+ * @file_hash: file hash list
+ *
+ * Iterate through each of the entries and the chains and dump
+ * onto stdscr.
+ */
+static void file_hash_list__display(struct hash_table *file_hash)
+{
+ struct file_sdt_ent *file_pos;
+ struct list_head *sdt_head;
+ struct hlist_head *ent_head;
+ struct sdt_note *sdt_ptr;
+ int i;
+
+ /* Go through all entries */
+ for (i = 0; i < SDT_HASH_SIZE; i++) {
+ /* Obtain the list head */
+ ent_head = &file_hash->ent[i];
+
+ /* ':' are used here as delimiters */
+ hlist_for_each_entry(file_pos, ent_head, file_list) {
+ fprintf(stdout, "%s:\n", file_pos->name);
+ sdt_head = &file_pos->sdt_list;
+ list_for_each_entry(sdt_ptr, sdt_head, note_list) {
+ fprintf(stdout, "%4s%s:%s\n", "%",
+ sdt_ptr->provider, sdt_ptr->name);
+ }
+ printf("\n");
+ }
+ }
+}
+
+/**
+ * dump_sdt_events: Dump the SDT events on stdout
+ */
+int dump_sdt_events(void)
+{
+ struct hash_table file_hash;
+ int ret;
+
+ ret = file_hash_list__init(&file_hash);
+ if (!ret)
+ file_hash_list__display(&file_hash);
+ file_hash_list__cleanup(&file_hash);
+ return ret;
+}
^ permalink raw reply [flat|nested] 49+ messages in thread* [PATCH v4 4/5] perf/sdt: Delete SDT events from cache
2014-11-02 12:40 [PATCH v4 0/5] perf/sdt: SDT events listing/probing Hemant Kumar
2014-11-02 12:41 ` [PATCH v4 1/5] perf/sdt: ELF support for SDT Hemant Kumar
2014-11-02 12:42 ` [PATCH v4 3/5] perf/sdt: Show SDT cache contents Hemant Kumar
@ 2014-11-02 12:42 ` Hemant Kumar
2014-11-02 12:42 ` [PATCH v4 2/5] perf/sdt: Add SDT events into a cache Hemant Kumar
2014-11-02 12:43 ` [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events Hemant Kumar
4 siblings, 0 replies; 49+ messages in thread
From: Hemant Kumar @ 2014-11-02 12:42 UTC (permalink / raw)
To: linux-kernel
Cc: srikar, peterz, oleg, hegdevasant, mingo, anton, systemtap,
namhyung, masami.hiramatsu.pt, aravinda, penberg
This patch adds the support to delete SDT events from the cache.
To delete an event corresponding to a file, first the cache is read into
the file_hash list. The key is calculated from the file name.
And then, the file_list for that file_hash entry is traversed to find out
the target file_list entry. Once, it is found, its contents are all freed up.
# ./perf sdt-cache --del /usr/lib64/libc-2.16.so
8 event(s) removed for /usr/lib64/libc-2.16.so
Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
---
tools/perf/Documentation/perf-sdt-cache.txt | 6 +++-
tools/perf/builtin-sdt-cache.c | 21 +++++++++++++-
tools/perf/util/parse-events.h | 1 +
tools/perf/util/sdt.c | 41 +++++++++++++++++++++++++++
4 files changed, 67 insertions(+), 2 deletions(-)
diff --git a/tools/perf/Documentation/perf-sdt-cache.txt b/tools/perf/Documentation/perf-sdt-cache.txt
index e57a867..523dcda 100644
--- a/tools/perf/Documentation/perf-sdt-cache.txt
+++ b/tools/perf/Documentation/perf-sdt-cache.txt
@@ -8,7 +8,7 @@ perf-sdt-cache - Manage SDT events' cache.
SYNOPSIS
--------
[verse]
-'perf sdt-cache --add <file_name>'
+'perf sdt-cache [--add | --del] <file_name>'
'perf sdt-cache --dump'
DESCRIPTION
@@ -23,6 +23,10 @@ OPTIONS
--add::
Add SDT events in the specified file to the cache. Takes file name
as an argument.
+-d::
+--del::
+ Remove SDT events in the specified file from the cache. Takes file as an
+ argument.
-D::
--dump::
Dump the contents of SDT events cache onto stdout.
diff --git a/tools/perf/builtin-sdt-cache.c b/tools/perf/builtin-sdt-cache.c
index 11f41a2..b35d844 100644
--- a/tools/perf/builtin-sdt-cache.c
+++ b/tools/perf/builtin-sdt-cache.c
@@ -16,6 +16,7 @@
/* Session management structure */
static struct {
bool add;
+ bool del;
bool dump;
const char *target;
} params;
@@ -29,6 +30,16 @@ static int opt_add_sdt_events(const struct option *opt __maybe_unused,
return 0;
}
+static int opt_del_sdt_events(const struct option *opt __maybe_unused,
+ const char *str, int unset __maybe_unused)
+{
+ params.del = true;
+ params.target = str;
+
+ return 0;
+}
+
+
static int opt_show_sdt_events(const struct option *opt __maybe_unused,
const char *str, int unset __maybe_unused)
{
@@ -45,6 +56,9 @@ int cmd_sdt_cache(int argc, const char **argv, const char *prefix __maybe_unused
OPT_CALLBACK('a', "add", NULL, "filename",
"add SDT events from a file.",
opt_add_sdt_events),
+ OPT_CALLBACK('d', "del", NULL, "filename",
+ "Remove SDT events corresponding to a file from the sdt cache.",
+ opt_del_sdt_events),
OPT_CALLBACK_NOOPT('D', "dump", NULL, "show SDT events",
"Read SDT events from cache and display.",
opt_show_sdt_events),
@@ -52,13 +66,14 @@ int cmd_sdt_cache(int argc, const char **argv, const char *prefix __maybe_unused
};
const char * const sdt_cache_usage[] = {
- "perf sdt-cache --add filename",
+ "perf sdt-cache [--add | --del] filename",
"perf sdt-cache --dump",
NULL
};
set_option_flag(sdt_cache_options, 'a', "add", PARSE_OPT_EXCLUSIVE);
set_option_flag(sdt_cache_options, 'D', "dump", PARSE_OPT_EXCLUSIVE);
+ set_option_flag(sdt_cache_options, 'd', "del", PARSE_OPT_EXCLUSIVE);
argc = parse_options(argc, argv, sdt_cache_options,
sdt_cache_usage,
@@ -71,6 +86,10 @@ int cmd_sdt_cache(int argc, const char **argv, const char *prefix __maybe_unused
ret = add_sdt_events(params.target);
if (ret < 0)
pr_err("Cannot add SDT events to cache\n");
+ } else if (params.del) {
+ ret = remove_sdt_events(params.target);
+ if (ret < 0)
+ pr_err("Cannot remove SDT events from cache\n");
} else if (params.dump) {
if (argc == 0) {
ret = dump_sdt_events();
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index cca6859..cafdab6 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -125,5 +125,6 @@ extern int valid_debugfs_mount(const char *debugfs);
int add_sdt_events(const char *file);
int dump_sdt_events(void);
+int remove_sdt_events(const char *str);
#endif /* __PERF_PARSE_EVENTS_H */
diff --git a/tools/perf/util/sdt.c b/tools/perf/util/sdt.c
index 5f70dd6..9dd7e4da 100644
--- a/tools/perf/util/sdt.c
+++ b/tools/perf/util/sdt.c
@@ -192,6 +192,7 @@ static int file_hash_list__remove(struct hash_table *file_hash,
nr_del = cleanup_sdt_note_list(&fse->sdt_list);
hlist_del(&fse->file_list);
free(fse);
+ break;
}
free(res_path);
return nr_del;
@@ -661,3 +662,43 @@ int dump_sdt_events(void)
file_hash_list__cleanup(&file_hash);
return ret;
}
+
+/**
+ * remove_sdt_events: remove SDT events from cache
+ * @str: filename
+ *
+ * Removes the SDT events from the cache corresponding to the file name
+ * @str.
+ */
+int remove_sdt_events(const char *str)
+{
+ struct hash_table file_hash;
+ char *res_path;
+ int nr_del, ret = -ENOMEM;
+
+ res_path = realpath(str, NULL);
+ if (!res_path) {
+ pr_err("%s: Bad file name\n", str);
+ goto out_err;
+ }
+ /* Initialize the hash_lists */
+ ret = file_hash_list__init(&file_hash);
+ if (ret < 0)
+ goto out;
+
+ /* Remove the file_list entry from file_hash and update the file_hash */
+ nr_del = file_hash_list__remove(&file_hash, res_path);
+ if (nr_del > 0) {
+ printf("%4d event(s) removed for %s\n", nr_del, res_path);
+ flush_hash_list_to_cache(&file_hash);
+ ret = nr_del;
+ goto out;
+ } else if (!nr_del)
+ pr_err("Error: Events for %s not found\n", str);
+
+out:
+ free(res_path);
+ file_hash_list__cleanup(&file_hash);
+out_err:
+ return ret;
+}
^ permalink raw reply [flat|nested] 49+ messages in thread* [PATCH v4 2/5] perf/sdt: Add SDT events into a cache
2014-11-02 12:40 [PATCH v4 0/5] perf/sdt: SDT events listing/probing Hemant Kumar
` (2 preceding siblings ...)
2014-11-02 12:42 ` [PATCH v4 4/5] perf/sdt: Delete SDT events from cache Hemant Kumar
@ 2014-11-02 12:42 ` Hemant Kumar
2014-11-02 12:43 ` [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events Hemant Kumar
4 siblings, 0 replies; 49+ messages in thread
From: Hemant Kumar @ 2014-11-02 12:42 UTC (permalink / raw)
To: linux-kernel
Cc: srikar, peterz, oleg, hegdevasant, mingo, anton, systemtap,
namhyung, masami.hiramatsu.pt, aravinda, penberg
This patch adds a new sub-command to perf : sdt-cache.
sdt-cache command can be used to add SDT events.
When user invokes "perf sdt-cache add <file-name>", a hash table/list is
created named as file_hash list. A typical entry in a file_hash table looks
like:
file hash file_sdt_ent file_sdt_ent
|---------| --------------- -------------
| hlist ==|===|=>file_list =|===|=>file_list=|==..
key = 644 =>| | | sbuild_id | | sbuild_id |
|---------| | name | | name |
| | | sdt_list | | sdt_list |
key = 645 =>| hlist | | || | | || |
|---------| --------------- --------------
|| || || Connected to SDT notes
---------------
| note_list |
| name |sdt_note
| provider |
-----||--------
connected to other SDT notes
Each entry of the file_hash table is an hlist which connects to file_list
in file_sdt_ent. file_sdt_ent is allocated per file whenever a file is
mapped to file_hash list. File name serves as the key to this hash table.
If a file is added to this hash list, a file_sdt_ent is allocated and a
list of SDT events in that file is created and assigned to sdt_list of
file_sdt_ent.
Example usage :
# ./perf sdt-cache --add /home/user_app
4 events added for /home/user_app
# ./perf sdt-cache --add /lib64/libc.so.6
8 events added for /usr/lib64/libc-2.16.so
Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
---
tools/perf/Documentation/perf-sdt-cache.txt | 27 +
tools/perf/Makefile.perf | 4
tools/perf/builtin-sdt-cache.c | 59 +++
tools/perf/builtin.h | 1
tools/perf/command-list.txt | 1
tools/perf/perf.c | 1
tools/perf/util/parse-events.h | 2
tools/perf/util/sdt.c | 615 +++++++++++++++++++++++++++
8 files changed, 710 insertions(+)
create mode 100644 tools/perf/Documentation/perf-sdt-cache.txt
create mode 100644 tools/perf/builtin-sdt-cache.c
create mode 100644 tools/perf/util/sdt.c
diff --git a/tools/perf/Documentation/perf-sdt-cache.txt b/tools/perf/Documentation/perf-sdt-cache.txt
new file mode 100644
index 0000000..08b9985
--- /dev/null
+++ b/tools/perf/Documentation/perf-sdt-cache.txt
@@ -0,0 +1,27 @@
+perf-sdt-cache(1)
+=====================
+
+NAME
+----
+perf-sdt-cache - Manage SDT events' cache.
+
+SYNOPSIS
+--------
+[verse]
+'perf sdt-cache --add <file_name>'
+
+DESCRIPTION
+-----------
+This command manages the SDT events cache. It can add/remove SDT events
+associated with an ELF to the cache.
+
+OPTIONS
+-------
+-a::
+--add::
+ Add SDT events in the specified file to the cache. Takes file name
+ as an argument.
+
+SEE ALSO
+--------
+linkperf:perf-record[1], linkperf:perf-report[1]
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 3caf7da..4f5c696 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -353,6 +353,7 @@ LIB_OBJS += $(OUTPUT)util/sigchain.o
LIB_OBJS += $(OUTPUT)util/dso.o
LIB_OBJS += $(OUTPUT)util/symbol.o
LIB_OBJS += $(OUTPUT)util/symbol-elf.o
+LIB_OBJS += $(OUTPUT)util/sdt.o
LIB_OBJS += $(OUTPUT)util/color.o
LIB_OBJS += $(OUTPUT)util/pager.o
LIB_OBJS += $(OUTPUT)util/header.o
@@ -472,6 +473,7 @@ BUILTIN_OBJS += $(OUTPUT)builtin-timechart.o
BUILTIN_OBJS += $(OUTPUT)builtin-top.o
BUILTIN_OBJS += $(OUTPUT)builtin-script.o
BUILTIN_OBJS += $(OUTPUT)builtin-probe.o
+BUILTIN_OBJS += $(OUTPUT)builtin-sdt-cache.o
BUILTIN_OBJS += $(OUTPUT)builtin-kmem.o
BUILTIN_OBJS += $(OUTPUT)builtin-lock.o
BUILTIN_OBJS += $(OUTPUT)builtin-kvm.o
@@ -499,8 +501,10 @@ LIB_OBJS := $(filter-out $(OUTPUT)util/symbol-elf.o,$(LIB_OBJS))
LIB_OBJS := $(filter-out $(OUTPUT)util/dwarf-aux.o,$(LIB_OBJS))
LIB_OBJS := $(filter-out $(OUTPUT)util/probe-event.o,$(LIB_OBJS))
LIB_OBJS := $(filter-out $(OUTPUT)util/probe-finder.o,$(LIB_OBJS))
+LIB_OBJS := $(filter-out $(OUTPUT)util/sdt.o,$(LIB_OBJS))
BUILTIN_OBJS := $(filter-out $(OUTPUT)builtin-probe.o,$(BUILTIN_OBJS))
+BUILTIN_OBJS := $(filter-out $(OUTPUT)builtin-sdt-cache.o,$(BUILTIN_OBJS))
# Use minimal symbol handling
LIB_OBJS += $(OUTPUT)util/symbol-minimal.o
diff --git a/tools/perf/builtin-sdt-cache.c b/tools/perf/builtin-sdt-cache.c
new file mode 100644
index 0000000..0e012f6
--- /dev/null
+++ b/tools/perf/builtin-sdt-cache.c
@@ -0,0 +1,59 @@
+/*
+ * builtin-sdt-cache.c
+ *
+ * Builtin sdt command: Add/remove/show SDT events
+ */
+#include "builtin.h"
+
+#include "perf.h"
+
+#include "util/parse-events.h"
+#include "util/cache.h"
+#include "util/parse-options.h"
+#include "symbol.h"
+#include "debug.h"
+
+/* Session management structure */
+static struct {
+ bool add;
+ const char *target;
+} params;
+
+static int opt_add_sdt_events(const struct option *opt __maybe_unused,
+ const char *str, int unset __maybe_unused)
+{
+ params.add = true;
+ params.target = str;
+
+ return 0;
+}
+
+int cmd_sdt_cache(int argc, const char **argv, const char *prefix __maybe_unused)
+{
+ int ret;
+ const struct option sdt_cache_options[] = {
+ OPT_CALLBACK('a', "add", NULL, "filename",
+ "add SDT events from a file.",
+ opt_add_sdt_events),
+ OPT_END()
+ };
+ const char * const sdt_cache_usage[] = {
+ "perf sdt-cache --add filename",
+ NULL
+ };
+
+ argc = parse_options(argc, argv, sdt_cache_options,
+ sdt_cache_usage,
+ PARSE_OPT_STOP_AT_NON_OPTION);
+
+ setup_pager();
+
+ symbol__elf_init();
+ if (params.add) {
+ ret = add_sdt_events(params.target);
+ if (ret < 0)
+ pr_err("Cannot add SDT events to cache\n");
+ } else
+ usage_with_options(sdt_cache_usage, sdt_cache_options);
+ return 0;
+}
diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
index b210d62..2746358 100644
--- a/tools/perf/builtin.h
+++ b/tools/perf/builtin.h
@@ -37,6 +37,7 @@ extern int cmd_test(int argc, const char **argv, const char *prefix);
extern int cmd_trace(int argc, const char **argv, const char *prefix);
extern int cmd_inject(int argc, const char **argv, const char *prefix);
extern int cmd_mem(int argc, const char **argv, const char *prefix);
+extern int cmd_sdt_cache(int argc, const char **argv, const char *prefix);
extern int find_scripts(char **scripts_array, char **scripts_path_array);
#endif
diff --git a/tools/perf/command-list.txt b/tools/perf/command-list.txt
index 0906fc4..e36047f 100644
--- a/tools/perf/command-list.txt
+++ b/tools/perf/command-list.txt
@@ -25,3 +25,4 @@ perf-test mainporcelain common
perf-timechart mainporcelain common
perf-top mainporcelain common
perf-trace mainporcelain common
+perf-sdt-cache mainporcelain full
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 452a847..8db763d 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -52,6 +52,7 @@ static struct cmd_struct commands[] = {
{ "sched", cmd_sched, 0 },
#ifdef HAVE_LIBELF_SUPPORT
{ "probe", cmd_probe, 0 },
+ { "sdt-cache", cmd_sdt_cache, 0 },
#endif
{ "kmem", cmd_kmem, 0 },
{ "lock", cmd_lock, 0 },
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index db2cf78..8bde8ea 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -123,4 +123,6 @@ extern int is_valid_tracepoint(const char *event_string);
extern int valid_debugfs_mount(const char *debugfs);
+int add_sdt_events(const char *file);
+
#endif /* __PERF_PARSE_EVENTS_H */
diff --git a/tools/perf/util/sdt.c b/tools/perf/util/sdt.c
new file mode 100644
index 0000000..3b84355
--- /dev/null
+++ b/tools/perf/util/sdt.c
@@ -0,0 +1,615 @@
+/*
+ * util/sdt.c
+ * This contains the relevant functions needed to find the SDT events
+ * in a binary and adding them to a cache.
+ *
+ * TODOS:
+ * - Listing SDT events in most of the binaries present in the system.
+ * - Looking into directories provided by the user for binaries with SDTs,
+ * etc.
+ * - Support SDT event arguments.
+ * - Support SDT event semaphores.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/stat.h>
+
+#include "parse-events.h"
+#include "probe-event.h"
+#include "linux/list.h"
+#include "symbol.h"
+#include "build-id.h"
+#include "debug.h"
+
+#include "build-id.h"
+
+#include <linux/bitops.h>
+#include <linux/hash.h>
+
+#define SDT_HASH_BITS 11
+#define SDT_HASH_SIZE (1 << SDT_HASH_BITS)
+#define HASH_PRIME_BASE 37
+#define SDT_CACHE_DIR ".debug"
+#define SDT_FILE_CACHE ".sdt-cache"
+
+#define DELIM ':'
+#define MAX_ADDR 17
+
+struct file_sdt_ent {
+ char name[PATH_MAX]; /* file name */
+ char sbuild_id[BUILD_ID_SIZE * 2 + 1]; /* Build id of the file */
+ struct hlist_node file_list;
+ struct list_head sdt_list; /* SDT notes in this file */
+};
+
+struct hash_table {
+ struct hlist_head ent[SDT_HASH_SIZE];
+};
+
+struct file_info {
+ char *name; /* File name */
+ char sbuild_id[BUILD_ID_SIZE * 2 + 1]; /* Build id of the file */
+};
+
+/**
+ * get_hash_key: calculates the key to hash tables
+ * @str: input string
+ *
+ * A simple basic function to calculate hash keys.
+ * adds the ascii values for all the chars in @str, multiplies with a prime
+ * and finds the modulo with SDT_HASH_SIZE.
+ *
+ * Return : An integer key
+ */
+static unsigned get_hash_key(const char *str)
+{
+ unsigned value = 0, key;
+ unsigned c;
+
+ for (c = 0; str[c] != '\0'; c++)
+ value += str[c];
+
+ key = (value * HASH_PRIME_BASE) % SDT_HASH_SIZE;
+
+ return key;
+}
+
+/**
+ * sdt_err: print SDT related error
+ * @val: error code
+ * @target: input file
+ *
+ * Display error corresponding to @val for file @target
+ */
+static int sdt_err(int val, const char *target)
+{
+ char buf[PATH_MAX];
+
+ switch (-val) {
+ case 0:
+ break;
+ case ENOENT:
+ /* Absence of SDT markers */
+ pr_err("%s: No SDT events found\n", target);
+ break;
+ case EBADF:
+ pr_err("%s: Bad file name\n", target);
+ break;
+ default:
+ pr_err("%s\n", strerror_r(val, buf, sizeof(buf)));
+ }
+
+ return val;
+}
+
+/**
+ * file_list_entry__init: Initialize a file_list_entry
+ * @fse: file_list_entry
+ * @file: file information
+ *
+ * Initializes @fse with the build id from @file.
+ */
+static void file_list_entry__init(struct file_sdt_ent *fse,
+ struct file_info *file)
+{
+ strcpy(fse->sbuild_id, file->sbuild_id);
+ INIT_HLIST_NODE(&fse->file_list);
+ INIT_LIST_HEAD(&fse->sdt_list);
+}
+
+/**
+ * file_hash_list__add: add an entry to file_hash_list
+ * @file_hash: hash table for file and sdt notes
+ * @sdt_notes: list of sdt_notes
+ * @file: struct having file name and build id
+ *
+ * Get the key corresponding to @file->name. Fetch the entry
+ * for that key. Build a file_list_entry fse, assign the SDT notes
+ * to it and then assign fse to the fetched entry into the hash.
+ */
+static int file_hash_list__add(struct hash_table *file_hash,
+ struct list_head *sdt_notes,
+ struct file_info *file)
+{
+ struct file_sdt_ent *fse;
+ struct hlist_head *head;
+ int nr_evs, key;
+
+ key = get_hash_key(file->name);
+ head = &file_hash->ent[key];
+
+ /* Initialize the file entry */
+ fse = malloc(sizeof(*fse));
+ if (!fse)
+ return -ENOMEM;
+ file_list_entry__init(fse, file);
+ nr_evs = sdt_notes__get_count(sdt_notes);
+ /* Add the sdt_notes list */
+ list_splice(sdt_notes, &fse->sdt_list);
+
+ strcpy(fse->name, file->name);
+ /* Add the file to the file hash entry */
+ hlist_add_head(&fse->file_list, head);
+
+ return nr_evs;
+}
+
+/**
+ * file_hash_list__remove: Remove a file entry from the file_hash table
+ * @file_hash: file_hash_table
+ * @target: file name
+ *
+ * Removes the entries from file_hash table corresponding to @target.
+ * Gets the key from @target. Also frees up the SDT events for that
+ * file.
+ */
+static int file_hash_list__remove(struct hash_table *file_hash,
+ const char *target)
+{
+ struct file_sdt_ent *fse;
+ struct hlist_head *ent_head;
+ struct hlist_node *tmp1;
+ char *res_path;
+ int key, nr_del = 0;
+
+ res_path = realpath(target, NULL);
+ if (!res_path)
+ return -ENOMEM;
+
+ key = get_hash_key(target);
+ ent_head = &file_hash->ent[key];
+
+ /* Got the file hash entry */
+ hlist_for_each_entry_safe(fse, tmp1, ent_head, file_list) {
+ if (strcmp(res_path, fse->name))
+ continue;
+
+ /* Got the file list entry, now start removing */
+ nr_del = cleanup_sdt_note_list(&fse->sdt_list);
+ hlist_del(&fse->file_list);
+ free(fse);
+ }
+ free(res_path);
+ return nr_del;
+}
+
+/**
+ * file_hash_list__entry_exists: Checks if a file is already present
+ * @file_hash: file_hash table
+ * @file: file name and build id to check
+ *
+ * Obtains the key from @file->name, fetches the ent[key] from file_hash,
+ * and goes through the chain to find out the correct file list entry.
+ * Compares the build id, if they match, return true, else, false.
+ */
+static bool file_hash_list__entry_exists(struct hash_table *file_hash,
+ struct file_info *file)
+{
+ struct file_sdt_ent *fse;
+ struct hlist_head *head;
+ int key;
+
+ key = get_hash_key(file->name);
+ head = &file_hash->ent[key];
+ hlist_for_each_entry(fse, head, file_list) {
+ if (!strcmp(file->name, fse->name))
+ if (!strcmp(file->sbuild_id, fse->sbuild_id))
+ return true;
+ }
+ return false;
+}
+
+/**
+ * sdt_note__read: Parse SDT note info
+ * @data: string containing the SDT note's info
+ * @sdt_list: empty list
+ *
+ * Parse @data to find out SDT note name, provider, location and semaphore.
+ * All these data are separated by DELIM:
+ * provider:marker_name:loc1:loc2
+ */
+static struct sdt_note *sdt_note__read(char *data)
+{
+ struct sdt_note *sn = NULL;
+ int value = 0;
+ char *loc = NULL, *sem = NULL;
+
+ sn = malloc(sizeof(*sn));
+ if (!sn) {
+ pr_err("Out of memory\n");
+ goto out_err;
+ }
+ sn->provider = NULL;
+ sn->name = NULL;
+ INIT_LIST_HEAD(&sn->note_list);
+
+ value = sscanf(data, "%m[^':']:%m[^':']:%m[^':']:%ms\n", &sn->provider,
+ &sn->name, &loc, &sem);
+ if (value != 4)
+ goto out_free_note;
+ value = sscanf(loc, "%lx", &sn->addr.a64[0]);
+ if (value != 1)
+ goto out_free_note;
+ value = sscanf(sem, "%lx", &sn->addr.a64[2]);
+ if (value != 1)
+ goto out_free_note;
+ goto out;
+
+out_free_note:
+ free(sn->provider);
+ free(sn->name);
+ free(sn);
+ sn = NULL;
+out:
+ free(loc);
+ free(sem);
+out_err:
+ return sn;
+}
+
+/**
+ * file_hash_list__populate: Fill up the file hash table
+ * @file_hash: empty file hash table
+ * @cache: FILE * to read from
+ *
+ * Parse @cache for file_name and its SDT events.
+ * The format of the cache is :
+ *
+ * file_name:build_id\n
+ * %provider:marker:location:semaphore\n
+ * %provider:marker:location:semaphore\n
+ * file_name:build_id\n
+ * ...
+ *
+ * Parse according to the above format. Find out the file_name and build_id
+ * first and then use sdt_note__read() to parse the SDT note info.
+ * Find out the hash key from the file_name and use that to add this new
+ * entry to file hash.
+ */
+static int file_hash_list__populate(struct hash_table *file_hash, FILE *cache)
+{
+ struct file_sdt_ent *fse = NULL;
+ struct sdt_note *sn;
+ int key, val, ret = -EBADF;
+ char *ptr, *tmp, *data = NULL;
+ size_t len = 2 * PATH_MAX;
+
+ data = zalloc(sizeof(char) * len);
+ if (!data) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ for (val = getline(&data, &len, cache); val != -1;
+ val = getline(&data, &len, cache)) {
+ if (!data)
+ goto out;
+ if (*data == '%') {
+ /*
+ * Its an SDT event entry :
+ * %provider:marker:addr1:addr2\n
+ */
+ if (!fse)
+ goto out;
+ sn = sdt_note__read(data + 1);
+ if (sn == NULL) {
+ ret = -EBADF;
+ goto out;
+ }
+ list_add(&sn->note_list, &fse->sdt_list);
+ } else {
+ /*
+ * Its a file entry:
+ * file_name:build_id\n
+ */
+ fse = malloc(sizeof(*fse));
+ if (!fse) {
+ ret = -ENOMEM;
+ break;
+ }
+ INIT_LIST_HEAD(&fse->sdt_list);
+ INIT_HLIST_NODE(&fse->file_list);
+ /* File name */
+ ptr = strtok_r(data, ":", &tmp);
+ if (!ptr)
+ break;
+ strcpy(fse->name, ptr);
+ /* build id */
+ ptr = strtok_r(NULL, "\n", &tmp);
+ if (!ptr)
+ break;
+ strcpy(fse->sbuild_id, ptr);
+ key = get_hash_key(fse->name);
+ hlist_add_head(&fse->file_list, &file_hash->ent[key]);
+ ret = 0;
+ }
+ }
+
+out:
+ free(data);
+ return ret;
+}
+
+/**
+ * file_hash_list__init: Initializes the file hash list
+ * @file_hash: empty file_hash
+ *
+ * Initializes the entries(ent's) of file_hash and opens the cache file.
+ * To look for the cache file, look into the directory in HOME env variable.
+ */
+static int file_hash_list__init(struct hash_table *file_hash)
+{
+ FILE *cache;
+ int i, ret = 0;
+ char sdt_cache_path[PATH_MAX], *val;
+ struct stat fs;
+
+ for (i = 0; i < SDT_HASH_SIZE; i++)
+ INIT_HLIST_HEAD(&file_hash->ent[i]);
+
+ val = getenv("HOME");
+ if (val)
+ scnprintf(sdt_cache_path, sizeof(sdt_cache_path), "%s/%s/%s",
+ val, SDT_CACHE_DIR, SDT_FILE_CACHE);
+ else {
+ pr_err("Error: Couldn't get user's home directory\n");
+ ret = -1;
+ goto out;
+ }
+
+ cache = fopen(sdt_cache_path, "r");
+ if (cache == NULL)
+ goto out;
+
+ /* To see if the cache contains anything */
+ ret = stat(sdt_cache_path, &fs);
+ if (ret)
+ goto out;
+ /* Populate the hash list */
+ if (fs.st_size > 0)
+ ret = file_hash_list__populate(file_hash, cache);
+ fclose(cache);
+out:
+ return ret;
+}
+
+/**
+ * file_hash_list__cleanup: Frees up all the space taken by file_hash list
+ * @file_hash: file_hash table
+ */
+static void file_hash_list__cleanup(struct hash_table *file_hash)
+{
+ struct file_sdt_ent *file_pos;
+ struct hlist_head *ent_head;
+ struct hlist_node *tmp;
+ int i;
+
+ /* Go through all the entries */
+ for (i = 0; i < SDT_HASH_SIZE; i++) {
+ ent_head = &file_hash->ent[i];
+
+ hlist_for_each_entry_safe(file_pos, tmp, ent_head, file_list) {
+ /* Cleanup the corresponding SDT notes' list */
+ cleanup_sdt_note_list(&file_pos->sdt_list);
+ hlist_del(&file_pos->file_list);
+ free(file_pos);
+ }
+ }
+}
+
+
+/**
+ * add_to_hash_list: add an entry to file_hash_list
+ * @file_hash: file hash table
+ * @target: file name
+ *
+ * Finds out the build_id of @target, checks if @target is already present
+ * in file hash list. If not present, delete any stale entries with this
+ * file name (i.e., entries matching this file name but having older
+ * build ids). And then, adds the file entry to file hash list and also
+ * updates the SDT events in the event hash list.
+ */
+static int add_to_hash_list(struct hash_table *file_hash, const char *target)
+{
+ struct file_info *file;
+ int ret = -EBADF;
+ u8 build_id[BUILD_ID_SIZE];
+
+ LIST_HEAD(sdt_notes);
+
+ file = malloc(sizeof(*file));
+ if (!file)
+ return -ENOMEM;
+
+ file->name = realpath(target, NULL);
+ if (!file->name) {
+ pr_err("%s: Bad file name\n", target);
+ goto out;
+ }
+
+ if (filename__read_build_id(file->name, &build_id,
+ sizeof(build_id)) < 0) {
+ pr_err("Couldn't read build-id in %s\n", file->name);
+ sdt_err(ret, file->name);
+ goto out;
+ }
+ build_id__sprintf(build_id, sizeof(build_id), file->sbuild_id);
+
+ /* File entry already exists ?*/
+ if (file_hash_list__entry_exists(file_hash, file)) {
+ pr_err("Error: SDT events for %s already exist\n",
+ file->name);
+ ret = 0;
+ goto out;
+ }
+
+ /*
+ * This should remove any stale entries (if any) from the file hash.
+ * Stale entries will have the same file name but different build ids.
+ */
+ ret = file_hash_list__remove(file_hash, file->name);
+ if (ret < 0)
+ goto out;
+ ret = get_sdt_note_list(&sdt_notes, file->name);
+ if (ret < 0)
+ sdt_err(ret, target);
+ /* Add the entry to file hash list */
+ ret = file_hash_list__add(file_hash, &sdt_notes, file);
+out:
+ free(file->name);
+ free(file);
+ return ret;
+}
+
+/**
+ * file_hash_list__flush: Flush the file_hash list to cache
+ * @file_hash: file_hash list
+ * @fcache: opened SDT events cache
+ *
+ * Iterate through all the entries of @file_hash and flush them
+ * onto fcache.
+ * The complete file hash list is flushed into the cache. Write the
+ * file entries for every ent of file_hash, alongwith the SDT notes. The
+ * delimiter used is DELIM.
+ */
+static void file_hash_list__flush(struct hash_table *file_hash,
+ FILE *fcache)
+{
+ struct file_sdt_ent *file_pos;
+ struct list_head *sdt_head;
+ struct hlist_head *ent_head;
+ struct sdt_note *sdt_ptr;
+ int i;
+
+ /* Go through all entries */
+ for (i = 0; i < SDT_HASH_SIZE; i++) {
+ /* Obtain the list head */
+ ent_head = &file_hash->ent[i];
+
+ /* DELIM is used here as delimiter */
+ hlist_for_each_entry(file_pos, ent_head, file_list) {
+ fprintf(fcache, "%s%c%s\n", file_pos->name, DELIM,
+ file_pos->sbuild_id);
+ sdt_head = &file_pos->sdt_list;
+ list_for_each_entry(sdt_ptr, sdt_head, note_list) {
+ fprintf(fcache, "%%%s%c%s%c%lx%c%lx\n",
+ sdt_ptr->provider, DELIM,
+ sdt_ptr->name, DELIM,
+ sdt_ptr->addr.a64[0], DELIM,
+ sdt_ptr->addr.a64[2]);
+ }
+ }
+ }
+}
+
+/**
+ * flush_hash_list_to_cache: Flush everything from file_hash to disk
+ * @file_hash: file_hash list
+ *
+ * Opens a cache and calls file_hash_list__flush() to dump everything
+ * on to the cache. The cache file is to be opened in HOME env variable
+ * inside the directory ".debug". The path for the cache file should be
+ * then "/home/user/.debug/.sdt-cache
+ */
+static int flush_hash_list_to_cache(struct hash_table *file_hash)
+{
+ FILE *cache;
+ int ret;
+ struct stat buf;
+ char sdt_cache_path[PATH_MAX], sdt_dir[PATH_MAX], *val;
+
+ val = getenv("HOME");
+ if (val) {
+ scnprintf(sdt_dir, sizeof(sdt_dir), "%s/%s", val, SDT_CACHE_DIR);
+ scnprintf(sdt_cache_path, sizeof(sdt_cache_path), "%s/%s",
+ sdt_dir, SDT_FILE_CACHE);
+ } else {
+ pr_err("Error: Couldn't get the user's home directory\n");
+ ret = -1;
+ goto out_err;
+ }
+ ret = stat(sdt_dir, &buf);
+ if (ret) {
+ ret = mkdir(sdt_dir, buf.st_mode);
+ if (ret) {
+ pr_err("Error: Couldn't create %s\n", sdt_dir);
+ goto out_err;
+ }
+ }
+
+ cache = fopen(sdt_cache_path, "w");
+ if (!cache) {
+ pr_err("Error in creating %s file\n", sdt_cache_path);
+ ret = -errno;
+ goto out_err;
+ }
+
+ file_hash_list__flush(file_hash, cache);
+ fclose(cache);
+out_err:
+ return ret;
+}
+
+/**
+ * add_sdt_events: Add SDT events
+ * @arg: filename
+ *
+ * Initializes a hash table 'file_hash', calls add_to_hash_list() to add
+ * SDT events of @arg to the cache and then cleans them up.
+ * 'file_hash' is a hash table which maintains all the information
+ * related to the files with the SDT events in them. The file name serves
+ * as the key to this hash list. Each entry of the file_hash table is a
+ * list head which contains a chain of 'file_list' entries. Each 'file_list'
+ * entry contains the list of SDT events found in that file. This hash
+ * serves as a mapping from file name to the SDT events.
+ */
+int add_sdt_events(const char *arg)
+{
+ struct hash_table file_hash;
+ int ret, val;
+
+ /* Initialize the file hash_list */
+ ret = file_hash_list__init(&file_hash);
+ if (ret < 0) {
+ pr_err("Error: Couldn't initialize the SDT hash tables\n");
+ goto out;
+ }
+ /* Try to add the events to the file hash_list */
+ ret = add_to_hash_list(&file_hash, arg);
+ if (ret > 0) {
+ val = flush_hash_list_to_cache(&file_hash);
+ if (val < 0) {
+ ret = val;
+ goto out;
+ }
+ printf("%4d events added for %s\n", ret, arg);
+ ret = 0;
+ }
+
+out:
+ file_hash_list__cleanup(&file_hash);
+ return ret;
+}
^ permalink raw reply [flat|nested] 49+ messages in thread* [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-11-02 12:40 [PATCH v4 0/5] perf/sdt: SDT events listing/probing Hemant Kumar
` (3 preceding siblings ...)
2014-11-02 12:42 ` [PATCH v4 2/5] perf/sdt: Add SDT events into a cache Hemant Kumar
@ 2014-11-02 12:43 ` Hemant Kumar
2014-11-04 7:38 ` Namhyung Kim
4 siblings, 1 reply; 49+ messages in thread
From: Hemant Kumar @ 2014-11-02 12:43 UTC (permalink / raw)
To: linux-kernel
Cc: srikar, peterz, oleg, hegdevasant, mingo, anton, systemtap,
namhyung, masami.hiramatsu.pt, aravinda, penberg
This patch adds support to perf to record SDT events. When invoked,
the SDT event is looked up in the sdt-cache. If its found, an entry is
made silently to uprobe_events file and then recording is invoked, and
then the entry for the SDT event in uprobe_events is silently discarded.
The SDT events are already stored in a cache file
(/var/cache/perf/perf-sdt-file.cache).
Although the file_hash table helps in addition or deletion of SDT events
from the cache, its not of much use when it comes to probing the actual
SDT event, because the key to this hash list is a file name and not the
SDT event name (which is given as an argument to perf record). So, we
won't be able to hash into it.
To avoid this problem, we can create another hash list "event_hash" list
which will be maintained along with the file_hash list.
Whenever a user invokes 'perf record -e %provider:event, perf should
initialize the event_hash list and the file_hash list.
The key to event_hash list is calculated from the event name and its
provider name.
event_hash sdt_note
|---------| ----------------
| | | file_ptr |==> container file_sdt_ent
key = 129 =>| hlist ==|===|=> event_list=|==> to sdt notes hashed to
| | | name | same entry
|---------| | provider |
| | | note_list==|==> to other notes in the
key = 130 =>| hlist | --------------- same file
|---------|
The entry at that key in event_hash contains a list of SDT notes hashed to
the same entry. It compares the name and provider to see if that is the SDT
note we are looking for. If yes, find out the file that contains this SDT
note. There is a file_ptr pointer embedded in this note which points to the
struct file_sdt_ent contained in the file_hash. From "file_sdt_ent" we will
find out the file name.
Convert this sdt note into a perf event and then write this into
uprobe_events file to be able to record the event.
Then, corresponding entries are added to uprobe_events file for
the SDT events.
After recording is done, these events are silently deleted from uprobe_events
file. The uprobe_events file is present in debugfs/tracing directory.
To support the addition and deletion of SDT events to/from uprobe_events
file, a record_sdt struct is maintained which has the event data.
An example usage:
# perf record -e %libc:setjmp -aR sleep 10
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.277 MB perf.data (~12103 samples) ]
# perf report --stdio
# To display the perf.data header info, please use --header/--header-only
#
# Samples: 1 of event 'libc:setjmp'
# Event count (approx.): 1
#
# Overhead Command Shared Object Symbol
# ........ ....... ............. ...............
#
100.00% sleep libc-2.16.so [.] __sigsetjmp
Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
---
tools/perf/builtin-probe.c | 5 +
tools/perf/builtin-record.c | 23 ++++
tools/perf/util/parse-events.c | 6 +
tools/perf/util/parse-events.h | 3 +
tools/perf/util/probe-event.c | 85 ++++++++++++++--
tools/perf/util/probe-event.h | 8 ++
tools/perf/util/probe-finder.c | 3 +
tools/perf/util/sdt.c | 213 ++++++++++++++++++++++++++++++++++++++--
tools/perf/util/symbol.h | 2
9 files changed, 329 insertions(+), 19 deletions(-)
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 921bb69..7a64e5b 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -509,3 +509,8 @@ int cmd_probe(int argc, const char **argv, const char *prefix)
return ret;
}
+
+void goto_quiet_mode(void)
+{
+ verbose = -1;
+}
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 5091a27..1e5fc84 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -47,6 +47,25 @@ struct record {
long samples;
};
+/* Session specific to SDT tracing */
+struct record_sdt {
+ bool sdt; /* is this SDT event tracing? */
+ char *str; /* hold the event name */
+} rec_sdt;
+
+int trace_sdt_event(const char *str)
+{
+ int ret = 0;
+
+ rec_sdt.sdt = true;
+ rec_sdt.str = strdup(str);
+ if (!rec_sdt.str)
+ return -ENOMEM;
+ ret = event_hash_list__lookup(str);
+ return ret;
+}
+
+
static int record__write(struct record *rec, void *bf, size_t size)
{
if (perf_data_file__write(rec->session->file, bf, size) < 0) {
@@ -879,6 +898,10 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
}
err = __cmd_record(&record, argc, argv);
+ if (rec_sdt.sdt) {
+ err = remove_perf_sdt_events(rec_sdt.str);
+ free(rec_sdt.str);
+ }
out_symbol_exit:
perf_evlist__delete(rec->evlist);
symbol__exit();
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index c659a3c..532ef83 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -990,6 +990,12 @@ static int parse_events__scanner(const char *str, void *data, int start_token)
return ret;
buffer = parse_events__scan_string(str, scanner);
+ /* '%' means it can be an SDT event */
+ if (*str == '%')
+ if (strchr(str, ':')) {
+ ret = trace_sdt_event(str);
+ str++;
+ }
#ifdef PARSER_DEBUG
parse_events_debug = 1;
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index cafdab6..15bd431 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -126,5 +126,8 @@ extern int valid_debugfs_mount(const char *debugfs);
int add_sdt_events(const char *file);
int dump_sdt_events(void);
int remove_sdt_events(const char *str);
+int event_hash_list__lookup(const char *str);
+int remove_perf_sdt_events(const char *str);
+int trace_sdt_event(const char *str);
#endif /* __PERF_PARSE_EVENTS_H */
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 28eb141..f644b46 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -364,7 +364,8 @@ error:
}
static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
- int ntevs, const char *exec)
+ int ntevs, const char *exec,
+ struct perf_probe_event *pev)
{
int i, ret = 0;
unsigned long stext = 0;
@@ -378,7 +379,10 @@ static int add_exec_to_probe_trace_events(struct probe_trace_event *tevs,
for (i = 0; i < ntevs && ret >= 0; i++) {
/* point.address is the addres of point.symbol + point.offset */
- tevs[i].point.address -= stext;
+ if (pev->sdt)
+ tevs[i].point.address = pev->point.offset;
+ else
+ tevs[i].point.address -= stext;
tevs[i].point.module = strdup(exec);
if (!tevs[i].point.module) {
ret = -ENOMEM;
@@ -426,15 +430,14 @@ static int add_module_to_probe_trace_events(struct probe_trace_event *tevs,
/* Post processing the probe events */
static int post_process_probe_trace_events(struct probe_trace_event *tevs,
int ntevs, const char *module,
- bool uprobe)
+ struct perf_probe_event *pev)
{
struct ref_reloc_sym *reloc_sym;
char *tmp;
int i;
- if (uprobe)
- return add_exec_to_probe_trace_events(tevs, ntevs, module);
-
+ if (pev->uprobes)
+ return add_exec_to_probe_trace_events(tevs, ntevs, module, pev);
/* Note that currently ref_reloc_sym based probe is not for drivers */
if (module)
return add_module_to_probe_trace_events(tevs, ntevs, module);
@@ -486,7 +489,7 @@ static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
if (ntevs > 0) { /* Succeeded to find trace events */
pr_debug("Found %d probe_trace_events.\n", ntevs);
ret = post_process_probe_trace_events(*tevs, ntevs,
- target, pev->uprobes);
+ target, pev);
if (ret < 0) {
clear_probe_trace_events(*tevs, ntevs);
zfree(tevs);
@@ -1117,6 +1120,43 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
return 0;
}
+/* Parse an SDT event */
+static int parse_perf_sdt_event(struct perf_sdt_event *sev,
+ struct perf_probe_event *pev)
+{
+ struct perf_probe_point *pp = &pev->point;
+
+ pev->uprobes = true;
+ pev->sdt = true;
+ pev->event = strdup(sev->note->name);
+ if (pev->event == NULL)
+ return -ENOMEM;
+ pev->group = strdup(sev->note->provider);
+ if (pev->event == NULL)
+ return -ENOMEM;
+
+ pp->file = strdup(sev->file_name);
+ if (pp->file == NULL)
+ return -ENOMEM;
+
+ pp->function = strdup(sev->note->name);
+ pp->offset = sev->note->addr.a64[0];
+ return 0;
+}
+
+int add_perf_sdt_event(struct perf_sdt_event *sev)
+{
+ struct perf_probe_event pev;
+ int ret;
+
+ goto_quiet_mode();
+ ret = parse_perf_sdt_event(sev, &pev);
+ if (!ret)
+ add_perf_probe_events(&pev, 1, MAX_PROBES,
+ sev->file_name, true);
+ return ret;
+}
+
/* Parse perf-probe event argument */
static int parse_perf_probe_arg(char *str, struct perf_probe_arg *arg)
{
@@ -2163,6 +2203,11 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
group = pev->group;
pev->event = tev->event;
pev->group = tev->group;
+ /* Arguments currently not supported with SDT events */
+ if (pev->sdt) {
+ pev->nargs = 0;
+ tev->nargs = 0;
+ }
show_perf_probe_event(pev, tev->point.module);
/* Trick here - restore current event/group */
pev->event = (char *)event;
@@ -2371,6 +2416,7 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs,
{
int i, j, ret;
struct __event_package *pkgs;
+ bool sdt = pevs->sdt;
ret = 0;
pkgs = zalloc(sizeof(struct __event_package) * npevs);
@@ -2412,7 +2458,8 @@ end:
zfree(&pkgs[i].tevs);
}
free(pkgs);
- exit_symbol_maps();
+ if (!sdt) /* We didn't initialize symbol maps for SDT events */
+ exit_symbol_maps();
return ret;
}
@@ -2453,7 +2500,7 @@ error:
}
static int del_trace_probe_event(int fd, const char *buf,
- struct strlist *namelist)
+ struct strlist *namelist)
{
struct str_node *ent, *n;
int ret = -1;
@@ -2478,7 +2525,7 @@ static int del_trace_probe_event(int fd, const char *buf,
return ret;
}
-int del_perf_probe_events(struct strlist *dellist)
+static int __del_perf_probe_events(struct strlist *dellist)
{
int ret = -1, ufd = -1, kfd = -1;
char buf[128];
@@ -2556,6 +2603,24 @@ error:
return ret;
}
+int del_perf_probe_events(struct strlist *dellist)
+{
+ return __del_perf_probe_events(dellist);
+}
+
+int remove_perf_sdt_events(const char *str)
+{
+ struct strlist *dellist;
+ int ret = 0;
+
+ dellist = strlist__new(true, NULL);
+ strlist__add(dellist, str + 1);
+ if (dellist)
+ ret = __del_perf_probe_events(dellist);
+
+ return ret;
+}
+
/* TODO: don't use a global variable for filter ... */
static struct strfilter *available_func_filter;
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index e01e994..dd7a0b06 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -74,9 +74,15 @@ struct perf_probe_event {
struct perf_probe_point point; /* Probe point */
int nargs; /* Number of arguments */
bool uprobes;
+ bool sdt; /* An SDT event? */
struct perf_probe_arg *args; /* Arguments */
};
+struct perf_sdt_event {
+ struct sdt_note *note; /* SDT note info */
+ char *file_name; /* File name */
+};
+
/* Line range */
struct line_range {
char *file; /* File name */
@@ -135,7 +141,9 @@ extern int show_available_vars(struct perf_probe_event *pevs, int npevs,
struct strfilter *filter, bool externs);
extern int show_available_funcs(const char *module, struct strfilter *filter,
bool user);
+extern int add_perf_sdt_event(struct perf_sdt_event *sev);
+void goto_quiet_mode(void);
/* Maximum index number of event-name postfix */
#define MAX_EVENT_INDEX 1024
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index c7918f8..1dd89db8 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -1197,6 +1197,9 @@ int debuginfo__find_trace_events(struct debuginfo *dbg,
tf.tevs = *tevs;
tf.ntevs = 0;
+ /* Number of trace events for SDT event is 1 */
+ if (pev->sdt)
+ return 1;
ret = debuginfo__find_probes(dbg, &tf.pf);
if (ret < 0) {
zfree(tevs);
diff --git a/tools/perf/util/sdt.c b/tools/perf/util/sdt.c
index 9dd7e4da..e94a469 100644
--- a/tools/perf/util/sdt.c
+++ b/tools/perf/util/sdt.c
@@ -273,6 +273,37 @@ out_err:
}
/**
+ * event_hash_list__add : add SDT events to event hash table
+ * @fse: obtained from the file_hash
+ * @event_hash: event_hash list
+ *
+ * Iterate through the SDT notes list one by one and add them
+ * to event hash table.
+ */
+static int event_hash_list__add(struct sdt_note *sn,
+ struct hash_table *event_hash)
+{
+ struct hlist_head *ent_head;
+ int event_key, len;
+ char *str;
+
+ len = strlen(sn->provider) + strlen(sn->name);
+ str = (char *)zalloc(len + 1);
+ if (!str)
+ return -ENOMEM;
+
+ /* Concatenate SDT name and provider and find out the key */
+ sprintf(str, "%s%s", sn->provider, sn->name);
+ event_key = get_hash_key(str);
+ free(str);
+ /* List adding */
+ ent_head = &event_hash->ent[event_key];
+ hlist_add_head(&sn->event_list, ent_head);
+
+ return 0;
+}
+
+/**
* file_hash_list__populate: Fill up the file hash table
* @file_hash: empty file hash table
* @cache: FILE * to read from
@@ -291,11 +322,12 @@ out_err:
* Find out the hash key from the file_name and use that to add this new
* entry to file hash.
*/
-static int file_hash_list__populate(struct hash_table *file_hash, FILE *cache)
+static int file_hash_list__populate(struct hash_table *file_hash, FILE *cache,
+ struct hash_table *event_hash)
{
struct file_sdt_ent *fse = NULL;
struct sdt_note *sn;
- int key, val, ret = -EBADF;
+ int key, val, nr_add = 0, ret = -EBADF;
char *ptr, *tmp, *data = NULL;
size_t len = 2 * PATH_MAX;
@@ -321,6 +353,13 @@ static int file_hash_list__populate(struct hash_table *file_hash, FILE *cache)
goto out;
}
list_add(&sn->note_list, &fse->sdt_list);
+ sn->file_ptr = fse;
+ if (event_hash) {
+ ret = event_hash_list__add(sn, event_hash);
+ if (ret < 0)
+ break;
+ nr_add++;
+ }
} else {
/*
* Its a file entry:
@@ -345,7 +384,7 @@ static int file_hash_list__populate(struct hash_table *file_hash, FILE *cache)
strcpy(fse->sbuild_id, ptr);
key = get_hash_key(fse->name);
hlist_add_head(&fse->file_list, &file_hash->ent[key]);
- ret = 0;
+ ret = nr_add;
}
}
@@ -360,8 +399,10 @@ out:
*
* Initializes the entries(ent's) of file_hash and opens the cache file.
* To look for the cache file, look into the directory in HOME env variable.
+ * Updates the event_hash list if needed.
*/
-static int file_hash_list__init(struct hash_table *file_hash)
+static int file_hash_list__init(struct hash_table *file_hash,
+ struct hash_table *event_hash)
{
FILE *cache;
int i, ret = 0;
@@ -389,9 +430,9 @@ static int file_hash_list__init(struct hash_table *file_hash)
ret = stat(sdt_cache_path, &fs);
if (ret)
goto out;
- /* Populate the hash list */
+ /* Populate the hash lists */
if (fs.st_size > 0)
- ret = file_hash_list__populate(file_hash, cache);
+ ret = file_hash_list__populate(file_hash, cache, event_hash);
fclose(cache);
out:
return ret;
@@ -421,6 +462,34 @@ static void file_hash_list__cleanup(struct hash_table *file_hash)
}
}
+/**
+ * event_hash_list__init: Initialize the event_hash list
+ * @event_hash: event_hash ptr
+ */
+static void event_hash_list__init(struct hash_table *event_hash)
+{
+ int i;
+
+ for (i = 0; i < SDT_HASH_SIZE; i++)
+ INIT_HLIST_HEAD(&event_hash->ent[i]);
+}
+
+/**
+ * init_hash_lists: Initialize the hash_lists
+ * @file_hash: file_hash ptr
+ * @event_hash: event_hash ptr
+ *
+ * Wrapper function to initialize both the hash lists.
+ */
+static int init_hash_lists(struct hash_table *file_hash,
+ struct hash_table *event_hash)
+{
+ if (event_hash)
+ event_hash_list__init(event_hash);
+
+ /* event_hash gets updated in file_hash too */
+ return file_hash_list__init(file_hash, event_hash);
+}
/**
* add_to_hash_list: add an entry to file_hash_list
@@ -593,7 +662,7 @@ int add_sdt_events(const char *arg)
int ret, val;
/* Initialize the file hash_list */
- ret = file_hash_list__init(&file_hash);
+ ret = init_hash_lists(&file_hash, NULL);
if (ret < 0) {
pr_err("Error: Couldn't initialize the SDT hash tables\n");
goto out;
@@ -656,7 +725,7 @@ int dump_sdt_events(void)
struct hash_table file_hash;
int ret;
- ret = file_hash_list__init(&file_hash);
+ ret = init_hash_lists(&file_hash, NULL);
if (!ret)
file_hash_list__display(&file_hash);
file_hash_list__cleanup(&file_hash);
@@ -682,7 +751,7 @@ int remove_sdt_events(const char *str)
goto out_err;
}
/* Initialize the hash_lists */
- ret = file_hash_list__init(&file_hash);
+ ret = init_hash_lists(&file_hash, NULL);
if (ret < 0)
goto out;
@@ -702,3 +771,129 @@ out:
out_err:
return ret;
}
+
+/**
+ * convert_to_sdt_event : Converts a SDT note into a perf consumable event
+ * @sn: sdt note
+ * @sdt_event: converted sdt_event
+ *
+ * Copies the file name and assigns a reference to @sn to @sdt_event->note
+ */
+static int convert_to_sdt_event(struct sdt_note *sn,
+ struct perf_sdt_event *sdt_event)
+{
+ sdt_event->file_name = strdup(sn->file_ptr->name);
+ if (!sdt_event->file_name) {
+ pr_err("Error: Not enough memory!");
+ return -ENOMEM;
+ }
+ sdt_event->note = sn;
+
+ return 0;
+}
+
+/**
+ * build_id__matches: Function to compare build-ids
+ * @fse: file_entry
+ *
+ * This function finds out the current build id of the file @fse->name
+ * and then compares that to the build id stored in @fse->sbuild_id.
+ * This is used to see if the file has changed since addition of that
+ * file's sdt events to the cache.
+ * Returns 0 if the build-ids match and non-zero in case of an error.
+ */
+static int build_id__matches(struct file_sdt_ent *fse)
+{
+ u8 curr_build_id[BUILD_ID_SIZE];
+ char curr_sbuild_id[BUILD_ID_SIZE * 2 + 1];
+
+ symbol__elf_init();
+ if (filename__read_build_id(fse->name, &curr_build_id,
+ sizeof(curr_build_id)) < 0) {
+ pr_err("Couldn't read build-id in %s\n", fse->name);
+ goto out_err;
+ }
+ build_id__sprintf(curr_build_id, sizeof(curr_build_id),
+ curr_sbuild_id);
+ if (!strcmp(curr_sbuild_id, fse->sbuild_id))
+ return 0;
+out_err:
+ return -1;
+}
+
+/**
+ * event_hash_list__lookup: Function to lookup an SDT event
+ * @str: provider:name
+ *
+ * file_hash list is not designed to lookup for an SDT event. To do that, we
+ * need another structure : "event_hash". This hash list is built up along
+ * with file_hash list but is based on SDT event names as keys as opposed to
+ * the file names (who serve as keys in file_hash list).
+ * To lookup for the SDT event name, initialize file_hash list first along
+ * with the event_hash. init_hash_lists() will do that for us. Now, obtain
+ * the key for event_hash using @str (provider:name). Go to that entry,
+ * obtain the list_head for that entry, start traversing the list of events.
+ * Once, we get the SDT note we were looking for, change the SDT note to a
+ * perf consumable event.
+ */
+int event_hash_list__lookup(const char *str)
+{
+ struct hash_table file_hash, event_hash;
+ struct sdt_note *sn;
+ struct perf_sdt_event *sdt_event = NULL;
+ struct hlist_head *ent_head;
+ char group[PATH_MAX], event[PATH_MAX];
+ char *ptr, *tmp, s[PATH_MAX], delim[2] = {DELIM, '\0'};
+ int event_key, ret = 0;
+
+ /* Initialize the hash_lists */
+ ret = init_hash_lists(&file_hash, &event_hash);
+ if (ret < 0)
+ goto out;
+ strcpy(s, str);
+ /* Get the SDT provider name */
+ ptr = strtok_r(s + 1, delim, &tmp); /* s + 1 to get rid of '%' */
+ if (!ptr)
+ goto out;
+ strcpy(group, ptr);
+ /* Get the SDT event name */
+ ptr = strtok_r(NULL, delim, &tmp);
+ if (!ptr)
+ goto out;
+ strcpy(event, ptr);
+
+ /* Get the SDT event name */
+ memset(ptr, '\0', strlen(ptr));
+ sprintf(ptr, "%s%s", group, event);
+ /* Calculate the event hash key */
+ event_key = get_hash_key(ptr);
+ ent_head = &event_hash.ent[event_key];
+
+ /* Found event(s) */
+ hlist_for_each_entry(sn, ent_head, event_list) {
+ if (!strcmp(sn->name, event) && !strcmp(sn->provider, group)) {
+ sdt_event = malloc(sizeof(*sdt_event));
+ if (!sdt_event) {
+ pr_err("Error: Not enough memory!");
+ ret = -ENOMEM;
+ goto out;
+ }
+ sdt_event->file_name = NULL;
+ ret = build_id__matches(sn->file_ptr);
+ if (ret) {
+ pr_err("File versions don't match\nPlease run \"perf sdt-cache --add <file>\" before doing \"perf record\"\n");
+ ret = 0;
+ goto out;
+ }
+ ret = convert_to_sdt_event(sn, sdt_event);
+ /* Add the SDT event to uprobes */
+ if (!ret)
+ ret = add_perf_sdt_event(sdt_event);
+ }
+ }
+out:
+ file_hash_list__cleanup(&file_hash);
+ if (sdt_event)
+ free(sdt_event->file_name);
+ return ret;
+}
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index ed4ecfa..245f093 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -323,6 +323,8 @@ struct sdt_note {
Elf32_Addr a32[3];
} addr;
struct list_head note_list; /* SDT notes' list */
+ struct hlist_node event_list; /* Link to event_hash_list entry */
+ struct file_sdt_ent *file_ptr; /* ptr to the containing file_sdt_ent */
};
int get_sdt_note_list(struct list_head *head, const char *target);
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-11-02 12:43 ` [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events Hemant Kumar
@ 2014-11-04 7:38 ` Namhyung Kim
2014-11-04 8:06 ` Hemant Kumar
0 siblings, 1 reply; 49+ messages in thread
From: Namhyung Kim @ 2014-11-04 7:38 UTC (permalink / raw)
To: Hemant Kumar
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo, anton,
systemtap, masami.hiramatsu.pt, aravinda, penberg
Hi Hemant,
As you know, you need to keep an eye on how (kprobes) event cache
patchset from Masami settles down. For those who aren't CC'ed, please
see the link below:
https://lkml.org/lkml/2014/10/31/207
On Sun, 02 Nov 2014 16:26:28 +0530, Hemant Kumar wrote:
> This patch adds support to perf to record SDT events. When invoked,
> the SDT event is looked up in the sdt-cache. If its found, an entry is
> made silently to uprobe_events file and then recording is invoked, and
> then the entry for the SDT event in uprobe_events is silently discarded.
>
> The SDT events are already stored in a cache file
> (/var/cache/perf/perf-sdt-file.cache).
> Although the file_hash table helps in addition or deletion of SDT events
> from the cache, its not of much use when it comes to probing the actual
> SDT event, because the key to this hash list is a file name and not the
> SDT event name (which is given as an argument to perf record). So, we
> won't be able to hash into it.
It likely to be ended up with per-file or per-buildid cache files under
~/.debug directory. In this case we also need to have the (central)
event-to-cache table anyway IMHO.
>
> To avoid this problem, we can create another hash list "event_hash" list
> which will be maintained along with the file_hash list.
> Whenever a user invokes 'perf record -e %provider:event, perf should
> initialize the event_hash list and the file_hash list.
> The key to event_hash list is calculated from the event name and its
> provider name.
Isn't it enough just to use provide name? I guess the provider names
are (should be?) unique among a system although there's no absolute
guarantee for that.
>
> event_hash sdt_note
> |---------| ----------------
> | | | file_ptr |==> container file_sdt_ent
> key = 129 =>| hlist ==|===|=> event_list=|==> to sdt notes hashed to
> | | | name | same entry
> |---------| | provider |
> | | | note_list==|==> to other notes in the
> key = 130 =>| hlist | --------------- same file
> |---------|
>
> The entry at that key in event_hash contains a list of SDT notes hashed to
> the same entry. It compares the name and provider to see if that is the SDT
> note we are looking for. If yes, find out the file that contains this SDT
> note. There is a file_ptr pointer embedded in this note which points to the
> struct file_sdt_ent contained in the file_hash. From "file_sdt_ent" we will
> find out the file name.
> Convert this sdt note into a perf event and then write this into
> uprobe_events file to be able to record the event.
> Then, corresponding entries are added to uprobe_events file for
> the SDT events.
> After recording is done, these events are silently deleted from uprobe_events
> file. The uprobe_events file is present in debugfs/tracing directory.
>
> To support the addition and deletion of SDT events to/from uprobe_events
> file, a record_sdt struct is maintained which has the event data.
>
> An example usage:
>
> # perf record -e %libc:setjmp -aR sleep 10
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.277 MB perf.data (~12103 samples) ]
>
> # perf report --stdio
> # To display the perf.data header info, please use --header/--header-only
> #
> # Samples: 1 of event 'libc:setjmp'
> # Event count (approx.): 1
> #
> # Overhead Command Shared Object Symbol
> # ........ ....... ............. ...............
> #
> 100.00% sleep libc-2.16.so [.] __sigsetjmp
>
>
> Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
[SNIP]
> +/* Session specific to SDT tracing */
> +struct record_sdt {
> + bool sdt; /* is this SDT event tracing? */
> + char *str; /* hold the event name */
> +} rec_sdt;
> +
> +int trace_sdt_event(const char *str)
> +{
> + int ret = 0;
> +
> + rec_sdt.sdt = true;
> + rec_sdt.str = strdup(str);
> + if (!rec_sdt.str)
> + return -ENOMEM;
> + ret = event_hash_list__lookup(str);
> + return ret;
> +}
Hmm.. does it support multiple SDT events recorded in a session like:
# perf record -e %libc:setjmp -e %libc:longjmp -aR sleep 10
?
[SNIP]
> +/* Parse an SDT event */
> +static int parse_perf_sdt_event(struct perf_sdt_event *sev,
> + struct perf_probe_event *pev)
> +{
> + struct perf_probe_point *pp = &pev->point;
> +
> + pev->uprobes = true;
> + pev->sdt = true;
> + pev->event = strdup(sev->note->name);
> + if (pev->event == NULL)
> + return -ENOMEM;
> + pev->group = strdup(sev->note->provider);
> + if (pev->event == NULL)
s/event/group/
> + return -ENOMEM;
> +
> + pp->file = strdup(sev->file_name);
> + if (pp->file == NULL)
> + return -ENOMEM;
> +
> + pp->function = strdup(sev->note->name);
Missing NULL check. Also you need to free prior allocations in case of
error.
Thanks,
Namhyung
> + pp->offset = sev->note->addr.a64[0];
> + return 0;
> +}
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-11-04 7:38 ` Namhyung Kim
@ 2014-11-04 8:06 ` Hemant Kumar
2014-11-04 12:57 ` Masami Hiramatsu
0 siblings, 1 reply; 49+ messages in thread
From: Hemant Kumar @ 2014-11-04 8:06 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo, anton,
systemtap, masami.hiramatsu.pt, aravinda, penberg
Hi Namhyung,
On 11/04/2014 01:08 PM, Namhyung Kim wrote:
> Hi Hemant,
>
> As you know, you need to keep an eye on how (kprobes) event cache
> patchset from Masami settles down. For those who aren't CC'ed, please
> see the link below:
>
> https://lkml.org/lkml/2014/10/31/207
>
> On Sun, 02 Nov 2014 16:26:28 +0530, Hemant Kumar wrote:
>> This patch adds support to perf to record SDT events. When invoked,
>> the SDT event is looked up in the sdt-cache. If its found, an entry is
>> made silently to uprobe_events file and then recording is invoked, and
>> then the entry for the SDT event in uprobe_events is silently discarded.
>>
>> The SDT events are already stored in a cache file
>> (/var/cache/perf/perf-sdt-file.cache).
>> Although the file_hash table helps in addition or deletion of SDT events
>> from the cache, its not of much use when it comes to probing the actual
>> SDT event, because the key to this hash list is a file name and not the
>> SDT event name (which is given as an argument to perf record). So, we
>> won't be able to hash into it.
> It likely to be ended up with per-file or per-buildid cache files under
> ~/.debug directory. In this case we also need to have the (central)
> event-to-cache table anyway IMHO.
>
>
>> To avoid this problem, we can create another hash list "event_hash" list
>> which will be maintained along with the file_hash list.
>> Whenever a user invokes 'perf record -e %provider:event, perf should
>> initialize the event_hash list and the file_hash list.
>> The key to event_hash list is calculated from the event name and its
>> provider name.
> Isn't it enough just to use provide name? I guess the provider names
> are (should be?) unique among a system although there's no absolute
> guarantee for that.
>
Yes, there is no guarantee for the provider names to be unique.
If we use only provider name with "perf record", then, what if a user
wants to trace
only a specific SDT event (not all the events for that provider)?
What do you think?
>> event_hash sdt_note
>> |---------| ----------------
>> | | | file_ptr |==> container file_sdt_ent
>> key = 129 =>| hlist ==|===|=> event_list=|==> to sdt notes hashed to
>> | | | name | same entry
>> |---------| | provider |
>> | | | note_list==|==> to other notes in the
>> key = 130 =>| hlist | --------------- same file
>> |---------|
>>
>> The entry at that key in event_hash contains a list of SDT notes hashed to
>> the same entry. It compares the name and provider to see if that is the SDT
>> note we are looking for. If yes, find out the file that contains this SDT
>> note. There is a file_ptr pointer embedded in this note which points to the
>> struct file_sdt_ent contained in the file_hash. From "file_sdt_ent" we will
>> find out the file name.
>> Convert this sdt note into a perf event and then write this into
>> uprobe_events file to be able to record the event.
>> Then, corresponding entries are added to uprobe_events file for
>> the SDT events.
>> After recording is done, these events are silently deleted from uprobe_events
>> file. The uprobe_events file is present in debugfs/tracing directory.
>>
>> To support the addition and deletion of SDT events to/from uprobe_events
>> file, a record_sdt struct is maintained which has the event data.
>>
>> An example usage:
>>
>> # perf record -e %libc:setjmp -aR sleep 10
>> [ perf record: Woken up 1 times to write data ]
>> [ perf record: Captured and wrote 0.277 MB perf.data (~12103 samples) ]
>>
>> # perf report --stdio
>> # To display the perf.data header info, please use --header/--header-only
>> #
>> # Samples: 1 of event 'libc:setjmp'
>> # Event count (approx.): 1
>> #
>> # Overhead Command Shared Object Symbol
>> # ........ ....... ............. ...............
>> #
>> 100.00% sleep libc-2.16.so [.] __sigsetjmp
>>
>>
>> Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
>
> [SNIP]
>> +/* Session specific to SDT tracing */
>> +struct record_sdt {
>> + bool sdt; /* is this SDT event tracing? */
>> + char *str; /* hold the event name */
>> +} rec_sdt;
>> +
>> +int trace_sdt_event(const char *str)
>> +{
>> + int ret = 0;
>> +
>> + rec_sdt.sdt = true;
>> + rec_sdt.str = strdup(str);
>> + if (!rec_sdt.str)
>> + return -ENOMEM;
>> + ret = event_hash_list__lookup(str);
>> + return ret;
>> +}
> Hmm.. does it support multiple SDT events recorded in a session like:
>
> # perf record -e %libc:setjmp -e %libc:longjmp -aR sleep 10
>
> ?
Yes, it supports multiple SDT events recorded in a session :
# ./perf record -e %libc:lll_lock_wait_private -e %libc:setjmp -aR sleep 10
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.264 MB perf.data (~11526 samples) ]
> [SNIP]
>> +/* Parse an SDT event */
>> +static int parse_perf_sdt_event(struct perf_sdt_event *sev,
>> + struct perf_probe_event *pev)
>> +{
>> + struct perf_probe_point *pp = &pev->point;
>> +
>> + pev->uprobes = true;
>> + pev->sdt = true;
>> + pev->event = strdup(sev->note->name);
>> + if (pev->event == NULL)
>> + return -ENOMEM;
>> + pev->group = strdup(sev->note->provider);
>> + if (pev->event == NULL)
> s/event/group/
Will fix that.
>> + return -ENOMEM;
>> +
>> + pp->file = strdup(sev->file_name);
>> + if (pp->file == NULL)
>> + return -ENOMEM;
>> +
>> + pp->function = strdup(sev->note->name);
> Missing NULL check. Also you need to free prior allocations in case of
> error.
Yes, will do that.
> Thanks,
> Namhyung
>
>
>> + pp->offset = sev->note->addr.a64[0];
>> + return 0;
>> +}
--
Thanks,
Hemant Kumar
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: Re: [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-11-04 8:06 ` Hemant Kumar
@ 2014-11-04 12:57 ` Masami Hiramatsu
[not found] ` <5459BD3E.7010804@linux.vnet.ibm.com>
2014-11-05 7:06 ` Namhyung Kim
0 siblings, 2 replies; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-04 12:57 UTC (permalink / raw)
To: Hemant Kumar
Cc: Namhyung Kim, linux-kernel, srikar, peterz, oleg, hegdevasant,
mingo, anton, systemtap, aravinda, penberg
Hi,
(2014/11/04 17:06), Hemant Kumar wrote:
> Hi Namhyung,
>
> On 11/04/2014 01:08 PM, Namhyung Kim wrote:
>> Hi Hemant,
>>
>> As you know, you need to keep an eye on how (kprobes) event cache
>> patchset from Masami settles down. For those who aren't CC'ed, please
>> see the link below:
>>
>> https://lkml.org/lkml/2014/10/31/207
>>
>> On Sun, 02 Nov 2014 16:26:28 +0530, Hemant Kumar wrote:
>>> This patch adds support to perf to record SDT events. When invoked,
>>> the SDT event is looked up in the sdt-cache. If its found, an entry is
>>> made silently to uprobe_events file and then recording is invoked, and
>>> then the entry for the SDT event in uprobe_events is silently discarded.
>>>
>>> The SDT events are already stored in a cache file
>>> (/var/cache/perf/perf-sdt-file.cache).
>>> Although the file_hash table helps in addition or deletion of SDT events
>>> from the cache, its not of much use when it comes to probing the actual
>>> SDT event, because the key to this hash list is a file name and not the
>>> SDT event name (which is given as an argument to perf record). So, we
>>> won't be able to hash into it.
>> It likely to be ended up with per-file or per-buildid cache files under
>> ~/.debug directory. In this case we also need to have the (central)
>> event-to-cache table anyway IMHO.
What we are talking is to make a new caching file with buildid under
.debug/.
We already has ~/.debug/.build-id/<build-id> for string the binary
symbol maps. I think there are 2 options, one is expanding the current
build-id file format to include sdt and probe-event caches. The other is
to add ~/.debug/.build-id/<build-id>.probe and
~/.debug/.build-id/<build-id>.sdt for caching probe/sdt information.
And also, user interface is a discussion point. This series defines new
sdt-cache command, and we already have buildid-cache command. We should
have probe-cache command too? or consolidate those cache managing commands?
This question should be involving your series too.
>>> To avoid this problem, we can create another hash list "event_hash" list
>>> which will be maintained along with the file_hash list.
>>> Whenever a user invokes 'perf record -e %provider:event, perf should
>>> initialize the event_hash list and the file_hash list.
>>> The key to event_hash list is calculated from the event name and its
>>> provider name.
>> Isn't it enough just to use provide name? I guess the provider names
>> are (should be?) unique among a system although there's no absolute
>> guarantee for that.
>>
>
> Yes, there is no guarantee for the provider names to be unique.
> If we use only provider name with "perf record", then, what if a user
> wants to trace
> only a specific SDT event (not all the events for that provider)?
> What do you think?
How about failing if the provider name is not unique unless user
gives the actual binary path?
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread[parent not found: <5459BD3E.7010804@linux.vnet.ibm.com>]
* Re: Re: Re: [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events
[not found] ` <5459BD3E.7010804@linux.vnet.ibm.com>
@ 2014-11-05 6:51 ` Hemant Kumar
2014-11-05 9:07 ` Masami Hiramatsu
0 siblings, 1 reply; 49+ messages in thread
From: Hemant Kumar @ 2014-11-05 6:51 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Namhyung Kim, LKML, Srikar Dronamraju, Peter Zijlstra, oleg,
hegdevasant, mingo, anton, systemtap, aravinda, penberg
Hi Masami,
> Hi,
>
> (2014/11/04 17:06), Hemant Kumar wrote:
>> Hi Namhyung,
>>
>> On 11/04/2014 01:08 PM, Namhyung Kim wrote:
>>> Hi Hemant,
>>>
>>> As you know, you need to keep an eye on how (kprobes) event cache
>>> patchset from Masami settles down. For those who aren't CC'ed, please
>>> see the link below:
>>>
>>> https://lkml.org/lkml/2014/10/31/207
>>>
>>> On Sun, 02 Nov 2014 16:26:28 +0530, Hemant Kumar wrote:
>>>> This patch adds support to perf to record SDT events. When invoked,
>>>> the SDT event is looked up in the sdt-cache. If its found, an entry is
>>>> made silently to uprobe_events file and then recording is invoked, and
>>>> then the entry for the SDT event in uprobe_events is silently
>>>> discarded.
>>>>
>>>> The SDT events are already stored in a cache file
>>>> (/var/cache/perf/perf-sdt-file.cache).
>>>> Although the file_hash table helps in addition or deletion of SDT
>>>> events
>>>> from the cache, its not of much use when it comes to probing the
>>>> actual
>>>> SDT event, because the key to this hash list is a file name and not
>>>> the
>>>> SDT event name (which is given as an argument to perf record). So, we
>>>> won't be able to hash into it.
>>> It likely to be ended up with per-file or per-buildid cache files under
>>> ~/.debug directory. In this case we also need to have the (central)
>>> event-to-cache table anyway IMHO.
>
> What we are talking is to make a new caching file with buildid under
> .debug/.
> We already has ~/.debug/.build-id/<build-id> for string the binary
> symbol maps. I think there are 2 options, one is expanding the current
> build-id file format to include sdt and probe-event caches. The other is
Like a single cache to manage all the events for that file? How do we
distinguish between the events as we will be having perf record to read
SDT events from this cache?
> to add ~/.debug/.build-id/<build-id>.probe and
> ~/.debug/.build-id/<build-id>.sdt for caching probe/sdt information.
>
This approach looks convenient.
> And also, user interface is a discussion point. This series defines new
> sdt-cache command, and we already have buildid-cache command. We should
> have probe-cache command too? or consolidate those cache managing
> commands?
> This question should be involving your series too.
>
I think, we need not have multiple sub-commands to manage the cache. We
can consolidate the cache management of probe events (including SDT
events) to a single command.
>>>> To avoid this problem, we can create another hash list "event_hash"
>>>> list
>>>> which will be maintained along with the file_hash list.
>>>> Whenever a user invokes 'perf record -e %provider:event, perf should
>>>> initialize the event_hash list and the file_hash list.
>>>> The key to event_hash list is calculated from the event name and its
>>>> provider name.
>>> Isn't it enough just to use provide name? I guess the provider names
>>> are (should be?) unique among a system although there's no absolute
>>> guarantee for that.
>>>
>>
>> Yes, there is no guarantee for the provider names to be unique.
>> If we use only provider name with "perf record", then, what if a user
>> wants to trace
>> only a specific SDT event (not all the events for that provider)?
>> What do you think?
>
> How about failing if the provider name is not unique unless user
> gives the actual binary path?
>
>
You mean something like this:
# perf record -e %provider @/path/to/file ...?
--
Thanks,
Hemant Kumar
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: Re: Re: Re: [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-11-05 6:51 ` Hemant Kumar
@ 2014-11-05 9:07 ` Masami Hiramatsu
2014-11-05 13:28 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-05 9:07 UTC (permalink / raw)
To: Hemant Kumar
Cc: Namhyung Kim, LKML, Srikar Dronamraju, Peter Zijlstra, oleg,
hegdevasant, mingo, anton, systemtap, aravinda, penberg,
Arnaldo Carvalho de Melo
(2014/11/05 15:50), Hemant Kumar wrote:
> Hi Masami,
>
>> Hi,
>>
>> (2014/11/04 17:06), Hemant Kumar wrote:
>>> Hi Namhyung,
>>>
>>> On 11/04/2014 01:08 PM, Namhyung Kim wrote:
>>>> Hi Hemant,
>>>>
>>>> As you know, you need to keep an eye on how (kprobes) event cache
>>>> patchset from Masami settles down. For those who aren't CC'ed, please
>>>> see the link below:
>>>>
>>>> https://lkml.org/lkml/2014/10/31/207
>>>>
>>>> On Sun, 02 Nov 2014 16:26:28 +0530, Hemant Kumar wrote:
>>>>> This patch adds support to perf to record SDT events. When invoked,
>>>>> the SDT event is looked up in the sdt-cache. If its found, an entry is
>>>>> made silently to uprobe_events file and then recording is invoked, and
>>>>> then the entry for the SDT event in uprobe_events is silently
>>>>> discarded.
>>>>>
>>>>> The SDT events are already stored in a cache file
>>>>> (/var/cache/perf/perf-sdt-file.cache).
>>>>> Although the file_hash table helps in addition or deletion of SDT
>>>>> events
>>>>> from the cache, its not of much use when it comes to probing the
>>>>> actual
>>>>> SDT event, because the key to this hash list is a file name and not
>>>>> the
>>>>> SDT event name (which is given as an argument to perf record). So, we
>>>>> won't be able to hash into it.
>>>> It likely to be ended up with per-file or per-buildid cache files under
>>>> ~/.debug directory. In this case we also need to have the (central)
>>>> event-to-cache table anyway IMHO.
>>
>> What we are talking is to make a new caching file with buildid under
>> .debug/.
>> We already has ~/.debug/.build-id/<build-id> for string the binary
>> symbol maps. I think there are 2 options, one is expanding the current
>> build-id file format to include sdt and probe-event caches. The other is
>
> Like a single cache to manage all the events for that file? How do we
> distinguish between the events as we will be having perf record to read
> SDT events from this cache?
>
>> to add ~/.debug/.build-id/<build-id>.probe and
>> ~/.debug/.build-id/<build-id>.sdt for caching probe/sdt information.
>>
>
> This approach looks convenient.
>
>> And also, user interface is a discussion point. This series defines new
>> sdt-cache command, and we already have buildid-cache command. We should
>> have probe-cache command too? or consolidate those cache managing
>> commands?
>> This question should be involving your series too.
>>
>
> I think, we need not have multiple sub-commands to manage the cache. We
> can consolidate the cache management of probe events (including SDT
> events) to a single command.
Agreed. maybe perf-cache --buildid/--sdt/--probe would be good.
>>>>> To avoid this problem, we can create another hash list "event_hash"
>>>>> list
>>>>> which will be maintained along with the file_hash list.
>>>>> Whenever a user invokes 'perf record -e %provider:event, perf should
>>>>> initialize the event_hash list and the file_hash list.
>>>>> The key to event_hash list is calculated from the event name and its
>>>>> provider name.
>>>> Isn't it enough just to use provide name? I guess the provider names
>>>> are (should be?) unique among a system although there's no absolute
>>>> guarantee for that.
>>>>
>>>
>>> Yes, there is no guarantee for the provider names to be unique.
>>> If we use only provider name with "perf record", then, what if a user
>>> wants to trace
>>> only a specific SDT event (not all the events for that provider)?
>>> What do you think?
>>
>> How about failing if the provider name is not unique unless user
>> gives the actual binary path?
>>
>>
>
> You mean something like this:
> # perf record -e %provider @/path/to/file ...?
Yes, that is what I meant. :)
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Re: Re: Re: [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-11-05 9:07 ` Masami Hiramatsu
@ 2014-11-05 13:28 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 49+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-11-05 13:28 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Hemant Kumar, Namhyung Kim, LKML, Srikar Dronamraju,
Peter Zijlstra, oleg, hegdevasant, mingo, anton, systemtap,
aravinda, penberg, acme
Em Wed, Nov 05, 2014 at 06:07:08PM +0900, Masami Hiramatsu escreveu:
> (2014/11/05 15:50), Hemant Kumar wrote:
> >> And also, user interface is a discussion point. This series defines new
> >> sdt-cache command, and we already have buildid-cache command. We should
> >> have probe-cache command too? or consolidate those cache managing
> >> commands?
> >> This question should be involving your series too.
> > I think, we need not have multiple sub-commands to manage the cache. We
> > can consolidate the cache management of probe events (including SDT
> > events) to a single command.
> Agreed. maybe perf-cache --buildid/--sdt/--probe would be good.
We have it already, its called 'perf buildid-cache':
[root@zoo ~]# perf buildid-cache --hell
Error: unknown option `hell'
usage: perf buildid-cache [<options>]
-a, --add <file list>
file(s) to add
-k, --kcore <file> kcore file to add
-r, --remove <file list>
file(s) to remove
-M, --missing <file> to find missing build ids in the cache
-f, --force don't complain, do it
-u, --update <file list>
file(s) to update
-v, --verbose be more verbose
[root@zoo ~]#
We can rename it at some point to 'perf cache', perhaps.
'perf cache --buildid' makes no sense, everything is keyed by build-id
(hence the name, which is albeit long, admit), I guess what you meant
was 'elf'.
It currently stores content of the form:
- ELF
- kcore (which is also ELF, but has no pathname)
- kallsyms
We would be adding other types of content, from this discussion:
- sdt
- [ku]probes (built from some other content, like ELF or kallsyms)
- Arnaldo
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-11-04 12:57 ` Masami Hiramatsu
[not found] ` <5459BD3E.7010804@linux.vnet.ibm.com>
@ 2014-11-05 7:06 ` Namhyung Kim
2014-11-05 9:05 ` Masami Hiramatsu
2014-11-05 18:24 ` [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events Hemant Kumar
1 sibling, 2 replies; 49+ messages in thread
From: Namhyung Kim @ 2014-11-05 7:06 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Hemant Kumar, linux-kernel, srikar, peterz, oleg, hegdevasant,
mingo, anton, systemtap, aravinda, penberg
On Tue, 04 Nov 2014 21:56:53 +0900, Masami Hiramatsu wrote:
> Hi,
>
> (2014/11/04 17:06), Hemant Kumar wrote:
>> Hi Namhyung,
>>
>> On 11/04/2014 01:08 PM, Namhyung Kim wrote:
>>> Hi Hemant,
>>>
>>> As you know, you need to keep an eye on how (kprobes) event cache
>>> patchset from Masami settles down. For those who aren't CC'ed, please
>>> see the link below:
>>>
>>> https://lkml.org/lkml/2014/10/31/207
>>>
>>> On Sun, 02 Nov 2014 16:26:28 +0530, Hemant Kumar wrote:
>>>> This patch adds support to perf to record SDT events. When invoked,
>>>> the SDT event is looked up in the sdt-cache. If its found, an entry is
>>>> made silently to uprobe_events file and then recording is invoked, and
>>>> then the entry for the SDT event in uprobe_events is silently discarded.
>>>>
>>>> The SDT events are already stored in a cache file
>>>> (/var/cache/perf/perf-sdt-file.cache).
>>>> Although the file_hash table helps in addition or deletion of SDT events
>>>> from the cache, its not of much use when it comes to probing the actual
>>>> SDT event, because the key to this hash list is a file name and not the
>>>> SDT event name (which is given as an argument to perf record). So, we
>>>> won't be able to hash into it.
>>> It likely to be ended up with per-file or per-buildid cache files under
>>> ~/.debug directory. In this case we also need to have the (central)
>>> event-to-cache table anyway IMHO.
>
> What we are talking is to make a new caching file with buildid under
> .debug/.
> We already has ~/.debug/.build-id/<build-id> for string the binary
> symbol maps.
??
The ~/.debug/.build-id/<build-id> (actually first 2 hexdigits are used
for directory name) is a symlink to a cached binary.
$ file .debug/.build-id/00/08a6c4028b3826f8905324c770e7aa450e5d3b
.debug/.build-id/00/08a6c4028b3826f8905324c770e7aa450e5d3b: symbolic link to `../../usr/lib64/libgtk-3.so.0.400.4/0008a6c4028b3826f8905324c770e7aa450e5d3b'
$ file .debug/usr/lib64/libgtk-3.so.0.400.4/0008a6c4028b3826f8905324c770e7aa450e5d3b
.debug/usr/lib64/libgtk-3.so.0.400.4/0008a6c4028b3826f8905324c770e7aa450e5d3b: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, BuildID[sha1]=0xc4a6080026388b02245390f8aae770c73b5d0e45, stripped
Hmm.. it seems the file utility prints the build-id as a sequence of 4
byte little-endian integers.
> I think there are 2 options, one is expanding the current
> build-id file format to include sdt and probe-event caches. The other is
> to add ~/.debug/.build-id/<build-id>.probe and
> ~/.debug/.build-id/<build-id>.sdt for caching probe/sdt information.
I think a single .probe file is enough for this, no?
>
> And also, user interface is a discussion point. This series defines new
> sdt-cache command, and we already have buildid-cache command. We should
> have probe-cache command too? or consolidate those cache managing commands?
> This question should be involving your series too.
>
>>>> To avoid this problem, we can create another hash list "event_hash" list
>>>> which will be maintained along with the file_hash list.
>>>> Whenever a user invokes 'perf record -e %provider:event, perf should
>>>> initialize the event_hash list and the file_hash list.
>>>> The key to event_hash list is calculated from the event name and its
>>>> provider name.
>>> Isn't it enough just to use provide name? I guess the provider names
>>> are (should be?) unique among a system although there's no absolute
>>> guarantee for that.
>>>
>>
>> Yes, there is no guarantee for the provider names to be unique.
>> If we use only provider name with "perf record", then, what if a user
>> wants to trace
>> only a specific SDT event (not all the events for that provider)?
>> What do you think?
What I'm saying is for managing cache not the usage of the cached
events. IIUC you keep hash entry for all events to find matching file,
but I think it can be managed in provider level as events in a single
provider will live in a single binary.
Btw, I think we should support such multiple events to like
# perf record -e %provider_xxx:* -e %provider_yyy:prefix_*
>
> How about failing if the provider name is not unique unless user
> gives the actual binary path?
It looks like a possible option. :)
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: Re: [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-11-05 7:06 ` Namhyung Kim
@ 2014-11-05 9:05 ` Masami Hiramatsu
2014-11-06 2:16 ` Josh Stone
2014-11-06 7:06 ` Hemant Kumar
2014-11-05 18:24 ` [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events Hemant Kumar
1 sibling, 2 replies; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-05 9:05 UTC (permalink / raw)
To: Namhyung Kim
Cc: Hemant Kumar, linux-kernel, srikar, peterz, oleg, hegdevasant,
mingo, anton, systemtap, aravinda, penberg,
Arnaldo Carvalho de Melo
(2014/11/05 16:06), Namhyung Kim wrote:
> On Tue, 04 Nov 2014 21:56:53 +0900, Masami Hiramatsu wrote:
>> Hi,
>>
>> (2014/11/04 17:06), Hemant Kumar wrote:
>>> Hi Namhyung,
>>>
>>> On 11/04/2014 01:08 PM, Namhyung Kim wrote:
>>>> Hi Hemant,
>>>>
>>>> As you know, you need to keep an eye on how (kprobes) event cache
>>>> patchset from Masami settles down. For those who aren't CC'ed, please
>>>> see the link below:
>>>>
>>>> https://lkml.org/lkml/2014/10/31/207
>>>>
>>>> On Sun, 02 Nov 2014 16:26:28 +0530, Hemant Kumar wrote:
>>>>> This patch adds support to perf to record SDT events. When invoked,
>>>>> the SDT event is looked up in the sdt-cache. If its found, an entry is
>>>>> made silently to uprobe_events file and then recording is invoked, and
>>>>> then the entry for the SDT event in uprobe_events is silently discarded.
>>>>>
>>>>> The SDT events are already stored in a cache file
>>>>> (/var/cache/perf/perf-sdt-file.cache).
>>>>> Although the file_hash table helps in addition or deletion of SDT events
>>>>> from the cache, its not of much use when it comes to probing the actual
>>>>> SDT event, because the key to this hash list is a file name and not the
>>>>> SDT event name (which is given as an argument to perf record). So, we
>>>>> won't be able to hash into it.
>>>> It likely to be ended up with per-file or per-buildid cache files under
>>>> ~/.debug directory. In this case we also need to have the (central)
>>>> event-to-cache table anyway IMHO.
>>
>> What we are talking is to make a new caching file with buildid under
>> .debug/.
>> We already has ~/.debug/.build-id/<build-id> for string the binary
>> symbol maps.
>
> ??
>
> The ~/.debug/.build-id/<build-id> (actually first 2 hexdigits are used
> for directory name) is a symlink to a cached binary.
>
> $ file .debug/.build-id/00/08a6c4028b3826f8905324c770e7aa450e5d3b
> .debug/.build-id/00/08a6c4028b3826f8905324c770e7aa450e5d3b: symbolic link to `../../usr/lib64/libgtk-3.so.0.400.4/0008a6c4028b3826f8905324c770e7aa450e5d3b'
>
> $ file .debug/usr/lib64/libgtk-3.so.0.400.4/0008a6c4028b3826f8905324c770e7aa450e5d3b
> .debug/usr/lib64/libgtk-3.so.0.400.4/0008a6c4028b3826f8905324c770e7aa450e5d3b: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, BuildID[sha1]=0xc4a6080026388b02245390f8aae770c73b5d0e45, stripped
>
>
> Hmm.. it seems the file utility prints the build-id as a sequence of 4
> byte little-endian integers.
Ah, I saw the kernel's build-id file...
>> I think there are 2 options, one is expanding the current
>> build-id file format to include sdt and probe-event caches. The other is
>> to add ~/.debug/.build-id/<build-id>.probe and
>> ~/.debug/.build-id/<build-id>.sdt for caching probe/sdt information.
>
> I think a single .probe file is enough for this, no?
I hope to be so, but SDT is a bit different, that may have a semaphore. Currently
we just ignore it. But in some application, SDT events never be hit without enabling
the semaphores. I'm not sure how we can enable it. Systemtap enables it by modifying
application memory(data) on the fly. Maybe perftools can be done it by using ptrace,
but it's not for ftrace.
[Off topic] I really don't like that the current SDT's semaphore. If the user apps
see the instruction at the probe point, it is easy to check whether the event is
enabled or not. Thus I recommend to change its implementation and update version
instead of supporting current semaphore by perftools.
>> And also, user interface is a discussion point. This series defines new
>> sdt-cache command, and we already have buildid-cache command. We should
>> have probe-cache command too? or consolidate those cache managing commands?
>> This question should be involving your series too.
>>
>>>>> To avoid this problem, we can create another hash list "event_hash" list
>>>>> which will be maintained along with the file_hash list.
>>>>> Whenever a user invokes 'perf record -e %provider:event, perf should
>>>>> initialize the event_hash list and the file_hash list.
>>>>> The key to event_hash list is calculated from the event name and its
>>>>> provider name.
>>>> Isn't it enough just to use provide name? I guess the provider names
>>>> are (should be?) unique among a system although there's no absolute
>>>> guarantee for that.
>>>>
>>>
>>> Yes, there is no guarantee for the provider names to be unique.
>>> If we use only provider name with "perf record", then, what if a user
>>> wants to trace
>>> only a specific SDT event (not all the events for that provider)?
>>> What do you think?
>
> What I'm saying is for managing cache not the usage of the cached
> events. IIUC you keep hash entry for all events to find matching file,
> but I think it can be managed in provider level as events in a single
> provider will live in a single binary.
Usually, the SDT provider names are managed by hand, as it is unique.
>
> Btw, I think we should support such multiple events to like
>
> # perf record -e %provider_xxx:* -e %provider_yyy:prefix_*
>
what will be placed in the xxx and yyy ?
Thank you,
>>
>> How about failing if the provider name is not unique unless user
>> gives the actual binary path?
>
> It looks like a possible option. :)
>
> Thanks,
> Namhyung
>
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-11-05 9:05 ` Masami Hiramatsu
@ 2014-11-06 2:16 ` Josh Stone
2014-11-06 5:34 ` Masami Hiramatsu
2014-11-06 7:06 ` Hemant Kumar
1 sibling, 1 reply; 49+ messages in thread
From: Josh Stone @ 2014-11-06 2:16 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Namhyung Kim, Hemant Kumar, linux-kernel, srikar, peterz, oleg,
hegdevasant, mingo, systemtap, aravinda, penberg,
Arnaldo Carvalho de Melo
On 11/05/2014 01:05 AM, Masami Hiramatsu wrote:
> [Off topic] I really don't like that the current SDT's semaphore. If the user apps
> see the instruction at the probe point, it is easy to check whether the event is
> enabled or not. Thus I recommend to change its implementation and update version
> instead of supporting current semaphore by perftools.
You and I have banged heads on this before, but I don't think checking
the instruction is a simple as you seem to think. I invite you to
prototype this, and if you get it working we can discuss the tradeoffs.
The good news is that other tools (stap and gdb) won't need to care. If
the SDT semaphore goes automatic, then we can just set that note field
to zero, unused from the tool's perspective.
Another tactic is to just discourage developers from using the semaphore
in the first place, as it's a completely optional feature. The marker
is just a NOP, so adding some "if (enabled) {...}" around it is often a
useless load and branch. It does make sense if the probe wants to
provide some expensively-computed arguments though, like cpython does to
prepare a function name string. So if you see a project testing the
semaphore around simple arguments, I'd suggest they just probe directly
instead.
Thanks,
Josh
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: Re: [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-11-06 2:16 ` Josh Stone
@ 2014-11-06 5:34 ` Masami Hiramatsu
0 siblings, 0 replies; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-06 5:34 UTC (permalink / raw)
To: Josh Stone
Cc: Namhyung Kim, Hemant Kumar, linux-kernel, srikar, peterz, oleg,
hegdevasant, mingo, systemtap, aravinda, penberg,
Arnaldo Carvalho de Melo
(2014/11/06 11:15), Josh Stone wrote:
> On 11/05/2014 01:05 AM, Masami Hiramatsu wrote:
>> [Off topic] I really don't like that the current SDT's semaphore. If the user apps
>> see the instruction at the probe point, it is easy to check whether the event is
>> enabled or not. Thus I recommend to change its implementation and update version
>> instead of supporting current semaphore by perftools.
>
> You and I have banged heads on this before, but I don't think checking
> the instruction is a simple as you seem to think. I invite you to
> prototype this, and if you get it working we can discuss the tradeoffs.
Would you have the prototype? I'd like to look :)
> The good news is that other tools (stap and gdb) won't need to care. If
> the SDT semaphore goes automatic, then we can just set that note field
> to zero, unused from the tool's perspective.
>
> Another tactic is to just discourage developers from using the semaphore
> in the first place, as it's a completely optional feature. The marker
> is just a NOP, so adding some "if (enabled) {...}" around it is often a
> useless load and branch. It does make sense if the probe wants to
> provide some expensively-computed arguments though, like cpython does to
> prepare a function name string. So if you see a project testing the
> semaphore around simple arguments, I'd suggest they just probe directly
> instead.
I see, and we did that on qemu. I consider that someone maybe use
it in the future unless we remove it. If we can succeed to discourage
people using semaphore, we also should remove it.
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-11-05 9:05 ` Masami Hiramatsu
2014-11-06 2:16 ` Josh Stone
@ 2014-11-06 7:06 ` Hemant Kumar
2014-11-06 14:56 ` Masami Hiramatsu
2014-11-07 8:21 ` [RFC] perf-cache command interface design Masami Hiramatsu
1 sibling, 2 replies; 49+ messages in thread
From: Hemant Kumar @ 2014-11-06 7:06 UTC (permalink / raw)
To: Masami Hiramatsu, Namhyung Kim
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo, anton,
systemtap, aravinda, penberg, Arnaldo Carvalho de Melo
On 11/05/2014 02:35 PM, Masami Hiramatsu wrote:
> (2014/11/05 16:06), Namhyung Kim wrote:
>> On Tue, 04 Nov 2014 21:56:53 +0900, Masami Hiramatsu wrote:
>>> Hi,
>>>
>>> (2014/11/04 17:06), Hemant Kumar wrote:
>>>> Hi Namhyung,
>>>>
>>>> On 11/04/2014 01:08 PM, Namhyung Kim wrote:
>>>>> Hi Hemant,
>>>>>
>>>>> As you know, you need to keep an eye on how (kprobes) event cache
>>>>> patchset from Masami settles down. For those who aren't CC'ed, please
>>>>> see the link below:
>>>>>
>>>>> https://lkml.org/lkml/2014/10/31/207
>>>>>
>>>>> On Sun, 02 Nov 2014 16:26:28 +0530, Hemant Kumar wrote:
>>>>>> This patch adds support to perf to record SDT events. When invoked,
>>>>>> the SDT event is looked up in the sdt-cache. If its found, an entry is
>>>>>> made silently to uprobe_events file and then recording is invoked, and
>>>>>> then the entry for the SDT event in uprobe_events is silently discarded.
>>>>>>
>>>>>> The SDT events are already stored in a cache file
>>>>>> (/var/cache/perf/perf-sdt-file.cache).
>>>>>> Although the file_hash table helps in addition or deletion of SDT events
>>>>>> from the cache, its not of much use when it comes to probing the actual
>>>>>> SDT event, because the key to this hash list is a file name and not the
>>>>>> SDT event name (which is given as an argument to perf record). So, we
>>>>>> won't be able to hash into it.
>>>>> It likely to be ended up with per-file or per-buildid cache files under
>>>>> ~/.debug directory. In this case we also need to have the (central)
>>>>> event-to-cache table anyway IMHO.
>>> What we are talking is to make a new caching file with buildid under
>>> .debug/.
>>> We already has ~/.debug/.build-id/<build-id> for string the binary
>>> symbol maps.
>> ??
>>
>> The ~/.debug/.build-id/<build-id> (actually first 2 hexdigits are used
>> for directory name) is a symlink to a cached binary.
>>
>> $ file .debug/.build-id/00/08a6c4028b3826f8905324c770e7aa450e5d3b
>> .debug/.build-id/00/08a6c4028b3826f8905324c770e7aa450e5d3b: symbolic link to `../../usr/lib64/libgtk-3.so.0.400.4/0008a6c4028b3826f8905324c770e7aa450e5d3b'
>>
>> $ file .debug/usr/lib64/libgtk-3.so.0.400.4/0008a6c4028b3826f8905324c770e7aa450e5d3b
>> .debug/usr/lib64/libgtk-3.so.0.400.4/0008a6c4028b3826f8905324c770e7aa450e5d3b: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, BuildID[sha1]=0xc4a6080026388b02245390f8aae770c73b5d0e45, stripped
>>
>>
>> Hmm.. it seems the file utility prints the build-id as a sequence of 4
>> byte little-endian integers.
> Ah, I saw the kernel's build-id file...
>
>>> I think there are 2 options, one is expanding the current
>>> build-id file format to include sdt and probe-event caches. The other is
>>> to add ~/.debug/.build-id/<build-id>.probe and
>>> ~/.debug/.build-id/<build-id>.sdt for caching probe/sdt information.
>> I think a single .probe file is enough for this, no?
> I hope to be so, but SDT is a bit different, that may have a semaphore. Currently
> we just ignore it. But in some application, SDT events never be hit without enabling
> the semaphores. I'm not sure how we can enable it. Systemtap enables it by modifying
> application memory(data) on the fly. Maybe perftools can be done it by using ptrace,
> but it's not for ftrace.
>
> [Off topic] I really don't like that the current SDT's semaphore. If the user apps
> see the instruction at the probe point, it is easy to check whether the event is
> enabled or not. Thus I recommend to change its implementation and update version
> instead of supporting current semaphore by perftools.
>
>
>>> And also, user interface is a discussion point. This series defines new
>>> sdt-cache command, and we already have buildid-cache command. We should
>>> have probe-cache command too? or consolidate those cache managing commands?
>>> This question should be involving your series too.
>>>
>>>>>> To avoid this problem, we can create another hash list "event_hash" list
>>>>>> which will be maintained along with the file_hash list.
>>>>>> Whenever a user invokes 'perf record -e %provider:event, perf should
>>>>>> initialize the event_hash list and the file_hash list.
>>>>>> The key to event_hash list is calculated from the event name and its
>>>>>> provider name.
>>>>> Isn't it enough just to use provide name? I guess the provider names
>>>>> are (should be?) unique among a system although there's no absolute
>>>>> guarantee for that.
>>>>>
>>>> Yes, there is no guarantee for the provider names to be unique.
>>>> If we use only provider name with "perf record", then, what if a user
>>>> wants to trace
>>>> only a specific SDT event (not all the events for that provider)?
>>>> What do you think?
>> What I'm saying is for managing cache not the usage of the cached
>> events. IIUC you keep hash entry for all events to find matching file,
>> but I think it can be managed in provider level as events in a single
>> provider will live in a single binary.
> Usually, the SDT provider names are managed by hand, as it is unique.
>
>> Btw, I think we should support such multiple events to like
>>
>> # perf record -e %provider_xxx:* -e %provider_yyy:prefix_*
>>
> what will be placed in the xxx and yyy ?
>
> Thank you,
>
>>> How about failing if the provider name is not unique unless user
>>> gives the actual binary path?
>> It looks like a possible option. :)
>>
>> Thanks,
>> Namhyung
>>
>
So, what should be our way forward here in case of SDT patchset wrt
event_cache patchset? Shall we wait for event_cache patchset to be
merged and then redesign the sdt_cache patchset according to new
event_cache?
Or, we can go ahead with the current sdt patchset (implementing the
latest review comments) and we can change the sdt_cache according to the
new event_cache design as and when required?
What do you think?
--
Thanks,
Hemant Kumar
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Re: [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-11-06 7:06 ` Hemant Kumar
@ 2014-11-06 14:56 ` Masami Hiramatsu
2014-11-07 8:21 ` [RFC] perf-cache command interface design Masami Hiramatsu
1 sibling, 0 replies; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-06 14:56 UTC (permalink / raw)
To: Hemant Kumar
Cc: Namhyung Kim, linux-kernel, srikar, peterz, oleg, hegdevasant,
mingo, anton, systemtap, aravinda, penberg,
Arnaldo Carvalho de Melo
(2014/11/06 16:06), Hemant Kumar wrote:
> So, what should be our way forward here in case of SDT patchset wrt
> event_cache patchset? Shall we wait for event_cache patchset to be
> merged and then redesign the sdt_cache patchset according to new
> event_cache?
> Or, we can go ahead with the current sdt patchset (implementing the
> latest review comments) and we can change the sdt_cache according to the
> new event_cache design as and when required?
>
> What do you think?
Good question :)
In my opinion, we'd better consolidate sdt_cache to new cache subcommand
at first, since it is a user-visible change. If we consolidate it after
introducing sdt-cache, users will see that option is also banished.
And also, the cache-format may be a problem, since that involves a backward
compatibility issue. For now, I'm considering Namhyung's idea of merging
SDT and probe caches. If we can use SDT as a kind of probe cache, why do we
need to have both SDT cache and probe cache? SDT cache is currently have its
own format, but it also could be written as a probe format, as below.
In ~/.debug/probe/bu/ild-id:
%<PROVIDER>:<EVENT> _text+<OFFSET>
This lucks a semaphore location, but who cares? Anyway we can't change
the semaphore. We already have reader of this format and also this can
have arguments if you get it from sdt.note. :)
Moreover, we can share the cache file with perf-probe! :)
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread
* [RFC] perf-cache command interface design
2014-11-06 7:06 ` Hemant Kumar
2014-11-06 14:56 ` Masami Hiramatsu
@ 2014-11-07 8:21 ` Masami Hiramatsu
2014-11-07 8:42 ` Peter Zijlstra
` (5 more replies)
1 sibling, 6 replies; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-07 8:21 UTC (permalink / raw)
To: Hemant Kumar, Namhyung Kim, Arnaldo Carvalho de Melo
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo,
systemtap, aravinda, penberg, brendan.d.gregg, yrl.pp-manager.tt
Hello,
Here, I've tried to describe my idea of perf-cache subcommand interface.
It is just a design review, not implemented yet :)
Please give me your comments/ideas!
Command-line Synopsis
=====================
Current "perf buildid-cache [options]" are directly mapped to
"perf cache --buildid [options]".
And adding --sdt for managing SDT caches as below.
Add or update SDT events in <FILES>
perf cache --sdt --add|--update <FILES>
Remove all SDT events for <FILES>
perf cache --sdt --remove <FILES>
List all SDT events
perf cache --sdt --list
And --probes for managing probe-caches as below.
Add new probe-cache entries for kernel, <PATH> or <MOD>.
perf cache --probe [--exec <PATH>|--module <MOD>] --add <SPEC>
Delete existing probe-cache entries for kernel, <PATH> or/and <BUILDID>.
perf cache --probe --del [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
Or remove all entires for given FILES
perf cache --probe --remove <FILES>
List the probe caches(including SDT) for kernel, <PATH>, or/and <BUILDID>.
perf cache --probe --list [@<PATH>][#<BUILDID>]
Query the probe definitions.
perf cache --probe --query [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
Note that --probes also can be used for managing SDT events, which has % prefix
e.g.
Add all SDT events for <PATH>
perf cache --probe --exec <PATH> --add '%*:*'
Remove some SDT events for <PATH>
perf cache --probe --del '%some:events@<PATH>'
Or remove all SDT events for <BUILDID>
perf cache --probe --del '%*:*#<BUILDID>'
File Format
===========
All the cache files are placed under ~/.debug/ by default.
The paths of buildid cache of binary/symbols are not changed.
The SDT/probe caches are placed under the ~/.debug/.probes/path/to/bin/bu/ildid
and that is linked to ~/.debug/.probes/.buildid/bu/ildid
# To avoid conflict with files under /probes/*, I picked up .probes/.
This SDT/probe caches contain probe-definitions as following format.
----
#buildid:BUILDID
#path:PATH
p:%PROVIDER/EVENT PATH:OFFSET [ARGS]
p:PROBE/EVENT _text+OFFSET [ARGS]
----
Normal probes and SDT cache entries can be mixed in a cache file, we'll
load all the entries and filter by % prefixes.
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [RFC] perf-cache command interface design
2014-11-07 8:21 ` [RFC] perf-cache command interface design Masami Hiramatsu
@ 2014-11-07 8:42 ` Peter Zijlstra
2014-11-07 13:58 ` [PATCH RESEND 1/2] perf tools: Move disable_buildid_cache() to util/build-id.c Namhyung Kim
2014-11-07 15:16 ` [RFC] perf-cache command interface design David Ahern
2014-11-07 10:51 ` Hemant Kumar
` (4 subsequent siblings)
5 siblings, 2 replies; 49+ messages in thread
From: Peter Zijlstra @ 2014-11-07 8:42 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Hemant Kumar, Namhyung Kim, Arnaldo Carvalho de Melo,
linux-kernel, srikar, oleg, hegdevasant, mingo, systemtap,
aravinda, penberg, brendan.d.gregg, yrl.pp-manager.tt
On Fri, Nov 07, 2014 at 05:21:08PM +0900, Masami Hiramatsu wrote:
> The paths of buildid cache of binary/symbols are not changed.
Could we get an option in .perfconfig to disable the buildid cache? Its
pointless diskspace wastage for some of us.
^ permalink raw reply [flat|nested] 49+ messages in thread* [PATCH RESEND 1/2] perf tools: Move disable_buildid_cache() to util/build-id.c
2014-11-07 8:42 ` Peter Zijlstra
@ 2014-11-07 13:58 ` Namhyung Kim
2014-11-07 13:58 ` [PATCH 2/2] perf tools: Add record.use-buildid-cache config option Namhyung Kim
2014-11-07 15:16 ` [RFC] perf-cache command interface design David Ahern
1 sibling, 1 reply; 49+ messages in thread
From: Namhyung Kim @ 2014-11-07 13:58 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Masami Hiramatsu, Hemant Kumar, Arnaldo Carvalho de Melo, LKML,
Ingo Molnar, srikar, oleg, hegdevasant, systemtap, aravinda,
penberg, brendan.d.gregg
Also move static variable no_buildid_cache and check it in the
perf_session_cache_build_ids().
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/build-id.c | 11 +++++++++++
tools/perf/util/build-id.h | 1 +
| 10 +---------
tools/perf/util/util.h | 1 -
4 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index dd2a3e52ada1..e8d79e5bfaf7 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -18,6 +18,9 @@
#include "header.h"
#include "vdso.h"
+
+static bool no_buildid_cache;
+
int build_id__mark_dso_hit(struct perf_tool *tool __maybe_unused,
union perf_event *event,
struct perf_sample *sample,
@@ -251,6 +254,11 @@ int dsos__hit_all(struct perf_session *session)
return 0;
}
+void disable_buildid_cache(void)
+{
+ no_buildid_cache = true;
+}
+
int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
const char *name, bool is_kallsyms, bool is_vdso)
{
@@ -404,6 +412,9 @@ int perf_session__cache_build_ids(struct perf_session *session)
int ret;
char debugdir[PATH_MAX];
+ if (no_buildid_cache)
+ return 0;
+
snprintf(debugdir, sizeof(debugdir), "%s", buildid_dir);
if (mkdir(debugdir, 0755) != 0 && errno != EEXIST)
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index 666a3bd4f64e..8236319514d5 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -25,5 +25,6 @@ int perf_session__cache_build_ids(struct perf_session *session);
int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
const char *name, bool is_kallsyms, bool is_vdso);
int build_id_cache__remove_s(const char *sbuild_id, const char *debugdir);
+void disable_buildid_cache(void);
#endif
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 76442caca37e..e86fffd89e2b 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -24,8 +24,6 @@
#include "build-id.h"
#include "data.h"
-static bool no_buildid_cache = false;
-
static u32 header_argc;
static const char **header_argv;
@@ -191,8 +189,7 @@ static int write_build_id(int fd, struct perf_header *h,
pr_debug("failed to write buildid table\n");
return err;
}
- if (!no_buildid_cache)
- perf_session__cache_build_ids(session);
+ perf_session__cache_build_ids(session);
return 0;
}
@@ -2790,8 +2787,3 @@ int perf_event__process_build_id(struct perf_tool *tool __maybe_unused,
session);
return 0;
}
-
-void disable_buildid_cache(void)
-{
- no_buildid_cache = true;
-}
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 7dc44cfe25b3..76d23d83eae5 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -154,7 +154,6 @@ extern void set_die_routine(void (*routine)(const char *err, va_list params) NOR
extern int prefixcmp(const char *str, const char *prefix);
extern void set_buildid_dir(void);
-extern void disable_buildid_cache(void);
static inline const char *skip_prefix(const char *str, const char *prefix)
{
--
2.0.0
^ permalink raw reply [flat|nested] 49+ messages in thread* [PATCH 2/2] perf tools: Add record.use-buildid-cache config option
2014-11-07 13:58 ` [PATCH RESEND 1/2] perf tools: Move disable_buildid_cache() to util/build-id.c Namhyung Kim
@ 2014-11-07 13:58 ` Namhyung Kim
0 siblings, 0 replies; 49+ messages in thread
From: Namhyung Kim @ 2014-11-07 13:58 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Masami Hiramatsu, Hemant Kumar, Arnaldo Carvalho de Melo, LKML,
Ingo Molnar, srikar, oleg, hegdevasant, systemtap, aravinda,
penberg, brendan.d.gregg, Steven Rostedt
Add a new config option for auto-disable buildid-cache.
$ cat ~/.perfconfig
[record]
use-buildid-cache = false
$ rm -rf ~/.debug
$ perf record -av sleep 1
mmap size 528384B
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.439 MB perf.data (~19182 samples) ]
Looking at the vmlinux_path (7 entries long)
Using /proc/kallsyms for symbols
$ tree ~/.debug
/home/namhyung/.debug [error opening dir]
0 directories, 0 files
Requested-by: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-record.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 582c4da155ea..19083e715698 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -685,6 +685,12 @@ int record_callchain_opt(const struct option *opt __maybe_unused,
static int perf_record_config(const char *var, const char *value, void *cb)
{
+ struct record *rec = cb;
+
+ if (!strcmp(var, "record.use-buildid-cache")) {
+ rec->no_buildid_cache = !perf_config_bool(var, value);
+ return 0;
+ }
if (!strcmp(var, "record.call-graph"))
var = "call-graph.record-mode"; /* fall-through */
--
2.0.0
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-07 8:42 ` Peter Zijlstra
2014-11-07 13:58 ` [PATCH RESEND 1/2] perf tools: Move disable_buildid_cache() to util/build-id.c Namhyung Kim
@ 2014-11-07 15:16 ` David Ahern
2014-11-07 15:33 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 49+ messages in thread
From: David Ahern @ 2014-11-07 15:16 UTC (permalink / raw)
To: Peter Zijlstra, Masami Hiramatsu
Cc: Hemant Kumar, Namhyung Kim, Arnaldo Carvalho de Melo,
linux-kernel, srikar, oleg, hegdevasant, mingo, systemtap,
aravinda, penberg, brendan.d.gregg, yrl.pp-manager.tt
On 11/7/14, 1:42 AM, Peter Zijlstra wrote:
> On Fri, Nov 07, 2014 at 05:21:08PM +0900, Masami Hiramatsu wrote:
>> The paths of buildid cache of binary/symbols are not changed.
>
> Could we get an option in .perfconfig to disable the buildid cache? Its
> pointless diskspace wastage for some of us.
I was going to respond with the same. This:
[buildid]
# Default, disable using /dev/null
dir = /dev/null
disables the build-id cache. Hopefully that extends to all the other
caches getting discussed.
David
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [RFC] perf-cache command interface design
2014-11-07 15:16 ` [RFC] perf-cache command interface design David Ahern
@ 2014-11-07 15:33 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 49+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-11-07 15:33 UTC (permalink / raw)
To: David Ahern
Cc: Peter Zijlstra, Masami Hiramatsu, Hemant Kumar, Namhyung Kim,
linux-kernel, srikar, oleg, hegdevasant, mingo, systemtap,
aravinda, penberg, brendan.d.gregg, yrl.pp-manager.tt
Em Fri, Nov 07, 2014 at 08:16:09AM -0700, David Ahern escreveu:
> On 11/7/14, 1:42 AM, Peter Zijlstra wrote:
> >On Fri, Nov 07, 2014 at 05:21:08PM +0900, Masami Hiramatsu wrote:
> >>The paths of buildid cache of binary/symbols are not changed.
> >
> >Could we get an option in .perfconfig to disable the buildid cache? Its
> >pointless diskspace wastage for some of us.
>
> I was going to respond with the same. This:
>
> [buildid]
> # Default, disable using /dev/null
> dir = /dev/null
>
> disables the build-id cache. Hopefully that extends to all the other
> caches getting discussed.
I think this can even be something Kconfig'ed, and we can provide a
.config file for people like Peter that want as barebone a perf he can
get over with :-)
/me needs to try that last perf Kconfig patchkit...
- Arnaldo
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-07 8:21 ` [RFC] perf-cache command interface design Masami Hiramatsu
2014-11-07 8:42 ` Peter Zijlstra
@ 2014-11-07 10:51 ` Hemant Kumar
2014-11-08 4:15 ` Masami Hiramatsu
2014-11-07 14:38 ` Arnaldo Carvalho de Melo
` (3 subsequent siblings)
5 siblings, 1 reply; 49+ messages in thread
From: Hemant Kumar @ 2014-11-07 10:51 UTC (permalink / raw)
To: Masami Hiramatsu, Namhyung Kim, Arnaldo Carvalho de Melo
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo,
systemtap, aravinda, penberg, brendan.d.gregg, yrl.pp-manager.tt
On 11/07/2014 01:51 PM, Masami Hiramatsu wrote:
> Hello,
>
> Here, I've tried to describe my idea of perf-cache subcommand interface.
> It is just a design review, not implemented yet :)
> Please give me your comments/ideas!
Thanks, I have some comments below.
> Command-line Synopsis
> =====================
>
> Current "perf buildid-cache [options]" are directly mapped to
> "perf cache --buildid [options]".
>
> And adding --sdt for managing SDT caches as below.
>
> Add or update SDT events in <FILES>
> perf cache --sdt --add|--update <FILES>
> Remove all SDT events for <FILES>
> perf cache --sdt --remove <FILES>
>
> List all SDT events
> perf cache --sdt --list
>
> And --probes for managing probe-caches as below.
>
> Add new probe-cache entries for kernel, <PATH> or <MOD>.
> perf cache --probe [--exec <PATH>|--module <MOD>] --add <SPEC>
>
> Delete existing probe-cache entries for kernel, <PATH> or/and <BUILDID>.
> perf cache --probe --del [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
>
> Or remove all entires for given FILES
> perf cache --probe --remove <FILES>
>
> List the probe caches(including SDT) for kernel, <PATH>, or/and <BUILDID>.
> perf cache --probe --list [@<PATH>][#<BUILDID>]
>
> Query the probe definitions.
> perf cache --probe --query [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
>
> Note that --probes also can be used for managing SDT events, which has % prefix
> e.g.
> Add all SDT events for <PATH>
> perf cache --probe --exec <PATH> --add '%*:*'
>
> Remove some SDT events for <PATH>
> perf cache --probe --del '%some:events@<PATH>'
>
> Or remove all SDT events for <BUILDID>
> perf cache --probe --del '%*:*#<BUILDID>'
Looks nice to me :)
>
> File Format
> ===========
> All the cache files are placed under ~/.debug/ by default.
> The paths of buildid cache of binary/symbols are not changed.
>
> The SDT/probe caches are placed under the ~/.debug/.probes/path/to/bin/bu/ildid
> and that is linked to ~/.debug/.probes/.buildid/bu/ildid
> # To avoid conflict with files under /probes/*, I picked up .probes/.
>
> This SDT/probe caches contain probe-definitions as following format.
> ----
> #buildid:BUILDID
> #path:PATH
> p:%PROVIDER/EVENT PATH:OFFSET [ARGS]
I think this format isn't accepted by the uprobe_events file (because of
the '%') if we want to cat and grep into uprobe_events file directly.
Although, in case of perf record, we can process it to remove '%' and
then write into uprobe_events file. And, '%' is required as a flag for
the SDT entries.
So, either we can modify the uprobe_events file to ignore '%' in the
event group name (with an additional patch in the SDT series) or we can
leave it to the user to write a script which can do some processing to
remove '%' and then echo it to uprobe_events file.
What would you think?
> p:PROBE/EVENT _text+OFFSET [ARGS]
> ----
>
> Normal probes and SDT cache entries can be mixed in a cache file, we'll
> load all the entries and filter by % prefixes.
>
>
> Thank you,
>
--
Thanks,
Hemant Kumar
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [RFC] perf-cache command interface design
2014-11-07 10:51 ` Hemant Kumar
@ 2014-11-08 4:15 ` Masami Hiramatsu
0 siblings, 0 replies; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-08 4:15 UTC (permalink / raw)
To: Hemant Kumar
Cc: Namhyung Kim, Arnaldo Carvalho de Melo, linux-kernel, srikar,
peterz, oleg, hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
(2014/11/07 19:51), Hemant Kumar wrote:
>>
>> File Format
>> ===========
>> All the cache files are placed under ~/.debug/ by default.
>> The paths of buildid cache of binary/symbols are not changed.
>>
>> The SDT/probe caches are placed under the ~/.debug/.probes/path/to/bin/bu/ildid
>> and that is linked to ~/.debug/.probes/.buildid/bu/ildid
>> # To avoid conflict with files under /probes/*, I picked up .probes/.
>>
>> This SDT/probe caches contain probe-definitions as following format.
>> ----
>> #buildid:BUILDID
>> #path:PATH
>> p:%PROVIDER/EVENT PATH:OFFSET [ARGS]
>
> I think this format isn't accepted by the uprobe_events file (because of
> the '%') if we want to cat and grep into uprobe_events file directly.
> Although, in case of perf record, we can process it to remove '%' and
> then write into uprobe_events file. And, '%' is required as a flag for
> the SDT entries.
Ah, right...
> So, either we can modify the uprobe_events file to ignore '%' in the
> event group name (with an additional patch in the SDT series) or we can
> leave it to the user to write a script which can do some processing to
> remove '%' and then echo it to uprobe_events file.
> What would you think?
Hmm, ok. I think we can unify SDT and probe cache as just an cached
event. So replacing % with probe_ prefix is OK I think.
What I mean is if you add a file to cache, perf adds SDTs in the file,
and if you add a file with some expressions, like '* $params', perf
also adds probe caches. And you can use both events for not only
tracing but also profiling.
e.g.
perf cache --add /bin/foo # add foo's SDTs
perf cache --add /bin/foo --probe 'foofunc $params' # add a probe cache for foofunc
perf record -e %foo:sdtevent -e %foo:foofunc ... # we can record both SDT and cache
So, % prefix means "recall cached probe/sdt events to record if needed".
If you already set up the probe-events on those, you don't need % as below.
perf probe --add "%foo:sdtevent" # add new probe_foo:sdtevent for %foo:sdtevent
perf record -e probe_foo:sdtevent ...
Note that to avoid crash with existing event groups, we'll add "probe_" prefix
for cached events.
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-07 8:21 ` [RFC] perf-cache command interface design Masami Hiramatsu
2014-11-07 8:42 ` Peter Zijlstra
2014-11-07 10:51 ` Hemant Kumar
@ 2014-11-07 14:38 ` Arnaldo Carvalho de Melo
2014-11-08 4:26 ` Masami Hiramatsu
2014-11-07 14:43 ` Namhyung Kim
` (2 subsequent siblings)
5 siblings, 1 reply; 49+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-11-07 14:38 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Hemant Kumar, Adrian Hunter, Mark Wielaard, David Ahern,
Jiri Olsa, Namhyung Kim, linux-kernel, srikar, peterz, oleg,
hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, acme, yrl.pp-manager.tt
Em Fri, Nov 07, 2014 at 05:21:08PM +0900, Masami Hiramatsu escreveu:
> Hello,
>
> Here, I've tried to describe my idea of perf-cache subcommand interface.
> It is just a design review, not implemented yet :)
> Please give me your comments/ideas!
>
> Command-line Synopsis
> =====================
>
> Current "perf buildid-cache [options]" are directly mapped to
> "perf cache --buildid [options]".
>
> And adding --sdt for managing SDT caches as below.
Can't we avoid having to specify the content? I.e. the tool can surely
be smart enough to figure that out, no?
We should, as much as possible, to make things more compact yet
unambiguous, IMHO.
> Add or update SDT events in <FILES>
> perf cache --sdt --add|--update <FILES>
Can automagically figure this out, so, it would become just:
Add or update events in <FILES>
perf cache --add/--update <FILES>
> Remove all SDT events for <FILES>
> perf cache --sdt --remove <FILES>
Ditto.
> List all SDT events
> perf cache --sdt --list
That is ok, since the cache can hold different types of contents, but:
perf cache --list
should show everything, with some marking showing the content type.
> And --probes for managing probe-caches as below.
>
> Add new probe-cache entries for kernel, <PATH> or <MOD>.
> perf cache --probe [--exec <PATH>|--module <MOD>] --add <SPEC>
No need to specify --probe
> Delete existing probe-cache entries for kernel, <PATH> or/and <BUILDID>.
> perf cache --probe --del [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
Ditto
> Or remove all entires for given FILES
> perf cache --probe --remove <FILES>
Ok
> List the probe caches(including SDT) for kernel, <PATH>, or/and <BUILDID>.
> perf cache --probe --list [@<PATH>][#<BUILDID>]
Can figure out the kind by the initial character, so no need to specify
--probe
> Query the probe definitions.
> perf cache --probe --query [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
Ditto
> Note that --probes also can be used for managing SDT events, which has % prefix
> e.g.
> Add all SDT events for <PATH>
> perf cache --probe --exec <PATH> --add '%*:*'
See? --probe is smart enough to deal with SDT and probe caches, it can
disambiguate by looking at the first char.
> Remove some SDT events for <PATH>
> perf cache --probe --del '%some:events@<PATH>'
No need for --probe
> Or remove all SDT events for <BUILDID>
> perf cache --probe --del '%*:*#<BUILDID>'
Ditto
>
> File Format
> ===========
> All the cache files are placed under ~/.debug/ by default.
> The paths of buildid cache of binary/symbols are not changed.
>
> The SDT/probe caches are placed under the ~/.debug/.probes/path/to/bin/bu/ildid
Here I think we could group everything related to a /path/to/bin into:
~/.debug/path/to/bin/bu/ild/
With one file per content type:
~/.debug/path/to/bin/bu/ild/elf
~/.debug/path/to/bin/bu/ild/probe
That leaves room for us to add more file formats (CTF anyone? The Dtrace
one, Compact C Type Information, I mean).
So that if we wanted to pick everything related to a pathname we could
do:
tar cf gcc.debug.tar ~/.debug/usr/bin/gcc/
If we instead wanted to pick all files to a specific gcc build id, we
would do:
tar cf gcc.debug.tar ~/.debug/usr/bin/gcc/bu/ild/
> and that is linked to ~/.debug/.probes/.buildid/bu/ildid
Since this would break compatibility with the existing cache format, we
could as well allow collecting everything related to a build id, by
making the link be also in this fashion:
tar cf bu.ildid.tar ~/.debug/.build-id/bu/ildid/
Because we would have:
~/.debug/.build-id/bu/ildid/elf -> ~/.debug/usr/bin/gcc/bu/ild/elf
~/.debug/.build-id/bu/ildid/probe -> ~/.debug/usr/bin/gcc/bu/ild/probe
And also links to the files that have no pathnames:
~/.debug/.build-id/bu/ildid/vdso -> ~/.debug/[vdso]/bu/ild
~/.debug/.build-id/bu/ildid/kallsyms -> ~/.debug/[kernel.kallsyms]/bu/ild
~/.debug/.build-id/bu/ildid/kcore -> ~/.debug/[kcore]/bu/ild
> # To avoid conflict with files under /probes/*, I picked up .probes/.
>
> This SDT/probe caches contain probe-definitions as following format.
> ----
> #buildid:BUILDID
> #path:PATH
> p:%PROVIDER/EVENT PATH:OFFSET [ARGS]
> p:PROBE/EVENT _text+OFFSET [ARGS]
> ----
>
> Normal probes and SDT cache entries can be mixed in a cache file, we'll
> load all the entries and filter by % prefixes.
- Arnaldo
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [RFC] perf-cache command interface design
2014-11-07 14:38 ` Arnaldo Carvalho de Melo
@ 2014-11-08 4:26 ` Masami Hiramatsu
0 siblings, 0 replies; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-08 4:26 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Hemant Kumar, Adrian Hunter, Mark Wielaard, David Ahern,
Jiri Olsa, Namhyung Kim, linux-kernel, srikar, peterz, oleg,
hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, acme, yrl.pp-manager.tt
(2014/11/07 23:38), Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 07, 2014 at 05:21:08PM +0900, Masami Hiramatsu escreveu:
>> Hello,
>>
>> Here, I've tried to describe my idea of perf-cache subcommand interface.
>> It is just a design review, not implemented yet :)
>> Please give me your comments/ideas!
>>
>> Command-line Synopsis
>> =====================
>>
>> Current "perf buildid-cache [options]" are directly mapped to
>> "perf cache --buildid [options]".
>>
>> And adding --sdt for managing SDT caches as below.
>
> Can't we avoid having to specify the content? I.e. the tool can surely
> be smart enough to figure that out, no?
>
> We should, as much as possible, to make things more compact yet
> unambiguous, IMHO.
Agreed.
>
>> Add or update SDT events in <FILES>
>> perf cache --sdt --add|--update <FILES>
>
> Can automagically figure this out, so, it would become just:
>
> Add or update events in <FILES>
>
> perf cache --add/--update <FILES>
Ah, nice idea :-)
>
>> Remove all SDT events for <FILES>
>> perf cache --sdt --remove <FILES>
>
> Ditto.
>
>> List all SDT events
>> perf cache --sdt --list
>
> That is ok, since the cache can hold different types of contents, but:
>
> perf cache --list
>
> should show everything, with some marking showing the content type.
Yeah, as I replied to Hemant, --sdt/--probe can be unified.
>
>> And --probes for managing probe-caches as below.
>>
>> Add new probe-cache entries for kernel, <PATH> or <MOD>.
>> perf cache --probe [--exec <PATH>|--module <MOD>] --add <SPEC>
>
> No need to specify --probe
>
>> Delete existing probe-cache entries for kernel, <PATH> or/and <BUILDID>.
>> perf cache --probe --del [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
>
> Ditto
>
>> Or remove all entires for given FILES
>> perf cache --probe --remove <FILES>
>
> Ok
>
>> List the probe caches(including SDT) for kernel, <PATH>, or/and <BUILDID>.
>> perf cache --probe --list [@<PATH>][#<BUILDID>]
>
> Can figure out the kind by the initial character, so no need to specify
> --probe
OK.
>> Query the probe definitions.
>> perf cache --probe --query [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
>
> Ditto
>
>> Note that --probes also can be used for managing SDT events, which has % prefix
>> e.g.
>> Add all SDT events for <PATH>
>> perf cache --probe --exec <PATH> --add '%*:*'
>
> See? --probe is smart enough to deal with SDT and probe caches, it can
> disambiguate by looking at the first char.
OK, but --add already have FILES arguments (according to buildid-cache)
So I'd like to move that pattern argument to --probe as I've described
in previous reply to Hemant.
>
>> Remove some SDT events for <PATH>
>> perf cache --probe --del '%some:events@<PATH>'
>
> No need for --probe
>
>> Or remove all SDT events for <BUILDID>
>> perf cache --probe --del '%*:*#<BUILDID>'
>
> Ditto
>
>>
>> File Format
>> ===========
>> All the cache files are placed under ~/.debug/ by default.
>> The paths of buildid cache of binary/symbols are not changed.
>>
>> The SDT/probe caches are placed under the ~/.debug/.probes/path/to/bin/bu/ildid
>
> Here I think we could group everything related to a /path/to/bin into:
>
> ~/.debug/path/to/bin/bu/ild/
>
> With one file per content type:
>
> ~/.debug/path/to/bin/bu/ild/elf
> ~/.debug/path/to/bin/bu/ild/probe
Ah, OK, this is good for me too. :)
>
> That leaves room for us to add more file formats (CTF anyone? The Dtrace
> one, Compact C Type Information, I mean).
Perhaps.
> So that if we wanted to pick everything related to a pathname we could
> do:
>
> tar cf gcc.debug.tar ~/.debug/usr/bin/gcc/
>
> If we instead wanted to pick all files to a specific gcc build id, we
> would do:
>
> tar cf gcc.debug.tar ~/.debug/usr/bin/gcc/bu/ild/
OK.
>> and that is linked to ~/.debug/.probes/.buildid/bu/ildid
>
> Since this would break compatibility with the existing cache format, we
> could as well allow collecting everything related to a build id, by
> making the link be also in this fashion:
>
> tar cf bu.ildid.tar ~/.debug/.build-id/bu/ildid/
>
> Because we would have:
>
> ~/.debug/.build-id/bu/ildid/elf -> ~/.debug/usr/bin/gcc/bu/ild/elf
> ~/.debug/.build-id/bu/ildid/probe -> ~/.debug/usr/bin/gcc/bu/ild/probe
>
> And also links to the files that have no pathnames:
>
> ~/.debug/.build-id/bu/ildid/vdso -> ~/.debug/[vdso]/bu/ild
> ~/.debug/.build-id/bu/ildid/kallsyms -> ~/.debug/[kernel.kallsyms]/bu/ild
> ~/.debug/.build-id/bu/ildid/kcore -> ~/.debug/[kcore]/bu/ild
OK, so let's make bu/ildid to a directory :)
Thank you!
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-07 8:21 ` [RFC] perf-cache command interface design Masami Hiramatsu
` (2 preceding siblings ...)
2014-11-07 14:38 ` Arnaldo Carvalho de Melo
@ 2014-11-07 14:43 ` Namhyung Kim
2014-11-08 4:38 ` Masami Hiramatsu
2014-11-10 10:59 ` Masami Hiramatsu
2014-11-10 12:05 ` Hagen Paul Pfeifer
5 siblings, 1 reply; 49+ messages in thread
From: Namhyung Kim @ 2014-11-07 14:43 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Hemant Kumar, Arnaldo Carvalho de Melo, linux-kernel, srikar,
peterz, oleg, hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
Hi Masami,
2014-11-07 (금), 17:21 +0900, Masami Hiramatsu:
> Hello,
>
> Here, I've tried to describe my idea of perf-cache subcommand interface.
> It is just a design review, not implemented yet :)
> Please give me your comments/ideas!
>
> Command-line Synopsis
> =====================
>
> Current "perf buildid-cache [options]" are directly mapped to
> "perf cache --buildid [options]".
>
> And adding --sdt for managing SDT caches as below.
>
> Add or update SDT events in <FILES>
> perf cache --sdt --add|--update <FILES>
>
> Remove all SDT events for <FILES>
> perf cache --sdt --remove <FILES>
>
> List all SDT events
> perf cache --sdt --list
>
> And --probes for managing probe-caches as below.
>
> Add new probe-cache entries for kernel, <PATH> or <MOD>.
> perf cache --probe [--exec <PATH>|--module <MOD>] --add <SPEC>
>
> Delete existing probe-cache entries for kernel, <PATH> or/and <BUILDID>.
> perf cache --probe --del [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
>
> Or remove all entires for given FILES
> perf cache --probe --remove <FILES>
>
> List the probe caches(including SDT) for kernel, <PATH>, or/and <BUILDID>.
> perf cache --probe --list [@<PATH>][#<BUILDID>]
>
> Query the probe definitions.
> perf cache --probe --query [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
>
> Note that --probes also can be used for managing SDT events, which has % prefix
> e.g.
> Add all SDT events for <PATH>
> perf cache --probe --exec <PATH> --add '%*:*'
>
> Remove some SDT events for <PATH>
> perf cache --probe --del '%some:events@<PATH>'
>
> Or remove all SDT events for <BUILDID>
> perf cache --probe --del '%*:*#<BUILDID>'
I'd prefer sub-command to option for this mandatory (and exclusive)
behavior. What about like this?
perf cache kprobe add [-m <module>] <spec> [<spec>...]
perf cache kprobe del [<group>:]<event> [<event>...]
perf cache kprobe list [-m <module>]
perf cache uprobe add -x <path> <spec> [<spec>...]
perf cache uprobe del [<group>:]<event> [<event>...]
perf cache uprobe list [-x <path>]
perf cache sdt [add|del|update] <file> [<file>...]
perf cache sdt list [-b <build-id>] [<file>...]
>
>
> File Format
> ===========
> All the cache files are placed under ~/.debug/ by default.
> The paths of buildid cache of binary/symbols are not changed.
>
> The SDT/probe caches are placed under the ~/.debug/.probes/path/to/bin/bu/ildid
> and that is linked to ~/.debug/.probes/.buildid/bu/ildid
> # To avoid conflict with files under /probes/*, I picked up .probes/.
However, to be used with perf record, we need a way to find a matching
probe cache file from a event name (or group/provider name, preferably).
What about having something like below:
$ cat ~/.debug/.probes/event.map
PROVIDER1 /path/to/some/where
PROVIDER2 /path/to/other/place
When perf record see a cached event used, it first searches its
provider/group name from the event.map file. And then read bulid-id
from the matching file on the path and finally find a cached event
definition from ~/.debug/.probes/.buildid/bu/ildid file.
>
> This SDT/probe caches contain probe-definitions as following format.
> ----
> #buildid:BUILDID
> #path:PATH
> p:%PROVIDER/EVENT PATH:OFFSET [ARGS]
It seems PATH is redundant and we don't need to repeat it everyline
IMHO. Since it need post-processing anyway, maybe it's better to just
make it simpler, like:
%:PROVIDER/EVENT OFFSET [ARGS]
Thanks,
Namhyung
> p:PROBE/EVENT _text+OFFSET [ARGS]
> ----
>
> Normal probes and SDT cache entries can be mixed in a cache file, we'll
> load all the entries and filter by % prefixes.
>
>
> Thank you,
>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [RFC] perf-cache command interface design
2014-11-07 14:43 ` Namhyung Kim
@ 2014-11-08 4:38 ` Masami Hiramatsu
0 siblings, 0 replies; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-08 4:38 UTC (permalink / raw)
To: Namhyung Kim
Cc: Hemant Kumar, Arnaldo Carvalho de Melo, linux-kernel, srikar,
peterz, oleg, hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
(2014/11/07 23:43), Namhyung Kim wrote:
> Hi Masami,
>
> 2014-11-07 (금), 17:21 +0900, Masami Hiramatsu:
>> Hello,
>>
>> Here, I've tried to describe my idea of perf-cache subcommand interface.
>> It is just a design review, not implemented yet :)
>> Please give me your comments/ideas!
>>
>> Command-line Synopsis
>> =====================
>>
>> Current "perf buildid-cache [options]" are directly mapped to
>> "perf cache --buildid [options]".
>>
>> And adding --sdt for managing SDT caches as below.
>>
>> Add or update SDT events in <FILES>
>> perf cache --sdt --add|--update <FILES>
>>
>> Remove all SDT events for <FILES>
>> perf cache --sdt --remove <FILES>
>>
>> List all SDT events
>> perf cache --sdt --list
>>
>> And --probes for managing probe-caches as below.
>>
>> Add new probe-cache entries for kernel, <PATH> or <MOD>.
>> perf cache --probe [--exec <PATH>|--module <MOD>] --add <SPEC>
>>
>> Delete existing probe-cache entries for kernel, <PATH> or/and <BUILDID>.
>> perf cache --probe --del [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
>>
>> Or remove all entires for given FILES
>> perf cache --probe --remove <FILES>
>>
>> List the probe caches(including SDT) for kernel, <PATH>, or/and <BUILDID>.
>> perf cache --probe --list [@<PATH>][#<BUILDID>]
>>
>> Query the probe definitions.
>> perf cache --probe --query [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
>>
>> Note that --probes also can be used for managing SDT events, which has % prefix
>> e.g.
>> Add all SDT events for <PATH>
>> perf cache --probe --exec <PATH> --add '%*:*'
>>
>> Remove some SDT events for <PATH>
>> perf cache --probe --del '%some:events@<PATH>'
>>
>> Or remove all SDT events for <BUILDID>
>> perf cache --probe --del '%*:*#<BUILDID>'
>
> I'd prefer sub-command to option for this mandatory (and exclusive)
> behavior. What about like this?
I consider this will also confusing users because it's different from
other subcommands. And Arnaldo gives us a better solution :)
>
> perf cache kprobe add [-m <module>] <spec> [<spec>...]
> perf cache kprobe del [<group>:]<event> [<event>...]
> perf cache kprobe list [-m <module>]
>
> perf cache uprobe add -x <path> <spec> [<spec>...]
> perf cache uprobe del [<group>:]<event> [<event>...]
> perf cache uprobe list [-x <path>]
Since perf has only "probe" subcommand, we shouldn't separate
uprobe and kprobe.
>
> perf cache sdt [add|del|update] <file> [<file>...]
> perf cache sdt list [-b <build-id>] [<file>...]
>
>
>>
>>
>> File Format
>> ===========
>> All the cache files are placed under ~/.debug/ by default.
>> The paths of buildid cache of binary/symbols are not changed.
>>
>> The SDT/probe caches are placed under the ~/.debug/.probes/path/to/bin/bu/ildid
>> and that is linked to ~/.debug/.probes/.buildid/bu/ildid
>> # To avoid conflict with files under /probes/*, I picked up .probes/.
>
> However, to be used with perf record, we need a way to find a matching
> probe cache file from a event name (or group/provider name, preferably).
> What about having something like below:
>
> $ cat ~/.debug/.probes/event.map
> PROVIDER1 /path/to/some/where
> PROVIDER2 /path/to/other/place
Hmm, agreed. We'd better have this kind of provider index file to
accelerate searching.
> When perf record see a cached event used, it first searches its
> provider/group name from the event.map file. And then read bulid-id
> from the matching file on the path and finally find a cached event
> definition from ~/.debug/.probes/.buildid/bu/ildid file.
>
>
>>
>> This SDT/probe caches contain probe-definitions as following format.
>> ----
>> #buildid:BUILDID
>> #path:PATH
>> p:%PROVIDER/EVENT PATH:OFFSET [ARGS]
>
> It seems PATH is redundant and we don't need to repeat it everyline
> IMHO. Since it need post-processing anyway, maybe it's better to just
> make it simpler, like:
>
> %:PROVIDER/EVENT OFFSET [ARGS]
Hmm, this is not good because this line will not be able to pass to
tracing/uprobe_events...
And also, I think we can compress this cache with gzip, then such
redundancy will not be a problem.
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-07 8:21 ` [RFC] perf-cache command interface design Masami Hiramatsu
` (3 preceding siblings ...)
2014-11-07 14:43 ` Namhyung Kim
@ 2014-11-10 10:59 ` Masami Hiramatsu
2014-11-10 12:23 ` Arnaldo Carvalho de Melo
2014-11-10 12:05 ` Hagen Paul Pfeifer
5 siblings, 1 reply; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-10 10:59 UTC (permalink / raw)
To: Hemant Kumar, Namhyung Kim, Arnaldo Carvalho de Melo
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo,
systemtap, aravinda, penberg, brendan.d.gregg, yrl.pp-manager.tt
Hello,
Here is the second try for the probe-cache. This version simplifies
the synopsis, and unifies the SDT and probe caches.
Please give me your comments/ideas!
Command-line Synopsis
=====================
Add elf(or symbols) and probe-caches of SDT if exists in <FILES>
perf cache --add <FILES> [--probe <SPEC>] # for user programs
perf cache --kcore <FILE> [--probe <SPEC>] # for kcore ?
perf cache --probe <SPEC> # for the current kernel
Remove caches related to <FILES> or <BUILDIDS>
perf cache --remove <FILES>|<BUILDIDS>
Show all probe caches(including SDT) or buildids
perf cache --list [probe|buildid]
Delete existing probe-cache entries for kernel, <PATH> or/and <BUILDID>.
perf cache --probe-del [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
Query the probe definitions.
perf cache --query [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
File Format
===========
All the cache files are placed under ~/.debug/ by default.
Elf caches are ~/.debug/path/to/file/bu/ildid/elf
Symbols caches are ~/.debug/path/to/file/bu/ildid/syms
Probes(and SDT) are ~/.debug/path/to/file/bu/ildid/probes
And ~/.debug/path/to/file/bu/ildid/ is linked to ~/.debug/.buildid/bu/ildid
Optionally, we can gzip the probes file.
This probe caches contain probe-definitions as following format.
----
#buildid:BUILDID
#spec:SDT
p:sdt_<PROVIDER>/<EVENT> PATH:OFFSET [ARGS]
...
#spec:* $params
p:probe_<GROUP>/<EVENT> _text+OFFSET [ARGS]
...
----
So the #spec: line gives the information what probe spec has been given.
This will be used for updating.
And all the "probe_" and "sdt_" prefix will be replaced by % in the command line,
e.g.
perf record -e %<PROVIDER>/<EVENT> -> this records sdt_PROVIDER/EVENT
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [RFC] perf-cache command interface design
2014-11-10 10:59 ` Masami Hiramatsu
@ 2014-11-10 12:23 ` Arnaldo Carvalho de Melo
2014-11-11 6:53 ` Masami Hiramatsu
0 siblings, 1 reply; 49+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-11-10 12:23 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Hemant Kumar, Namhyung Kim, linux-kernel, srikar, peterz, oleg,
hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
Em Mon, Nov 10, 2014 at 07:59:24PM +0900, Masami Hiramatsu escreveu:
> Hello,
>
> Here is the second try for the probe-cache. This version simplifies
> the synopsis, and unifies the SDT and probe caches.
> Please give me your comments/ideas!
>
> Command-line Synopsis
> =====================
>
> Add elf(or symbols) and probe-caches of SDT if exists in <FILES>
> perf cache --add <FILES> [--probe <SPEC>] # for user programs
Why the --probe above? Shouldn't this be just (if you are talking about
ELF files only):
perf cache --add <FILES>
> perf cache --kcore <FILE> [--probe <SPEC>] # for kcore ?
Adrian, aren't kcore files easily identifiable as such and thus could be
added as:
perf cache --add <FILES>
> perf cache --probe <SPEC> # for the current kernel
Why do we need a --probe here? Don't they always start with a character
that is seldomly used in ELF file names and thus we could get away with
not requiring --probe?
> Remove caches related to <FILES> or <BUILDIDS>
> perf cache --remove <FILES>|<BUILDIDS>
>
> Show all probe caches(including SDT) or buildids
> perf cache --list [probe|buildid]
>
> Delete existing probe-cache entries for kernel, <PATH> or/and <BUILDID>.
> perf cache　--probe-del [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
Ditto, i.e. can't we just use:
perf cache　--remove [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
And it figure out that this is a probe that is being removed?
> Query the probe definitions.
> perf cache --query [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
Replace --query with --show?
> File Format
> ===========
> All the cache files are placed under ~/.debug/ by default.
> Elf caches are ~/.debug/path/to/file/bu/ildid/elf
> Symbols caches are ~/.debug/path/to/file/bu/ildid/syms
> Probes(and SDT) are ~/.debug/path/to/file/bu/ildid/probes
> And ~/.debug/path/to/file/bu/ildid/ is linked to ~/.debug/.buildid/bu/ildid
> Optionally, we can gzip the probes file.
Ok!
> This probe caches contain probe-definitions as following format.
> ----
> #buildid:BUILDID
> #spec:SDT
> p:sdt_<PROVIDER>/<EVENT> PATH:OFFSET [ARGS]
> ...
> #spec:* $params
> p:probe_<GROUP>/<EVENT> _text+OFFSET [ARGS]
> ...
> ----
> So the #spec: line gives the information what probe spec has been given.
> This will be used for updating.
>
> And all the "probe_" and "sdt_" prefix will be replaced by % in the command line,
> e.g.
>
> perf record -e %<PROVIDER>/<EVENT> -> this records sdt_PROVIDER/EVENT
>
> Thank you,
>
> --
> Masami HIRAMATSU
> Software Platform Research Dept. Linux Technology Research Center
> Hitachi, Ltd., Yokohama Research Laboratory
> E-mail: masami.hiramatsu.pt@hitachi.com
>
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-10 12:23 ` Arnaldo Carvalho de Melo
@ 2014-11-11 6:53 ` Masami Hiramatsu
2014-11-11 13:10 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-11 6:53 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Hemant Kumar, Namhyung Kim, linux-kernel, srikar, peterz, oleg,
hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
(2014/11/10 21:23), Arnaldo Carvalho de Melo wrote:
> Em Mon, Nov 10, 2014 at 07:59:24PM +0900, Masami Hiramatsu escreveu:
>> Hello,
>>
>> Here is the second try for the probe-cache. This version simplifies
>> the synopsis, and unifies the SDT and probe caches.
>> Please give me your comments/ideas!
>>
>> Command-line Synopsis
>> =====================
>>
>> Add elf(or symbols) and probe-caches of SDT if exists in <FILES>
>> perf cache --add <FILES> [--probe <SPEC>] # for user programs
>
> Why the --probe above? Shouldn't this be just (if you are talking about
> ELF files only):
>
> perf cache --add <FILES>
Yes, for the elf and sdt cache, we don't need --probe.
Note that "[]" means optional. If we would like to add some probe cache,
we need a spec of probe definition.
>> perf cache --kcore <FILE> [--probe <SPEC>] # for kcore ?
>
> Adrian, aren't kcore files easily identifiable as such and thus could be
> added as:
>
> perf cache --add <FILES>
>
>> perf cache --probe <SPEC> # for the current kernel
>
> Why do we need a --probe here? Don't they always start with a character
> that is seldomly used in ELF file names and thus we could get away with
> not requiring --probe?
This is only for adding the probe cache (not elf, nor sdt), which requires
a probe definition. Moreover, I'd like to unify the specification of the
probe definition with perf-probe. In that case, --probe is more natural.
>> Remove caches related to <FILES> or <BUILDIDS>
>> perf cache --remove <FILES>|<BUILDIDS>
>>
>> Show all probe caches(including SDT) or buildids
>> perf cache --list [probe|buildid]
>>
>> Delete existing probe-cache entries for kernel, <PATH> or/and <BUILDID>.
>> perf cache　--probe-del [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
>
> Ditto, i.e. can't we just use:
>
> perf cache　--remove [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
>
> And it figure out that this is a probe that is being removed?
In most cases, it may be OK, but it is also possible to cause unexpected
result when mis-typing. I think if <FILE> is always starting at '/', it
is easy to identify.
>> Query the probe definitions.
>> perf cache --query [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
>
> Replace --query with --show?
OK.
Thank you,
>
>> File Format
>> ===========
>> All the cache files are placed under ~/.debug/ by default.
>> Elf caches are ~/.debug/path/to/file/bu/ildid/elf
>> Symbols caches are ~/.debug/path/to/file/bu/ildid/syms
>> Probes(and SDT) are ~/.debug/path/to/file/bu/ildid/probes
>> And ~/.debug/path/to/file/bu/ildid/ is linked to ~/.debug/.buildid/bu/ildid
>> Optionally, we can gzip the probes file.
>
> Ok!
>
>> This probe caches contain probe-definitions as following format.
>> ----
>> #buildid:BUILDID
>> #spec:SDT
>> p:sdt_<PROVIDER>/<EVENT> PATH:OFFSET [ARGS]
>> ...
>> #spec:* $params
>> p:probe_<GROUP>/<EVENT> _text+OFFSET [ARGS]
>> ...
>> ----
>> So the #spec: line gives the information what probe spec has been given.
>> This will be used for updating.
>>
>> And all the "probe_" and "sdt_" prefix will be replaced by % in the command line,
>> e.g.
>>
>> perf record -e %<PROVIDER>/<EVENT> -> this records sdt_PROVIDER/EVENT
>>
>> Thank you,
>>
>> --
>> Masami HIRAMATSU
>> Software Platform Research Dept. Linux Technology Research Center
>> Hitachi, Ltd., Yokohama Research Laboratory
>> E-mail: masami.hiramatsu.pt@hitachi.com
>>
>
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-11 6:53 ` Masami Hiramatsu
@ 2014-11-11 13:10 ` Arnaldo Carvalho de Melo
2014-11-12 15:26 ` Masami Hiramatsu
0 siblings, 1 reply; 49+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-11-11 13:10 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Hemant Kumar, Namhyung Kim, linux-kernel, srikar, peterz, oleg,
hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
Em Tue, Nov 11, 2014 at 03:53:42PM +0900, Masami Hiramatsu escreveu:
> (2014/11/10 21:23), Arnaldo Carvalho de Melo wrote:
> > Em Mon, Nov 10, 2014 at 07:59:24PM +0900, Masami Hiramatsu escreveu:
> >> Here is the second try for the probe-cache. This version simplifies
> >> the synopsis, and unifies the SDT and probe caches.
> >> Please give me your comments/ideas!
> >>
> >> Command-line Synopsis
> >> =====================
> >> Add elf(or symbols) and probe-caches of SDT if exists in <FILES>
> >> perf cache --add <FILES> [--probe <SPEC>] # for user programs
> > Why the --probe above? Shouldn't this be just (if you are talking about
> > ELF files only):
> > perf cache --add <FILES>
> Yes, for the elf and sdt cache, we don't need --probe.
> Note that "[]" means optional. If we would like to add some probe cache,
> we need a spec of probe definition.
I understand that, its just that it looked superfluous at that specific
place, where you are explaining how to add ELF files.
> >> perf cache --kcore <FILE> [--probe <SPEC>] # for kcore ?
> > Adrian, aren't kcore files easily identifiable as such and thus could be
> > added as:
> > perf cache --add <FILES>
> >> perf cache --probe <SPEC> # for the current kernel
> > Why do we need a --probe here? Don't they always start with a character
> > that is seldomly used in ELF file names and thus we could get away with
> > not requiring --probe?
> This is only for adding the probe cache (not elf, nor sdt), which requires
> a probe definition. Moreover, I'd like to unify the specification of the
> probe definition with perf-probe. In that case, --probe is more natural.
What I meant was, what is wrong with replacing:
perf cache --probe <SPEC> # for the current kernel
With:
perf cache --add <PROBE-SPEC> # for the current kernel
And have it figure out that what is being added is a probe and do the
right thing?
> >> Remove caches related to <FILES> or <BUILDIDS>
> >> perf cache --remove <FILES>|<BUILDIDS>
> >>
> >> Show all probe caches(including SDT) or buildids
> >> perf cache --list [probe|buildid]
> >>
> >> Delete existing probe-cache entries for kernel, <PATH> or/and <BUILDID>.
> >> perf cache　--probe-del [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
> >
> > Ditto, i.e. can't we just use:
> >
> > perf cache　--remove [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
> >
> > And it figure out that this is a probe that is being removed?
>
> In most cases, it may be OK, but it is also possible to cause unexpected
> result when mis-typing. I think if <FILE> is always starting at '/', it
> is easy to identify.
We can keep the explicit switch (--probe-del) perhaps to resolve
ambiguities, if they happen, but make it so that it is not strictly
required for the common case.
- Thanks,
- Arnaldo
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-11 13:10 ` Arnaldo Carvalho de Melo
@ 2014-11-12 15:26 ` Masami Hiramatsu
2014-11-17 3:09 ` Namhyung Kim
0 siblings, 1 reply; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-12 15:26 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Hemant Kumar, Namhyung Kim, linux-kernel, srikar, peterz, oleg,
hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
(2014/11/11 22:10), Arnaldo Carvalho de Melo wrote:
> Em Tue, Nov 11, 2014 at 03:53:42PM +0900, Masami Hiramatsu escreveu:
>> (2014/11/10 21:23), Arnaldo Carvalho de Melo wrote:
>>> Em Mon, Nov 10, 2014 at 07:59:24PM +0900, Masami Hiramatsu escreveu:
>>>> Here is the second try for the probe-cache. This version simplifies
>>>> the synopsis, and unifies the SDT and probe caches.
>>>> Please give me your comments/ideas!
>>>>
>>>> Command-line Synopsis
>>>> =====================
>
>>>> Add elf(or symbols) and probe-caches of SDT if exists in <FILES>
>>>> perf cache --add <FILES> [--probe <SPEC>] # for user programs
>
>>> Why the --probe above? Shouldn't this be just (if you are talking about
>>> ELF files only):
>
>>> perf cache --add <FILES>
>
>> Yes, for the elf and sdt cache, we don't need --probe.
>> Note that "[]" means optional. If we would like to add some probe cache,
>> we need a spec of probe definition.
>
> I understand that, its just that it looked superfluous at that specific
> place, where you are explaining how to add ELF files.
>
>>>> perf cache --kcore <FILE> [--probe <SPEC>] # for kcore ?
>
>>> Adrian, aren't kcore files easily identifiable as such and thus could be
>>> added as:
>
>>> perf cache --add <FILES>
>
>>>> perf cache --probe <SPEC> # for the current kernel
>
>>> Why do we need a --probe here? Don't they always start with a character
>>> that is seldomly used in ELF file names and thus we could get away with
>>> not requiring --probe?
>
>> This is only for adding the probe cache (not elf, nor sdt), which requires
>> a probe definition. Moreover, I'd like to unify the specification of the
>> probe definition with perf-probe. In that case, --probe is more natural.
>
> What I meant was, what is wrong with replacing:
>
> perf cache --probe <SPEC> # for the current kernel
>
> With:
>
> perf cache --add <PROBE-SPEC> # for the current kernel
>
> And have it figure out that what is being added is a probe and do the
> right thing?
As I've said previously, PROBE-SPEC can be same as FILES (imagine that a binary
file which has same name function in the kernel.)
Moreover, PROBE-SPEC requires the target binary(or kernel module) except for
kernel probes. In that case, anyway we need -x or -m options with file-path
for --add, that is very strange.
e.g.
For me,
perf cache --add ./binary --probe '*'
looks more natural than
perf cache --add '*' -exec ./binary
since in other cases(sdt/elf), we'll just do
perf cache --add ./binary
:-/
>>>> Remove caches related to <FILES> or <BUILDIDS>
>>>> perf cache --remove <FILES>|<BUILDIDS>
>>>>
>>>> Show all probe caches(including SDT) or buildids
>>>> perf cache --list [probe|buildid]
>>>>
>>>> Delete existing probe-cache entries for kernel, <PATH> or/and <BUILDID>.
>>>> perf cache　--probe-del [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
>>>
>>> Ditto, i.e. can't we just use:
>>>
>>> perf cache　--remove [<GROUP>:]<EVENT>[@<PATH>][#<BUILDID>]
>>>
>>> And it figure out that this is a probe that is being removed?
>>
>> In most cases, it may be OK, but it is also possible to cause unexpected
>> result when mis-typing. I think if <FILE> is always starting at '/', it
>> is easy to identify.
>
> We can keep the explicit switch (--probe-del) perhaps to resolve
> ambiguities, if they happen, but make it so that it is not strictly
> required for the common case.
OK, it'll take a longer time to remove, since we need to load all caches
to find matching entries of probe caches, but is feasible.
Thank you!
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-12 15:26 ` Masami Hiramatsu
@ 2014-11-17 3:09 ` Namhyung Kim
2014-11-17 3:17 ` Masami Hiramatsu
2014-11-17 18:59 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 49+ messages in thread
From: Namhyung Kim @ 2014-11-17 3:09 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Arnaldo Carvalho de Melo, Hemant Kumar, linux-kernel, srikar,
peterz, oleg, hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
Hi Masami,
On Thu, 13 Nov 2014 00:25:57 +0900, Masami Hiramatsu wrote:
> (2014/11/11 22:10), Arnaldo Carvalho de Melo wrote:
>> What I meant was, what is wrong with replacing:
>>
>> perf cache --probe <SPEC> # for the current kernel
>>
>> With:
>>
>> perf cache --add <PROBE-SPEC> # for the current kernel
>>
>> And have it figure out that what is being added is a probe and do the
>> right thing?
>
> As I've said previously, PROBE-SPEC can be same as FILES (imagine that a binary
> file which has same name function in the kernel.)
> Moreover, PROBE-SPEC requires the target binary(or kernel module) except for
> kernel probes. In that case, anyway we need -x or -m options with file-path
> for --add, that is very strange.
>
> e.g.
>
> For me,
>
> perf cache --add ./binary --probe '*'
>
> looks more natural than
>
> perf cache --add '*' -exec ./binary
>
> since in other cases(sdt/elf), we'll just do
>
> perf cache --add ./binary
I prefer this too. But I'd like make the 'add' part a subcommand rather
than option like we do in perf kmem/kvm/list/lock/mem/sched ... And it
can handle multiple files at once. What about this?
perf cache add [--elf|--sdt|--probe <spec>] <binary> [<binary>...]
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Re: [RFC] perf-cache command interface design
2014-11-17 3:09 ` Namhyung Kim
@ 2014-11-17 3:17 ` Masami Hiramatsu
2014-11-17 22:09 ` Masami Hiramatsu
2014-11-18 4:41 ` Namhyung Kim
2014-11-17 18:59 ` Arnaldo Carvalho de Melo
1 sibling, 2 replies; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-17 3:17 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Hemant Kumar, linux-kernel, srikar,
peterz, oleg, hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
(2014/11/17 12:08), Namhyung Kim wrote:
> Hi Masami,
>
> On Thu, 13 Nov 2014 00:25:57 +0900, Masami Hiramatsu wrote:
>> (2014/11/11 22:10), Arnaldo Carvalho de Melo wrote:
>>> What I meant was, what is wrong with replacing:
>>>
>>> perf cache --probe <SPEC> # for the current kernel
>>>
>>> With:
>>>
>>> perf cache --add <PROBE-SPEC> # for the current kernel
>>>
>>> And have it figure out that what is being added is a probe and do the
>>> right thing?
>>
>> As I've said previously, PROBE-SPEC can be same as FILES (imagine that a binary
>> file which has same name function in the kernel.)
>> Moreover, PROBE-SPEC requires the target binary(or kernel module) except for
>> kernel probes. In that case, anyway we need -x or -m options with file-path
>> for --add, that is very strange.
>>
>> e.g.
>>
>> For me,
>>
>> perf cache --add ./binary --probe '*'
>>
>> looks more natural than
>>
>> perf cache --add '*' -exec ./binary
>>
>> since in other cases(sdt/elf), we'll just do
>>
>> perf cache --add ./binary
>
> I prefer this too. But I'd like make the 'add' part a subcommand rather
> than option like we do in perf kmem/kvm/list/lock/mem/sched ... And it
> can handle multiple files at once. What about this?
>
> perf cache add [--elf|--sdt|--probe <spec>] <binary> [<binary>...]
OK, that's good to me. And I think --elf/--sdt is meaningless.
Only --probe option is required, since we can scan the elf file to
add sdt cache when adding elf binary :)
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Re: Re: [RFC] perf-cache command interface design
2014-11-17 3:17 ` Masami Hiramatsu
@ 2014-11-17 22:09 ` Masami Hiramatsu
2014-11-18 4:51 ` Namhyung Kim
2014-11-18 4:41 ` Namhyung Kim
1 sibling, 1 reply; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-17 22:09 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Hemant Kumar, linux-kernel, srikar,
peterz, oleg, hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
(2014/11/17 12:17), Masami Hiramatsu wrote:
> (2014/11/17 12:08), Namhyung Kim wrote:
>> Hi Masami,
>>
>> On Thu, 13 Nov 2014 00:25:57 +0900, Masami Hiramatsu wrote:
>>> (2014/11/11 22:10), Arnaldo Carvalho de Melo wrote:
>>>> What I meant was, what is wrong with replacing:
>>>>
>>>> perf cache --probe <SPEC> # for the current kernel
>>>>
>>>> With:
>>>>
>>>> perf cache --add <PROBE-SPEC> # for the current kernel
>>>>
>>>> And have it figure out that what is being added is a probe and do the
>>>> right thing?
>>>
>>> As I've said previously, PROBE-SPEC can be same as FILES (imagine that a binary
>>> file which has same name function in the kernel.)
>>> Moreover, PROBE-SPEC requires the target binary(or kernel module) except for
>>> kernel probes. In that case, anyway we need -x or -m options with file-path
>>> for --add, that is very strange.
>>>
>>> e.g.
>>>
>>> For me,
>>>
>>> perf cache --add ./binary --probe '*'
>>>
>>> looks more natural than
>>>
>>> perf cache --add '*' -exec ./binary
>>>
>>> since in other cases(sdt/elf), we'll just do
>>>
>>> perf cache --add ./binary
>>
>> I prefer this too. But I'd like make the 'add' part a subcommand rather
>> than option like we do in perf kmem/kvm/list/lock/mem/sched ... And it
>> can handle multiple files at once. What about this?
>>
>> perf cache add [--elf|--sdt|--probe <spec>] <binary> [<binary>...]
>
> OK, that's good to me. And I think --elf/--sdt is meaningless.
> Only --probe option is required, since we can scan the elf file to
> add sdt cache when adding elf binary :)
BTW, what should we do if we put the probe cache on current running kernel?
perf cache add --probe <probe-spec>
and have no binary argument, is it OK?
Thanks,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-17 22:09 ` Masami Hiramatsu
@ 2014-11-18 4:51 ` Namhyung Kim
2014-11-18 11:16 ` Masami Hiramatsu
0 siblings, 1 reply; 49+ messages in thread
From: Namhyung Kim @ 2014-11-18 4:51 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Arnaldo Carvalho de Melo, Hemant Kumar, linux-kernel, srikar,
peterz, oleg, hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
On Tue, 18 Nov 2014 07:09:02 +0900, Masami Hiramatsu wrote:
> (2014/11/17 12:17), Masami Hiramatsu wrote:
>> (2014/11/17 12:08), Namhyung Kim wrote:
>>> I prefer this too. But I'd like make the 'add' part a subcommand rather
>>> than option like we do in perf kmem/kvm/list/lock/mem/sched ... And it
>>> can handle multiple files at once. What about this?
>>>
>>> perf cache add [--elf|--sdt|--probe <spec>] <binary> [<binary>...]
>>
>> OK, that's good to me. And I think --elf/--sdt is meaningless.
>> Only --probe option is required, since we can scan the elf file to
>> add sdt cache when adding elf binary :)
>
> BTW, what should we do if we put the probe cache on current running kernel?
>
> perf cache add --probe <probe-spec>
>
> and have no binary argument, is it OK?
Hmm.. what about passing /proc/kallsyms and/or /boot/vmlinux for that?
Now I found that we need to special-case the kallsyms non-ELF file :)
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-18 4:51 ` Namhyung Kim
@ 2014-11-18 11:16 ` Masami Hiramatsu
0 siblings, 0 replies; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-18 11:16 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Hemant Kumar, linux-kernel, srikar,
peterz, oleg, hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
(2014/11/18 13:51), Namhyung Kim wrote:
> On Tue, 18 Nov 2014 07:09:02 +0900, Masami Hiramatsu wrote:
>> (2014/11/17 12:17), Masami Hiramatsu wrote:
>>> (2014/11/17 12:08), Namhyung Kim wrote:
>>>> I prefer this too. But I'd like make the 'add' part a subcommand rather
>>>> than option like we do in perf kmem/kvm/list/lock/mem/sched ... And it
>>>> can handle multiple files at once. What about this?
>>>>
>>>> perf cache add [--elf|--sdt|--probe <spec>] <binary> [<binary>...]
>>>
>>> OK, that's good to me. And I think --elf/--sdt is meaningless.
>>> Only --probe option is required, since we can scan the elf file to
>>> add sdt cache when adding elf binary :)
>>
>> BTW, what should we do if we put the probe cache on current running kernel?
>>
>> perf cache add --probe <probe-spec>
>>
>> and have no binary argument, is it OK?
>
> Hmm.. what about passing /proc/kallsyms and/or /boot/vmlinux for that?
Yeah, but I think it is just optional (as same as perf probe's -k option),
we can skip that too.
> Now I found that we need to special-case the kallsyms non-ELF file :)
So, I wonder that we'd better use "bu/ildid/dso" instead of "bu/ildid/elf".
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-17 3:17 ` Masami Hiramatsu
2014-11-17 22:09 ` Masami Hiramatsu
@ 2014-11-18 4:41 ` Namhyung Kim
2014-11-18 10:32 ` Masami Hiramatsu
1 sibling, 1 reply; 49+ messages in thread
From: Namhyung Kim @ 2014-11-18 4:41 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Arnaldo Carvalho de Melo, Hemant Kumar, linux-kernel, srikar,
peterz, oleg, hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
Hi Masami,
On Mon, 17 Nov 2014 12:17:31 +0900, Masami Hiramatsu wrote:
> (2014/11/17 12:08), Namhyung Kim wrote:
>> I prefer this too. But I'd like make the 'add' part a subcommand rather
>> than option like we do in perf kmem/kvm/list/lock/mem/sched ... And it
>> can handle multiple files at once. What about this?
>>
>> perf cache add [--elf|--sdt|--probe <spec>] <binary> [<binary>...]
>
> OK, that's good to me. And I think --elf/--sdt is meaningless.
Maybe not :)
I'm considering the opposite side - by providing the options, we also
support the negative ones too. So --no-elf and/or --no-sdt options are
possible. Also the positive options can be used with del(ete)
subcommand to remove some contents selectively.
I think it'd be helpful as we sometimes don't want to do that for some
reason. For example, current perf record adds binary (elf) files to the
cache automatically iff it's accessed. But what about SDTs? Should we
add SDTs at the same time? If not, what if we try to add existing elf
files only for SDTs?
Thanks,
Namhyung
> Only --probe option is required, since we can scan the elf file to
> add sdt cache when adding elf binary :)
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: Re: [RFC] perf-cache command interface design
2014-11-18 4:41 ` Namhyung Kim
@ 2014-11-18 10:32 ` Masami Hiramatsu
0 siblings, 0 replies; 49+ messages in thread
From: Masami Hiramatsu @ 2014-11-18 10:32 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Hemant Kumar, linux-kernel, srikar,
peterz, oleg, hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
(2014/11/18 13:41), Namhyung Kim wrote:
> Hi Masami,
>
> On Mon, 17 Nov 2014 12:17:31 +0900, Masami Hiramatsu wrote:
>> (2014/11/17 12:08), Namhyung Kim wrote:
>>> I prefer this too. But I'd like make the 'add' part a subcommand rather
>>> than option like we do in perf kmem/kvm/list/lock/mem/sched ... And it
>>> can handle multiple files at once. What about this?
>>>
>>> perf cache add [--elf|--sdt|--probe <spec>] <binary> [<binary>...]
>>
>> OK, that's good to me. And I think --elf/--sdt is meaningless.
>
> Maybe not :)
>
> I'm considering the opposite side - by providing the options, we also
> support the negative ones too. So --no-elf and/or --no-sdt options are
> possible. Also the positive options can be used with del(ete)
> subcommand to remove some contents selectively.
>
> I think it'd be helpful as we sometimes don't want to do that for some
> reason. For example, current perf record adds binary (elf) files to the
> cache automatically iff it's accessed. But what about SDTs? Should we
> add SDTs at the same time? If not, what if we try to add existing elf
> files only for SDTs?
Ah, I see. Indeed, in this case we'd better have perf cache add --sdt <bin>
for explicitly adding SDTs. (Of course perf cache add <bin> can also
add SDTs automagically, but adding --sdt is more natural)
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-17 3:09 ` Namhyung Kim
2014-11-17 3:17 ` Masami Hiramatsu
@ 2014-11-17 18:59 ` Arnaldo Carvalho de Melo
2014-11-18 4:46 ` Namhyung Kim
1 sibling, 1 reply; 49+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-11-17 18:59 UTC (permalink / raw)
To: Namhyung Kim
Cc: Masami Hiramatsu, Hemant Kumar, linux-kernel, srikar, peterz,
oleg, hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
Em Mon, Nov 17, 2014 at 12:08:59PM +0900, Namhyung Kim escreveu:
> On Thu, 13 Nov 2014 00:25:57 +0900, Masami Hiramatsu wrote:
> > (2014/11/11 22:10), Arnaldo Carvalho de Melo wrote:
> >> What I meant was, what is wrong with replacing:
> >> perf cache --probe <SPEC> # for the current kernel
> >> With:
> >> perf cache --add <PROBE-SPEC> # for the current kernel
> >> And have it figure out that what is being added is a probe and do the
> >> right thing?
> > As I've said previously, PROBE-SPEC can be same as FILES (imagine that a binary
> > file which has same name function in the kernel.)
> > Moreover, PROBE-SPEC requires the target binary(or kernel module) except for
> > kernel probes. In that case, anyway we need -x or -m options with file-path
> > for --add, that is very strange.
> > e.g.
> > For me,
> > perf cache --add ./binary --probe '*'
> > looks more natural than
> > perf cache --add '*' -exec ./binary
> > since in other cases(sdt/elf), we'll just do
> > perf cache --add ./binary
> I prefer this too. But I'd like make the 'add' part a subcommand rather
> than option like we do in perf kmem/kvm/list/lock/mem/sched ... And it
> can handle multiple files at once. What about this?
> perf cache add [--elf|--sdt|--probe <spec>] <binary> [<binary>...]
In the end we can have it both ways, i.e. if the user does just:
perf cache add something
or:
perf cache add --elf something
And 'something' is an ELF file, then in the first case (no --elf
specified) it will figure it out (checking the magic number, etc) and do
the right thing.
In the second case since we're being more verbose and think we know what
'something' is (an ELF file) the tool can check if it indeed is an ELF
file and if not, bail out.
- Arnaldo
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [RFC] perf-cache command interface design
2014-11-17 18:59 ` Arnaldo Carvalho de Melo
@ 2014-11-18 4:46 ` Namhyung Kim
0 siblings, 0 replies; 49+ messages in thread
From: Namhyung Kim @ 2014-11-18 4:46 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Masami Hiramatsu, Hemant Kumar, linux-kernel, srikar, peterz,
oleg, hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
Hi Arnaldo,
On Mon, 17 Nov 2014 16:58:57 -0200, Arnaldo Carvalho de Melo wrote:
> Em Mon, Nov 17, 2014 at 12:08:59PM +0900, Namhyung Kim escreveu:
>> I prefer this too. But I'd like make the 'add' part a subcommand rather
>> than option like we do in perf kmem/kvm/list/lock/mem/sched ... And it
>> can handle multiple files at once. What about this?
>
>> perf cache add [--elf|--sdt|--probe <spec>] <binary> [<binary>...]
>
> In the end we can have it both ways, i.e. if the user does just:
>
> perf cache add something
>
> or:
>
> perf cache add --elf something
>
> And 'something' is an ELF file, then in the first case (no --elf
> specified) it will figure it out (checking the magic number, etc) and do
> the right thing.
>
> In the second case since we're being more verbose and think we know what
> 'something' is (an ELF file) the tool can check if it indeed is an ELF
> file and if not, bail out.
Ah, that's possible.
I'm not sure what we can do if it's not an ELF file though. :)
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-07 8:21 ` [RFC] perf-cache command interface design Masami Hiramatsu
` (4 preceding siblings ...)
2014-11-10 10:59 ` Masami Hiramatsu
@ 2014-11-10 12:05 ` Hagen Paul Pfeifer
2014-11-10 12:31 ` Arnaldo Carvalho de Melo
2014-11-10 12:50 ` Peter Zijlstra
5 siblings, 2 replies; 49+ messages in thread
From: Hagen Paul Pfeifer @ 2014-11-10 12:05 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Hemant Kumar, Namhyung Kim, Arnaldo Carvalho de Melo,
linux-kernel, srikar, peterz, oleg, hegdevasant, mingo,
systemtap, aravinda, penberg, brendan.d.gregg, yrl.pp-manager.tt
On 7 November 2014 09:21, Masami Hiramatsu
<masami.hiramatsu.pt@hitachi.com> wrote:
> File Format
> ===========
> All the cache files are placed under ~/.debug/ by default.
> The paths of buildid cache of binary/symbols are not changed.
>
> The SDT/probe caches are placed under the ~/.debug/.probes/path/to/bin/bu/ildid
> and that is linked to ~/.debug/.probes/.buildid/bu/ildid
> # To avoid conflict with files under /probes/*, I picked up .probes/.
A little bit late: but why ~/.debug? Why not $XDG_CACHE_HOME/perf/ as
the root for all perf related files? debug is not unique nor is it sufficient
to meet file hierarchy: man file-hierarchy(8)
Hagen
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [RFC] perf-cache command interface design
2014-11-10 12:05 ` Hagen Paul Pfeifer
@ 2014-11-10 12:31 ` Arnaldo Carvalho de Melo
2014-11-10 12:50 ` Peter Zijlstra
1 sibling, 0 replies; 49+ messages in thread
From: Arnaldo Carvalho de Melo @ 2014-11-10 12:31 UTC (permalink / raw)
To: Hagen Paul Pfeifer
Cc: Masami Hiramatsu, Hemant Kumar, Namhyung Kim, linux-kernel,
srikar, peterz, oleg, hegdevasant, mingo, systemtap, aravinda,
penberg, brendan.d.gregg, yrl.pp-manager.tt
Em Mon, Nov 10, 2014 at 01:05:45PM +0100, Hagen Paul Pfeifer escreveu:
> On 7 November 2014 09:21, Masami Hiramatsu
> <masami.hiramatsu.pt@hitachi.com> wrote:
>
> > File Format
> > ===========
> > All the cache files are placed under ~/.debug/ by default.
> > The paths of buildid cache of binary/symbols are not changed.
> >
> > The SDT/probe caches are placed under the ~/.debug/.probes/path/to/bin/bu/ildid
> > and that is linked to ~/.debug/.probes/.buildid/bu/ildid
> > # To avoid conflict with files under /probes/*, I picked up .probes/.
>
> A little bit late: but why ~/.debug? Why not $XDG_CACHE_HOME/perf/ as
Why "perf"?
> the root for all perf related files? debug is not unique nor is it sufficient
Probably should be some other name then, but ~/.debug/ is there since
the build-id cache was introduced, guess this is why it is being kept so
far in Masami's proposal.
> to meet file hierarchy: man file-hierarchy(8)
Humm,
It starts in $HOME/.debug/ because this is not supposed to be a system
wide cache, a developer can, for instance:
1. perf record myapp
2. edit myapp.c
3. rebuild it
4. perf record myapp
5. perf diff
And it will show the difference from the previous version, for which it
stored a copy of its binary on its private, keyed by build-id, cache.
So this is not merely a place where we will read stuff from.
If somebody decides to have it in a place accessible by multiple users
on the same system, then, yes, some more suitable place outside its
$HOME is needed, and then tooling should look there as well as on the
places it already looks for such files, i.e. $HOME/.debug/,
/usr/lib/debug (where foo-debuginfo packages store stuff, also keyed by
build-id), /lib/modules/$KVR/, etc.
- Arnaldo
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-10 12:05 ` Hagen Paul Pfeifer
2014-11-10 12:31 ` Arnaldo Carvalho de Melo
@ 2014-11-10 12:50 ` Peter Zijlstra
2014-11-10 13:37 ` Hagen Paul Pfeifer
1 sibling, 1 reply; 49+ messages in thread
From: Peter Zijlstra @ 2014-11-10 12:50 UTC (permalink / raw)
To: Hagen Paul Pfeifer
Cc: Masami Hiramatsu, Hemant Kumar, Namhyung Kim,
Arnaldo Carvalho de Melo, linux-kernel, srikar, oleg,
hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
On Mon, Nov 10, 2014 at 01:05:45PM +0100, Hagen Paul Pfeifer wrote:
> On 7 November 2014 09:21, Masami Hiramatsu
> <masami.hiramatsu.pt@hitachi.com> wrote:
>
> > File Format
> > ===========
> > All the cache files are placed under ~/.debug/ by default.
> > The paths of buildid cache of binary/symbols are not changed.
> >
> > The SDT/probe caches are placed under the ~/.debug/.probes/path/to/bin/bu/ildid
> > and that is linked to ~/.debug/.probes/.buildid/bu/ildid
> > # To avoid conflict with files under /probes/*, I picked up .probes/.
>
> A little bit late: but why ~/.debug? Why not $XDG_CACHE_HOME/perf/ as
> the root for all perf related files? debug is not unique nor is it sufficient
> to meet file hierarchy: man file-hierarchy(8)
# env | grep XDG | wc -l
0
Which renders it useless crap in my book.
I agree on the .debug name being somewhat generic, also, it would be:
man 8 file-hierarchy, but that too fails:
$ man file-hierarchy
No manual entry for file-hierarchy
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [RFC] perf-cache command interface design
2014-11-10 12:50 ` Peter Zijlstra
@ 2014-11-10 13:37 ` Hagen Paul Pfeifer
0 siblings, 0 replies; 49+ messages in thread
From: Hagen Paul Pfeifer @ 2014-11-10 13:37 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Masami Hiramatsu, Hemant Kumar, Namhyung Kim,
Arnaldo Carvalho de Melo, linux-kernel, srikar, oleg,
hegdevasant, mingo, systemtap, aravinda, penberg,
brendan.d.gregg, yrl.pp-manager.tt
On 10 November 2014 13:50, Peter Zijlstra <peterz@infradead.org> wrote:
> # env | grep XDG | wc -l
> 0
>
> Which renders it useless crap in my book.
>
> I agree on the .debug name being somewhat generic, also, it would be:
> man 8 file-hierarchy, but that too fails:
>
> $ man file-hierarchy
> No manual entry for file-hierarchy
Ok, but then we are not even consistent to ourself, see "perf: Add
support for full Intel event lists" which download already to
$XDG_CACHE_HOME [1]. It is not useless to agree on one standard.
Reinvent the place where caches and configurations are stored again
and again is somewhat stupid. E.g. ~/.cache (or $XDG_CACHE_HOME) can
be deleted without problems, ~/.config not, and so on. Someone
(Rusty?) thought about this several years ago, IMHO we should give at
least a try.
Hagen
[1] https://lkml.org/lkml/2014/7/30/686
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-11-05 7:06 ` Namhyung Kim
2014-11-05 9:05 ` Masami Hiramatsu
@ 2014-11-05 18:24 ` Hemant Kumar
1 sibling, 0 replies; 49+ messages in thread
From: Hemant Kumar @ 2014-11-05 18:24 UTC (permalink / raw)
To: Namhyung Kim, Masami Hiramatsu
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo, anton,
systemtap, aravinda, penberg
On 11/05/2014 12:36 PM, Namhyung Kim wrote:
> On Tue, 04 Nov 2014 21:56:53 +0900, Masami Hiramatsu wrote:
>> Hi,
>>
>> (2014/11/04 17:06), Hemant Kumar wrote:
>>> Hi Namhyung,
>>>
>>> On 11/04/2014 01:08 PM, Namhyung Kim wrote:
>>>> Hi Hemant,
>>>>
>>>> As you know, you need to keep an eye on how (kprobes) event cache
>>>> patchset from Masami settles down. For those who aren't CC'ed, please
>>>> see the link below:
>>>>
>>>> https://lkml.org/lkml/2014/10/31/207
>>>>
>>>> On Sun, 02 Nov 2014 16:26:28 +0530, Hemant Kumar wrote:
>>>>> This patch adds support to perf to record SDT events. When invoked,
>>>>> the SDT event is looked up in the sdt-cache. If its found, an entry is
>>>>> made silently to uprobe_events file and then recording is invoked, and
>>>>> then the entry for the SDT event in uprobe_events is silently discarded.
>>>>>
>>>>> The SDT events are already stored in a cache file
>>>>> (/var/cache/perf/perf-sdt-file.cache).
>>>>> Although the file_hash table helps in addition or deletion of SDT events
>>>>> from the cache, its not of much use when it comes to probing the actual
>>>>> SDT event, because the key to this hash list is a file name and not the
>>>>> SDT event name (which is given as an argument to perf record). So, we
>>>>> won't be able to hash into it.
>>>> It likely to be ended up with per-file or per-buildid cache files under
>>>> ~/.debug directory. In this case we also need to have the (central)
>>>> event-to-cache table anyway IMHO.
>> What we are talking is to make a new caching file with buildid under
>> .debug/.
>> We already has ~/.debug/.build-id/<build-id> for string the binary
>> symbol maps.
> ??
>
> The ~/.debug/.build-id/<build-id> (actually first 2 hexdigits are used
> for directory name) is a symlink to a cached binary.
>
> $ file .debug/.build-id/00/08a6c4028b3826f8905324c770e7aa450e5d3b
> .debug/.build-id/00/08a6c4028b3826f8905324c770e7aa450e5d3b: symbolic link to `../../usr/lib64/libgtk-3.so.0.400.4/0008a6c4028b3826f8905324c770e7aa450e5d3b'
>
> $ file .debug/usr/lib64/libgtk-3.so.0.400.4/0008a6c4028b3826f8905324c770e7aa450e5d3b
> .debug/usr/lib64/libgtk-3.so.0.400.4/0008a6c4028b3826f8905324c770e7aa450e5d3b: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, BuildID[sha1]=0xc4a6080026388b02245390f8aae770c73b5d0e45, stripped
>
>
> Hmm.. it seems the file utility prints the build-id as a sequence of 4
> byte little-endian integers.
>
>
>> I think there are 2 options, one is expanding the current
>> build-id file format to include sdt and probe-event caches. The other is
>> to add ~/.debug/.build-id/<build-id>.probe and
>> ~/.debug/.build-id/<build-id>.sdt for caching probe/sdt information.
> I think a single .probe file is enough for this, no?
>
>
>> And also, user interface is a discussion point. This series defines new
>> sdt-cache command, and we already have buildid-cache command. We should
>> have probe-cache command too? or consolidate those cache managing commands?
>> This question should be involving your series too.
>>
>>>>> To avoid this problem, we can create another hash list "event_hash" list
>>>>> which will be maintained along with the file_hash list.
>>>>> Whenever a user invokes 'perf record -e %provider:event, perf should
>>>>> initialize the event_hash list and the file_hash list.
>>>>> The key to event_hash list is calculated from the event name and its
>>>>> provider name.
>>>> Isn't it enough just to use provide name? I guess the provider names
>>>> are (should be?) unique among a system although there's no absolute
>>>> guarantee for that.
>>>>
>>> Yes, there is no guarantee for the provider names to be unique.
>>> If we use only provider name with "perf record", then, what if a user
>>> wants to trace
>>> only a specific SDT event (not all the events for that provider)?
>>> What do you think?
> What I'm saying is for managing cache not the usage of the cached
> events. IIUC you keep hash entry for all events to find matching file,
> but I think it can be managed in provider level as events in a single
> provider will live in a single binary.
Oh! I see.
So, the events should be mapped to their respective files based on only
the provider names. And in case of any ambiguity, it should fail and ask
for the binary path as Masami suggested below.
Yeah, that's a nice idea. :)
> Btw, I think we should support such multiple events to like
>
> # perf record -e %provider_xxx:* -e %provider_yyy:prefix_*
>
>
>> How about failing if the provider name is not unique unless user
>> gives the actual binary path?
> It looks like a possible option. :)
Agreed.
--
Thanks,
Hemant Kumar
^ permalink raw reply [flat|nested] 49+ messages in thread