linux-kernel - Re: [PATCH v1 06/15] perf session: load data directory into tool process memory

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]

Date:   Mon, 12 Oct 2020 09:09:21 -0700
From:   Andi Kleen <ak@...ux.intel.com>
To:     Alexey Budankov <alexey.budankov@...ux.intel.com>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 06/15] perf session: load data directory into tool
 process memory

On Mon, Oct 12, 2020 at 11:58:58AM +0300, Alexey Budankov wrote:
> 
> Read trace files located at data directory into tool process memory.
> Basic analysis support of data directories is provided for report
> mode. Raw dump (-D) and aggregated reports are available for data
> directories, still with no memory consumption optimizations. However
> data directories collected with --compression-level option enabled
> can be analyzed with little less memory because trace files are
> unmaped from tool process memory after loading collected data.
> The implementation is based on the prototype [1], [2].

Should credit the author(s) of the prototypes.
> 
> [1] git clone https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git -b perf/record_threads
> [2] https://lore.kernel.org/lkml/20180913125450.21342-1-jolsa@kernel.org/
> 
> Signed-off-by: Alexey Budankov <alexey.budankov@...ux.intel.com>
> ---
>  tools/perf/util/session.c | 48 +++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/session.h |  1 +
>  2 files changed, 49 insertions(+)
> 
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 6afc670fdf0c..0752eec19813 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -2212,6 +2212,17 @@ reader__process_events(struct reader *rd, struct perf_session *session,
>  		goto more;
>  
>  out:
> +	if (rd->unmap_file) {
> +		int i;
> +
> +		for (i = 0; i < NUM_MMAPS; i++) {
> +			if (mmaps[i]) {
> +				munmap(mmaps[i], mmap_size);
> +				mmaps[i] = NULL;

Okay so where is the mmap? Would make more sense to put that
into the same patch as who adds the mmap. Or is the mmap
code already in the perf source? In that case it should
probably be some common helper with the existing users.