linux-kernel - Re: [PATCH] perf tools: Fix ordering with unstable tsc

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: <20120322001028.GA7024@somewhere.redhat.com>
Date:	Thu, 22 Mar 2012 01:10:31 +0100
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Arnaldo Carvalho de Melo <acme@...stprotocols.net>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	LKML <linux-kernel@...r.kernel.org>,
	David Ahern <dsahern@...il.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Stephane Eranian <eranian@...gle.com>,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH] perf tools: Fix ordering with unstable tsc

On Wed, Mar 14, 2012 at 04:55:35PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Sat, Feb 18, 2012 at 05:50:37PM +0100, Frederic Weisbecker escreveu:
> > +static int alloc_cpus_timestamp_array(struct perf_session *s,
> > +				      struct perf_sample *sample,
> > +				      struct ordered_samples *os)
> > +{
> > +	int i;
> > +	int nr_cpus;
> > +
> > +	if (sample->cpu < s->nr_cpus)
> > +		return 0;
> 
> 
> Shouldn't we try to robustify this checking against HEADER_NRCPUS (if
> present)?
> 
> I just tried to see how to find that info, but unfortunately it just
> prints it when calling perf_header__fprintf_info, it can be made to
> work.
> 
> I need to work on storing those values in a struct accessible thru
> perf_session or perf_evlist, so that I can show that info on the TUI,
> perhaps at that time I can robustify this way.

Yeah I thought about that too. I can try to make that working.
I just thought this was an optimization that I could later add (ie: first
try to see if the core idea of the patch is accepted).

Of course the real long term optimization is going to have one file per
CPU. There, the ordering will be much easier and deterministically
correct.

> 
> > +
> > +	nr_cpus = sample->cpu + 1;
> > +
> > +	if (!os->last_cpu_timestamp)
> > +		os->last_cpu_timestamp = malloc(sizeof(u64) * nr_cpus);
> > +	else
> > +		os->last_cpu_timestamp = realloc(os->last_cpu_timestamp,
> > +						 sizeof(u64) * nr_cpus);
> 
> If realloc fails we return -ENOMEM, but leak the old buffer.

Doh! the common trap with realloc...

> 
> At some point we can have in the TUI/GUI a way for the user to ask for
> an specific perf.data file to be processed, if it fails to process due
> to the above realloc, we'll continue, allowing other perf.data files to
> be processed, but will have this leak.

Ok. Will fix.

Thanks.
--
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/