linux-kernel - Re: [PATCH 05/33] perf record: Allow the user to disable time stamps

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20140805133347.GH13375@kernel.org>
Date:	Tue, 5 Aug 2014 10:33:47 -0300
From:	Arnaldo Carvalho de Melo <acme@...nel.org>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	linux-kernel@...r.kernel.org, Andi Kleen <ak@...ux.intel.com>,
	Andi Kleen <andi@...stfloor.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Stephane Eranian <eranian@...gle.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Jiri Olsa <jolsa@...hat.com>
Subject: Re: [PATCH 05/33] perf record: Allow the user to disable time stamps

Em Tue, Aug 05, 2014 at 08:08:56AM +0200, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo <acme@...nel.org> wrote:
> > From: Andi Kleen <ak@...ux.intel.com>
 
> This patch looks dangerous and misleading to me.

I took it more from the angle: hey, it fixes a regression, i.e.
-T/--time somehow became a noop, and also it doesn't change any default.

It also states that you will get misaccounting, why that would happen,
althought wording could've been better, as mmaps won't be "missing",
i.e. they will be there as this governs just PERF_SAMPLE_TIME, not
perf_event_attr.mmap, so it MMAPs may come after samples for its maps.

So, I took it.

How should we proceed?

- Arnaldo
 
> > Time stamps are always implicitely enabled for record currently.  The
> > old --time/-T option is a nop.
> > 
> > Allow the user to disable timestamps by using --no-time
> > 
> > This can cause some minor misaccounting (by missing mmaps), but
> > significantly lowers the size of perf.data
> 
> I'm not any big change in size:
> 
>  -rw------- 1 mingo mingo 384768 Aug  5 08:01 perf.data.timestamps
>  -rw------- 1 mingo mingo 336952 Aug  5 08:00 perf.data.notimestamps
> 
> That's a ~14% reduction - it's certainly noticeable but not 
> earth-shattering.

I wonder if this is for things like intel PT or some other super high
volume scenario where 14% indeed looks like huge, but then not using
PERF_SAMPLE_TIME there may have implications, but then, this is one of
those use-only-if-you-know-what-are-the-implications knob, where the
default is the sane one.
 
> And what is called 'minor misaccounting' in the description is 
> actually a potentially broken profile that people might rely on.
> 
> So the patch description misleads twice: by exaggerating the 
> advantages and also by downplaying the disadvantages.
> 
> So either remove the --time option altogether, or fix its 
> 'misaccounting' so that the profile can be relied on.
> 
> Thanks,
> 
> 	Ingo
--
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/