[PATCH v2 1/2] perf tools: Honor namespace when synthesizing build-id

[PATCH v2 1/2] perf tools: Honor namespace when synthesizing build-id

[Namhyung Kim]

It needs to go into a namespace before reading a file.

Fixes: 4183a8d70a28 ("perf tools: Allow synthesizing the build id for kernel/modules/tasks in PERF_RECORD_MMAP2")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/synthetic-events.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
index 0ff57ca24577..289ea17ac5f7 100644
--- a/tools/perf/util/synthetic-events.c
+++ b/tools/perf/util/synthetic-events.c
@@ -367,13 +367,24 @@ static void perf_record_mmap2__read_build_id(struct perf_record_mmap2 *event,
 					     bool is_kernel)
 {
 	struct build_id bid;
+	struct nsinfo *nsi;
+	struct nscookie nc;
 	int rc;
 
-	if (is_kernel)
+	if (is_kernel) {
 		rc = sysfs__read_build_id("/sys/kernel/notes", &bid);
-	else
-		rc = filename__read_build_id(event->filename, &bid) > 0 ? 0 : -1;
+		goto out;
+	}
+
+	nsi = nsinfo__new(event->pid);
+	nsinfo__mountns_enter(nsi, &nc);
 
+	rc = filename__read_build_id(event->filename, &bid) > 0 ? 0 : -1;
+
+	nsinfo__mountns_exit(&nc);
+	nsinfo__put(nsi);
+
+out:
 	if (rc == 0) {
 		memcpy(event->build_id, bid.data, sizeof(bid.data));
 		event->build_id_size = (u8) bid.size;
-- 
2.37.3.968.ga6b4b080e4-goog

[PATCH v2 2/2] perf record: Save DSO build-ID for synthesizing

[Namhyung Kim]

When synthesizing MMAP2 with build-id, it'd read the same file repeatedly as
it has no idea if it's done already.  Maintain a dsos to check that and skip
the file access if possible.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
v2 change)
 * Remove perf_event__synthesize_{start,stop} and use machine->dsos  (Adrian)

 tools/perf/util/synthetic-events.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
index 289ea17ac5f7..cccd293b5312 100644
--- a/tools/perf/util/synthetic-events.c
+++ b/tools/perf/util/synthetic-events.c
@@ -364,11 +364,14 @@ static bool read_proc_maps_line(struct io *io, __u64 *start, __u64 *end,
 }
 
 static void perf_record_mmap2__read_build_id(struct perf_record_mmap2 *event,
+					     struct machine *machine,
 					     bool is_kernel)
 {
 	struct build_id bid;
 	struct nsinfo *nsi;
 	struct nscookie nc;
+	struct dso *dso = NULL;
+	struct dso_id id;
 	int rc;
 
 	if (is_kernel) {
@@ -376,6 +379,18 @@ static void perf_record_mmap2__read_build_id(struct perf_record_mmap2 *event,
 		goto out;
 	}
 
+	id.maj = event->maj;
+	id.min = event->min;
+	id.ino = event->ino;
+	id.ino_generation = event->ino_generation;
+
+	dso = dsos__findnew_id(&machine->dsos, event->filename, &id);
+	if (dso && dso->has_build_id) {
+		bid = dso->bid;
+		rc = 0;
+		goto out;
+	}
+
 	nsi = nsinfo__new(event->pid);
 	nsinfo__mountns_enter(nsi, &nc);
 
@@ -391,12 +406,16 @@ static void perf_record_mmap2__read_build_id(struct perf_record_mmap2 *event,
 		event->header.misc |= PERF_RECORD_MISC_MMAP_BUILD_ID;
 		event->__reserved_1 = 0;
 		event->__reserved_2 = 0;
+
+		if (dso && !dso->has_build_id)
+			dso__set_build_id(dso, &bid);
 	} else {
 		if (event->filename[0] == '/') {
 			pr_debug2("Failed to read build ID for %s\n",
 				  event->filename);
 		}
 	}
+	dso__put(dso);
 }
 
 int perf_event__synthesize_mmap_events(struct perf_tool *tool,
@@ -507,7 +526,7 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
 		event->mmap2.tid = pid;
 
 		if (symbol_conf.buildid_mmap2)
-			perf_record_mmap2__read_build_id(&event->mmap2, false);
+			perf_record_mmap2__read_build_id(&event->mmap2, machine, false);
 
 		if (perf_tool__process_synth_event(tool, event, machine, process) != 0) {
 			rc = -1;
@@ -690,7 +709,7 @@ int perf_event__synthesize_modules(struct perf_tool *tool, perf_event__handler_t
 			memcpy(event->mmap2.filename, pos->dso->long_name,
 			       pos->dso->long_name_len + 1);
 
-			perf_record_mmap2__read_build_id(&event->mmap2, false);
+			perf_record_mmap2__read_build_id(&event->mmap2, machine, false);
 		} else {
 			size = PERF_ALIGN(pos->dso->long_name_len + 1, sizeof(u64));
 			event->mmap.header.type = PERF_RECORD_MMAP;
@@ -1126,7 +1145,7 @@ static int __perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 		event->mmap2.len   = map->end - event->mmap.start;
 		event->mmap2.pid   = machine->pid;
 
-		perf_record_mmap2__read_build_id(&event->mmap2, true);
+		perf_record_mmap2__read_build_id(&event->mmap2, machine, true);
 	} else {
 		size = snprintf(event->mmap.filename, sizeof(event->mmap.filename),
 				"%s%s", machine->mmap_name, kmap->ref_reloc_sym->name) + 1;
-- 
2.37.3.968.ga6b4b080e4-goog

Re: [PATCH v2 2/2] perf record: Save DSO build-ID for synthesizing

[Arnaldo Carvalho de Melo]

Em Tue, Sep 20, 2022 at 03:28:22PM -0700, Namhyung Kim escreveu:
> When synthesizing MMAP2 with build-id, it'd read the same file repeatedly as
> it has no idea if it's done already.  Maintain a dsos to check that and skip
> the file access if possible.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> v2 change)
>  * Remove perf_event__synthesize_{start,stop} and use machine->dsos  (Adrian)

Will wait till I merge perf/urgent into perf/core so that this applies.

- Arnaldo
 
>  tools/perf/util/synthetic-events.c | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 289ea17ac5f7..cccd293b5312 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -364,11 +364,14 @@ static bool read_proc_maps_line(struct io *io, __u64 *start, __u64 *end,
>  }
>  
>  static void perf_record_mmap2__read_build_id(struct perf_record_mmap2 *event,
> +					     struct machine *machine,
>  					     bool is_kernel)
>  {
>  	struct build_id bid;
>  	struct nsinfo *nsi;
>  	struct nscookie nc;
> +	struct dso *dso = NULL;
> +	struct dso_id id;
>  	int rc;
>  
>  	if (is_kernel) {
> @@ -376,6 +379,18 @@ static void perf_record_mmap2__read_build_id(struct perf_record_mmap2 *event,
>  		goto out;
>  	}
>  
> +	id.maj = event->maj;
> +	id.min = event->min;
> +	id.ino = event->ino;
> +	id.ino_generation = event->ino_generation;
> +
> +	dso = dsos__findnew_id(&machine->dsos, event->filename, &id);
> +	if (dso && dso->has_build_id) {
> +		bid = dso->bid;
> +		rc = 0;
> +		goto out;
> +	}
> +
>  	nsi = nsinfo__new(event->pid);
>  	nsinfo__mountns_enter(nsi, &nc);
>  
> @@ -391,12 +406,16 @@ static void perf_record_mmap2__read_build_id(struct perf_record_mmap2 *event,
>  		event->header.misc |= PERF_RECORD_MISC_MMAP_BUILD_ID;
>  		event->__reserved_1 = 0;
>  		event->__reserved_2 = 0;
> +
> +		if (dso && !dso->has_build_id)
> +			dso__set_build_id(dso, &bid);
>  	} else {
>  		if (event->filename[0] == '/') {
>  			pr_debug2("Failed to read build ID for %s\n",
>  				  event->filename);
>  		}
>  	}
> +	dso__put(dso);
>  }
>  
>  int perf_event__synthesize_mmap_events(struct perf_tool *tool,
> @@ -507,7 +526,7 @@ int perf_event__synthesize_mmap_events(struct perf_tool *tool,
>  		event->mmap2.tid = pid;
>  
>  		if (symbol_conf.buildid_mmap2)
> -			perf_record_mmap2__read_build_id(&event->mmap2, false);
> +			perf_record_mmap2__read_build_id(&event->mmap2, machine, false);
>  
>  		if (perf_tool__process_synth_event(tool, event, machine, process) != 0) {
>  			rc = -1;
> @@ -690,7 +709,7 @@ int perf_event__synthesize_modules(struct perf_tool *tool, perf_event__handler_t
>  			memcpy(event->mmap2.filename, pos->dso->long_name,
>  			       pos->dso->long_name_len + 1);
>  
> -			perf_record_mmap2__read_build_id(&event->mmap2, false);
> +			perf_record_mmap2__read_build_id(&event->mmap2, machine, false);
>  		} else {
>  			size = PERF_ALIGN(pos->dso->long_name_len + 1, sizeof(u64));
>  			event->mmap.header.type = PERF_RECORD_MMAP;
> @@ -1126,7 +1145,7 @@ static int __perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
>  		event->mmap2.len   = map->end - event->mmap.start;
>  		event->mmap2.pid   = machine->pid;
>  
> -		perf_record_mmap2__read_build_id(&event->mmap2, true);
> +		perf_record_mmap2__read_build_id(&event->mmap2, machine, true);
>  	} else {
>  		size = snprintf(event->mmap.filename, sizeof(event->mmap.filename),
>  				"%s%s", machine->mmap_name, kmap->ref_reloc_sym->name) + 1;
> -- 
> 2.37.3.968.ga6b4b080e4-goog

-- 

- Arnaldo

Re: [PATCH v2 2/2] perf record: Save DSO build-ID for synthesizing

[Namhyung Kim]

On Wed, Sep 21, 2022 at 6:50 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Tue, Sep 20, 2022 at 03:28:22PM -0700, Namhyung Kim escreveu:
> > When synthesizing MMAP2 with build-id, it'd read the same file repeatedly as
> > it has no idea if it's done already.  Maintain a dsos to check that and skip
> > the file access if possible.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > v2 change)
> >  * Remove perf_event__synthesize_{start,stop} and use machine->dsos  (Adrian)
>
> Will wait till I merge perf/urgent into perf/core so that this applies.

Sounds good, thanks!
Namhyung

Re: [PATCH v2 2/2] perf record: Save DSO build-ID for synthesizing

[Namhyung Kim]

Hi Arnaldo,

On Wed, Sep 21, 2022 at 10:55 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Wed, Sep 21, 2022 at 6:50 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Tue, Sep 20, 2022 at 03:28:22PM -0700, Namhyung Kim escreveu:
> > > When synthesizing MMAP2 with build-id, it'd read the same file repeatedly as
> > > it has no idea if it's done already.  Maintain a dsos to check that and skip
> > > the file access if possible.
> > >
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> > > v2 change)
> > >  * Remove perf_event__synthesize_{start,stop} and use machine->dsos  (Adrian)
> >
> > Will wait till I merge perf/urgent into perf/core so that this applies.

I think it's doable now :)

Thanks,
Namhyung

Re: [PATCH v2 2/2] perf record: Save DSO build-ID for synthesizing

[Arnaldo Carvalho de Melo]

Em Wed, Oct 05, 2022 at 10:50:01AM -0700, Namhyung Kim escreveu:
> On Wed, Sep 21, 2022 at 10:55 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > On Wed, Sep 21, 2022 at 6:50 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > Em Tue, Sep 20, 2022 at 03:28:22PM -0700, Namhyung Kim escreveu:
> > > > When synthesizing MMAP2 with build-id, it'd read the same file repeatedly as
> > > > it has no idea if it's done already.  Maintain a dsos to check that and skip
> > > > the file access if possible.
> > > >
> > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > > ---
> > > > v2 change)
> > > >  * Remove perf_event__synthesize_{start,stop} and use machine->dsos  (Adrian)
> > >
> > > Will wait till I merge perf/urgent into perf/core so that this applies.
> 
> I think it's doable now :)

Sure, applied! :-)

- Arnaldo

Re: [PATCH v2 1/2] perf tools: Honor namespace when synthesizing build-id

[Arnaldo Carvalho de Melo]

Em Tue, Sep 20, 2022 at 03:28:21PM -0700, Namhyung Kim escreveu:
> It needs to go into a namespace before reading a file.
> 
> Fixes: 4183a8d70a28 ("perf tools: Allow synthesizing the build id for kernel/modules/tasks in PERF_RECORD_MMAP2")
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Thanks, applied to perf/urgent.

- Arnaldo

> ---
>  tools/perf/util/synthetic-events.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 0ff57ca24577..289ea17ac5f7 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -367,13 +367,24 @@ static void perf_record_mmap2__read_build_id(struct perf_record_mmap2 *event,
>  					     bool is_kernel)
>  {
>  	struct build_id bid;
> +	struct nsinfo *nsi;
> +	struct nscookie nc;
>  	int rc;
>  
> -	if (is_kernel)
> +	if (is_kernel) {
>  		rc = sysfs__read_build_id("/sys/kernel/notes", &bid);
> -	else
> -		rc = filename__read_build_id(event->filename, &bid) > 0 ? 0 : -1;
> +		goto out;
> +	}
> +
> +	nsi = nsinfo__new(event->pid);
> +	nsinfo__mountns_enter(nsi, &nc);
>  
> +	rc = filename__read_build_id(event->filename, &bid) > 0 ? 0 : -1;
> +
> +	nsinfo__mountns_exit(&nc);
> +	nsinfo__put(nsi);
> +
> +out:
>  	if (rc == 0) {
>  		memcpy(event->build_id, bid.data, sizeof(bid.data));
>  		event->build_id_size = (u8) bid.size;
> -- 
> 2.37.3.968.ga6b4b080e4-goog

-- 

- Arnaldo