Re: [ckrm-tech] [RFC][PATCH] Add timestamp to process event connector message

[Matt Helsley <matthltc@us.ibm.com>]

On Mon, 2005-12-05 at 18:39 -0800, Dave Hansen wrote: 
> On Mon, 2005-12-05 at 18:21 -0800, Matt Helsley wrote:
> > +static inline void get_timestamp(struct timespec *ts)
> > +{
> > +       unsigned int seq;
> > +       struct timespec wall2mono;
> > +
> > +       /* synchronize with settimeofday() changes */
> > +       do {
> > +               seq = read_seqbegin(&xtime_lock);
> > +               getnstimeofday(ts);
> > +               wall2mono = wall_to_monotonic;
> > +       } while(read_seqretry(&xtime_lock, seq));
> > +
> > +       /* adjust to monotonicaly-increasing values */
> > +       ts += wall2mono.tv_sec;
> > +       ts += wall2mono.tv_nsec;
> > +       while ((ts->tv_nsec - NSEC_PER_SEC) >= 0) {
> > +               ts->tv_nsec -= NSEC_PER_SEC;
> > +               ts->tv_sec++;
> > +       }
> > +}
> 
> This seems like something a bit too generic to have in your
> drivers/connector/cn_proc.c file.  Is there a generic timekeeping
> function that should be used instead?  Or, should this go into one of
> the timekeeping files?
> 
> -- Dave

An excellent point! 

There are two functions that might seem appropriate for a timestamp:

current_kernel_time()
do_gettimeofday()


	The former has jiffies resolution and hence may not be sufficient to
distinguish ordering between two events in the same process. The second
is interpolated wall time and may not necessarily monotonic. 

	Most other methods, such as get_cycles(), reading the tsc, etc. are not
SMP-safe or do not provide the needed resolution. Hence the patch gets
the time in nanoseconds and converts to a monotonicly increasing value.

	I'll split out a similarly-named function (get_timestamp exists in
ibmasm.c) to kernel/time.c and repost as two patches.

Thanks,
	-Matt Helsley