Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 17 Nov 2020 09:46:52 -0300
From:   Arnaldo Carvalho de Melo <acme@kernel.org>
To:     Jiri Olsa <jolsa@kernel.org>
Cc:     lkml <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>,
        Ingo Molnar <mingo@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Michael Petlan <mpetlan@redhat.com>,
        Song Liu <songliubraving@fb.com>,
        Ian Rogers <irogers@google.com>,
        Stephane Eranian <eranian@google.com>,
        Alexey Budankov <alexey.budankov@linux.intel.com>,
        Andi Kleen <ak@linux.intel.com>,
        Adrian Hunter <adrian.hunter@intel.com>
Subject: Re: [PATCH 14/24] perf tools: Allow mmap2 event to synthesize modules
Message-ID: <20201117124652.GP614220@kernel.org>
References: <20201117110053.1303113-1-jolsa@kernel.org>
 <20201117110053.1303113-15-jolsa@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201117110053.1303113-15-jolsa@kernel.org>
Precedence: bulk

Em Tue, Nov 17, 2020 at 12:00:43PM +0100, Jiri Olsa escreveu:
> Allow mmap2 event to synthesize kernel modules,
> so we can synthesize module's build id data in
> following changes.
> 
> It's enabled by new symbol_conf.buildid_mmap2
> bool, which will be switched in following
> changes.

Ditto as for the kernel mmap event, don't we do this probing before
generating the synthetic events? If not perhaps we should, to avoid
synthesizing things and then failing on creating the events? If we do it
that way, we can switch from symbol_conf.buildid_mmap2 to
!perf_missing_features.mmap2.

- Arnaldo
 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/synthetic-events.c | 49 +++++++++++++++++++-----------
>  1 file changed, 32 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 872df6d6dbef..a18ae502d765 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -593,16 +593,17 @@ int perf_event__synthesize_modules(struct perf_tool *tool, perf_event__handler_t
>  	int rc = 0;
>  	struct map *pos;
>  	struct maps *maps = machine__kernel_maps(machine);
> -	union perf_event *event = zalloc((sizeof(event->mmap) +
> -					  machine->id_hdr_size));
> +	union perf_event *event;
> +	size_t size = symbol_conf.buildid_mmap2 ?
> +			sizeof(event->mmap2) : sizeof(event->mmap);
> +
> +	event = zalloc(size + machine->id_hdr_size);
>  	if (event == NULL) {
>  		pr_debug("Not enough memory synthesizing mmap event "
>  			 "for kernel modules\n");
>  		return -1;
>  	}
>  
> -	event->header.type = PERF_RECORD_MMAP;
> -
>  	/*
>  	 * kernel uses 0 for user space maps, see kernel/perf_event.c
>  	 * __perf_event_mmap
> @@ -613,23 +614,37 @@ int perf_event__synthesize_modules(struct perf_tool *tool, perf_event__handler_t
>  		event->header.misc = PERF_RECORD_MISC_GUEST_KERNEL;
>  
>  	maps__for_each_entry(maps, pos) {
> -		size_t size;
> -
>  		if (!__map__is_kmodule(pos))
>  			continue;
>  
> -		size = PERF_ALIGN(pos->dso->long_name_len + 1, sizeof(u64));
> -		event->mmap.header.type = PERF_RECORD_MMAP;
> -		event->mmap.header.size = (sizeof(event->mmap) -
> -				        (sizeof(event->mmap.filename) - size));
> -		memset(event->mmap.filename + size, 0, machine->id_hdr_size);
> -		event->mmap.header.size += machine->id_hdr_size;
> -		event->mmap.start = pos->start;
> -		event->mmap.len   = pos->end - pos->start;
> -		event->mmap.pid   = machine->pid;
> +		if (symbol_conf.buildid_mmap2) {
> +			size = PERF_ALIGN(pos->dso->long_name_len + 1, sizeof(u64));
> +			event->mmap2.header.type = PERF_RECORD_MMAP2;
> +			event->mmap2.header.size = (sizeof(event->mmap2) -
> +						(sizeof(event->mmap2.filename) - size));
> +			memset(event->mmap2.filename + size, 0, machine->id_hdr_size);
> +			event->mmap2.header.size += machine->id_hdr_size;
> +			event->mmap2.start = pos->start;
> +			event->mmap2.len   = pos->end - pos->start;
> +			event->mmap2.pid   = machine->pid;
> +
> +			memcpy(event->mmap2.filename, pos->dso->long_name,
> +			       pos->dso->long_name_len + 1);
> +		} else {
> +			size = PERF_ALIGN(pos->dso->long_name_len + 1, sizeof(u64));
> +			event->mmap.header.type = PERF_RECORD_MMAP;
> +			event->mmap.header.size = (sizeof(event->mmap) -
> +						(sizeof(event->mmap.filename) - size));
> +			memset(event->mmap.filename + size, 0, machine->id_hdr_size);
> +			event->mmap.header.size += machine->id_hdr_size;
> +			event->mmap.start = pos->start;
> +			event->mmap.len   = pos->end - pos->start;
> +			event->mmap.pid   = machine->pid;
> +
> +			memcpy(event->mmap.filename, pos->dso->long_name,
> +			       pos->dso->long_name_len + 1);
> +		}
>  
> -		memcpy(event->mmap.filename, pos->dso->long_name,
> -		       pos->dso->long_name_len + 1);
>  		if (perf_tool__process_synth_event(tool, event, machine, process) != 0) {
>  			rc = -1;
>  			break;
> -- 
> 2.26.2
> 

-- 

- Arnaldo