perf/sdt: SDT events listing/probing

This patch serves as the basic 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.

Made the necessary changes as suggested by Namhyung Kim.

Signed-off-by Hemant Kumar <[email protected]>

---
 tools/perf/util/symbol-elf.c |  207 ++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/symbol.h     |   19 ++++
 2 files changed, 226 insertions(+)

tools/perf/util/symbol-elf.c +3 -0

index 2a92e10..9702167 100644
@@ -1672,6 +1672,213 @@ void kcore_extract__delete(struct kcore_extract *kce)
 	unlink(kce->extract_filename);
 }
 
+/*
+ * populate_sdt_note() : Responsible for parsing the section .note.stapsdt and
+ * after adjusting the note's location, returns that to the calling functions.

tools/perf/util/symbol-elf.c +27 -0

+	int i;
+
+	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;
+	}

tools/perf/util/symbol-elf.c +59 -0

+
+	/* 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];
+		}
+	}
+
+	*note = tmp;
+	return 0;
+

Quoted 8 lines · Hemant Kumar

This patch serves as the basic 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.

Quoted 4 lines

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.

Made the necessary changes as suggested by Namhyung Kim.

Looks almost good!
I have some comments, please see below.

Quoted 14 lines

Signed-off-by Hemant Kumar <[email protected]>

---
  tools/perf/util/symbol-elf.c |  207 ++++++++++++++++++++++++++++++++++++++++++
  tools/perf/util/symbol.h     |   19 ++++
  2 files changed, 226 insertions(+)

tools/perf/util/symbol-elf.c +0 -0

index 2a92e10..9702167 100644
@@ -1672,6 +1672,213 @@ void kcore_extract__delete(struct kcore_extract *kce)
  	unlink(kce->extract_filename);
  }
 

tools/perf/util/symbol-elf.c void kcore_extract__delete(struct kcore_extract *kce) · line 1687

Quoted 13 lines

tools/perf/util/symbol-elf.c +13 -0

+/*
+ * populate_sdt_note() : Responsible for parsing the section .note.stapsdt and
+ * after adjusting the note's location, returns that to the calling functions.
+ */
+static int populate_sdt_note(Elf **elf, const char *data, size_t len, int type,
+			     struct sdt_note **note)
+{
+	const char *provider, *name;
+	struct sdt_note *tmp = NULL;
+	GElf_Ehdr ehdr;
+	GElf_Addr base_off = 0;
+	GElf_Shdr shdr;
+	int ret = -1;

-1 is not an error code. maybe, -EINVAL is better.

Quoted 30 lines

tools/perf/util/symbol-elf.c +30 -0

+	int i;
+
+	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);
+

tools/perf/util/symbol-elf.c void kcore_extract__delete(struct kcore_extract *kce) · line 1724

Quoted 7 lines

tools/perf/util/symbol-elf.c +7 -0

+	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)
+		printf("gelf_xlatetom : %s\n", elf_errmsg(-1));

pr_err? and shouldn't it goes to out_free_note?

Quoted 3 lines

[...]

This patch serves as the basic 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.

Made the necessary changes as suggested by Namhyung Kim.

tools/perf/util/symbol-elf.c +15 -0

+		}
+	}
+	if (list_empty(sdt_notes))
+		ret = -ENOENT;
+
+out_ret:
+	return ret;
+}
+
+/*
+ * get_sdt_note_list() : Takes two arguments "head" and "target", where head
+ * is the head of the SDT events' list and "target" is the file name as to
+ * where the SDT events should be looked for. This opens the file, initializes
+ * the ELF and then calls construct_sdt_notes_list.
+ */

tools/perf/util/symbol-elf.c +10 -0

+	if (!elf) {
+		ret = -EBADF;
+		goto out_close;
+	}
+	ret = construct_sdt_notes_list(elf, head);
+	elf_end(elf);
+out_close:
+	close(fd);
+	return ret;
+}

Hi Hemant,

Quoted 8 lines · Hemant Kumar

This patch serves as the basic 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.

Quoted 4 lines

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.

Made the necessary changes as suggested by Namhyung Kim.

Below are just few more nitpicks, other than that, looks good to me.

[SNIP]

tools/perf/util/symbol-elf.c void kcore_extract__delete(struct kcore_extract *kce) · line 1845

Quoted 12 lines

tools/perf/util/symbol-elf.c +12 -0

+	/* 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,
+						&tmp);
+			if (ret < 0)
+				goto out_ret;
+			list_add_tail(&tmp->note_list, sdt_notes);

I think we can just pass the sdt_notes list into populate_sdt_note() instead of passing a tmp pointer.

Quoted 18 lines

tools/perf/util/symbol-elf.c +18 -0

+		}
+	}
+	if (list_empty(sdt_notes))
+		ret = -ENOENT;
+
+out_ret:
+	return ret;
+}
+
+/*
+ * get_sdt_note_list() : Takes two arguments "head" and "target", where head
+ * is the head of the SDT events' list and "target" is the file name as to
+ * where the SDT events should be looked for. 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;

tools/perf/util/symbol-elf.c void kcore_extract__delete(struct kcore_extract *kce) · line 1870

Quoted 7 lines

tools/perf/util/symbol-elf.c +7 -0

+	int fd, ret;
+
+	fd = open(target, O_RDONLY);
+	if (fd < 0)
+		return -EBADF;
+
+	elf = elf_begin(fd, ELF_C_READ, NULL);

You may want to use PERF_ELF_C_READ_MMAP. :)

Thanks,
Namhyung

[...]

This patch adds a new sub-command to perf : sdt-cache. sdt-cache command can be used to add, remove and dump SDT events. This patch adds support to only "add" the SDT events. The rest of the patches add support to rest of them.

When user invokes "perf sdt-cache add <file-name>", two hash tables are
created: file_hash list and event_hash list. A typical entry in a file_hash table looks like:

            file hash      file_sdt_ent     file_sdt_ent
            |---------|   ---------------   -------------
            | list  ==|===|=>file_list =|===|=>file_list=|==..
key = 644 =>|         |   | sbuild_id   |   | sbuild_id  |
            |---------|   | name        |   | name       |
            |	        |   | sdt_list    |   | sdt_list   |
key = 645 =>| list    |   |   ||        |   |    ||      |
            |---------|   ---------------   --------------
	          ||            ||                 || Connected to SDT notes

---------------
| note_list |
sdt_note| name |
| provider |
------||-------
connected to other SDT notes

Each entry of the file_hash table is a list 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 <[email protected]>

---
 tools/perf/Makefile.perf       |    3 
 tools/perf/builtin-sdt-cache.c |   59 ++++
 tools/perf/builtin.h           |    1 
 tools/perf/perf.c              |    1 
 tools/perf/util/parse-events.h |    2 
 tools/perf/util/probe-event.h  |    1 
 tools/perf/util/sdt.c          |  666 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/sdt.h          |   30 ++
 8 files changed, 763 insertions(+)
 create mode 100644 tools/perf/builtin-sdt-cache.c
 create mode 100644 tools/perf/util/sdt.c
 create mode 100644 tools/perf/util/sdt.h

tools/perf/Makefile.perf +2 -0

index 262916f..09b3325 100644
@@ -274,6 +274,7 @@ LIB_H += util/run-command.h
 LIB_H += util/sigchain.h
 LIB_H += util/dso.h
 LIB_H += util/symbol.h
+LIB_H += util/sdt.h
 LIB_H += util/color.h
 LIB_H += util/values.h
 LIB_H += util/sort.h

@@ -339,6 +340,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

tools/perf/Makefile.perf +0 -0

 BUILTIN_OBJS += $(OUTPUT)builtin-kmem.o
 BUILTIN_OBJS += $(OUTPUT)builtin-lock.o
 BUILTIN_OBJS += $(OUTPUT)builtin-kvm.o

tools/perf/perf.c +0 -0

 	{ "kmem",	cmd_kmem,	0 },
 	{ "lock",	cmd_lock,	0 },

tools/perf/util/parse-events.h +2 -0

index df094b4..e6efe2c 100644
@@ -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 */

tools/perf/util/probe-event.h +1 -0

index e01e994..f1ddca6 100644
@@ -5,6 +5,7 @@
 #include "intlist.h"
 #include "strlist.h"
 #include "strfilter.h"
+#include "sdt.h"
 
 extern bool probe_event_dry_run;
 

tools/perf/util/sdt.c new file +69 -0

new file mode 100644
index 0000000..9a39d36
@@ -0,0 +1,666 @@
+/*
+ * 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 <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <limits.h>
+#include <sys/mman.h>
+#include <fcntl.h>
+
+#include "parse-events.h"
+#include "probe-event.h"
+#include "linux/list.h"
+#include "symbol.h"
+#include "build-id.h"
+#include "debug.h"
+
+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
+ *
+ * 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 * 37) % HASH_TABLE_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)
+{
+	switch (-val) {
+	case 0:

tools/perf/util/sdt.c new file +7 -0

+	}
+
+	return val;
+}
+
+/**
+ * cleanup_sdt_note_list : free the sdt notes' list

tools/perf/util/sdt.c new file +54 -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);
+	}
+}
+
+/**
+ * file_list_entry__init: Initialize a file_list_entry
+ * @fse: file_list_entry
+ * @file: file information
+ *
+ * Initializes @fse with the build id of a @file.
+ */
+static void file_list_entry__init(struct file_sdt_ent *fse,
+				 struct file_info *file)
+{
+	strcpy(fse->sbuild_id, file->sbuild_id);
+	INIT_LIST_HEAD(&fse->file_list);
+	INIT_LIST_HEAD(&fse->sdt_list);
+}
+
+/**
+ * sdt_events__get_count: Counts the number of sdt events
+ * @start: list_head to sdt_notes list
+ *
+ * Returns the number of SDT notes in a list
+ */
+static int sdt_events__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;
+}
+
+/**
+ * file_hash_list__add: add an entry to file_hash_list
+ * @sdt_notes: list of sdt_notes
+ * @file: file name and build id
+ * @file_hash: hash table for file and sdt notes
+ *
+ * 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 list_head *sdt_notes,
+				struct file_info *file,
+				struct hash_list *file_hash)

tools/perf/util/sdt.c new file +17 -0

+}
+
+/**
+ * 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_list *file_hash,
+				   const char *target)
+{
+	struct file_sdt_ent *fse, *tmp1;
+	struct list_head *sdt_head, *ent_head;
+	struct sdt_note *sdt_ptr, *tmp2;

tools/perf/util/sdt.c new file +32 -0

+
+	/* Got the file hash entry */
+	list_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 */
+		list_for_each_entry_safe(sdt_ptr, tmp2, sdt_head, note_list) {
+			list_del(&sdt_ptr->note_list);
+			free(sdt_ptr->name);
+			free(sdt_ptr->provider);
+			free(sdt_ptr);
+			nr_del++;
+		}
+		list_del(&fse->file_list);
+		list_del(&fse->sdt_list);
+		free(fse);
+	}
+
+	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 correct entry,
+ * 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_list *file_hash,

tools/perf/util/sdt.c new file +50 -0

+
+	list_for_each_entry(fse, ent_head, file_list) {
+		if (!strcmp(file->name, fse->name)) {
+			if (!strcmp(file->sbuild_id, fse->sbuild_id))
+				return true;
+		}
+	}
+
+	return false;
+}
+
+/**
+ * copy_delim: copy till a character
+ * @src: source string
+ * @target: dest string
+ * @delim: till this character
+ * @len: length of @target
+ * @end: end of @src
+ *
+ * Returns the number of chars copied.
+ * NOTE: We need @end as @src may or may not be terminated by NULL
+ */
+static int copy_delim(char *src, char *target, char delim, size_t len,
+		      char *end)
+{
+	int i;
+
+	memset(target, '\0', len);
+	for (i = 0; (src[i] != delim) && !(src + i > end) ; i++)
+		target[i] = src[i];
+
+	return i + 1;
+}
+
+/**
+ * sdt_note__read: Parse SDT note info
+ * @ptr: raw data
+ * @sdt_list: empty list
+ * @end: end of the raw data
+ *
+ * Parse @ptr to find out SDT note name, provider, location and semaphore.
+ * All these data are separated by DELIM.
+ */
+static void sdt_note__read(char **ptr, struct list_head *sdt_list, char *end)
+{
+	struct sdt_note *sn;
+	char addr[MAX_ADDR];
+	int len = 0;
+
+	while (1) {

tools/perf/util/sdt.c new file +27 -0

+		/* Extract the provider name */
+		len = copy_delim(*ptr, sn->provider, DELIM, PATH_MAX, end);
+		*ptr += len;
+
+		/* Extract the note name */
+		len = copy_delim(*ptr, sn->name, DELIM, PATH_MAX, end);
+		*ptr += len;
+
+		/* Extract the note's location */
+		len = copy_delim(*ptr, addr , DELIM, MAX_ADDR, end);
+		*ptr += len;
+		sscanf(addr, "%lx", &sn->addr.a64[0]);
+
+		/* Extract the sem location */
+		len = copy_delim(*ptr, addr , DELIM, MAX_ADDR, end);
+		*ptr += len;
+		sscanf(addr, "%lx", &sn->addr.a64[2]);
+		list_add(&sn->note_list, sdt_list);
+
+		/* If the entries for this file are finished */
+		if (**ptr == DELIM)
+			break;
+	}
+}
+
+/**
+ * file_hash_list__populate: Fill up the file hash table

tools/perf/util/sdt.c new file +113 -0

+ */
+static void file_hash_list__populate(struct hash_list *file_hash, char *data,
+				     off_t size)
+{
+	struct list_head *ent_head;
+	struct file_sdt_ent *fse;
+	char *end, tmp[PATH_MAX], *ptr;
+	int key, len = 0;
+
+	end = data + size - 1;
+	ptr = data;
+	if (ptr == end)
+		return;
+	do {
+		/* Obtain the key */
+		len = copy_delim(ptr, tmp, DELIM, PATH_MAX, end);
+		ptr += len;
+		key = atoi(tmp);
+		/* Obtain the file_hash list entry */
+		ent_head = &file_hash->ent[key].list;
+		while (1) {
+			fse = (struct file_sdt_ent *)
+				malloc(sizeof(struct file_sdt_ent));
+			if (!fse)
+				return;
+			INIT_LIST_HEAD(&fse->file_list);
+			INIT_LIST_HEAD(&fse->sdt_list);
+			/* Obtain the file name */
+			len = copy_delim(ptr, fse->name, DELIM,
+					 sizeof(fse->name), end);
+			ptr += len;
+			/* Obtain the build id */
+			len = copy_delim(ptr, fse->sbuild_id, DELIM,
+					 sizeof(fse->sbuild_id), end);
+			ptr += len;
+			sdt_note__read(&ptr, &fse->sdt_list, end);
+
+			list_add(&fse->file_list, ent_head);
+
+			/* Check for another file entry */
+			if ((*ptr++ == DELIM) && (*ptr == '-'))
+				break;
+		}
+		if (ptr == end)
+			break;
+		else
+			ptr++;
+
+	} while (1);
+}
+
+/**
+ * file_hash_list__init: Initializes the file hash list
+ * @file_hash: empty file_hash
+ *
+ * Opens the cache file, reads the data into 'data' and calls
+ * file_hash_list__populate() to populate the above hash list.
+ */
+static void file_hash_list__init(struct hash_list *file_hash)
+{
+	struct stat sb;
+	char *data;
+	int fd, i, ret;
+
+	for (i = 0; i < HASH_TABLE_SIZE; i++)
+		INIT_LIST_HEAD(&file_hash->ent[i].list);
+
+	fd = open(SDT_CACHE_DIR SDT_FILE_CACHE, O_RDONLY);
+	if (fd == -1)
+		return;
+
+	ret = fstat(fd, &sb);
+	if (ret == -1) {
+		pr_err("fstat : error\n");
+		return;
+	}
+
+	if (!S_ISREG(sb.st_mode)) {
+		pr_err("%s is not a regular file\n", SDT_CACHE_DIR
+		       SDT_FILE_CACHE);
+		return;
+	}
+	/* Is size of the cache zero ?*/
+	if (!sb.st_size)
+		return;
+	/* Read the data */
+	data = mmap(0, sb.st_size, PROT_READ, MAP_SHARED, fd, 0);
+	if (data == MAP_FAILED) {
+		pr_err("Error in mmap\n");
+		return;
+	}
+
+	ret = madvise(data, sb.st_size, MADV_SEQUENTIAL);
+	if (ret == -1)
+		pr_debug("Error in madvise\n");
+	ret = close(fd);
+	if (ret == -1) {
+		pr_err("Error in close\n");
+		return;
+	}
+
+	/* Populate the hash list */
+	file_hash_list__populate(file_hash, data, sb.st_size);
+	ret = munmap(data, sb.st_size);
+	if (ret == -1)
+		pr_err("Error in munmap\n");
+}
+
+/**
+ * file_hash_list__cleanup: Free up all the space for file_hash list
+ * @file_hash: file_hash table
+ */
+static void file_hash_list__cleanup(struct hash_list *file_hash)

tools/perf/util/sdt.c new file +17 -0

+
+		list_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);
+			list_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
+ *

tools/perf/util/sdt.c new file +68 -0

+	u8 build_id[BUILD_ID_SIZE];
+
+	LIST_HEAD(sdt_notes);
+
+	file = (struct file_info *)malloc(sizeof(struct file_info));
+	if (!file)
+		return -ENOMEM;
+
+	file->name = realpath(target, NULL);
+	if (!file->name) {
+		ret = -EBADF;
+		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);
+		ret = -EBADF;
+		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!",
+		       file->name);
+		ret = NO_MOD;
+		goto out;
+	}
+
+	/*
+	 * This will also remove any stale entries (if any) from the event hash.
+	 * Stale entries will have the same file name but older build ids.
+	 */
+	file_hash_list__remove(file_hash, file->name);
+	ret = get_sdt_note_list(&sdt_notes, file->name);
+	if (!ret) {
+		/* Add the entry to file hash list */
+		ret = file_hash_list__add(&sdt_notes, file, file_hash);
+	} else {
+		sdt_err(ret, target);
+	}
+
+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. First
+ * write the key for non-empty entry and then file entries for that
+ * key, and then the SDT notes. The delimiter used is ':'.
+ */
+static void file_hash_list__flush(struct hash_list *file_hash,
+				  FILE *fcache)
+{
+	struct file_sdt_ent *file_pos;
+	struct list_head *ent_head, *sdt_head;
+	struct sdt_note *sdt_ptr;

tools/perf/util/sdt.c new file +27 -0

+		fprintf(fcache, "%d:", i);
+		list_for_each_entry(file_pos, ent_head, file_list) {
+			fprintf(fcache, "%s:", file_pos->name);
+			fprintf(fcache, "%s:", file_pos->sbuild_id);
+			sdt_head = &file_pos->sdt_list;
+			list_for_each_entry(sdt_ptr, sdt_head, note_list) {
+				fprintf(fcache, "%s:%s:%lx:%lx:",
+					sdt_ptr->provider, sdt_ptr->name,
+					sdt_ptr->addr.a64[0],
+					sdt_ptr->addr.a64[2]);
+			}
+			fprintf(fcache, ":");
+		}
+		/* '-' marks the end of entries for that key */
+		fprintf(fcache, "-");
+	}
+
+}
+
+/**
+ * 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.
+ */
+static void flush_hash_list_to_cache(struct hash_list *file_hash)

tools/perf/util/sdt.c new file +28 -0

+			return;
+		}
+	}
+
+	cache = fopen(SDT_CACHE_DIR SDT_FILE_CACHE, "w");
+	if (!cache) {
+		pr_err("Error in creating %s file!\n",
+		       SDT_CACHE_DIR SDT_FILE_CACHE);
+		return;
+	}
+
+	file_hash_list__flush(file_hash, cache);
+	fclose(cache);
+}
+
+/**
+ * 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' list 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. 'file_hash' is used to add/del SDT events
+ * to the SDT cache.

tools/perf/util/sdt.c new file +21 -0

+		return -1;
+	}
+	/* Initialize the file hash_list */
+	file_hash_list__init(&file_hash);
+
+	/* Try to add the events to the file hash_list */
+	ret = add_to_hash_list(&file_hash, arg);
+	switch (ret) {
+	case MOD:
+		/* file hash table is dirty, flush it */
+		flush_hash_list_to_cache(&file_hash);
+	case NO_MOD:
+		/* file hash isn't dirty, do not flush */
+		file_hash_list__cleanup(&file_hash);
+		ret = 0;
+		break;
+	default:
+		file_hash_list__cleanup(&file_hash);
+	}
+	return ret;
+}

tools/perf/util/sdt.h new file +0 -0

new file mode 100644
index 0000000..4ed27d7

tools/perf/util/sdt.h new file +27 -0

+#define SDT_CACHE_DIR "/var/cache/perf/"
+#define SDT_FILE_CACHE "perf-sdt-file.cache"
+#define SDT_EVENT_CACHE "perf-sdt-event.cache"
+
+#define DELIM ':'
+#define MAX_ADDR 17
+
+#define MOD 1
+#define NO_MOD 2
+
+struct file_sdt_ent {
+	char name[PATH_MAX];                    /* file name */
+	char sbuild_id[BUILD_ID_SIZE * 2 + 1];  /* Build id of the file */
+	struct list_head file_list;
+	struct list_head sdt_list;              /* SDT notes in this file */
+
+};
+
+/* hash table entry */
+struct hash_ent {
+	struct list_head list;
+};
+
+/* Hash table struct */
+struct hash_list {
+	struct hash_ent ent[HASH_TABLE_SIZE];
+};

Quoted 75 lines · Hemant Kumar

This patch adds a new sub-command to perf : sdt-cache. sdt-cache command can be used to add, remove and dump SDT events. This patch adds support to only "add" the SDT events. The rest of the patches add support to rest of them.

When user invokes "perf sdt-cache add <file-name>", two hash tables are
created: file_hash list and event_hash list. A typical entry in a file_hash table looks like:

             file hash      file_sdt_ent     file_sdt_ent
             |---------|   ---------------   -------------
             | list  ==|===|=>file_list =|===|=>file_list=|==..
key = 644 =>|         |   | sbuild_id   |   | sbuild_id  |
             |---------|   | name        |   | name       |
             |	        |   | sdt_list    |   | sdt_list   |
key = 645 =>| list    |   |   ||        |   |    ||      |
             |---------|   ---------------   --------------
	          ||            ||                 || Connected to SDT notes

---------------
| note_list |
sdt_note| name |
| provider |
------||-------
connected to other SDT notes

Each entry of the file_hash table is a list 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 <[email protected]>

---
  tools/perf/Makefile.perf       |    3
  tools/perf/builtin-sdt-cache.c |   59 ++++
  tools/perf/builtin.h           |    1
  tools/perf/perf.c              |    1
  tools/perf/util/parse-events.h |    2
  tools/perf/util/probe-event.h  |    1
  tools/perf/util/sdt.c          |  666 ++++++++++++++++++++++++++++++++++++++++
  tools/perf/util/sdt.h          |   30 ++
  8 files changed, 763 insertions(+)

create mode 100644 tools/perf/builtin-sdt-cache.c create mode 100644 tools/perf/util/sdt.c create mode 100644 tools/perf/util/sdt.h

tools/perf/Makefile.perf +2 -0

index 262916f..09b3325 100644
@@ -274,6 +274,7 @@ LIB_H += util/run-command.h
  LIB_H += util/sigchain.h
  LIB_H += util/dso.h
  LIB_H += util/symbol.h
+LIB_H += util/sdt.h
  LIB_H += util/color.h
  LIB_H += util/values.h
  LIB_H += util/sort.h

@@ -339,6 +340,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

tools/perf/Makefile.perf BUILTIN_OBJS += $(OUTPUT)builtin-timechart.o · line 463

Quoted 6 lines

tools/perf/Makefile.perf +1 -0

  LIB_OBJS += $(OUTPUT)util/header.o

@@ -458,6 +460,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

Hmm.. did you test it without libelf support? I guess we need to guard those files under the feature test.

Quoted 3 lines

tools/perf/Makefile.perf +0 -0

  BUILTIN_OBJS += $(OUTPUT)builtin-kmem.o
  BUILTIN_OBJS += $(OUTPUT)builtin-lock.o
  BUILTIN_OBJS += $(OUTPUT)builtin-kvm.o

[SNIP]

tools/perf/perf.c static struct cmd_struct commands[] = { · line 56

Quoted 8 lines

tools/perf/perf.c +1 -0

@@ -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

Like here..

Quoted 104 lines

tools/perf/perf.c +0 -0

  	{ "kmem",	cmd_kmem,	0 },
  	{ "lock",	cmd_lock,	0 },

tools/perf/util/parse-events.h +2 -0

index df094b4..e6efe2c 100644
@@ -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 */

tools/perf/util/probe-event.h +1 -0

index e01e994..f1ddca6 100644
@@ -5,6 +5,7 @@
  #include "intlist.h"
  #include "strlist.h"
  #include "strfilter.h"
+#include "sdt.h"
 
  extern bool probe_event_dry_run;
 

tools/perf/util/sdt.c new file +73 -0

new file mode 100644
index 0000000..9a39d36
@@ -0,0 +1,666 @@
+/*
+ * 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 <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <limits.h>
+#include <sys/mman.h>
+#include <fcntl.h>
+
+#include "parse-events.h"
+#include "probe-event.h"
+#include "linux/list.h"
+#include "symbol.h"
+#include "build-id.h"
+#include "debug.h"
+
+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
+ *
+ * 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 * 37) % HASH_TABLE_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)
+{
+	switch (-val) {
+	case 0:
+		break;
+	case ENOENT:
+		/* Absence of SDT markers */
+		pr_err("%s: No SDT events found\n", target);

tools/perf/util/sdt.c line 79

Quoted 6 lines

tools/perf/util/sdt.c new file +6 -0

+		break;
+	case EBADF:
+		pr_err("%s: Bad file name\n", target);
+		break;
+	default:
+		pr_err("%s\n", strerror(val));

Please don't add strerror() anymore - use (GNU-style) strerror_r() instead.

Quoted 7 lines

tools/perf/util/sdt.c new file +7 -0

+	}
+
+	return val;
+}
+
+/**
+ * cleanup_sdt_note_list : free the sdt notes' list

tools/perf/util/sdt.c line 96

Quoted 10 lines

tools/perf/util/sdt.c new file +10 -0

+ * @sdt_notes: sdt notes' list
+ *
+ * Free up the SDT notes in @sdt_notes.
+ */
+static void cleanup_sdt_note_list(struct list_head *sdt_notes)
+{
+	struct sdt_note *tmp, *pos;
+
+	if (list_empty(sdt_notes))
+		return;

Not needed.

Quoted 54 lines

tools/perf/util/sdt.c new file +54 -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);
+	}
+}
+
+/**
+ * file_list_entry__init: Initialize a file_list_entry
+ * @fse: file_list_entry
+ * @file: file information
+ *
+ * Initializes @fse with the build id of a @file.
+ */
+static void file_list_entry__init(struct file_sdt_ent *fse,
+				 struct file_info *file)
+{
+	strcpy(fse->sbuild_id, file->sbuild_id);
+	INIT_LIST_HEAD(&fse->file_list);
+	INIT_LIST_HEAD(&fse->sdt_list);
+}
+
+/**
+ * sdt_events__get_count: Counts the number of sdt events
+ * @start: list_head to sdt_notes list
+ *
+ * Returns the number of SDT notes in a list
+ */
+static int sdt_events__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;
+}
+
+/**
+ * file_hash_list__add: add an entry to file_hash_list
+ * @sdt_notes: list of sdt_notes
+ * @file: file name and build id
+ * @file_hash: hash table for file and sdt notes
+ *
+ * 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 list_head *sdt_notes,
+				struct file_info *file,
+				struct hash_list *file_hash)

tools/perf/util/sdt.c line 160

Quoted 10 lines

tools/perf/util/sdt.c new file +10 -0

+{
+	struct file_sdt_ent *fse;
+	struct list_head *ent_head;
+	int key, nr_evs;
+
+	key = get_hash_key(file->name);
+	ent_head = &file_hash->ent[key].list;
+
+	/* Initialize the file entry */
+	fse = (struct file_sdt_ent *)malloc(sizeof(struct file_sdt_ent));

You can use following style instead (for other call-sites too). It's shorter and easier to change.

	fse = malloc(sizeof(*fse));

Quoted 4 lines

tools/perf/util/sdt.c new file +3 -0

+	if (!fse)
+		return -ENOMEM;
+	file_list_entry__init(fse, file);

tools/perf/util/sdt.c line 173

Quoted 10 lines

tools/perf/util/sdt.c new file +10 -0

+	nr_evs = sdt_events__get_count(sdt_notes);
+	list_splice(sdt_notes, &fse->sdt_list);
+
+	printf("%d events added for %s\n", nr_evs, file->name);
+	strcpy(fse->name, file->name);
+
+	/* Add the file to the file hash entry */
+	list_add(&fse->file_list, ent_head);
+
+	return MOD;

What is MOD?

Quoted 20 lines

tools/perf/util/sdt.c new file +20 -0

+}
+
+/**
+ * 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_list *file_hash,
+				   const char *target)
+{
+	struct file_sdt_ent *fse, *tmp1;
+	struct list_head *sdt_head, *ent_head;
+	struct sdt_note *sdt_ptr, *tmp2;
+
+	char *res_path;
+	int key, nr_del = 0;

tools/perf/util/sdt.c line 200

Quoted 7 lines

tools/perf/util/sdt.c new file +7 -0

+
+	key = get_hash_key(target);
+	ent_head = &file_hash->ent[key].list;
+
+	res_path = realpath(target, NULL);
+	if (!res_path)
+		return -ENOMEM;

Why did you get hash key before getting realpath()?

Also it seems you need to free res_path at the end.

tools/perf/util/sdt.c line 203

Quoted 5 lines

tools/perf/util/sdt.c new file +3 -0

+
+	if (list_empty(ent_head))
+		return 0;

Not needed.

Quoted 32 lines

tools/perf/util/sdt.c new file +32 -0

+
+	/* Got the file hash entry */
+	list_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 */
+		list_for_each_entry_safe(sdt_ptr, tmp2, sdt_head, note_list) {
+			list_del(&sdt_ptr->note_list);
+			free(sdt_ptr->name);
+			free(sdt_ptr->provider);
+			free(sdt_ptr);
+			nr_del++;
+		}
+		list_del(&fse->file_list);
+		list_del(&fse->sdt_list);
+		free(fse);
+	}
+
+	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 correct entry,
+ * 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_list *file_hash,

tools/perf/util/sdt.c line 245

Quoted 10 lines

tools/perf/util/sdt.c new file +10 -0

+					  struct file_info *file)
+{
+	struct file_sdt_ent *fse;
+	struct list_head *ent_head;
+	int key;
+
+	key = get_hash_key(file->name);
+	ent_head = &file_hash->ent[key].list;
+	if (list_empty(ent_head))
+		return false;

Not needed.

Quoted 54 lines

tools/perf/util/sdt.c new file +54 -0

+
+	list_for_each_entry(fse, ent_head, file_list) {
+		if (!strcmp(file->name, fse->name)) {
+			if (!strcmp(file->sbuild_id, fse->sbuild_id))
+				return true;
+		}
+	}
+
+	return false;
+}
+
+/**
+ * copy_delim: copy till a character
+ * @src: source string
+ * @target: dest string
+ * @delim: till this character
+ * @len: length of @target
+ * @end: end of @src
+ *
+ * Returns the number of chars copied.
+ * NOTE: We need @end as @src may or may not be terminated by NULL
+ */
+static int copy_delim(char *src, char *target, char delim, size_t len,
+		      char *end)
+{
+	int i;
+
+	memset(target, '\0', len);
+	for (i = 0; (src[i] != delim) && !(src + i > end) ; i++)
+		target[i] = src[i];
+
+	return i + 1;
+}
+
+/**
+ * sdt_note__read: Parse SDT note info
+ * @ptr: raw data
+ * @sdt_list: empty list
+ * @end: end of the raw data
+ *
+ * Parse @ptr to find out SDT note name, provider, location and semaphore.
+ * All these data are separated by DELIM.
+ */
+static void sdt_note__read(char **ptr, struct list_head *sdt_list, char *end)
+{
+	struct sdt_note *sn;
+	char addr[MAX_ADDR];
+	int len = 0;
+
+	while (1) {
+		sn = (struct sdt_note *)malloc(sizeof(struct sdt_note));
+		if (!sn)
+			return;
+		INIT_LIST_HEAD(&sn->note_list);

tools/perf/util/sdt.c line 305

Quoted 6 lines

tools/perf/util/sdt.c new file +6 -0

+		sn->name = (char *)malloc(PATH_MAX);
+		if (!sn->name)
+			return;
+		sn->provider = (char *)malloc(PATH_MAX);
+		if (!sn->provider)
+			return;

Hmm.. this seems too much.. Why not use a local temp buffer for copy_delim() and then strdup() for real provider and name?

Quoted 32 lines

tools/perf/util/sdt.c new file +32 -0

+		/* Extract the provider name */
+		len = copy_delim(*ptr, sn->provider, DELIM, PATH_MAX, end);
+		*ptr += len;
+
+		/* Extract the note name */
+		len = copy_delim(*ptr, sn->name, DELIM, PATH_MAX, end);
+		*ptr += len;
+
+		/* Extract the note's location */
+		len = copy_delim(*ptr, addr , DELIM, MAX_ADDR, end);
+		*ptr += len;
+		sscanf(addr, "%lx", &sn->addr.a64[0]);
+
+		/* Extract the sem location */
+		len = copy_delim(*ptr, addr , DELIM, MAX_ADDR, end);
+		*ptr += len;
+		sscanf(addr, "%lx", &sn->addr.a64[2]);
+		list_add(&sn->note_list, sdt_list);
+
+		/* If the entries for this file are finished */
+		if (**ptr == DELIM)
+			break;
+	}
+}
+
+/**
+ * file_hash_list__populate: Fill up the file hash table
+ * @file_hash: empty file hash table
+ * @data: raw cache data
+ * @size: size of data
+ *
+ * Find out the end of data with help of @size. Start parsing @data.

tools/perf/util/sdt.c line 342

Quoted 5 lines

tools/perf/util/sdt.c new file +5 -0

+ * In the outer loop, it reads the 'key' delimited by "::-". In the inner
+ * loop, it reads the file name, build id and calls sdt_note__read() to
+ * read the SDT notes. Multiple entries for the same key are delimited
+ * by "::". Then, it assigns the file name, build id and SDT notes to the
+ * list entry corresponding to 'key'.

Do we really need to save 'key' in the cache file? It's an implementation detail and can be changed later IMHO. Why not just saving file name and calculate hash key at runtime?

Also I think it'd be better having a single line for each SDT note entry. It's more human-readable and easier to deal with.

Quoted 113 lines

tools/perf/util/sdt.c new file +113 -0

+ */
+static void file_hash_list__populate(struct hash_list *file_hash, char *data,
+				     off_t size)
+{
+	struct list_head *ent_head;
+	struct file_sdt_ent *fse;
+	char *end, tmp[PATH_MAX], *ptr;
+	int key, len = 0;
+
+	end = data + size - 1;
+	ptr = data;
+	if (ptr == end)
+		return;
+	do {
+		/* Obtain the key */
+		len = copy_delim(ptr, tmp, DELIM, PATH_MAX, end);
+		ptr += len;
+		key = atoi(tmp);
+		/* Obtain the file_hash list entry */
+		ent_head = &file_hash->ent[key].list;
+		while (1) {
+			fse = (struct file_sdt_ent *)
+				malloc(sizeof(struct file_sdt_ent));
+			if (!fse)
+				return;
+			INIT_LIST_HEAD(&fse->file_list);
+			INIT_LIST_HEAD(&fse->sdt_list);
+			/* Obtain the file name */
+			len = copy_delim(ptr, fse->name, DELIM,
+					 sizeof(fse->name), end);
+			ptr += len;
+			/* Obtain the build id */
+			len = copy_delim(ptr, fse->sbuild_id, DELIM,
+					 sizeof(fse->sbuild_id), end);
+			ptr += len;
+			sdt_note__read(&ptr, &fse->sdt_list, end);
+
+			list_add(&fse->file_list, ent_head);
+
+			/* Check for another file entry */
+			if ((*ptr++ == DELIM) && (*ptr == '-'))
+				break;
+		}
+		if (ptr == end)
+			break;
+		else
+			ptr++;
+
+	} while (1);
+}
+
+/**
+ * file_hash_list__init: Initializes the file hash list
+ * @file_hash: empty file_hash
+ *
+ * Opens the cache file, reads the data into 'data' and calls
+ * file_hash_list__populate() to populate the above hash list.
+ */
+static void file_hash_list__init(struct hash_list *file_hash)
+{
+	struct stat sb;
+	char *data;
+	int fd, i, ret;
+
+	for (i = 0; i < HASH_TABLE_SIZE; i++)
+		INIT_LIST_HEAD(&file_hash->ent[i].list);
+
+	fd = open(SDT_CACHE_DIR SDT_FILE_CACHE, O_RDONLY);
+	if (fd == -1)
+		return;
+
+	ret = fstat(fd, &sb);
+	if (ret == -1) {
+		pr_err("fstat : error\n");
+		return;
+	}
+
+	if (!S_ISREG(sb.st_mode)) {
+		pr_err("%s is not a regular file\n", SDT_CACHE_DIR
+		       SDT_FILE_CACHE);
+		return;
+	}
+	/* Is size of the cache zero ?*/
+	if (!sb.st_size)
+		return;
+	/* Read the data */
+	data = mmap(0, sb.st_size, PROT_READ, MAP_SHARED, fd, 0);
+	if (data == MAP_FAILED) {
+		pr_err("Error in mmap\n");
+		return;
+	}
+
+	ret = madvise(data, sb.st_size, MADV_SEQUENTIAL);
+	if (ret == -1)
+		pr_debug("Error in madvise\n");
+	ret = close(fd);
+	if (ret == -1) {
+		pr_err("Error in close\n");
+		return;
+	}
+
+	/* Populate the hash list */
+	file_hash_list__populate(file_hash, data, sb.st_size);
+	ret = munmap(data, sb.st_size);
+	if (ret == -1)
+		pr_err("Error in munmap\n");
+}
+
+/**
+ * file_hash_list__cleanup: Free up all the space for file_hash list
+ * @file_hash: file_hash table
+ */
+static void file_hash_list__cleanup(struct hash_list *file_hash)

tools/perf/util/sdt.c line 465

Quoted 10 lines

tools/perf/util/sdt.c new file +10 -0

+{
+	struct file_sdt_ent *file_pos, *tmp;
+	struct list_head *ent_head, *sdt_head;
+	int i;
+
+	/* Go through all the entries */
+	for (i = 0; i < HASH_TABLE_SIZE; i++) {
+		ent_head = &file_hash->ent[i].list;
+		if (list_empty(ent_head))
+			continue;

Not needed.

Quoted 20 lines

tools/perf/util/sdt.c new file +20 -0

+
+		list_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);
+			list_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
+ *
+ * Does a sanity check for the @target, finds out its build id,
+ * 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

tools/perf/util/sdt.c line 492

Quoted 7 lines

tools/perf/util/sdt.c new file +7 -0

+ * 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_list *file_hash, const char *target)
+{
+	struct file_info *file;
+	int ret = 0;

Maybe it'd be better to initialize it to -EBADF.

Quoted 72 lines

tools/perf/util/sdt.c new file +71 -0

+	u8 build_id[BUILD_ID_SIZE];
+
+	LIST_HEAD(sdt_notes);
+
+	file = (struct file_info *)malloc(sizeof(struct file_info));
+	if (!file)
+		return -ENOMEM;
+
+	file->name = realpath(target, NULL);
+	if (!file->name) {
+		ret = -EBADF;
+		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);
+		ret = -EBADF;
+		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!",
+		       file->name);
+		ret = NO_MOD;
+		goto out;
+	}
+
+	/*
+	 * This will also remove any stale entries (if any) from the event hash.
+	 * Stale entries will have the same file name but older build ids.
+	 */
+	file_hash_list__remove(file_hash, file->name);
+	ret = get_sdt_note_list(&sdt_notes, file->name);
+	if (!ret) {
+		/* Add the entry to file hash list */
+		ret = file_hash_list__add(&sdt_notes, file, file_hash);
+	} else {
+		sdt_err(ret, target);
+	}
+
+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. First
+ * write the key for non-empty entry and then file entries for that
+ * key, and then the SDT notes. The delimiter used is ':'.
+ */
+static void file_hash_list__flush(struct hash_list *file_hash,
+				  FILE *fcache)
+{
+	struct file_sdt_ent *file_pos;
+	struct list_head *ent_head, *sdt_head;
+	struct sdt_note *sdt_ptr;
+	int i;
+
+	/* Go through all entries */

tools/perf/util/sdt.c line 570

Quoted 7 lines

tools/perf/util/sdt.c new file +7 -0

+	for (i = 0; i < HASH_TABLE_SIZE; i++) {
+		/* Obtain the list head */
+		ent_head = &file_hash->ent[i].list;
+		/* Empty ? */
+		if (list_empty(ent_head))
+			continue;
+		/* ':' are used here as delimiters */

Wouldn't it be better using DELIM for consistency then?

Quoted 28 lines

tools/perf/util/sdt.c new file +27 -0

+		fprintf(fcache, "%d:", i);
+		list_for_each_entry(file_pos, ent_head, file_list) {
+			fprintf(fcache, "%s:", file_pos->name);
+			fprintf(fcache, "%s:", file_pos->sbuild_id);
+			sdt_head = &file_pos->sdt_list;
+			list_for_each_entry(sdt_ptr, sdt_head, note_list) {
+				fprintf(fcache, "%s:%s:%lx:%lx:",
+					sdt_ptr->provider, sdt_ptr->name,
+					sdt_ptr->addr.a64[0],
+					sdt_ptr->addr.a64[2]);
+			}
+			fprintf(fcache, ":");
+		}
+		/* '-' marks the end of entries for that key */
+		fprintf(fcache, "-");
+	}
+
+}
+
+/**
+ * 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.
+ */
+static void flush_hash_list_to_cache(struct hash_list *file_hash)

tools/perf/util/sdt.c line 607

Quoted 10 lines

tools/perf/util/sdt.c new file +10 -0

+{
+	FILE *cache;
+	int ret;
+	struct stat buf;
+
+	ret = stat(SDT_CACHE_DIR, &buf);
+	if (ret) {
+		ret = mkdir(SDT_CACHE_DIR, buf.st_mode);
+		if (ret) {
+			pr_err("%s : %s\n", SDT_CACHE_DIR, strerror(errno));

Ditto. Please use strerror_r().

Quoted 31 lines

tools/perf/util/sdt.c new file +31 -0

+			return;
+		}
+	}
+
+	cache = fopen(SDT_CACHE_DIR SDT_FILE_CACHE, "w");
+	if (!cache) {
+		pr_err("Error in creating %s file!\n",
+		       SDT_CACHE_DIR SDT_FILE_CACHE);
+		return;
+	}
+
+	file_hash_list__flush(file_hash, cache);
+	fclose(cache);
+}
+
+/**
+ * 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' list 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. 'file_hash' is used to add/del SDT events
+ * to the SDT cache.
+ */
+int add_sdt_events(const char *arg)
+{

tools/perf/util/sdt.c line 645

Quoted 7 lines

tools/perf/util/sdt.c new file +7 -0

+	struct hash_list file_hash;
+	int ret;
+
+	if (!arg) {
+		pr_err("Error : File Name must be specified with \"--add\""
+		       "option!\n"
+	       "Usage :\n  perf sdt-cache --add <file-name>\n");

I think this message should be a part of the builtin-sdt-cache.c file.

Also please don't wrap lines (especially for string literals) just to fit to the 80 column limit.

Quoted 23 lines

tools/perf/util/sdt.c new file +21 -0

+		return -1;
+	}
+	/* Initialize the file hash_list */
+	file_hash_list__init(&file_hash);
+
+	/* Try to add the events to the file hash_list */
+	ret = add_to_hash_list(&file_hash, arg);
+	switch (ret) {
+	case MOD:
+		/* file hash table is dirty, flush it */
+		flush_hash_list_to_cache(&file_hash);
+	case NO_MOD:
+		/* file hash isn't dirty, do not flush */
+		file_hash_list__cleanup(&file_hash);
+		ret = 0;
+		break;
+	default:
+		file_hash_list__cleanup(&file_hash);
+	}
+	return ret;
+}

tools/perf/util/sdt.h new file +0 -0

new file mode 100644

tools/perf/util/sdt.h line 3

Quoted 7 lines

index 0000000..4ed27d7

tools/perf/util/sdt.h new file +3 -0

@@ -0,0 +1,30 @@
+#include "build-id.h"
+
+#define HASH_TABLE_SIZE 2063

Hmm.. look like an unusual size of a hash table.

Quoted 20 lines

tools/perf/util/sdt.h new file +19 -0

+#define SDT_CACHE_DIR "/var/cache/perf/"
+#define SDT_FILE_CACHE "perf-sdt-file.cache"
+#define SDT_EVENT_CACHE "perf-sdt-event.cache"
+
+#define DELIM ':'
+#define MAX_ADDR 17
+
+#define MOD 1
+#define NO_MOD 2
+
+struct file_sdt_ent {
+	char name[PATH_MAX];                    /* file name */
+	char sbuild_id[BUILD_ID_SIZE * 2 + 1];  /* Build id of the file */
+	struct list_head file_list;
+	struct list_head sdt_list;              /* SDT notes in this file */
+
+};
+
+/* hash table entry */

tools/perf/util/sdt.h line 30

Quoted 8 lines

tools/perf/util/sdt.h new file +8 -0

+struct hash_ent {
+	struct list_head list;
+};
+
+/* Hash table struct */
+struct hash_list {
+	struct hash_ent ent[HASH_TABLE_SIZE];
+};

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 <[email protected]>

---
 tools/perf/builtin-sdt-cache.c |   26 +++++++++++++++++++++-
 tools/perf/util/parse-events.h |    1 +
 tools/perf/util/sdt.c          |   47 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 1 deletion(-)

tools/perf/builtin-sdt-cache.c +25 -1

index 3754896..5faf8e5 100644
@@ -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,13 @@ 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('s', "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 filename | --dump]",
 		NULL
 	};
 

@@ -50,9 +63,20 @@ 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;

tools/perf/util/parse-events.h +1 -0

index e6efe2c..f43e6aa 100644
@@ -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 */

tools/perf/util/sdt.c +47 -0

index 9a39d36..f2c7dc0 100644
@@ -664,3 +664,50 @@ int add_sdt_events(const char *arg)
 	}
 	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_list *file_hash)
+{
+	struct file_sdt_ent *file_pos;
+	struct list_head *ent_head, *sdt_head;
+	struct sdt_note *sdt_pos;
+	int i;
+
+	for (i = 0; i < HASH_TABLE_SIZE; i++) {
+		/* Get the list_head for this entry */
+		ent_head = &file_hash->ent[i].list;
+		/* No entries ?*/
+		if (list_empty(ent_head))
+			continue;
+
+		/* Iterate through the chain in this entry */
+		list_for_each_entry(file_pos, ent_head, file_list) {
+			printf("%s :\n", file_pos->name);
+			/* Get he SDT events' head */
+			sdt_head = &file_pos->sdt_list;
+			list_for_each_entry(sdt_pos, sdt_head, note_list) {
+				printf("\t%%%s:%s\n", sdt_pos->provider,
+				       sdt_pos->name);
+			}
+			printf("\n");
+		}
+	}
+}
+
+/**
+ * dump_sdt_events: Dump the SDT events on stdout
+ */
+int dump_sdt_events(void)
+{
+	struct hash_list file_hash;
+
+	file_hash_list__init(&file_hash);
+	file_hash_list__display(&file_hash);
+	return 0;
+}

tools/perf/builtin-sdt-cache.c int cmd_sdt_cache(int argc, const char **argv, const char *prefix __maybe_unused · line 50

Quoted 7 lines · Hemant Kumar

tools/perf/builtin-sdt-cache.c +3 -0

@@ -35,10 +45,13 @@ 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('s', "dump", NULL, "show SDT events",
+				   "Read SDT events from cache and display.",
+				   opt_show_sdt_events),

Just one note here. -s and --dump are a bit odd. how about using -D for short ops?

Thank you,

This patch adds the support to delete SDT events from the cache. To delete an event corresponding to a file, first the cache is read into the file_hash list. The key is calculated from the file name.
And then, the file_list for that file_hash entry is traversed to find out the target file_list entry. Once, it is found, its contents are all freed up.

# ./perf sdt-cache --del /usr/lib64/libc-2.16.so

8 events removed for /usr/lib64/libc-2.16.so!

Signed-off-by Hemant Kumar <[email protected]>

---
 tools/perf/builtin-sdt-cache.c |   28 +++++++++++++++++++++++++++-
 tools/perf/util/parse-events.h |    1 +
 tools/perf/util/sdt.c          |   35 +++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 1 deletion(-)

tools/perf/builtin-sdt-cache.c +12 -0

index 5faf8e5..12276da 100644
@@ -16,6 +16,7 @@
 /* Session management structure */
 static struct {
 	bool add;
+	bool del;
 	bool dump;
 	const char *target;
 } params;

@@ -29,6 +30,15 @@ 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 +55,17 @@ 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"

tools/perf/builtin-sdt-cache.c +5 -0

 		NULL
 	};
 

@@ -63,6 +77,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);

@@ -70,6 +88,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) {

tools/perf/util/parse-events.h +1 -0

index f43e6aa..2297af7 100644
@@ -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 */

tools/perf/util/sdt.c +35 -0

index f2c7dc0..e8eea48 100644
@@ -711,3 +711,38 @@ int dump_sdt_events(void)
 	file_hash_list__display(&file_hash);
 	return 0;
 }
+
+/**
+ * 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_list file_hash;
+	char *res_path;
+	int nr_del;
+
+	/* Initialize the hash_lists */
+	file_hash_list__init(&file_hash);
+	res_path = realpath(str, NULL);
+	if (!res_path)
+		return -ENOMEM;
+
+	/* Remove the file_list entry from file_hash and update the event_hash */
+	nr_del = file_hash_list__remove(&file_hash, res_path);
+	if (nr_del > 0) {
+		printf("%d events removed for %s!\n", nr_del, res_path);
+		flush_hash_list_to_cache(&file_hash);
+		goto out;
+	} else if (!nr_del) {
+		pr_err("Events for %s not found!\n", str);
+	}
+
+out:
+	free(res_path);
+	file_hash_list__cleanup(&file_hash);
+	return nr_del;
+}

Quoted 51 lines · Hemant Kumar

This patch adds the support to delete SDT events from the cache. To delete an event corresponding to a file, first the cache is read into the file_hash list. The key is calculated from the file name.
And then, the file_list for that file_hash entry is traversed to find out the target file_list entry. Once, it is found, its contents are all freed up.

# ./perf sdt-cache --del /usr/lib64/libc-2.16.so

8 events removed for /usr/lib64/libc-2.16.so!

Signed-off-by Hemant Kumar <[email protected]>

---
  tools/perf/builtin-sdt-cache.c |   28 +++++++++++++++++++++++++++-
  tools/perf/util/parse-events.h |    1 +
  tools/perf/util/sdt.c          |   35 +++++++++++++++++++++++++++++++++++
  3 files changed, 63 insertions(+), 1 deletion(-)

tools/perf/builtin-sdt-cache.c +12 -0

index 5faf8e5..12276da 100644
@@ -16,6 +16,7 @@
  /* Session management structure */
  static struct {
  	bool add;
+	bool del;
  	bool dump;
  	const char *target;
  } params;

@@ -29,6 +30,15 @@ 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 +55,17 @@ 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"

tools/perf/builtin-sdt-cache.c int cmd_sdt_cache(int argc, const char **argv, const char *prefix __maybe_unused · line 68

Quoted 10 lines

tools/perf/builtin-sdt-cache.c +3 -1

+			     " sdt cache.",
+			     opt_del_sdt_events),
  		OPT_CALLBACK_NOOPT('s', "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 | --dump]",
+		"perf sdt_cache [--add filename | --del filename | --dump]",

I think it'd be better split the usage into two lines:

		"perf sdt_cache [--add | --del] <filename>"
		"perf sdt_cache --dump"

Quoted 24 lines

tools/perf/builtin-sdt-cache.c +9 -0

  		NULL
  	};
 

@@ -63,6 +77,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);

@@ -70,6 +88,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);
+		}

tools/perf/builtin-sdt-cache.c int cmd_sdt_cache(int argc, const char **argv, const char *prefix __maybe_unused · line 101

Quoted 6 lines

tools/perf/builtin-sdt-cache.c +3 -0

+		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();

I think we need to a flag for exclusive options so that it can emit warnings like this automatically during option parsing.

Thanks,
Namhyung

[...]