linux-kernel - Re: [PATCH] perf record: mmap output file

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: <87txgj9eir.fsf@sejong.aot.lge.com>
Date:	Tue, 15 Oct 2013 16:31:08 +0900
From:	Namhyung Kim <namhyung@...nel.org>
To:	David Ahern <dsahern@...il.com>
Cc:	acme@...stprotocols.net, linux-kernel@...r.kernel.org,
	Ingo Molnar <mingo@...nel.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Jiri Olsa <jolsa@...hat.com>, Mike Galbraith <efault@....de>,
	Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH] perf record: mmap output file - v2

Hi David,

On Mon, 14 Oct 2013 20:55:31 -0600, David Ahern wrote:
> When recording raw_syscalls for the entire system, e.g.,
>     perf record -e raw_syscalls:*,sched:sched_switch -a -- sleep 1
>
> you end up with a negative feedback loop as perf itself calls
> write() fairly often. This patch handles the problem by mmap'ing the
> file in chunks of 64M at a time and copies events from the event buffers
> to the file avoiding write system calls.
>
> Before (with write syscall):
>
> perf record -o /tmp/perf.data -e raw_syscalls:*,sched:sched_switch -a -- sleep 1
> [ perf record: Woken up 0 times to write data ]
> [ perf record: Captured and wrote 81.843 MB /tmp/perf.data (~3575786 samples) ]
>
> After (using mmap):
>
> perf record -o /tmp/perf.data -e raw_syscalls:*,sched:sched_switch -a -- sleep 1
> [ perf record: Woken up 31 times to write data ]
> [ perf record: Captured and wrote 8.203 MB /tmp/perf.data (~358388 samples) ]

Why do they have that different size?

>
[SNIP]
> +/* mmap file big chunks at a time */
> +#define MMAP_OUTPUT_SIZE   (64*1024*1024)

Why did you choose 64MB for the size?  Did you also test other sizes?


[SNIP]
> +
> +		rec->mmap_addr = mmap(NULL, rec->mmap_size,
> +				     PROT_WRITE | PROT_READ,
> +				     MAP_SHARED,
> +				     rec->output,
> +				     offset);
> +
> +		if (rec->mmap_addr == MAP_FAILED) {
> +			pr_err("mmap failed: %d: %s\n", errno, strerror(errno));
> +			return -1;
> +		}
> +
> +		/* expand file to include this mmap segment */
> +		if (ftruncate(rec->output, offset + rec->mmap_size) != 0) {
> +			pr_err("ftruncate failed\n");
> +			return -1;
> +		}

I think this mmap + ftruncate should be reordered.  Although it looks
work without problems the mmap man pages says it's unspecified behavior.

       A file is mapped in multiples of the page size.  For a file that is not
       a multiple of the page  size,  the  remaining  memory  is  zeroed  when
       mapped, and writes to that region are not written out to the file.  The
       effect of changing the size of the underlying file of a mapping on  the
       pages  that  correspond  to  added  or  removed  regions of the file is
       unspecified.


Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/