* [PATCH v3 1/5] perf/sdt: ELF support for SDT
2014-10-10 10:58 [PATCH v3 0/5] perf/sdt: SDT events listing/probing Hemant Kumar
@ 2014-10-10 10:58 ` Hemant Kumar
2014-10-22 2:39 ` Namhyung Kim
2014-10-10 10:59 ` [PATCH v3 3/5] perf/sdt: Show SDT cache contents Hemant Kumar
` (3 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Hemant Kumar @ 2014-10-10 10:58 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.
Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
---
tools/perf/util/symbol-elf.c | 257 ++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/symbol.h | 21 +++
2 files changed, 278 insertions(+)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 1e23a5b..e403df6 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1677,6 +1677,263 @@ 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, int type,
+ 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 i, ret = -EINVAL;
+
+ union {
+ Elf64_Addr a64[3];
+ Elf32_Addr a32[3];
+ } buf;
+
+ Elf_Data dst = {
+ .d_buf = &buf, .d_type = ELF_T_ADDR, .d_version = EV_CURRENT,
+ .d_size = gelf_fsize((*elf), ELF_T_ADDR, 3, EV_CURRENT),
+ .d_off = 0, .d_align = 0
+ };
+ Elf_Data src = {
+ .d_buf = (void *) data, .d_type = ELF_T_ADDR,
+ .d_version = EV_CURRENT, .d_size = dst.d_size, .d_off = 0,
+ .d_align = 0
+ };
+
+ /* Check the type of each of the notes */
+ if (type != SDT_NOTE_TYPE)
+ goto out_err;
+
+ tmp = (struct sdt_note *)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;
+ }
+
+ /* Obtain the addresses */
+ if (gelf_getclass(*elf) == ELFCLASS32) {
+ for (i = 0; i < 3; i++)
+ tmp->addr.a32[i] = buf.a32[i];
+ tmp->bit32 = true;
+ } else {
+ for (i = 0; i < 3; i++)
+ tmp->addr.a64[i] = buf.a64[i];
+ tmp->bit32 = false;
+ }
+
+ /* Now Adjust the prelink effect */
+ if (!gelf_getehdr(*elf, &ehdr)) {
+ pr_debug("%s : cannot get elf header.\n", __func__);
+ ret = -EBADF;
+ goto out_free_name;
+ }
+
+ /*
+ * 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))) {
+ ret = populate_sdt_note(&elf, ((data->d_buf) + desc_off),
+ nhdr.n_descsz, nhdr.n_type,
+ 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 bec4b7b..23d3d41 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"
+
#endif /* __PERF_SYMBOL */
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v3 1/5] perf/sdt: ELF support for SDT
2014-10-10 10:58 ` [PATCH v3 1/5] perf/sdt: ELF support for SDT Hemant Kumar
@ 2014-10-22 2:39 ` Namhyung Kim
2014-10-22 10:26 ` Hemant Kumar
0 siblings, 1 reply; 20+ messages in thread
From: Namhyung Kim @ 2014-10-22 2:39 UTC (permalink / raw)
To: Hemant Kumar
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo, anton,
systemtap, masami.hiramatsu.pt, aravinda, penberg
Hi Hemant,
On Fri, 10 Oct 2014 16:27:53 +0530, Hemant Kumar wrote:
> 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.
>
> Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Just a nitpick below..
> +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)) {
I think the below is more readable:
if ((shdr.sh_type != SHT_NOTE) || (shdr.sh_flags & SHF_ALLOC)) {
Other than that, looks good to me!
Thanks,
Namhyung
> + 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))) {
> + ret = populate_sdt_note(&elf, ((data->d_buf) + desc_off),
> + nhdr.n_descsz, nhdr.n_type,
> + sdt_notes);
> + if (ret < 0)
> + goto out_ret;
> + }
> + }
> + if (list_empty(sdt_notes))
> + ret = -ENOENT;
> +
> +out_ret:
> + return ret;
> +}
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v3 1/5] perf/sdt: ELF support for SDT
2014-10-22 2:39 ` Namhyung Kim
@ 2014-10-22 10:26 ` Hemant Kumar
0 siblings, 0 replies; 20+ messages in thread
From: Hemant Kumar @ 2014-10-22 10:26 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo, anton,
systemtap, masami.hiramatsu.pt, aravinda, penberg
On 10/22/2014 08:09 AM, Namhyung Kim wrote:
> Hi Hemant,
>
> On Fri, 10 Oct 2014 16:27:53 +0530, Hemant Kumar wrote:
>> 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.
>>
>> Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
> Acked-by: Namhyung Kim <namhyung@kernel.org>
>
> Just a nitpick below..
>
>
>> +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)) {
> I think the below is more readable:
Yes.
>
> if ((shdr.sh_type != SHT_NOTE) || (shdr.sh_flags & SHF_ALLOC)) {
Will change to this.
>
> Other than that, looks good to me!
>
> Thanks,
> Namhyung
>
>
>> + 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))) {
>> + ret = populate_sdt_note(&elf, ((data->d_buf) + desc_off),
>> + nhdr.n_descsz, nhdr.n_type,
>> + sdt_notes);
>> + if (ret < 0)
>> + goto out_ret;
>> + }
>> + }
>> + if (list_empty(sdt_notes))
>> + ret = -ENOENT;
>> +
>> +out_ret:
>> + return ret;
>> +}
--
Thanks,
Hemant Kumar
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 3/5] perf/sdt: Show SDT cache contents
2014-10-10 10:58 [PATCH v3 0/5] perf/sdt: SDT events listing/probing Hemant Kumar
2014-10-10 10:58 ` [PATCH v3 1/5] perf/sdt: ELF support for SDT Hemant Kumar
@ 2014-10-10 10:59 ` Hemant Kumar
2014-10-22 4:48 ` Namhyung Kim
2014-10-10 10:59 ` [PATCH v3 2/5] perf/sdt: Add SDT events into a cache Hemant Kumar
` (2 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Hemant Kumar @ 2014-10-10 10:59 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 | 27 +++++++++++++++
tools/perf/util/parse-events.h | 1 +
tools/perf/util/sdt.c | 48 +++++++++++++++++++++++++++
4 files changed, 82 insertions(+), 1 deletion(-)
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..31041db 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,6 +29,15 @@ 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;
@@ -35,10 +45,14 @@ 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_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 sd-cache --dump",
NULL
};
@@ -50,9 +64,22 @@ int cmd_sdt_cache(int argc, const char **argv, const char *prefix __maybe_unused
symbol__elf_init();
if (params.add) {
+ if (params.dump) {
+ pr_err("Error: Don't use --dump with --add\n");
+ usage_with_options(sdt_cache_usage, sdt_cache_options);
+ }
+
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 e6efe2c..f43e6aa 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -110,5 +110,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 626f07e..4e64f3e 100644
--- a/tools/perf/util/sdt.c
+++ b/tools/perf/util/sdt.c
@@ -607,3 +607,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, "\t%%%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] 20+ messages in thread* Re: [PATCH v3 3/5] perf/sdt: Show SDT cache contents
2014-10-10 10:59 ` [PATCH v3 3/5] perf/sdt: Show SDT cache contents Hemant Kumar
@ 2014-10-22 4:48 ` Namhyung Kim
0 siblings, 0 replies; 20+ messages in thread
From: Namhyung Kim @ 2014-10-22 4:48 UTC (permalink / raw)
To: Hemant Kumar
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo, anton,
systemtap, masami.hiramatsu.pt, aravinda, penberg
On Fri, 10 Oct 2014 16:28:43 +0530, Hemant Kumar wrote:
> 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
Hmm.. it seems the output is prefixed with '\t' in the code..
[SNIP]
> @@ -35,10 +45,14 @@ 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_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 sd-cache --dump",
s/sd-cache/sdt-cache/
> NULL
> };
[SNIP]
> +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, "\t%%%s:%s\n",
> + sdt_ptr->provider, sdt_ptr->name);
.. Here you printed it with \t.
Other than that, looks good to me.
Thanks,
Namhyung
> + }
> + 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] 20+ messages in thread
* [PATCH v3 2/5] perf/sdt: Add SDT events into a cache
2014-10-10 10:58 [PATCH v3 0/5] perf/sdt: SDT events listing/probing Hemant Kumar
2014-10-10 10:58 ` [PATCH v3 1/5] perf/sdt: ELF support for SDT Hemant Kumar
2014-10-10 10:59 ` [PATCH v3 3/5] perf/sdt: Show SDT cache contents Hemant Kumar
@ 2014-10-10 10:59 ` Hemant Kumar
2014-10-22 4:41 ` Namhyung Kim
2014-10-23 11:51 ` Masami Hiramatsu
2014-10-10 10:59 ` [PATCH v3 4/5] perf/sdt: Delete SDT events from cache Hemant Kumar
2014-10-10 11:00 ` [PATCH v3 5/5] perf/sdt: Add support to perf record to trace SDT events Hemant Kumar
4 siblings, 2 replies; 20+ messages in thread
From: Hemant Kumar @ 2014-10-10 10:59 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 | 609 +++++++++++++++++++++++++++
8 files changed, 704 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 262916f..e36f721 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -339,6 +339,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
@@ -458,6 +459,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
@@ -485,8 +487,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..1af1f21c 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 common
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 df094b4..e6efe2c 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -109,4 +109,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..626f07e
--- /dev/null
+++ b/tools/perf/util/sdt.c
@@ -0,0 +1,609 @@
+/*
+ * 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 "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 ".sdt"
+#define SDT_FILE_CACHE "perf-sdt-file.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 HASH_TABLE_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 list_head *sdt_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) {
+ sdt_head = &fse->sdt_list;
+ if (strcmp(res_path, fse->name))
+ continue;
+
+ /* Got the file list entry, now start removing */
+ nr_del = cleanup_sdt_note_list(sdt_head);
+ hlist_del(&fse->file_list);
+ list_del(&fse->sdt_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.
+ */
+static int sdt_note__read(char *data, struct list_head *sdt_list)
+{
+ struct sdt_note *sn;
+ char *tmp, *ptr, delim[2];
+ int ret = -EBADF, val;
+
+ /* Initialize the delimiter with DELIM */
+ delim[0] = DELIM;
+ sn = malloc(sizeof(*sn));
+ if (!sn) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ INIT_LIST_HEAD(&sn->note_list);
+
+ /* Provider name */
+ ptr = strtok_r(data, delim, &tmp);
+ if (!ptr)
+ goto out;
+ sn->provider = strdup(ptr);
+ if (!sn->provider) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ /* Marker name */
+ ptr = strtok_r(NULL, delim, &tmp);
+ if (!ptr)
+ goto out;
+ sn->name = strdup(ptr);
+ if (!sn->name) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ /* Location */
+ ptr = strtok_r(NULL, delim, &tmp);
+ if (!ptr)
+ goto out;
+ val = sscanf(ptr, "%lx", &sn->addr.a64[0]);
+ if (val == EOF)
+ goto out;
+ /* Sempahore */
+ ptr = strtok_r(NULL, delim, &tmp);
+ if (!ptr)
+ goto out;
+ val = sscanf(ptr, "%lx", &sn->addr.a64[2]);
+ if (val == EOF)
+ goto out;
+ /* Add to the SDT list */
+ list_add(&sn->note_list, sdt_list);
+ ret = 0;
+out:
+ return ret;
+}
+
+/**
+ * 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
+ * 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;
+ int key, val, ret = -EBADF;
+ char data[2 * PATH_MAX], *ptr, *tmp;
+
+ for (val = fscanf(cache, "%s", data); val != EOF;
+ val = fscanf(cache, "%s", data)) {
+ fse = malloc(sizeof(*fse));
+ if (!fse) {
+ ret = -ENOMEM;
+ break;
+ }
+ INIT_LIST_HEAD(&fse->sdt_list);
+ INIT_HLIST_NODE(&fse->file_list);
+
+ /* First line is file name and build id */
+ ptr = strtok_r(data, ":", &tmp);
+ if (!ptr)
+ break;
+ strcpy(fse->name, ptr);
+ ptr = strtok_r(NULL, ":", &tmp);
+ if (!ptr)
+ break;
+ strcpy(fse->sbuild_id, ptr);
+
+ /* Now, try to get the SDT notes for that file */
+ while ((fscanf(cache, "%s", data)) != EOF) {
+ if (!strcmp(data, ":"))
+ break;
+ ret = sdt_note__read(data, &fse->sdt_list);
+ if (ret < 0)
+ goto out;
+ }
+ key = get_hash_key(fse->name);
+ hlist_add_head(&fse->file_list, &file_hash->ent[key]);
+ ret = 0;
+ }
+out:
+ return ret;
+}
+
+/**
+ * file_hash_list__init: Initializes the file hash list
+ * @file_hash: empty file_hash
+ *
+ * Initializes the 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[MAXPATHLEN], *val;
+
+ for (i = 0; i < SDT_HASH_SIZE; i++)
+ INIT_HLIST_HEAD(&file_hash->ent[i]);
+
+ val = getenv("HOME");
+ if (val)
+ snprintf(sdt_cache_path, MAXPATHLEN-1, "%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;
+
+ /* Populate the hash list */
+ 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 list_head *sdt_head;
+ 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) {
+ sdt_head = &file_pos->sdt_list;
+
+ /* Cleanup the corresponding SDT notes' list */
+ cleanup_sdt_note_list(sdt_head);
+ hlist_del(&file_pos->file_list);
+ list_del(&file_pos->sdt_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;
+ }
+
+ symbol__elf_init();
+ 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", file_pos->name, DELIM);
+ fprintf(fcache, "%s\n", 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]);
+ }
+ /* This marks the end of the SDT notes for a file */
+ fprintf(fcache, "%c\n", DELIM);
+ }
+ }
+}
+
+/**
+ * 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 a directory ".sdt".
+ */
+static int flush_hash_list_to_cache(struct hash_table *file_hash)
+{
+ FILE *cache;
+ int ret;
+ struct stat buf;
+ char sdt_cache_path[MAXPATHLEN], sdt_dir[MAXPATHLEN], *val;
+
+ val = getenv("HOME");
+ if (val) {
+ snprintf(sdt_dir, MAXPATHLEN-1, "%s/%s", val, SDT_CACHE_DIR);
+ snprintf(sdt_cache_path, MAXPATHLEN-1, "%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] 20+ messages in thread* Re: [PATCH v3 2/5] perf/sdt: Add SDT events into a cache
2014-10-10 10:59 ` [PATCH v3 2/5] perf/sdt: Add SDT events into a cache Hemant Kumar
@ 2014-10-22 4:41 ` Namhyung Kim
2014-10-24 11:28 ` Hemant Kumar
2014-10-23 11:51 ` Masami Hiramatsu
1 sibling, 1 reply; 20+ messages in thread
From: Namhyung Kim @ 2014-10-22 4:41 UTC (permalink / raw)
To: Hemant Kumar
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo, anton,
systemtap, masami.hiramatsu.pt, aravinda, penberg
On Fri, 10 Oct 2014 16:28:24 +0530, Hemant Kumar wrote:
> 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>
[SNIP]
> +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 list_head *sdt_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) {
> + sdt_head = &fse->sdt_list;
> + if (strcmp(res_path, fse->name))
> + continue;
> +
> + /* Got the file list entry, now start removing */
> + nr_del = cleanup_sdt_note_list(sdt_head);
> + hlist_del(&fse->file_list);
> + list_del(&fse->sdt_list);
It seems you don't need to call list_del() here. And what about just
calling cleanup_sdt_note_list(&fse->sdt_list) instead?
> + free(fse);
> + }
> + free(res_path);
> + return nr_del;
> +}
[SNIP]
> +static int sdt_note__read(char *data, struct list_head *sdt_list)
> +{
> + struct sdt_note *sn;
> + char *tmp, *ptr, delim[2];
> + int ret = -EBADF, val;
> +
> + /* Initialize the delimiter with DELIM */
> + delim[0] = DELIM;
What about delim[1] then?
> + sn = malloc(sizeof(*sn));
> + if (!sn) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + INIT_LIST_HEAD(&sn->note_list);
> +
> + /* Provider name */
> + ptr = strtok_r(data, delim, &tmp);
> + if (!ptr)
> + goto out;
> + sn->provider = strdup(ptr);
> + if (!sn->provider) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + /* Marker name */
> + ptr = strtok_r(NULL, delim, &tmp);
> + if (!ptr)
> + goto out;
> + sn->name = strdup(ptr);
> + if (!sn->name) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + /* Location */
> + ptr = strtok_r(NULL, delim, &tmp);
> + if (!ptr)
> + goto out;
> + val = sscanf(ptr, "%lx", &sn->addr.a64[0]);
> + if (val == EOF)
> + goto out;
> + /* Sempahore */
> + ptr = strtok_r(NULL, delim, &tmp);
> + if (!ptr)
> + goto out;
> + val = sscanf(ptr, "%lx", &sn->addr.a64[2]);
> + if (val == EOF)
> + goto out;
> + /* Add to the SDT list */
> + list_add(&sn->note_list, sdt_list);
> + ret = 0;
> +out:
> + return ret;
> +}
> +
> +/**
> + * 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
You seem forgot to add a file delimiter before new file.
:\n
> + * file_name:build_id\n
> + * ...
Anyway IMHO it's bit uncomfortable. How about changing the format same
as it dumps? A line starts with '%' is a sdt description, otherwise
file info:
file_name1:build_id\n
%provide:marker1:location:semaphore\n
%provide:marker2:location:semaphore\n
%...
file_name2:build_id\n
%...
And we can deal with it by usual fgets/getline and strtok.
Oh, it's just a suggestion..
> + *
> + * 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;
> + int key, val, ret = -EBADF;
> + char data[2 * PATH_MAX], *ptr, *tmp;
> +
> + for (val = fscanf(cache, "%s", data); val != EOF;
> + val = fscanf(cache, "%s", data)) {
> + fse = malloc(sizeof(*fse));
> + if (!fse) {
> + ret = -ENOMEM;
> + break;
> + }
> + INIT_LIST_HEAD(&fse->sdt_list);
> + INIT_HLIST_NODE(&fse->file_list);
> +
> + /* First line is file name and build id */
> + ptr = strtok_r(data, ":", &tmp);
> + if (!ptr)
> + break;
> + strcpy(fse->name, ptr);
> + ptr = strtok_r(NULL, ":", &tmp);
> + if (!ptr)
> + break;
> + strcpy(fse->sbuild_id, ptr);
> +
> + /* Now, try to get the SDT notes for that file */
> + while ((fscanf(cache, "%s", data)) != EOF) {
> + if (!strcmp(data, ":"))
> + break;
> + ret = sdt_note__read(data, &fse->sdt_list);
> + if (ret < 0)
> + goto out;
> + }
> + key = get_hash_key(fse->name);
> + hlist_add_head(&fse->file_list, &file_hash->ent[key]);
> + ret = 0;
> + }
> +out:
> + return ret;
> +}
> +
> +/**
> + * file_hash_list__init: Initializes the file hash list
> + * @file_hash: empty file_hash
> + *
> + * Initializes the ent's of file_hash and opens the cache file.
What is the ent's?
> + * 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[MAXPATHLEN], *val;
s/MAXPATHLEN/PATH_MAX/ ?
> +
> + for (i = 0; i < SDT_HASH_SIZE; i++)
> + INIT_HLIST_HEAD(&file_hash->ent[i]);
> +
> + val = getenv("HOME");
> + if (val)
> + snprintf(sdt_cache_path, MAXPATHLEN-1, "%s/%s/%s",
> + val, SDT_CACHE_DIR, SDT_FILE_CACHE);
Then you can use scnprintf(sdt_cache_path, sizeof(sdt_cache_path), ...).
> + 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;
> +
> + /* Populate the hash list */
> + 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 list_head *sdt_head;
> + 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) {
> + sdt_head = &file_pos->sdt_list;
> +
> + /* Cleanup the corresponding SDT notes' list */
> + cleanup_sdt_note_list(sdt_head);
> + hlist_del(&file_pos->file_list);
> + list_del(&file_pos->sdt_list);
Ditto. Don't call list_del() but call cleanup_sdt_note_list directly.
> + 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;
> + }
> +
> + symbol__elf_init();
No need to call symbol__elf_init() here since we already called it in
cmd_sdt_cache().
> + 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;
> +}
[SNIP]
> +static int flush_hash_list_to_cache(struct hash_table *file_hash)
> +{
> + FILE *cache;
> + int ret;
> + struct stat buf;
> + char sdt_cache_path[MAXPATHLEN], sdt_dir[MAXPATHLEN], *val;
> +
> + val = getenv("HOME");
> + if (val) {
> + snprintf(sdt_dir, MAXPATHLEN-1, "%s/%s", val, SDT_CACHE_DIR);
> + snprintf(sdt_cache_path, MAXPATHLEN-1, "%s/%s",
> + sdt_dir, SDT_FILE_CACHE);
Ditto. s/MAXPATHLEN/PATH_MAX/g and use scnprintf().
Thanks,
Namhyung
> + } 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;
> +}
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v3 2/5] perf/sdt: Add SDT events into a cache
2014-10-22 4:41 ` Namhyung Kim
@ 2014-10-24 11:28 ` Hemant Kumar
0 siblings, 0 replies; 20+ messages in thread
From: Hemant Kumar @ 2014-10-24 11:28 UTC (permalink / raw)
To: Namhyung Kim
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo, anton,
systemtap, masami.hiramatsu.pt, aravinda, penberg
On 10/22/2014 10:11 AM, Namhyung Kim wrote:
> On Fri, 10 Oct 2014 16:28:24 +0530, Hemant Kumar wrote:
>> 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>
> [SNIP]
>> +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 list_head *sdt_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) {
>> + sdt_head = &fse->sdt_list;
>> + if (strcmp(res_path, fse->name))
>> + continue;
>> +
>> + /* Got the file list entry, now start removing */
>> + nr_del = cleanup_sdt_note_list(sdt_head);
>> + hlist_del(&fse->file_list);
>> + list_del(&fse->sdt_list);
> It seems you don't need to call list_del() here. And what about just
> calling cleanup_sdt_note_list(&fse->sdt_list) instead?
Yeah, will remove this.
>
>> + free(fse);
>> + }
>> + free(res_path);
>> + return nr_del;
>> +}
> [SNIP]
>> +static int sdt_note__read(char *data, struct list_head *sdt_list)
>> +{
>> + struct sdt_note *sn;
>> + char *tmp, *ptr, delim[2];
>> + int ret = -EBADF, val;
>> +
>> + /* Initialize the delimiter with DELIM */
>> + delim[0] = DELIM;
> What about delim[1] then?
>
It should be '\0' here, will initialize it.
>> + sn = malloc(sizeof(*sn));
>> + if (!sn) {
>> + ret = -ENOMEM;
>> + goto out;
>> + }
>> + INIT_LIST_HEAD(&sn->note_list);
>> +
>> + /* Provider name */
>> + ptr = strtok_r(data, delim, &tmp);
>> + if (!ptr)
>> + goto out;
>> + sn->provider = strdup(ptr);
>> + if (!sn->provider) {
>> + ret = -ENOMEM;
>> + goto out;
>> + }
>> + /* Marker name */
>> + ptr = strtok_r(NULL, delim, &tmp);
>> + if (!ptr)
>> + goto out;
>> + sn->name = strdup(ptr);
>> + if (!sn->name) {
>> + ret = -ENOMEM;
>> + goto out;
>> + }
>> + /* Location */
>> + ptr = strtok_r(NULL, delim, &tmp);
>> + if (!ptr)
>> + goto out;
>> + val = sscanf(ptr, "%lx", &sn->addr.a64[0]);
>> + if (val == EOF)
>> + goto out;
>> + /* Sempahore */
>> + ptr = strtok_r(NULL, delim, &tmp);
>> + if (!ptr)
>> + goto out;
>> + val = sscanf(ptr, "%lx", &sn->addr.a64[2]);
>> + if (val == EOF)
>> + goto out;
>> + /* Add to the SDT list */
>> + list_add(&sn->note_list, sdt_list);
>> + ret = 0;
>> +out:
>> + return ret;
>> +}
>> +
>> +/**
>> + * 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
> You seem forgot to add a file delimiter before new file.
>
> :\n
>> + * file_name:build_id\n
>> + * ...
> Anyway IMHO it's bit uncomfortable. How about changing the format same
> as it dumps? A line starts with '%' is a sdt description, otherwise
> file info:
>
> file_name1:build_id\n
> %provide:marker1:location:semaphore\n
> %provide:marker2:location:semaphore\n
> %...
> file_name2:build_id\n
> %...
>
> And we can deal with it by usual fgets/getline and strtok.
>
> Oh, it's just a suggestion..
No problem. Will change the format to above.
>
>> + *
>> + * 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;
>> + int key, val, ret = -EBADF;
>> + char data[2 * PATH_MAX], *ptr, *tmp;
>> +
>> + for (val = fscanf(cache, "%s", data); val != EOF;
>> + val = fscanf(cache, "%s", data)) {
>> + fse = malloc(sizeof(*fse));
>> + if (!fse) {
>> + ret = -ENOMEM;
>> + break;
>> + }
>> + INIT_LIST_HEAD(&fse->sdt_list);
>> + INIT_HLIST_NODE(&fse->file_list);
>> +
>> + /* First line is file name and build id */
>> + ptr = strtok_r(data, ":", &tmp);
>> + if (!ptr)
>> + break;
>> + strcpy(fse->name, ptr);
>> + ptr = strtok_r(NULL, ":", &tmp);
>> + if (!ptr)
>> + break;
>> + strcpy(fse->sbuild_id, ptr);
>> +
>> + /* Now, try to get the SDT notes for that file */
>> + while ((fscanf(cache, "%s", data)) != EOF) {
>> + if (!strcmp(data, ":"))
>> + break;
>> + ret = sdt_note__read(data, &fse->sdt_list);
>> + if (ret < 0)
>> + goto out;
>> + }
>> + key = get_hash_key(fse->name);
>> + hlist_add_head(&fse->file_list, &file_hash->ent[key]);
>> + ret = 0;
>> + }
>> +out:
>> + return ret;
>> +}
>> +
>> +/**
>> + * file_hash_list__init: Initializes the file hash list
>> + * @file_hash: empty file_hash
>> + *
>> + * Initializes the ent's of file_hash and opens the cache file.
> What is the ent's?
Hash entries. :) Will change to that to entries.
>> + * 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[MAXPATHLEN], *val;
> s/MAXPATHLEN/PATH_MAX/ ?
Ok, for consistency, will change it to PATH_MAX.
>> +
>> + for (i = 0; i < SDT_HASH_SIZE; i++)
>> + INIT_HLIST_HEAD(&file_hash->ent[i]);
>> +
>> + val = getenv("HOME");
>> + if (val)
>> + snprintf(sdt_cache_path, MAXPATHLEN-1, "%s/%s/%s",
>> + val, SDT_CACHE_DIR, SDT_FILE_CACHE);
> Then you can use scnprintf(sdt_cache_path, sizeof(sdt_cache_path), ...).
Ok.
>
>> + 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;
>> +
>> + /* Populate the hash list */
>> + 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 list_head *sdt_head;
>> + 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) {
>> + sdt_head = &file_pos->sdt_list;
>> +
>> + /* Cleanup the corresponding SDT notes' list */
>> + cleanup_sdt_note_list(sdt_head);
>> + hlist_del(&file_pos->file_list);
>> + list_del(&file_pos->sdt_list);
> Ditto. Don't call list_del() but call cleanup_sdt_note_list directly.
>
Sure.
>> + 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;
>> + }
>> +
>> + symbol__elf_init();
> No need to call symbol__elf_init() here since we already called it in
> cmd_sdt_cache().
Yeah, my bad. Will remove it.
>
>> + 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;
>> +}
> [SNIP]
>> +static int flush_hash_list_to_cache(struct hash_table *file_hash)
>> +{
>> + FILE *cache;
>> + int ret;
>> + struct stat buf;
>> + char sdt_cache_path[MAXPATHLEN], sdt_dir[MAXPATHLEN], *val;
>> +
>> + val = getenv("HOME");
>> + if (val) {
>> + snprintf(sdt_dir, MAXPATHLEN-1, "%s/%s", val, SDT_CACHE_DIR);
>> + snprintf(sdt_cache_path, MAXPATHLEN-1, "%s/%s",
>> + sdt_dir, SDT_FILE_CACHE);
> Ditto. s/MAXPATHLEN/PATH_MAX/g and use scnprintf().
>
> Thanks,
> Namhyung
>
>
>> + } 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;
>> +}
Thanks a lot for reviewing the patch.
--
Thanks,
Hemant Kumar
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 2/5] perf/sdt: Add SDT events into a cache
2014-10-10 10:59 ` [PATCH v3 2/5] perf/sdt: Add SDT events into a cache Hemant Kumar
2014-10-22 4:41 ` Namhyung Kim
@ 2014-10-23 11:51 ` Masami Hiramatsu
1 sibling, 0 replies; 20+ messages in thread
From: Masami Hiramatsu @ 2014-10-23 11:51 UTC (permalink / raw)
To: Hemant Kumar
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo, anton,
systemtap, namhyung, aravinda, penberg
Hi Hermant,
(2014/10/10 19:58), Hemant Kumar wrote:
> +
> +/**
> + * 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.
> + */
> +static int sdt_note__read(char *data, struct list_head *sdt_list)
It seems that you can simply use sscanf() instead of this code, like this.
n = sscanf(data, "%ms:%ms:%lx:%lx", &sn->provider, &sn->name,
&sn->addr.a64[0], &sn->addr.a64[2]);
if (n != 4)
goto error;
> +{
> + struct sdt_note *sn;
> + char *tmp, *ptr, delim[2];
> + int ret = -EBADF, val;
> +
> + /* Initialize the delimiter with DELIM */
> + delim[0] = DELIM;
> + sn = malloc(sizeof(*sn));
> + if (!sn) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + INIT_LIST_HEAD(&sn->note_list);
> +
> + /* Provider name */
> + ptr = strtok_r(data, delim, &tmp);
> + if (!ptr)
> + goto out;
> + sn->provider = strdup(ptr);
> + if (!sn->provider) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + /* Marker name */
> + ptr = strtok_r(NULL, delim, &tmp);
> + if (!ptr)
> + goto out;
> + sn->name = strdup(ptr);
> + if (!sn->name) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + /* Location */
> + ptr = strtok_r(NULL, delim, &tmp);
> + if (!ptr)
> + goto out;
> + val = sscanf(ptr, "%lx", &sn->addr.a64[0]);
> + if (val == EOF)
> + goto out;
> + /* Sempahore */
> + ptr = strtok_r(NULL, delim, &tmp);
> + if (!ptr)
> + goto out;
> + val = sscanf(ptr, "%lx", &sn->addr.a64[2]);
> + if (val == EOF)
> + goto out;
> + /* Add to the SDT list */
> + list_add(&sn->note_list, sdt_list);
> + ret = 0;
> +out:
> + return ret;
> +}
> +
> +/**
> + * 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
> + * 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;
> + int key, val, ret = -EBADF;
> + char data[2 * PATH_MAX], *ptr, *tmp;
> +
> + for (val = fscanf(cache, "%s", data); val != EOF;
> + val = fscanf(cache, "%s", data)) {
And no, to read one-line, please use fgets instead of fscanf.
> + fse = malloc(sizeof(*fse));
> + if (!fse) {
> + ret = -ENOMEM;
> + break;
> + }
> + INIT_LIST_HEAD(&fse->sdt_list);
> + INIT_HLIST_NODE(&fse->file_list);
> +
> + /* First line is file name and build id */
> + ptr = strtok_r(data, ":", &tmp);
> + if (!ptr)
> + break;
> + strcpy(fse->name, ptr);
> + ptr = strtok_r(NULL, ":", &tmp);
> + if (!ptr)
> + break;
> + strcpy(fse->sbuild_id, ptr);
> +
> + /* Now, try to get the SDT notes for that file */
> + while ((fscanf(cache, "%s", data)) != EOF) {
Ditto.
> + if (!strcmp(data, ":"))
> + break;
> + ret = sdt_note__read(data, &fse->sdt_list);
> + if (ret < 0)
> + goto out;
> + }
> + key = get_hash_key(fse->name);
> + hlist_add_head(&fse->file_list, &file_hash->ent[key]);
> + ret = 0;
> + }
> +out:
> + return ret;
> +}
> +
> +/**
> + * file_hash_list__init: Initializes the file hash list
> + * @file_hash: empty file_hash
> + *
> + * Initializes the 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[MAXPATHLEN], *val;
> +
> + for (i = 0; i < SDT_HASH_SIZE; i++)
> + INIT_HLIST_HEAD(&file_hash->ent[i]);
> +
> + val = getenv("HOME");
> + if (val)
> + snprintf(sdt_cache_path, MAXPATHLEN-1, "%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;
> +
> + /* Populate the hash list */
> + 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 list_head *sdt_head;
> + 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) {
> + sdt_head = &file_pos->sdt_list;
> +
> + /* Cleanup the corresponding SDT notes' list */
> + cleanup_sdt_note_list(sdt_head);
> + hlist_del(&file_pos->file_list);
> + list_del(&file_pos->sdt_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;
> + }
> +
> + symbol__elf_init();
> + 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", file_pos->name, DELIM);
> + fprintf(fcache, "%s\n", file_pos->sbuild_id);
Here, you also should merge these 2 sequential fprintfs.
> + 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]);
> + }
> + /* This marks the end of the SDT notes for a file */
> + fprintf(fcache, "%c\n", DELIM);
> + }
> + }
> +}
> +
> +/**
> + * 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 a directory ".sdt".
I think we can share $HOME/.debug/ with buildid, which uses .debug/.build-id/*.
So, perhaps, $HOME/.debug/.sdt-cache will be better filename (not dirname).
Thank you,
> + */
> +static int flush_hash_list_to_cache(struct hash_table *file_hash)
> +{
> + FILE *cache;
> + int ret;
> + struct stat buf;
> + char sdt_cache_path[MAXPATHLEN], sdt_dir[MAXPATHLEN], *val;
> +
> + val = getenv("HOME");
> + if (val) {
> + snprintf(sdt_dir, MAXPATHLEN-1, "%s/%s", val, SDT_CACHE_DIR);
> + snprintf(sdt_cache_path, MAXPATHLEN-1, "%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;
> +}
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
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] 20+ messages in thread
* [PATCH v3 4/5] perf/sdt: Delete SDT events from cache
2014-10-10 10:58 [PATCH v3 0/5] perf/sdt: SDT events listing/probing Hemant Kumar
` (2 preceding siblings ...)
2014-10-10 10:59 ` [PATCH v3 2/5] perf/sdt: Add SDT events into a cache Hemant Kumar
@ 2014-10-10 10:59 ` Hemant Kumar
2014-10-10 11:00 ` [PATCH v3 5/5] perf/sdt: Add support to perf record to trace SDT events Hemant Kumar
4 siblings, 0 replies; 20+ messages in thread
From: Hemant Kumar @ 2014-10-10 10:59 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 freed up.
# ./perf sdt-cache --del /usr/lib64/libc-2.16.so
8 events 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 | 28 +++++++++++++++++++
tools/perf/util/parse-events.h | 1 +
tools/perf/util/sdt.c | 39 +++++++++++++++++++++++++++
4 files changed, 72 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 31041db..b9ed5d4 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,13 +56,16 @@ 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),
OPT_END()
};
const char * const sdt_cache_usage[] = {
- "perf sdt-cache --add filename",
+ "perf sdt-cache [--add | --del] filename",
"perf sd-cache --dump",
NULL
};
@@ -64,6 +78,10 @@ int cmd_sdt_cache(int argc, const char **argv, const char *prefix __maybe_unused
symbol__elf_init();
if (params.add) {
+ if (params.del) {
+ pr_err("Error: Don't use --del with --add\n");
+ usage_with_options(sdt_cache_usage, sdt_cache_options);
+ }
if (params.dump) {
pr_err("Error: Don't use --dump with --add\n");
usage_with_options(sdt_cache_usage, sdt_cache_options);
@@ -72,6 +90,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.del) {
+ if (params.dump) {
+ pr_err("Error: Don't use --dump with --del\n");
+ usage_with_options(sdt_cache_usage, sdt_cache_options);
+ }
+ 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 f43e6aa..2297af7 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -111,5 +111,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 4e64f3e..880c089 100644
--- a/tools/perf/util/sdt.c
+++ b/tools/perf/util/sdt.c
@@ -655,3 +655,42 @@ 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)
+ goto out_clean;
+
+ /* 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("%d 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);
+out_clean:
+ file_hash_list__cleanup(&file_hash);
+ return ret;
+}
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v3 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-10-10 10:58 [PATCH v3 0/5] perf/sdt: SDT events listing/probing Hemant Kumar
` (3 preceding siblings ...)
2014-10-10 10:59 ` [PATCH v3 4/5] perf/sdt: Delete SDT events from cache Hemant Kumar
@ 2014-10-10 11:00 ` Hemant Kumar
2014-10-22 6:45 ` Masami Hiramatsu
4 siblings, 1 reply; 20+ messages in thread
From: Hemant Kumar @ 2014-10-10 11:00 UTC (permalink / raw)
To: linux-kernel
Cc: srikar, peterz, oleg, hegdevasant, mingo, anton, systemtap,
namhyung, masami.hiramatsu.pt, aravinda, penberg
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 %user_app:fun_start -aR /home/user_app
In main
This is foo
This is fun
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.265 MB perf.data (~11581 samples) ]
# ./perf report --stdio
...
# To display the perf.data header info, please use --header/--header-only options.
#
# Samples: 1 of event 'user_app:fun_start'
# Event count (approx.): 1
#
# Overhead Command Shared Object Symbol
# ........ ........ ............. .......
#
100.00% user_app user_app [.] fun
Multiple events can also be recorded simultaneously.
Signed-off-by: Hemant Kumar <hemant@linux.vnet.ibm.com>
---
tools/perf/builtin-record.c | 22 +++++
tools/perf/util/parse-events.c | 6 +
tools/perf/util/parse-events.h | 3 +
tools/perf/util/probe-event.c | 121 +++++++++++++++++++++-----
tools/perf/util/probe-event.h | 7 ++
tools/perf/util/probe-finder.c | 3 +
tools/perf/util/sdt.c | 186 ++++++++++++++++++++++++++++++++++++++--
tools/perf/util/symbol.h | 2
8 files changed, 319 insertions(+), 31 deletions(-)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 44c6f3d..b0f1cc8 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -45,6 +45,26 @@ 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) {
@@ -874,6 +894,8 @@ 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);
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 d76aa30..edc96d9 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -900,6 +900,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 2297af7..52a163a 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -112,5 +112,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 c150ca4..76277d6 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;
+
+ 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)
{
@@ -1910,9 +1950,12 @@ static int show_perf_probe_event(struct perf_probe_event *pev,
if (ret < 0)
return ret;
- printf(" %-20s (on %s", buf, place);
- if (module)
- printf(" in %s", module);
+ /* Do not display anything for SDTs */
+ if (!pev->sdt) {
+ printf(" %-20s (on %s", buf, place);
+ if (module)
+ printf(" in %s", module);
+ }
if (pev->nargs > 0) {
printf(" with");
@@ -1924,7 +1967,9 @@ static int show_perf_probe_event(struct perf_probe_event *pev,
printf(" %s", buf);
}
}
- printf(")\n");
+ /* Not for SDT events */
+ if (!pev->sdt)
+ printf(")\n");
free(place);
return ret;
}
@@ -2124,7 +2169,9 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
}
ret = 0;
- printf("Added new event%s\n", (ntevs > 1) ? "s:" : ":");
+ /* If SDT event, do not print anything */
+ if (!pev->sdt)
+ printf("Added new event%s\n", (ntevs > 1) ? "s:" : ":");
for (i = 0; i < ntevs; i++) {
tev = &tevs[i];
if (pev->event)
@@ -2163,6 +2210,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;
@@ -2177,8 +2229,8 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
allow_suffix = true;
}
- if (ret >= 0) {
- /* Show how to use the event. */
+ if (ret >= 0 && !pev->sdt) {
+ /* Show how to use the event except for SDT events. */
printf("\nYou can now use it in all perf tools, such as:\n\n");
printf("\tperf record -e %s:%s -aR sleep 1\n\n", tev->group,
tev->event);
@@ -2371,6 +2423,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,12 +2465,13 @@ 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;
}
-static int __del_trace_probe_event(int fd, struct str_node *ent)
+static int __del_trace_probe_event(int fd, struct str_node *ent, bool sdt)
{
char *p;
char buf[128];
@@ -2444,7 +2498,8 @@ static int __del_trace_probe_event(int fd, struct str_node *ent)
goto error;
}
- printf("Removed event: %s\n", ent->s);
+ if (!sdt)
+ printf("Removed event: %s\n", ent->s);
return 0;
error:
pr_warning("Failed to delete event: %s\n",
@@ -2453,7 +2508,7 @@ error:
}
static int del_trace_probe_event(int fd, const char *buf,
- struct strlist *namelist)
+ struct strlist *namelist, bool sdt)
{
struct str_node *ent, *n;
int ret = -1;
@@ -2461,7 +2516,7 @@ static int del_trace_probe_event(int fd, const char *buf,
if (strpbrk(buf, "*?")) { /* Glob-exp */
strlist__for_each_safe(ent, n, namelist)
if (strglobmatch(ent->s, buf)) {
- ret = __del_trace_probe_event(fd, ent);
+ ret = __del_trace_probe_event(fd, ent, sdt);
if (ret < 0)
break;
strlist__remove(namelist, ent);
@@ -2469,7 +2524,7 @@ static int del_trace_probe_event(int fd, const char *buf,
} else {
ent = strlist__find(namelist, buf);
if (ent) {
- ret = __del_trace_probe_event(fd, ent);
+ ret = __del_trace_probe_event(fd, ent, sdt);
if (ret >= 0)
strlist__remove(namelist, ent);
}
@@ -2478,7 +2533,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, bool sdt)
{
int ret = -1, ufd = -1, kfd = -1;
char buf[128];
@@ -2531,10 +2586,10 @@ int del_perf_probe_events(struct strlist *dellist)
pr_debug("Group: %s, Event: %s\n", group, event);
if (namelist)
- ret = del_trace_probe_event(kfd, buf, namelist);
+ ret = del_trace_probe_event(kfd, buf, namelist, sdt);
if (unamelist && ret != 0)
- ret = del_trace_probe_event(ufd, buf, unamelist);
+ ret = del_trace_probe_event(ufd, buf, unamelist, sdt);
if (ret != 0)
pr_info("Info: Event \"%s\" does not exist.\n", buf);
@@ -2556,6 +2611,26 @@ error:
return ret;
}
+int del_perf_probe_events(struct strlist *dellist)
+{
+ return __del_perf_probe_events(dellist, false);
+}
+
+int remove_perf_sdt_events(const char *str)
+{
+ struct strlist *dellist;
+ int ret = 0;
+
+ dellist = strlist__new(true, NULL);
+ strlist__add(dellist, str + 1);
+ printf("remove_perf_sdt_events\n");
+ if (dellist)
+ ret = __del_perf_probe_events(dellist, true);
+
+ 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..d647042 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,6 +141,7 @@ extern int show_available_vars(struct perf_probe_event *pevs, int npevs,
struct strfilter *filter, bool externs);
extern int show_available_funcs(const char *module, struct strfilter *filter,
bool user);
+extern int add_perf_sdt_event(struct perf_sdt_event *sev);
/* 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 880c089..5c3c46d 100644
--- a/tools/perf/util/sdt.c
+++ b/tools/perf/util/sdt.c
@@ -60,7 +60,7 @@ struct file_info {
*
* 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 HASH_TABLE_SIZE.
+ * and finds the modulo with SDT_HASH_SIZE.
*
* Return : An integer key
*/
@@ -288,6 +288,49 @@ out:
}
/**
+ * 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 file_sdt_ent *fse,
+ struct hash_table *event_hash)
+{
+ struct hlist_head *ent_head;
+ struct sdt_note *sn;
+ int event_key, nr_add = 0, len;
+ char *str;
+
+ /* Iterate through the sdt notes and add them to the event hash */
+ list_for_each_entry(sn, &fse->sdt_list, note_list) {
+ /*
+ * Assign the file_ptr so that we can always find its container
+ * file
+ */
+ sn->file_ptr = fse;
+
+ len = strlen(sn->provider) + strlen(sn->name);
+ str = (char *)malloc(len + 1);
+ if (!str)
+ return -ENOMEM;
+
+ memset(str, '\0', len + 1);
+ /* 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);
+ nr_add++;
+ }
+
+ return nr_add;
+}
+
+/**
* file_hash_list__populate: Fill up the file hash table
* @file_hash: empty file hash table
* @cache: FILE * to read from
@@ -305,7 +348,8 @@ out:
* 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;
int key, val, ret = -EBADF;
@@ -341,6 +385,11 @@ static int file_hash_list__populate(struct hash_table *file_hash, FILE *cache)
}
key = get_hash_key(fse->name);
hlist_add_head(&fse->file_list, &file_hash->ent[key]);
+ if (event_hash) {
+ ret = event_hash_list__add(fse, event_hash);
+ if (ret < 0)
+ break;
+ }
ret = 0;
}
out:
@@ -353,8 +402,10 @@ out:
*
* Initializes the 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;
@@ -377,8 +428,8 @@ static int file_hash_list__init(struct hash_table *file_hash)
if (cache == NULL)
goto out;
- /* Populate the hash list */
- ret = file_hash_list__populate(file_hash, cache);
+ /* Populate the hash list(s) */
+ ret = file_hash_list__populate(file_hash, cache, event_hash);
fclose(cache);
out:
return ret;
@@ -412,6 +463,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
@@ -586,7 +665,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;
@@ -649,7 +728,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);
@@ -674,7 +753,7 @@ int remove_sdt_events(const char *str)
goto out_clean;
/* Initialize the hash_lists */
- ret = file_hash_list__init(&file_hash);
+ ret = init_hash_lists(&file_hash, NULL);
if (ret < 0)
goto out;
@@ -694,3 +773,94 @@ out_clean:
file_hash_list__cleanup(&file_hash);
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;
+}
+
+/**
+ * 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, delim[2], s[PATH_MAX];
+ int event_key, ret = 0;
+
+ delim[0] = DELIM;
+ /* 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;
+ }
+ 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->file_name)
+ free(sdt_event->file_name);
+ return ret;
+}
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 23d3d41..c0dac7b 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] 20+ messages in thread* Re: [PATCH v3 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-10-10 11:00 ` [PATCH v3 5/5] perf/sdt: Add support to perf record to trace SDT events Hemant Kumar
@ 2014-10-22 6:45 ` Masami Hiramatsu
2014-10-22 8:20 ` Hemant Kumar
0 siblings, 1 reply; 20+ messages in thread
From: Masami Hiramatsu @ 2014-10-22 6:45 UTC (permalink / raw)
To: Hemant Kumar
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo, anton,
systemtap, namhyung, aravinda, penberg
Hi Hemant,
(2014/10/10 19:59), Hemant Kumar wrote:
> The SDT events are already stored in a cache file
> (/var/cache/perf/perf-sdt-file.cache).
Please describe what this patch does at first.
> 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.
OK, I have some comments on this.
> An example usage:
>
> # ./perf record -e %user_app:fun_start -aR /home/user_app
At first, I'd like to add SDT support for adding probes too, like below;
./perf probe -a '%user_app:fun_start $vars'
So, maybe we don't need to remove the SDT-based events silently, nor
hide it from users. I think you just need to add new sdt events and
verify it if there is.
BTW, for silently adding event, I'll introduce --quite(-q) option for
perf probe. So you'll just need to set silent flag with that.
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] 20+ messages in thread
* Re: [PATCH v3 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-10-22 6:45 ` Masami Hiramatsu
@ 2014-10-22 8:20 ` Hemant Kumar
2014-10-22 9:42 ` Masami Hiramatsu
0 siblings, 1 reply; 20+ messages in thread
From: Hemant Kumar @ 2014-10-22 8:20 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo, anton,
systemtap, namhyung, aravinda, penberg
Hi Masami,
On 10/22/2014 12:15 PM, Masami Hiramatsu wrote:
> Hi Hemant,
>
> (2014/10/10 19:59), Hemant Kumar wrote:
>> The SDT events are already stored in a cache file
>> (/var/cache/perf/perf-sdt-file.cache).
> Please describe what this patch does at first.
>
Sure, will do that.
>> 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.
> OK, I have some comments on this.
>
>> An example usage:
>>
>> # ./perf record -e %user_app:fun_start -aR /home/user_app
> At first, I'd like to add SDT support for adding probes too, like below;
>
> ./perf probe -a '%user_app:fun_start $vars'
But I think, previously we discussed that we won't be having "perf
probe" for SDT events.
We list them and probe/trace them using "perf record" directly.
> So, maybe we don't need to remove the SDT-based events silently, nor
> hide it from users. I think you just need to add new sdt events and
> verify it if there is.
>
> BTW, for silently adding event, I'll introduce --quite(-q) option for
> perf probe. So you'll just need to set silent flag with that.
>
>
> Thank you,
>
>
--
Thanks,
Hemant Kumar
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-10-22 8:20 ` Hemant Kumar
@ 2014-10-22 9:42 ` Masami Hiramatsu
2014-10-23 5:31 ` Hemant Kumar
2014-10-23 5:54 ` Srikar Dronamraju
0 siblings, 2 replies; 20+ messages in thread
From: Masami Hiramatsu @ 2014-10-22 9:42 UTC (permalink / raw)
To: Hemant Kumar
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo, anton,
systemtap, namhyung, aravinda, penberg
(2014/10/22 17:20), Hemant Kumar wrote:
>>> 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.
>> OK, I have some comments on this.
>>
>>> An example usage:
>>>
>>> # ./perf record -e %user_app:fun_start -aR /home/user_app
>> At first, I'd like to add SDT support for adding probes too, like below;
>>
>> ./perf probe -a '%user_app:fun_start $vars'
>
> But I think, previously we discussed that we won't be having "perf
> probe" for SDT events.
> We list them and probe/trace them using "perf record" directly.
Right, sorry for confusing you. I meant that I'd like to support SDT on both of
perf-record and perf-probe :)
And even if we'll hide sdt related events via perf, users can access it via ftrace.
So, I doubt that we can completely hide them, in that case, honesty is the best way;)
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] 20+ messages in thread
* Re: [PATCH v3 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-10-22 9:42 ` Masami Hiramatsu
@ 2014-10-23 5:31 ` Hemant Kumar
2014-10-23 5:54 ` Srikar Dronamraju
1 sibling, 0 replies; 20+ messages in thread
From: Hemant Kumar @ 2014-10-23 5:31 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: linux-kernel, srikar, peterz, oleg, hegdevasant, mingo, anton,
systemtap, namhyung, aravinda, penberg
On 10/22/2014 03:11 PM, Masami Hiramatsu wrote:
> (2014/10/22 17:20), Hemant Kumar wrote:
>>>> 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.
>>> OK, I have some comments on this.
>>>
>>>> An example usage:
>>>>
>>>> # ./perf record -e %user_app:fun_start -aR /home/user_app
>>> At first, I'd like to add SDT support for adding probes too, like below;
>>>
>>> ./perf probe -a '%user_app:fun_start $vars'
>> But I think, previously we discussed that we won't be having "perf
>> probe" for SDT events.
>> We list them and probe/trace them using "perf record" directly.
> Right, sorry for confusing you. I meant that I'd like to support SDT on both of
> perf-record and perf-probe :)
I plan to do this and add this subsequently but will it be okay if we go
with the current
implementation for the time being?
What do you think?
> And even if we'll hide sdt related events via perf, users can access it via ftrace.
> So, I doubt that we can completely hide them, in that case, honesty is the best way;)
>
> Thank you,
>
>
--
Thanks,
Hemant Kumar
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-10-22 9:42 ` Masami Hiramatsu
2014-10-23 5:31 ` Hemant Kumar
@ 2014-10-23 5:54 ` Srikar Dronamraju
2014-10-23 6:33 ` Masami Hiramatsu
1 sibling, 1 reply; 20+ messages in thread
From: Srikar Dronamraju @ 2014-10-23 5:54 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Hemant Kumar, linux-kernel, peterz, oleg, hegdevasant, mingo,
anton, systemtap, namhyung, aravinda, penberg
* Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> [2014-10-22 18:41:58]:
> (2014/10/22 17:20), Hemant Kumar wrote:
> >>> 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.
> >> OK, I have some comments on this.
> >>
> >>> An example usage:
> >>>
> >>> # ./perf record -e %user_app:fun_start -aR /home/user_app
> >> At first, I'd like to add SDT support for adding probes too, like below;
> >>
> >> ./perf probe -a '%user_app:fun_start $vars'
> >
> > But I think, previously we discussed that we won't be having "perf
> > probe" for SDT events.
> > We list them and probe/trace them using "perf record" directly.
>
> Right, sorry for confusing you. I meant that I'd like to support SDT on both of
> perf-record and perf-probe :)
> And even if we'll hide sdt related events via perf, users can access it via ftrace.
> So, I doubt that we can completely hide them, in that case, honesty is the best way;)
>
I am somehow not able to figure out how perf probe comes into the
current workflow.
I think the current design was
1. perf sdt-cache --add <file> (only once per file)
2. perf record -e <sdt-event>
So what is the additional thing that perf probe does or Is it going to
replace any of the above steps?
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-10-23 5:54 ` Srikar Dronamraju
@ 2014-10-23 6:33 ` Masami Hiramatsu
2014-10-23 8:21 ` Namhyung Kim
0 siblings, 1 reply; 20+ messages in thread
From: Masami Hiramatsu @ 2014-10-23 6:33 UTC (permalink / raw)
To: Srikar Dronamraju, Hemant Kumar
Cc: linux-kernel, peterz, oleg, hegdevasant, mingo, anton, systemtap,
namhyung, aravinda, penberg
(2014/10/23 14:54), Srikar Dronamraju wrote:
> * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> [2014-10-22 18:41:58]:
>
>> (2014/10/22 17:20), Hemant Kumar wrote:
>>>>> 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.
>>>> OK, I have some comments on this.
>>>>
>>>>> An example usage:
>>>>>
>>>>> # ./perf record -e %user_app:fun_start -aR /home/user_app
>>>> At first, I'd like to add SDT support for adding probes too, like below;
>>>>
>>>> ./perf probe -a '%user_app:fun_start $vars'
>>>
>>> But I think, previously we discussed that we won't be having "perf
>>> probe" for SDT events.
>>> We list them and probe/trace them using "perf record" directly.
>>
>> Right, sorry for confusing you. I meant that I'd like to support SDT on both of
>> perf-record and perf-probe :)
>> And even if we'll hide sdt related events via perf, users can access it via ftrace.
>> So, I doubt that we can completely hide them, in that case, honesty is the best way;)
>>
>
> I am somehow not able to figure out how perf probe comes into the
> current workflow.
>
> I think the current design was
> 1. perf sdt-cache --add <file> (only once per file)
> 2. perf record -e <sdt-event>
>
> So what is the additional thing that perf probe does or Is it going to
> replace any of the above steps?
3. perf probe -a <sdt-event>
And this will be done subsequently in this series (without user interface part).
However, current implementation of 2. will do the following steps
s1. get sdt event data from sdt-cache
s2. set up sdt events with suppressing messages
s3. do recording events
(s4. and hiding existing sdt events from perf-probe --list)
s5. remove sdt events
So, what I proposed were ;
- to implement s2., we can introduce --quiet(-q) option and use it
instead of ->sdt flag checking
- removing s4. and s5.
- and add verification of existing sdt events at s2. if needed.
This will simplify your patch and removing complex part of sdt-specific code.
What would you think about this?
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] 20+ messages in thread
* Re: [PATCH v3 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-10-23 6:33 ` Masami Hiramatsu
@ 2014-10-23 8:21 ` Namhyung Kim
2014-10-23 8:57 ` Masami Hiramatsu
0 siblings, 1 reply; 20+ messages in thread
From: Namhyung Kim @ 2014-10-23 8:21 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Srikar Dronamraju, Hemant Kumar, linux-kernel, peterz, oleg,
hegdevasant, mingo, anton, systemtap, aravinda, penberg
Hi Masami,
On Thu, 23 Oct 2014 15:33:37 +0900, Masami Hiramatsu wrote:
> (2014/10/23 14:54), Srikar Dronamraju wrote:
>> I am somehow not able to figure out how perf probe comes into the
>> current workflow.
>>
>> I think the current design was
>> 1. perf sdt-cache --add <file> (only once per file)
>> 2. perf record -e <sdt-event>
>>
>> So what is the additional thing that perf probe does or Is it going to
>> replace any of the above steps?
>
> 3. perf probe -a <sdt-event>
>
> And this will be done subsequently in this series (without user interface part).
> However, current implementation of 2. will do the following steps
>
> s1. get sdt event data from sdt-cache
> s2. set up sdt events with suppressing messages
> s3. do recording events
> (s4. and hiding existing sdt events from perf-probe --list)
> s5. remove sdt events
>
> So, what I proposed were ;
> - to implement s2., we can introduce --quiet(-q) option and use it
> instead of ->sdt flag checking
> - removing s4. and s5.
> - and add verification of existing sdt events at s2. if needed.
I'm okay with removing the s4 but not sure about the s5. In that case,
we might have many dynamic events in a system without noticing to users.
Thanks,
Namhyung
>
> This will simplify your patch and removing complex part of sdt-specific code.
> What would you think about this?
>
> Thank you,
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Re: [PATCH v3 5/5] perf/sdt: Add support to perf record to trace SDT events
2014-10-23 8:21 ` Namhyung Kim
@ 2014-10-23 8:57 ` Masami Hiramatsu
0 siblings, 0 replies; 20+ messages in thread
From: Masami Hiramatsu @ 2014-10-23 8:57 UTC (permalink / raw)
To: Namhyung Kim
Cc: Srikar Dronamraju, Hemant Kumar, linux-kernel, peterz, oleg,
hegdevasant, mingo, anton, systemtap, aravinda, penberg
(2014/10/23 17:21), Namhyung Kim wrote:
> Hi Masami,
>
> On Thu, 23 Oct 2014 15:33:37 +0900, Masami Hiramatsu wrote:
>> (2014/10/23 14:54), Srikar Dronamraju wrote:
>>> I am somehow not able to figure out how perf probe comes into the
>>> current workflow.
>>>
>>> I think the current design was
>>> 1. perf sdt-cache --add <file> (only once per file)
>>> 2. perf record -e <sdt-event>
>>>
>>> So what is the additional thing that perf probe does or Is it going to
>>> replace any of the above steps?
>>
>> 3. perf probe -a <sdt-event>
>>
>> And this will be done subsequently in this series (without user interface part).
>> However, current implementation of 2. will do the following steps
>>
>> s1. get sdt event data from sdt-cache
>> s2. set up sdt events with suppressing messages
>> s3. do recording events
>> (s4. and hiding existing sdt events from perf-probe --list)
>> s5. remove sdt events
>>
>> So, what I proposed were ;
>> - to implement s2., we can introduce --quiet(-q) option and use it
>> instead of ->sdt flag checking
>> - removing s4. and s5.
>> - and add verification of existing sdt events at s2. if needed.
>
> I'm okay with removing the s4 but not sure about the s5. In that case,
> we might have many dynamic events in a system without noticing to users.
Indeed, okey, so let's keep s5 then :)
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] 20+ messages in thread