From 81ca27c9b07ba29e06ac22482d9a4994e5bdf0cb Mon Sep 17 00:00:00 2001 From: patacongo Date: Sun, 11 Sep 2011 17:48:52 +0000 Subject: [PATCH] Fix some timer race conditions git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@3949 42af7a65-404d-4744-a932-0658087f49c3 --- ChangeLog | 7 ++-- sched/clock_gettime.c | 75 ++++++++++++++++++++++++++++++++++++++---- sched/clock_settime.c | 4 +-- sched/clock_systimer.c | 24 ++++++++++++-- 4 files changed, 97 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index 57eaf7a9964..fcf4df8e470 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2018,7 +2018,7 @@ * arch/arm/src/kinetis/kinetis_sdhc.c: SDHC driver for Kinetis parts. Initially check-in is just a crude port of the STM32 SDIO driver. - Much more is needed. + Much more work is needed. * graphics/, include/nuttx/nx: Add new NX interfaces for drawing circles -- both circular outlines and filled circles. * graphic/nxglib/nxglib_spitline.c: Add a "fudge factor" that eliminates @@ -2041,7 +2041,10 @@ * drivers/mtd/flash_eraseall.c: Add a callable function that accepts the path to a block driver and then erases the underlying FLASH memory (assuming that the block driver is an MTD driver wrapped in the FTL - layer). + layer). Hmmm... this is probably not the best long term solution; + flash_eraseall() should be a user-callable function that operates + one driver interfaces; not an internal, OS function that operates + on directly on block drivers. * drivers/bch: Fixed some important bugs in the BCH driver (noted by Li Zhuoyi (Lzyy)). This would have effected any large reads or writes (larger than the hardware sector size). diff --git a/sched/clock_gettime.c b/sched/clock_gettime.c index 3fdfe9602d1..1dc48aeb7a7 100644 --- a/sched/clock_gettime.c +++ b/sched/clock_gettime.c @@ -45,6 +45,8 @@ #include #include +#include + #include "clock_internal.h" /************************************************************************ @@ -93,7 +95,14 @@ int clock_gettime(clockid_t clock_id, struct timespec *tp) uint32_t msecs; uint32_t secs; uint32_t nsecs; +#else + uint32_t system_utc; + uint32_t tickcount; #endif +#if defined(CONFIG_RTC) || defined(CONFIG_SYSTEM_UTC) + irqstate_t flags; +#endif + int ret = OK; sdbg("clock_id=%d\n", clock_id); @@ -147,14 +156,58 @@ int clock_gettime(clockid_t clock_id, struct timespec *tp) #ifdef CONFIG_RTC if (g_rtc_enabled) { - tp->tv_sec = up_rtc_gettime(); - tp->tv_nsec = (up_rtc_getclock() & (RTC_CLOCKS_PER_SEC-1) ) * (1000000000/RTC_CLOCKS_PER_SEC); + /* up_rtc_gettime() returns the time in seconds and up_rtc_getclock() + * will return the time int RTC clock ticks. Under the hood, these + * are probably based on the same running time. However, since we + * sample this time twice, we have to add the following strange logic + * to assure that the fractional second value does not rollover to + * a full second between sampling times. + */ + + clock_t rtc_frac; /* Current fractional seconds in RTC ticks */ + clock_t rtc_last; /* Previous fractional seconds in RTC ticks */ + time_t rtc_sec; /* Current seconds */ + + /* Interrupts are disabled here only to prevent interrupts and context + * switches from interfering with the consecutive time samples. I + * expect to go through this loop 1 time 99.9% of the time and then + * only twice on the remaining cornercases. + */ + + flags = irqsave(); + rtc_frac = up_rtc_getclock() & (RTC_CLOCKS_PER_SEC-1); + do + { + rtc_last = rtc_frac; + rtc_sec = up_rtc_gettime(); + rtc_frac = up_rtc_getclock() & (RTC_CLOCKS_PER_SEC-1); + } + while (rtc_frac < rtc_last); + irqrestore(flags); + + /* Okay.. the samples should be as close together in time as possible + * and we can be assured that no fractional second rollover occurred + * between the samples. + */ + + tp->tv_sec = rtc_sec; + tp->tv_nsec = rtc_frac * (1000000000/RTC_CLOCKS_PER_SEC); } else #endif { - tp->tv_sec = g_system_utc; - tp->tv_nsec = g_tickcount * (1000000000/TICK_PER_SEC); + /* Disable interrupts while g_system_utc and g_tickcount are sampled + * so that we can be assured that g_system_utc and g_tickcount are based + * at the same point in time. + */ + + flags = irqsave(); + system_utc = g_system_utc; + tickcount = g_tickcount; + irqrestore(flags); + + tp->tv_sec = system_utc; + tp->tv_nsec = tickcount * (1000000000/TICK_PER_SEC); } #endif @@ -169,8 +222,18 @@ int clock_gettime(clockid_t clock_id, struct timespec *tp) #ifdef CONFIG_RTC else if (clock_id == CLOCK_ACTIVETIME && g_rtc_enabled && tp) { - tp->tv_sec = g_system_utc; - tp->tv_nsec = g_tickcount * (1000000000/TICK_PER_SEC); + /* Disable interrupts while g_system_utc and g_tickcount are sampled + * so that we can be assured that g_system_utc and g_tickcount are based + * at the same point in time. + */ + + flags = irqsave(); + system_utc = g_system_utc; + tickcount = g_tickcount; + irqrestore(flags); + + tp->tv_sec = system_utc; + tp->tv_nsec = tickcount * (1000000000/TICK_PER_SEC); } #endif diff --git a/sched/clock_settime.c b/sched/clock_settime.c index 72fb8276e6e..77b6ebecc97 100644 --- a/sched/clock_settime.c +++ b/sched/clock_settime.c @@ -116,7 +116,7 @@ int clock_settime(clockid_t clock_id, const struct timespec *tp) #ifdef CONFIG_RTC if (g_rtc_enabled) { - up_rtc_settime( tp->tv_sec ); + up_rtc_settime(tp->tv_sec); } else #endif @@ -143,7 +143,7 @@ int clock_settime(clockid_t clock_id, const struct timespec *tp) else { sdbg("Returning ERROR\n"); - *get_errno_ptr() = EINVAL; + set_errno(EINVAL); ret = ERROR; } diff --git a/sched/clock_systimer.c b/sched/clock_systimer.c index 71577df76e0..cfa019fc754 100644 --- a/sched/clock_systimer.c +++ b/sched/clock_systimer.c @@ -45,6 +45,8 @@ #include #include +#include + #if !defined(clock_systimer) /* See nuttx/clock.h */ /**************************************************************************** @@ -77,6 +79,12 @@ uint32_t clock_systimer(void) { +#ifdef CONFIG_SYSTEM_UTC + irqstate_t flags; + uint32_t system_utc; + uint32_t tickcount; +#endif + /* Fetch the g_system_timer value from timer hardware, if available */ #ifdef CONFIG_RTC @@ -89,14 +97,24 @@ uint32_t clock_systimer(void) if (g_rtc_enabled) { -// return up_rtc_getclock(); - } + /* return up_rtc_getclock(); */ + } #endif #ifndef CONFIG_SYSTEM_UTC return g_system_timer; #else - return g_system_utc * TICK_PER_SEC + g_tickcount; + /* Disable interrupts while g_system_utc and g_tickcount are sampled + * so that we can be assured that g_system_utc and g_tickcount are based + * at the same point in time. + */ + + flags = irqsave(); + system_utc = g_system_utc; + tickcount = g_tickcount; + irqrestore(flags); + + return system_utc * TICK_PER_SEC + tickcount; #endif }