Messages in this thread Patch in this message | | | Date | Fri, 17 Sep 1999 16:41:17 +0200 (CEST) | From | Andrea Arcangeli <> | Subject | [patch] stime/settimeofday/adjtimex SMP races (2.2.12 and 2.3.18ac5) |
| |
I reviewed the time-related stuff in 2.2.12 and I found potential SMP races in the time-related syscalls. Basically the only problem is that the time-syscalls are not protecting against each other. They are only protecting themself against the timer irq. And this may lead to SMP races.
Actually I don't know if these SMP races may be the source of the RTC screwedup with ntpd enabled (also considering that ntpd seems single threaded...). Anyway here it is the fix for the races I spotted so far. It's against 2.2.12 but it applyes cleanly to all 2.2.x 2.3.x kernels out there as there aren't been NTP/time changes since 2.2.0. You may want to try to reproduce the RTC SMP race with this fix applyed (I can't reproduce in any way here, but I am not sure I configured xntp hard enough to trigger the race).
--- 2.2.12/kernel/time.c Wed Mar 24 01:53:19 1999 +++ 2.2.12-ntp-SMP-races/kernel/time.c Fri Sep 17 16:29:11 1999 @@ -22,6 +22,8 @@ * "A Kernel Model for Precision Timekeeping" by Dave Mills * Allow time_constant larger than MAXTC(6) for NTP v4 (MAXTC == 10) * (Even though the technical memorandum forbids it) + * 1999-09-17 Andrea Arcangeli <andrea@suse.de> + * Fixed adjtimex/settimeofday/stime SMP races. */ #include <linux/mm.h> @@ -76,6 +78,13 @@ return i; } +/* The xtime_lock is not only serializing the xtime read/writes but it's also + serializing all accesses to the global NTP variables. + NOTE NOTE: We really need a spinlock here as the global irq locking + only protect us against the timer irq and not against other time-related + syscall running under us. */ +extern rwlock_t xtime_lock; + /* * sys_stime() can be implemented in user-level using * sys_settimeofday(). Is this for backwards compatibility? If so, @@ -91,14 +100,14 @@ return -EPERM; if (get_user(value, tptr)) return -EFAULT; - cli(); + write_lock_irq(&xtime_lock); xtime.tv_sec = value; xtime.tv_usec = 0; time_adjust = 0; /* stop active adjtime() */ time_status |= STA_UNSYNC; time_maxerror = NTP_PHASE_LIMIT; time_esterror = NTP_PHASE_LIMIT; - sti(); + write_unlock_irq(&xtime_lock); return 0; } @@ -137,9 +146,9 @@ */ inline static void warp_clock(void) { - cli(); + write_lock_irq(&xtime_lock); xtime.tv_sec += sys_tz.tz_minuteswest * 60; - sti(); + write_unlock_irq(&xtime_lock); } /* @@ -220,7 +229,7 @@ int do_adjtimex(struct timex *txc) { long ltemp, mtemp, save_adjust; - int result = time_state; /* mostly `TIME_OK' */ + int result; /* In order to modify anything, you gotta be super-user! */ if (txc->modes && !capable(CAP_SYS_TIME)) @@ -238,7 +247,8 @@ if (txc->tick < 900000/HZ || txc->tick > 1100000/HZ) return -EINVAL; - cli(); /* SMP: global cli() is enough protection. */ + write_lock(&xtime_lock); + result = time_state; /* mostly `TIME_OK' */ /* Save for later - semantics of adjtime is to return old value */ save_adjust = time_adjust; @@ -383,7 +393,6 @@ txc->constant = time_constant; txc->precision = time_precision; txc->tolerance = time_tolerance; - do_gettimeofday(&txc->time); txc->tick = tick; txc->ppsfreq = pps_freq; txc->jitter = pps_jitter >> PPS_AVG; @@ -393,8 +402,8 @@ txc->calcnt = pps_calcnt; txc->errcnt = pps_errcnt; txc->stbcnt = pps_stbcnt; - - sti(); + write_unlock_irq(&xtime_lock); + do_gettimeofday(&txc->time); return(result); } Andrea
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |