Skip to contentSkip to conversation

perf auxtrace: Alter addr_filter__entire_dso() to work if there are no symbols

PATCH In review Adrian Hunter 3 participants 6 messages Nov 27, 2018 – Dec 20, 2018 perf events

lore
Share & tools
lore

Adrian Hunter

author reviewer
#0 lore raw

PATCH 1/1 perf auxtrace: Alter addr_filter__entire_dso() to work if there are no symbols

addr_filter__entire_dso() uses the first and last symbols from a dso, and so does not work when there are no symbols. Alter it to filter the whole file instead.

---
 tools/perf/util/auxtrace.c | 11 ++++-------
 tools/perf/util/dso.c      |  6 +++---
 tools/perf/util/dso.h      |  1 +
 3 files changed, 8 insertions(+), 10 deletions(-)
tools/perf/util/auxtrace.c +4 -7
index 72d5ba2479bf..f69961c4a4f3 100644
@@ -1983,17 +1983,14 @@ static int find_dso_sym(struct dso *dso, const char *sym_name, u64 *start,
 
 static int addr_filter__entire_dso(struct addr_filter *filt, struct dso *dso)
 {
-	struct symbol *first_sym = dso__first_symbol(dso);
-	struct symbol *last_sym = dso__last_symbol(dso);
-
-	if (!first_sym || !last_sym) {
-		pr_err("Failed to determine filter for %s\nNo symbols found.\n",
+	if (dso__data_file_size(dso, NULL)) {
+		pr_err("Failed to determine filter for %s\nCannot determine file size.\n",
 		       filt->filename);
 		return -EINVAL;
 	}
 
-	filt->addr = first_sym->start;
-	filt->size = last_sym->end - first_sym->start;
+	filt->addr = 0;
+	filt->size = dso->data.file_size;
 
 	return 0;
 }
tools/perf/util/dso.c +3 -3
index bbed90e5d9bb..721f3d583945 100644
@@ -894,7 +894,7 @@ static ssize_t cached_read(struct dso *dso, struct machine *machine,
 	return r;
 }
 
-static int data_file_size(struct dso *dso, struct machine *machine)
+int dso__data_file_size(struct dso *dso, struct machine *machine)
 {
 	int ret = 0;
 	struct stat st;
@@ -943,7 +943,7 @@ static int data_file_size(struct dso *dso, struct machine *machine)
  */
 off_t dso__data_size(struct dso *dso, struct machine *machine)
 {
-	if (data_file_size(dso, machine))
+	if (dso__data_file_size(dso, machine))
 		return -1;
 
 	/* For now just estimate dso data size is close to file size */
@@ -953,7 +953,7 @@ off_t dso__data_size(struct dso *dso, struct machine *machine)
 static ssize_t data_read_offset(struct dso *dso, struct machine *machine,
 				u64 offset, u8 *data, ssize_t size)
 {
-	if (data_file_size(dso, machine))
+	if (dso__data_file_size(dso, machine))
 		return -1;
 
 	/* Check the offset sanity. */
tools/perf/util/dso.h +1 -0
index c5380500bed4..8c8a7abe809d 100644
@@ -322,6 +322,7 @@ int dso__data_get_fd(struct dso *dso, struct machine *machine);
 void dso__data_put_fd(struct dso *dso);
 void dso__data_close(struct dso *dso);
 
+int dso__data_file_size(struct dso *dso, struct machine *machine);
 off_t dso__data_size(struct dso *dso, struct machine *machine);
 ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine,
 			      u64 offset, u8 *data, ssize_t size);
Signature · 1 line
2.17.1
#1 lore raw

Em Tue, Nov 27, 2018 at 10:46:34AM +0200, Adrian Hunter escreveu:

Quoted 3 lines

addr_filter__entire_dso() uses the first and last symbols from a dso, and so does not work when there are no symbols. Alter it to filter the whole file instead.

I'm splitting this patch into a first prep one that renames and exports the static function dso__data_file_size() and a second that does what this commig log message states.

The reason is that sometimes we find out that the main part of the patch is wrong and we then decide to revert the patch, only to realize that the exported function is by that time already used elsewhere.

- Arnaldo

Quoted 78 lines
---
 tools/perf/util/auxtrace.c | 11 ++++-------
 tools/perf/util/dso.c      |  6 +++---
 tools/perf/util/dso.h      |  1 +
 3 files changed, 8 insertions(+), 10 deletions(-)
tools/perf/util/auxtrace.c +4 -7
index 72d5ba2479bf..f69961c4a4f3 100644
@@ -1983,17 +1983,14 @@ static int find_dso_sym(struct dso *dso, const char *sym_name, u64 *start,
 
 static int addr_filter__entire_dso(struct addr_filter *filt, struct dso *dso)
 {
-	struct symbol *first_sym = dso__first_symbol(dso);
-	struct symbol *last_sym = dso__last_symbol(dso);
-
-	if (!first_sym || !last_sym) {
-		pr_err("Failed to determine filter for %s\nNo symbols found.\n",
+	if (dso__data_file_size(dso, NULL)) {
+		pr_err("Failed to determine filter for %s\nCannot determine file size.\n",
 		       filt->filename);
 		return -EINVAL;
 	}
 
-	filt->addr = first_sym->start;
-	filt->size = last_sym->end - first_sym->start;
+	filt->addr = 0;
+	filt->size = dso->data.file_size;
 
 	return 0;
 }
tools/perf/util/dso.c +3 -3
index bbed90e5d9bb..721f3d583945 100644
@@ -894,7 +894,7 @@ static ssize_t cached_read(struct dso *dso, struct machine *machine,
 	return r;
 }
 
-static int data_file_size(struct dso *dso, struct machine *machine)
+int dso__data_file_size(struct dso *dso, struct machine *machine)
 {
 	int ret = 0;
 	struct stat st;
@@ -943,7 +943,7 @@ static int data_file_size(struct dso *dso, struct machine *machine)
  */
 off_t dso__data_size(struct dso *dso, struct machine *machine)
 {
-	if (data_file_size(dso, machine))
+	if (dso__data_file_size(dso, machine))
 		return -1;
 
 	/* For now just estimate dso data size is close to file size */
@@ -953,7 +953,7 @@ off_t dso__data_size(struct dso *dso, struct machine *machine)
 static ssize_t data_read_offset(struct dso *dso, struct machine *machine,
 				u64 offset, u8 *data, ssize_t size)
 {
-	if (data_file_size(dso, machine))
+	if (dso__data_file_size(dso, machine))
 		return -1;
 
 	/* Check the offset sanity. */
tools/perf/util/dso.h +1 -0
index c5380500bed4..8c8a7abe809d 100644
@@ -322,6 +322,7 @@ int dso__data_get_fd(struct dso *dso, struct machine *machine);
 void dso__data_put_fd(struct dso *dso);
 void dso__data_close(struct dso *dso);
 
+int dso__data_file_size(struct dso *dso, struct machine *machine);
 off_t dso__data_size(struct dso *dso, struct machine *machine);
 ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine,
 			      u64 offset, u8 *data, ssize_t size);
Signature · 1 line
2.17.1
Signature · 1 line
- Arnaldo
author reviewer
#2 lore raw
Quoted 5 lines · Arnaldo Carvalho de Melo

Em Tue, Nov 27, 2018 at 10:46:34AM +0200, Adrian Hunter escreveu:

Quoted 3 lines

addr_filter__entire_dso() uses the first and last symbols from a dso, and so does not work when there are no symbols. Alter it to filter the whole file instead.

Quoted 7 lines

I'm splitting this patch into a first prep one that renames and exports the static function dso__data_file_size() and a second that does what this commig log message states.

The reason is that sometimes we find out that the main part of the patch is wrong and we then decide to revert the patch, only to realize that the exported function is by that time already used elsewhere.

Thanks for looking at this, but did it get lost? I don't see it in your tree.

#3 lore raw

Em Mon, Dec 10, 2018 at 03:17:05PM +0200, Adrian Hunter escreveu:

Quoted 10 lines · Arnaldo Carvalho de Melo
Quoted 8 lines

Em Tue, Nov 27, 2018 at 10:46:34AM +0200, Adrian Hunter escreveu:

Quoted 3 lines

addr_filter__entire_dso() uses the first and last symbols from a dso, and so does not work when there are no symbols. Alter it to filter the whole file instead.

I'm splitting this patch into a first prep one that renames and exports the static function dso__data_file_size() and a second that does what this commig log message states.

Quoted 5 lines
Quoted 3 lines

The reason is that sometimes we find out that the main part of the patch is wrong and we then decide to revert the patch, only to realize that the exported function is by that time already used elsewhere.

Thanks for looking at this, but did it get lost? I don't see it in your tree.

Yeah, my bad, thanks for the reminder, did the split and added to my perf/core branch,

Also I'm adding this:

Cc Masami Hiramatsu <[email protected]>
Cc Mathieu Poirier <[email protected]>
Fixes 1b36c03e3569 ("perf record: Add support for using symbols in address filters")

So that people that were involved in the patch that introduced this feature gets notified of this fix, also for backporters to take notice, please try to do this in the future.

Thanks,

- Arnaldo

#4 lore raw

[tip:perf/core] perf dso: Export data_file_size() method there are no symbols

In reply to Adrian Hunter
perf dso: Export data_file_size() method there are no symbols
Commit-ID:  b5c2161cc415babb84d2b49599df8bd03b2b9b69
Gitweb:     https://git.kernel.org/tip/b5c2161cc415babb84d2b49599df8bd03b2b9b69
Author:     Adrian Hunter <[email protected]>
AuthorDate: Tue, 27 Nov 2018 10:46:34 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 18 Dec 2018 12:21:44 -0300

perf dso: Export data_file_size() method there are no symbols

Will be used outside dso.c in a followup patch, so rename it and make it non-static.

---
 tools/perf/util/dso.c | 6 +++---
 tools/perf/util/dso.h | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)
tools/perf/util/dso.c +3 -3
index cee717a3794f..62c8cf622607 100644
@@ -894,7 +894,7 @@ static ssize_t cached_read(struct dso *dso, struct machine *machine,
 	return r;
 }
 
-static int data_file_size(struct dso *dso, struct machine *machine)
+int dso__data_file_size(struct dso *dso, struct machine *machine)
 {
 	int ret = 0;
 	struct stat st;
@@ -943,7 +943,7 @@ out:
  */
 off_t dso__data_size(struct dso *dso, struct machine *machine)
 {
-	if (data_file_size(dso, machine))
+	if (dso__data_file_size(dso, machine))
 		return -1;
 
 	/* For now just estimate dso data size is close to file size */
@@ -953,7 +953,7 @@ off_t dso__data_size(struct dso *dso, struct machine *machine)
 static ssize_t data_read_offset(struct dso *dso, struct machine *machine,
 				u64 offset, u8 *data, ssize_t size)
 {
-	if (data_file_size(dso, machine))
+	if (dso__data_file_size(dso, machine))
 		return -1;
 
 	/* Check the offset sanity. */
tools/perf/util/dso.h +1 -0
index c5380500bed4..8c8a7abe809d 100644
@@ -322,6 +322,7 @@ int dso__data_get_fd(struct dso *dso, struct machine *machine);
 void dso__data_put_fd(struct dso *dso);
 void dso__data_close(struct dso *dso);
 
+int dso__data_file_size(struct dso *dso, struct machine *machine);
 off_t dso__data_size(struct dso *dso, struct machine *machine);
 ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine,
 			      u64 offset, u8 *data, ssize_t size);
#5 lore raw
In reply to Adrian Hunter
perf auxtrace: Alter addr_filter__entire_dso() to work if there are no symbols
Commit-ID:  571766010ea6bf9726b288eb2db1abb59b1841af
Gitweb:     https://git.kernel.org/tip/571766010ea6bf9726b288eb2db1abb59b1841af
Author:     Adrian Hunter <[email protected]>
AuthorDate: Tue, 27 Nov 2018 10:46:34 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 18 Dec 2018 12:21:44 -0300

perf auxtrace: Alter addr_filter__entire_dso() to work if there are no symbols

addr_filter__entire_dso() uses the first and last symbols from a dso, and so does not work when there are no symbols. Alter it to filter the whole file instead.

Cc Jiri Olsa <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Mathieu Poirier <[email protected]>
Fixes 1b36c03e3569 ("perf record: Add support for using symbols in address filters")
[ split from a larger patch ]
---
 tools/perf/util/auxtrace.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)
tools/perf/util/auxtrace.c +4 -7
index 72d5ba2479bf..f69961c4a4f3 100644
@@ -1983,17 +1983,14 @@ static int find_dso_sym(struct dso *dso, const char *sym_name, u64 *start,
 
 static int addr_filter__entire_dso(struct addr_filter *filt, struct dso *dso)
 {
-	struct symbol *first_sym = dso__first_symbol(dso);
-	struct symbol *last_sym = dso__last_symbol(dso);
-
-	if (!first_sym || !last_sym) {
-		pr_err("Failed to determine filter for %s\nNo symbols found.\n",
+	if (dso__data_file_size(dso, NULL)) {
+		pr_err("Failed to determine filter for %s\nCannot determine file size.\n",
 		       filt->filename);
 		return -EINVAL;
 	}
 
-	filt->addr = first_sym->start;
-	filt->size = last_sym->end - first_sym->start;
+	filt->addr = 0;
+	filt->size = dso->data.file_size;
 
 	return 0;
 }