Re: [RFC][PATCH] new timeofday core subsystem (v.A0)

George Anzinger
2004-09-03T01:48:38Z

john stultz wrote:
> All,
> This patch implements the architecture independent portion of the time
> of day subsystem. Included is timeofday.c (which includes all the time
> of day management and accessor functions), ntp.c (which includes the ntp
> scaling code, leap second processing, and ntp kernel state machine
> code), interface definition .h files, the example jiffies timesource
> (lowest common denominator time source, mainly for use as example code)
> and minimal hooks into arch independent code.
>
> The patch does not function without minimal architecture specific hooks
> (i386 example to follow), and it can be applied to a tree without
> affecting the code.
>
> I look forward to your comments and feedback.
>
> thanks
> -john
~

> +static cycle_t jiffies_read(void)
> +{
> + cycle_t ret = get_jiffies_64();
> + return ret;
> +}
> +
> +static cycle_t jiffies_delta(cycle_t now, cycle_t then)
> +{
> + /* simple subtraction, no need to mask */
> + return now - then;
> +}

This should be inline...

> +
> +static nsec_t jiffies_cyc2ns(cycle_t cyc, cycle_t* remainder)
> +{
> +
> + cyc *= NSEC_PER_SEC/HZ;

Hm... This assumes that 1/HZ is what is needed here. Today this value is
999898. Not exactly reachable by NSEC_PER_SEC/HZ. Or did I miss something,
like the relationship of jiffie to 1/HZ and to real time.

> +
~

> +int ntp_leapsecond(struct timespec now)
> +{
> + /*
> + * Leap second processing. If in leap-insert state at
> + * the end of the day, the system clock is set back one
> + * second; if in leap-delete state, the system clock is
> + * set ahead one second. The microtime() routine or
> + * external clock driver will insure that reported time
> + * is always monotonic. The ugly divides should be
> + * replaced.

?? Is this really to be hidden? I rather though this was just the same as a
user driven clock setting. In any case, it is a slew of the wall clock and
should be presented to abs timer code so that the abs timers can be corrected.
This means a call to "clock_was_set()" AND an update of the mono_to_wall value.
> + */
> + static time_t leaptime = 0;
> +
> + switch (ntp_state) {
> + case TIME_OK:
> + if (ntp_status & STA_INS) {
> + ntp_state = TIME_INS;
> + /* calculate end of today (23:59:59)*/
> + leaptime = now.tv_sec + SEC_PER_DAY - (now.tv_sec % SEC_PER_DAY) - 1;
> + }
> + else if (ntp_status & STA_DEL) {
> + ntp_state = TIME_DEL;
> + /* calculate end of today (23:59:59)*/
> + leaptime = now.tv_sec + SEC_PER_DAY - (now.tv_sec % SEC_PER_DAY) - 1;
> + }
> + break;
> +
> + case TIME_INS:
> + /* Once we are at (or past) leaptime, insert the second */
> + if (now.tv_sec > leaptime) {
> + ntp_state = TIME_OOP;
> + printk(KERN_NOTICE "Clock: inserting leap second 23:59:60 UTC\n");
> +
> + return -1;
> + }
> + break;
> +
> + case TIME_DEL:
> + /* Once we are at (or past) leaptime, delete the second */
> + if (now.tv_sec >= leaptime) {
> + ntp_state = TIME_WAIT;
> + printk(KERN_NOTICE "Clock: deleting leap second 23:59:59 UTC\n");
> +
> + return 1;
> + }
~

> +/* do_gettimeofday():
> + * Returns the time of day
> + */
> +void do_gettimeofday(struct timeval *tv)
> +{
> + nsec_t wall, sys;
> + unsigned long seq;
> +
> + /* atomically read wall and sys time */
> + do {
> + seq = read_seqbegin(&system_time_lock);
> +
> + wall = wall_time_offset;
> + sys = __monotonic_clock();
> +
> + } while (read_seqretry(&system_time_lock, seq));
> +
> + /* add them and convert to timeval */
> + *tv = ns2timeval(wall+sys);
> +}
I am not sure you don't want to seperate the locking from the clock read. This
so one lock can be put around larger bits of code. For example, in
posix-times.c we need to get all three clocks under the same lock (that being
monotonic, wall_time_offset, and jiffies (and possibly as sub jiffie value)).

Something like:
void get_tod_parts(nsec_t *wall, nsec_t *mon)
{
*wall = wall_time_offset;
*mon = __monotonic_clock();
}

This could then be used in a larger function with out the double locking.

> +
> +
> +/* do_settimeofday():
> + * Sets the time of day
> + */
> +int do_settimeofday(struct timespec *tv)
> +{
> + /* convert timespec to ns */
> + nsec_t newtime = timespec2ns(tv);
> +
> + /* atomically adjust wall_time_offset to the desired value */
> + write_seqlock_irq(&system_time_lock);
> +
> + wall_time_offset = newtime - __monotonic_clock();
> +
> + /* clear NTP settings */
> + ntp_clear();
> +
> + write_sequnlock_irq(&system_time_lock);

Also need a clock_was_set() call here.
> +
~
--
George Anzinger george@mvista.com
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml