Re: Experiences on ARM with LTTng

* Richard Purdie ([email protected]) wrote:
[...]
> I realise the bogus files shouldn't be there. Perhaps the best thing to
> do would be to improve some of the error messages in tracefile.c so it
> was obvious which tracefile was causing the problems and also consider
> turning some of the asserts into "proper" errors complete with
> filenames? If I'd been able to see which file was causing the problems I
> would have realised what was going wrong much sooner...
> 

Absolutely. A lot of g_error should be changed for more explicit error
messages.

> > I guess the work would overlap a little bit. It would be better to do a
> > pass on the code once I finish switching over to the markers.
> 
> Ok, that sounds sensible.
> 
> > I am a bit reluctant to use events to tell what happens with the TSC,
> > since TSC is itself used to reorder events coming from multiple traces.
> > I could be done, but goes against my motto about timekeeping: keep it
> > simple. I would rather prefer an approach that would create a wrapper
> > over the timestamp reads:
> > 
> > u64 sleep_cycles;
> > 
> > ltt_get_timestamp32()
> >   - return (u32)(get_cycles() + sleep_cycles);
> > 
> > ltt_get_timestamp64()
> >   - return get_cycles() + sleep_cycles;
> > 
> >  Called upon return from sleep mode:
> > arch_ltt_add_timestamp()
> >   - Updates sleep_cycles atomically
> > 
> > We would have to decide whether sleep_cycles should be per cpu.
> > 
> > What do you think of this approach ?
> 
> I've given it a bit of thought and I think it could work, my only
> concern is how to handle incrementing sleep_cycles safely given its 64
> bit.
> 
> My attention has been drawn to include/asm-arm/cnt32_to_63.h which
> provides a lockless 32 to 63 bit conversion which may or may not help.
> 

Interesting, but I think ltt/ltt-timestamp.c does a better job: it
extends a 32 bits cycle counter into a 64 bits cycle counter locklessly.

It does so by keeping an array of two 64 bits values and alternates the
current index between each of them, always updating the other one. This
array is per cpu, and we must protect against concurrent "updates"
(typically done only by the timer interrupt, so there is no need for
locking there).

You could use a similar data structure to keep your offset, given that
you would update it only from a single source.

The read side only needs to disable preemption.

You just have to be aware that events occuring before the update
(interrupts ?) would have the old offset.

> On the subject of whether it should be per cpu, on ARM I'd say there is
> no need at present, I'm not sure if there are other places where it
> would make sense...

Just be cautious about concurrent updates if you plan not to do this per
cpu.

Mathieu

> 
> Cheers,
> 
> Richard
> 

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68