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

[Jeremy Fitzhardinge <jeremy@goop.org>]

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