perf/sdt: SDT events listing/probing

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

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

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

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

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

Hi Hemant,

Quoted 8 lines · Hemant Kumar

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.

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.

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

Acked-by Namhyung Kim <[email protected]>

Just a nitpick below..

Quoted 21 lines

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

+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 */

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

Quoted 7 lines

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

+	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

Quoted 25 lines

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

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

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

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

+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];
+

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

+		free(fse);
+	}
+	free(res_path);
+	return nr_del;
+}

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

+	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_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.
+ */

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

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

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

+	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 */

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

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

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

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

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

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

Quoted 39 lines · Hemant Kumar

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

[SNIP]

Quoted 21 lines

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

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

tools/perf/util/sdt.c line 195

Quoted 6 lines

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

+			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?

Quoted 6 lines

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

+		free(fse);
+	}
+	free(res_path);
+	return nr_del;
+}

[SNIP]

tools/perf/util/sdt.c line 243

Quoted 8 lines

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

+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?

Quoted 46 lines

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

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

tools/perf/util/sdt.c line 299

Quoted 10 lines

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

+/**
+ * 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

Quoted 2 lines

+ * 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..

Quoted 47 lines

+ *
+ * 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.
+ */

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

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

tools/perf/util/sdt.c line 354

Quoted 7 lines

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

+}
+
+/**
+ * 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?

tools/perf/util/sdt.c line 361

Quoted 7 lines

+ * To look for the cache file, look into the directory in HOME env variable.
+ */

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

+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/ ?

tools/perf/util/sdt.c line 369

Quoted 8 lines

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

+
+	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), ...).

Quoted 34 lines

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

+	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];
+

tools/perf/util/sdt.c line 409

Quoted 7 lines

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

+		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.

Quoted 30 lines

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

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

tools/perf/util/sdt.c line 445

Quoted 6 lines

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

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

Quoted 34 lines

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

+	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]

tools/perf/util/sdt.c line 542

Quoted 12 lines

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

+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

Quoted 18 lines

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

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

tools/perf/util/sdt.c line 568

Quoted 8 lines

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

+		goto out_err;
+	}
+
+	file_hash_list__flush(file_hash, cache);
+	fclose(cache);
+out_err:
+	return ret;
+}

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

+{
+	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

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

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

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

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

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

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

+ */

tools/perf/util/sdt.c new file +79 -1

+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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

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

(quoted diff) +0 -0

 		NULL
 	};

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

+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++) {

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

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

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

Hi Hemant,

Quoted 2 lines · Hemant Kumar

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.

Quoted 35 lines

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.

Quoted 2 lines

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.

Quoted 3 lines

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,

Quoted 14 lines

Quoted 10 lines

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.

Quoted 2 lines

An example usage:

Quoted 4 lines

Quoted 1 line

# ./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.

Quoted 15 lines · Hemant Kumar

Quoted 15 lines

Quoted 10 lines

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.

Quoted 3 lines

An example usage:

# ./perf record -e %user_app:fun_start -aR /home/user_app

Quoted 6 lines

Quoted 3 lines

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,

Quoted 19 lines · Hemant Kumar

Quoted 18 lines

Quoted 10 lines

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.

Quoted 3 lines

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'

Quoted 3 lines

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

Quoted 20 lines · Masami Hiramatsu

Quoted 19 lines · Hemant Kumar

Quoted 18 lines

Quoted 10 lines

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.

Quoted 3 lines

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'

Quoted 9 lines

Quoted 3 lines

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?

Quoted 9 lines · Srikar Dronamraju

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,

Hi Masami,

Quoted 22 lines · Masami Hiramatsu

Quoted 9 lines · Srikar Dronamraju

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

Quoted 5 lines

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.