linux-kernel - Re: [PATCH 2/2] perf tools: Skip read of kernel maps once it failed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180126064830.GC22737@sejong>
Date:   Fri, 26 Jan 2018 15:48:30 +0900
From:   Namhyung Kim <namhyung@...nel.org>
To:     Jiri Olsa <jolsa@...nel.org>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        David Ahern <dsahern@...il.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>, kernel-team@....com
Subject: Re: [PATCH 2/2] perf tools: Skip read of kernel maps once it failed

On Fri, Jan 19, 2018 at 05:11:03PM +0100, Jiri Olsa wrote:
> Current perf report is real slow on newer kernels,
> with following commit:
>   c0f3ea158939 ("stop using '%pK' for /proc/kallsyms pointer values")
> 
> which prevent pointers in /proc/kallsyms, in case
> kernel.perf_event_paranoid=2.
> 
> That makes perf to fail in finding kernel map details,
> and keep parsing it again for every kernel sample.
> 
> Adding and setting a new machine::vmlinux_maps_failed
> flag after first failed parsing attempt and using it
> to prevent new pointless parsing.

Hmm.. is it because it's called from machine__resolve() right?

	/*
	 * Have we already created the kernel maps for this machine?
	 *
	 * This should have happened earlier, when we processed the kernel MMAP
	 * events, but for older perf.data files there was no such thing, so do
	 * it now.
	 */

It seems that it's only to be compatible with ancient versions.

Do we still need it?  I guess they are recorded many years ago (with
the ancient version) so using addresses of current kernel is just
meaningless.  If one still uses the ancient version, [s]he really
needs to update it.

Thanks,
Namhyung


> 
> TODO We might want to add some perf report warning
> about that.
> 
> Link: http://lkml.kernel.org/n/tip-ld2kp994rhz6i341igt8f98y@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> ---
>  tools/perf/util/machine.c | 6 ++++--
>  tools/perf/util/machine.h | 1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index b05a67464c03..5e9648817077 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1222,13 +1222,15 @@ int machine__create_kernel_maps(struct machine *machine)
>  	u64 addr = 0;
>  	int ret;
>  
> -	if (kernel == NULL)
> +	if (kernel == NULL || machine->vmlinux_maps_failed)
>  		return -1;
>  
>  	ret = __machine__create_kernel_maps(machine, kernel);
>  	dso__put(kernel);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		machine->vmlinux_maps_failed = true;
>  		return -1;
> +	}
>  
>  	if (symbol_conf.use_modules && machine__create_modules(machine) < 0) {
>  		if (machine__is_host(machine))
> diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
> index 5ce860b64c74..edcb007333cb 100644
> --- a/tools/perf/util/machine.h
> +++ b/tools/perf/util/machine.h
> @@ -42,6 +42,7 @@ struct machine {
>  	u16		  id_hdr_size;
>  	bool		  comm_exec;
>  	bool		  kptr_restrict_warned;
> +	bool		  vmlinux_maps_failed;
>  	char		  *root_dir;
>  	struct threads    threads[THREADS__TABLE_SIZE];
>  	struct vdso_info  *vdso_info;
> -- 
> 2.13.6
>