linux-kernel - Re: [RFC PATCH 1/3] Unified trace buffer

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: <48DC73C2.5080309@goop.org>
Date:	Thu, 25 Sep 2008 22:31:46 -0700
From:	Jeremy Fitzhardinge <jeremy@...p.org>
To:	Steven Rostedt <rostedt@...dmis.org>
CC:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...e.hu>, Martin Bligh <mbligh@...gle.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Martin Bligh <mbligh@...igh.org>, linux-kernel@...r.kernel.org,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	prasad@...ux.vnet.ibm.com,
	Mathieu Desnoyers <compudj@...stal.dyndns.org>,
	"Frank Ch. Eigler" <fche@...hat.com>,
	David Wilder <dwilder@...ibm.com>, hch@....de,
	Tom Zanussi <zanussi@...cast.net>,
	Steven Rostedt <srostedt@...hat.com>
Subject: Re: [RFC PATCH 1/3] Unified trace buffer

Steven Rostedt wrote:
> [
>   "When people ask me what language my mother tongue is,
>    I simply reply 'C'" - Steven Rostedt
> ]
>
> This is exactly why I have that saying ;-)
>
>
> On Thu, 25 Sep 2008, Jeremy Fitzhardinge wrote:
>
>   
>> Steven Rostedt wrote:
>>     
>>> OK, let me rephrase my question.
>>>
>>> How and where do we record this?  Do we keep this information in some 
>>> global variable that we must compare to every time we add a new item in 
>>> the trace?
>>>
>>> Do we have the buffer register a call back to record this information?
>>>   
>>>       
>> Something like (total pseudocode):
>>     
>
> Good enough.
>
>   
>> struct tsc_time_parameters {
>> 	int version;		/* even - values OK; odd - values being updated */
>> 	u64 tsc;
>> 	u32 tsc_freq;
>> 	u64 gtod;
>> };
>>
>> DEFINE_PERCPU(struct tsc_time_parameters, tsc_params);
>>     
>
> These are all global I presume (No "static" in front)
>   

No, they could probably be static, depending where everything ends up. 
It would only need to get accessed from a couple of places.

>> /* To be called after a tsc frequency change, before any new
>>    trace records are being emitted, in a context where we can call get_GTOD() */
>> void update_tsc_params(void)
>>     
>
> So this needs to be called by the cpu freq code?
>   

Yes, and any other place the tsc might get affected, like going into a
C-state which stops the tsc, and things like suspend/resume.

>> {
>> 	struct tsc_time_parameters *p = __get_percpu_var(tsc_params);
>>
>> 	p->version |= 1;
>> 	wmb();
>>
>> 	p->tsc = get_tsc();
>> 	p->tsc_freq = get_tsc_freq();
>> 	p->gtod = get_GTOD();
>>
>> 	wmb();
>> 	p->version++;
>> 	wmb();
>> }
>>
>> DEFINE_PERCPU(unsigned, current_tsc_version);
>> DEFINE_PERCPU(u64, prev_tsc);
>>
>> /* may be called in any context */
>> u64 get_trace_timestamp_delta(void)
>> {
>> 	const struct tsc_time_parameters *p = &__get_percpu_var(tsc_params);
>> 	unsigned *current_version = &__get_cpu_var(current_tsc_version);
>> 	u64 prev = __get_cpu_var(prev_tsc);
>> 	u64 now, ret;
>>
>> 	/* check the current tsc_params version against the last one we emitted;
>> 		if the version is odd, then we interrupted the parameters as they were
>> 		being updated, so just emit a new delta with the old parameters */
>> 	if (unlikely(*current_version != p->version && !(p->version & 1))) {
>> 		/* XXX probably need a loop to deal with p->version changing under our feet */
>> 		emit_tsc_freq_record(p);
>>     
>
> I take it the above is your record to the tracer?
>   

Yeah.  No doubt it needs a few more parameters.

>> 		prev = p->tsc;
>> 		__get_cpu_var(current_tsc_version) = p->version;
>> 	}
>>
>> 	now = read_tsc();
>>     
>
> We probably wont to check here that p didn't change again.
> and try again if it did.
>   

Yeah, you may want to put the whole thing in a loop to make sure that
the version is consistent.  You might end up emitting multiple redundant
tsc parameters, but that should be very rare.

>> 	ret = now - prev;
>> 	__get_cpu_var(prev_tsc) = now;
>>
>> 	return ret;
>> }
>>     
>
>
> Hmm, the beginning of each patch will need to record the global tsc, as 
> well as this information. Simply because in overwrite mode, we do not want 
> to lose it if the producer is faster than te consumer.
>   

Each patch?  Page?

    J
--
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/