subject:"Periodically sync RTC"

Re: Periodically sync RTC

2016-09-03 Thread Mark Kettenis

> Date: Fri, 2 Sep 2016 19:45:39 +0200
> From: Christian Weisgerber 
> 
> I would like to sync the system time periodically back to the RTC.
> 
> Currently we update the RTC
> (1) when the time is set with clock_settime() or settimeofday(), which
> never happens for a typical ntpd setup;
> (2) before suspend;
> (3) when the system is properly shut down.
> 
> This means if a machine has been running for a few months and it
> loses power, it may come back up with the time way off, leading to
> all sorts of problems.
> 
> Patch below, originally inspired by FreeBSD, and incorporating various
> suggestions from kettenis@ when I first proposed this:
> 
> * A timeout schedules a task that actually calls resettodr().
> * The RTC is synced back every 30 minutes.
> * The timeout and task are removed before shutdown and suspend;
>   the timeout is started up again on resume.
> * Suspend/resume paths: acpi, i386 apm, loongson "apm".
> 
> Comments?

looks good to me

> Index: arch/i386/i386/apm.c
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/apm.c,v
> retrieving revision 1.114
> diff -u -p -r1.114 apm.c
> --- arch/i386/i386/apm.c  28 Sep 2015 18:36:36 -  1.114
> +++ arch/i386/i386/apm.c  2 Sep 2016 17:28:23 -
> @@ -248,6 +248,7 @@ apm_suspend(int state)
>  #if NWSDISPLAY > 0
>   wsdisplay_suspend();
>  #endif /* NWSDISPLAY > 0 */
> + stop_periodic_resettodr();
>   config_suspend_all(DVACT_QUIESCE);
>   bufq_quiesce();
>  
> @@ -284,6 +285,7 @@ apm_suspend(int state)
>   bufq_restart();
>  
>   config_suspend_all(DVACT_WAKEUP);
> + start_periodic_resettodr();
>  
>  #if NWSDISPLAY > 0
>   wsdisplay_resume();
> Index: arch/loongson/dev/apm.c
> ===
> RCS file: /cvs/src/sys/arch/loongson/dev/apm.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 apm.c
> --- arch/loongson/dev/apm.c   28 Sep 2015 18:36:36 -  1.29
> +++ arch/loongson/dev/apm.c   2 Sep 2016 17:28:23 -
> @@ -373,6 +373,7 @@ apm_suspend(int state)
>   wsdisplay_suspend();
>  #endif
>  
> + stop_periodic_resettodr();
>   resettodr();
>  
>   config_suspend_all(DVACT_QUIESCE);
> @@ -422,6 +423,8 @@ apm_suspend(int state)
>   bufq_restart();
>  
>   config_suspend_all(DVACT_WAKEUP);
> +
> + start_periodic_resettodr();
>  
>  #if NWSDISPLAY > 0
>   wsdisplay_resume();
> Index: dev/acpi/acpi.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
> retrieving revision 1.314
> diff -u -p -r1.314 acpi.c
> --- dev/acpi/acpi.c   31 Aug 2016 15:40:42 -  1.314
> +++ dev/acpi/acpi.c   2 Sep 2016 17:28:23 -
> @@ -2383,6 +2383,8 @@ acpi_sleep_state(struct acpi_softc *sc, 
>   rw_enter_write(>sc_lck);
>  #endif /* NWSDISPLAY > 0 */
>  
> + stop_periodic_resettodr();
> +
>  #ifdef HIBERNATE
>   if (state == ACPI_STATE_S4) {
>   uvmpd_hibernate();
> @@ -2482,6 +2484,8 @@ fail_alloc:
>   hibernate_resume_bufcache();
>   }
>  #endif /* HIBERNATE */
> +
> + start_periodic_resettodr();
>  
>  #if NWSDISPLAY > 0
>   rw_exit_write(>sc_lck);
> Index: kern/init_main.c
> ===
> RCS file: /cvs/src/sys/kern/init_main.c,v
> retrieving revision 1.254
> diff -u -p -r1.254 init_main.c
> --- kern/init_main.c  2 Sep 2016 12:17:33 -   1.254
> +++ kern/init_main.c  2 Sep 2016 17:28:23 -
> @@ -551,6 +551,8 @@ main(void *framep)
>   pool_gc_pages(NULL);
>  #endif
>  
> + start_periodic_resettodr();
> +
>  /*
>   * proc0: nothing to do, back to sleep
>   */
> Index: kern/kern_time.c
> ===
> RCS file: /cvs/src/sys/kern/kern_time.c,v
> retrieving revision 1.97
> diff -u -p -r1.97 kern_time.c
> --- kern/kern_time.c  28 Apr 2016 20:11:20 -  1.97
> +++ kern/kern_time.c  2 Sep 2016 17:28:24 -
> @@ -41,6 +41,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  
>  #include 
> @@ -785,3 +787,37 @@ ppsratecheck(struct timeval *lasttime, i
>   return (rv);
>  }
>  
> +
> +#define RESETTODR_PERIOD 1800
> +
> +void periodic_resettodr(void *);
> +void perform_resettodr(void *);
> +
> +struct timeout resettodr_to = TIMEOUT_INITIALIZER(periodic_resettodr, NULL);
> +struct task resettodr_task = TASK_INITIALIZER(perform_resettodr, NULL);
> +
> +void
> +periodic_resettodr(void *arg __unused)
> +{
> + task_add(systq, _task);
> +}
> +
> +void
> +perform_resettodr(void *arg __unused)
> +{
> + resettodr();
> + timeout_add_sec(_to, RESETTODR_PERIOD);
> +}
> +
> +void
> +start_periodic_resettodr(void)
> +{
> + timeout_add_sec(_to, RESETTODR_PERIOD);
> +}
> +
> +void
> +stop_periodic_resettodr(void)
> +{
> +

Periodically sync RTC

2016-09-02 Thread Christian Weisgerber

I would like to sync the system time periodically back to the RTC.

Currently we update the RTC
(1) when the time is set with clock_settime() or settimeofday(), which
never happens for a typical ntpd setup;
(2) before suspend;
(3) when the system is properly shut down.

This means if a machine has been running for a few months and it
loses power, it may come back up with the time way off, leading to
all sorts of problems.

Patch below, originally inspired by FreeBSD, and incorporating various
suggestions from kettenis@ when I first proposed this:

* A timeout schedules a task that actually calls resettodr().
* The RTC is synced back every 30 minutes.
* The timeout and task are removed before shutdown and suspend;
  the timeout is started up again on resume.
* Suspend/resume paths: acpi, i386 apm, loongson "apm".

Comments?

Index: arch/i386/i386/apm.c
===
RCS file: /cvs/src/sys/arch/i386/i386/apm.c,v
retrieving revision 1.114
diff -u -p -r1.114 apm.c
--- arch/i386/i386/apm.c28 Sep 2015 18:36:36 -  1.114
+++ arch/i386/i386/apm.c2 Sep 2016 17:28:23 -
@@ -248,6 +248,7 @@ apm_suspend(int state)
 #if NWSDISPLAY > 0
wsdisplay_suspend();
 #endif /* NWSDISPLAY > 0 */
+   stop_periodic_resettodr();
config_suspend_all(DVACT_QUIESCE);
bufq_quiesce();
 
@@ -284,6 +285,7 @@ apm_suspend(int state)
bufq_restart();
 
config_suspend_all(DVACT_WAKEUP);
+   start_periodic_resettodr();
 
 #if NWSDISPLAY > 0
wsdisplay_resume();
Index: arch/loongson/dev/apm.c
===
RCS file: /cvs/src/sys/arch/loongson/dev/apm.c,v
retrieving revision 1.29
diff -u -p -r1.29 apm.c
--- arch/loongson/dev/apm.c 28 Sep 2015 18:36:36 -  1.29
+++ arch/loongson/dev/apm.c 2 Sep 2016 17:28:23 -
@@ -373,6 +373,7 @@ apm_suspend(int state)
wsdisplay_suspend();
 #endif
 
+   stop_periodic_resettodr();
resettodr();
 
config_suspend_all(DVACT_QUIESCE);
@@ -422,6 +423,8 @@ apm_suspend(int state)
bufq_restart();
 
config_suspend_all(DVACT_WAKEUP);
+
+   start_periodic_resettodr();
 
 #if NWSDISPLAY > 0
wsdisplay_resume();
Index: dev/acpi/acpi.c
===
RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.314
diff -u -p -r1.314 acpi.c
--- dev/acpi/acpi.c 31 Aug 2016 15:40:42 -  1.314
+++ dev/acpi/acpi.c 2 Sep 2016 17:28:23 -
@@ -2383,6 +2383,8 @@ acpi_sleep_state(struct acpi_softc *sc, 
rw_enter_write(>sc_lck);
 #endif /* NWSDISPLAY > 0 */
 
+   stop_periodic_resettodr();
+
 #ifdef HIBERNATE
if (state == ACPI_STATE_S4) {
uvmpd_hibernate();
@@ -2482,6 +2484,8 @@ fail_alloc:
hibernate_resume_bufcache();
}
 #endif /* HIBERNATE */
+
+   start_periodic_resettodr();
 
 #if NWSDISPLAY > 0
rw_exit_write(>sc_lck);
Index: kern/init_main.c
===
RCS file: /cvs/src/sys/kern/init_main.c,v
retrieving revision 1.254
diff -u -p -r1.254 init_main.c
--- kern/init_main.c2 Sep 2016 12:17:33 -   1.254
+++ kern/init_main.c2 Sep 2016 17:28:23 -
@@ -551,6 +551,8 @@ main(void *framep)
pool_gc_pages(NULL);
 #endif
 
+   start_periodic_resettodr();
+
 /*
  * proc0: nothing to do, back to sleep
  */
Index: kern/kern_time.c
===
RCS file: /cvs/src/sys/kern/kern_time.c,v
retrieving revision 1.97
diff -u -p -r1.97 kern_time.c
--- kern/kern_time.c28 Apr 2016 20:11:20 -  1.97
+++ kern/kern_time.c2 Sep 2016 17:28:24 -
@@ -41,6 +41,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 #include 
@@ -785,3 +787,37 @@ ppsratecheck(struct timeval *lasttime, i
return (rv);
 }
 
+
+#define RESETTODR_PERIOD   1800
+
+void periodic_resettodr(void *);
+void perform_resettodr(void *);
+
+struct timeout resettodr_to = TIMEOUT_INITIALIZER(periodic_resettodr, NULL);
+struct task resettodr_task = TASK_INITIALIZER(perform_resettodr, NULL);
+
+void
+periodic_resettodr(void *arg __unused)
+{
+   task_add(systq, _task);
+}
+
+void
+perform_resettodr(void *arg __unused)
+{
+   resettodr();
+   timeout_add_sec(_to, RESETTODR_PERIOD);
+}
+
+void
+start_periodic_resettodr(void)
+{
+   timeout_add_sec(_to, RESETTODR_PERIOD);
+}
+
+void
+stop_periodic_resettodr(void)
+{
+   timeout_del(_to);
+   task_del(systq, _task);
+}
Index: kern/kern_xxx.c
===
RCS file: /cvs/src/sys/kern/kern_xxx.c,v
retrieving revision 1.29
diff -u -p -r1.29 kern_xxx.c
--- kern/kern_xxx.c 5 Dec 2015 10:11:53 -   1.29
+++ kern/kern_xxx.c 2 Sep 2016 17:28:24 -
@@ -65,6

Re: Periodically sync RTC

2015-11-09 Thread Mark Kettenis

> Date: Sun, 8 Nov 2015 22:57:24 +0100
> From: Christian Weisgerber 
> 
> I would like to sync the system time periodically back to the RTC.
> 
> Currently we update the RTC
> (1) when the time is set with clock_settime() or settimeofday(), which
> never happens for a typical ntpd setup,
> (2) before suspend,
> (3) when the system is properly shut down.
> 
> This means if a machine has been running for a few months and it
> loses power, it may come back up with the time, say, 200 seconds
> off because of RTC drift, and then your SixXS tunnel won't come up
> and you can no longer reach your home network on the last day of
> u2k15.  For example.
> 
> FreeBSD uses a period of 30 minutes.  I have no idea what a good
> number would be so I went with that.  (Maybe a prime number of
> seconds?)  FreeBSD also has a sysctl knob to change the period,
> which is silly.
> 
> The patch below, inspired by FreeBSD, "seems to work for me", but
> I don't really know what I'm doing and if it's okay to just use a
> timeout(9) like that.

No.  Some of the implementattions of resettodr() may sleep.

> Do I need a task?

Probably.

> Any locking?

Kernel lock should be good enough.

> I'm also uncertain where to put the hook to kick off the initial
> timeout_add().

Most logical place would be inittodr() but that's MD code.  Unifying
that code (and therefore unifying the checks for silly times and
falling back to the filesystem time) would be good, but probably too
much to ask from you.  It's a serious can of worms!

You should also delete the timeout and task before rebooting.  And
across suspend/resume.


> Index: kern/init_main.c
> ===
> RCS file: /cvs/src/sys/kern/init_main.c,v
> retrieving revision 1.246
> diff -u -p -r1.246 init_main.c
> --- kern/init_main.c  8 Nov 2015 20:45:57 -   1.246
> +++ kern/init_main.c  8 Nov 2015 21:06:47 -
> @@ -118,6 +118,8 @@ structsigacts sigacts0;
>  struct   process *initprocess;
>  struct   proc *reaperproc;
>  
> +void start_periodic_resettodr(void);
> +
>  extern   struct user *proc0paddr;
>  
>  struct   vnode *rootvp, *swapdev_vp;
> @@ -550,6 +552,8 @@ main(void *framep)
>  #if !(defined(__m88k__) && defined(MULTIPROCESSOR))  /* XXX */
>   pool_gc_pages(NULL);
>  #endif
> +
> + start_periodic_resettodr();
>  
>  /*
>   * proc0: nothing to do, back to sleep
> Index: kern/kern_time.c
> ===
> RCS file: /cvs/src/sys/kern/kern_time.c,v
> retrieving revision 1.95
> diff -u -p -r1.95 kern_time.c
> --- kern/kern_time.c  1 Nov 2015 19:03:33 -   1.95
> +++ kern/kern_time.c  8 Nov 2015 16:46:46 -
> @@ -41,6 +41,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include 
> @@ -792,3 +793,21 @@ ppsratecheck(struct timeval *lasttime, i
>   return (rv);
>  }
>  
> +
> +#define RESETTODR_PERIOD 1800
> +
> +void periodic_resettodr(void *);
> +struct timeout resettodr_to = TIMEOUT_INITIALIZER(periodic_resettodr, NULL);
> +
> +void
> +periodic_resettodr(void *arg __unused)
> +{
> + resettodr();
> + timeout_add_sec(_to, RESETTODR_PERIOD);
> +}
> +
> +void
> +start_periodic_resettodr(void)
> +{
> + timeout_add_sec(_to, RESETTODR_PERIOD);
> +}
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 
>

Periodically sync RTC

2015-11-08 Thread Christian Weisgerber

I would like to sync the system time periodically back to the RTC.

Currently we update the RTC
(1) when the time is set with clock_settime() or settimeofday(), which
never happens for a typical ntpd setup,
(2) before suspend,
(3) when the system is properly shut down.

This means if a machine has been running for a few months and it
loses power, it may come back up with the time, say, 200 seconds
off because of RTC drift, and then your SixXS tunnel won't come up
and you can no longer reach your home network on the last day of
u2k15.  For example.

FreeBSD uses a period of 30 minutes.  I have no idea what a good
number would be so I went with that.  (Maybe a prime number of
seconds?)  FreeBSD also has a sysctl knob to change the period,
which is silly.

The patch below, inspired by FreeBSD, "seems to work for me", but
I don't really know what I'm doing and if it's okay to just use a
timeout(9) like that.  Do I need a task?  Any locking?  I'm also
uncertain where to put the hook to kick off the initial timeout_add().


Index: kern/init_main.c
===
RCS file: /cvs/src/sys/kern/init_main.c,v
retrieving revision 1.246
diff -u -p -r1.246 init_main.c
--- kern/init_main.c8 Nov 2015 20:45:57 -   1.246
+++ kern/init_main.c8 Nov 2015 21:06:47 -
@@ -118,6 +118,8 @@ struct  sigacts sigacts0;
 struct process *initprocess;
 struct proc *reaperproc;
 
+void   start_periodic_resettodr(void);
+
 extern struct user *proc0paddr;
 
 struct vnode *rootvp, *swapdev_vp;
@@ -550,6 +552,8 @@ main(void *framep)
 #if !(defined(__m88k__) && defined(MULTIPROCESSOR))/* XXX */
pool_gc_pages(NULL);
 #endif
+
+   start_periodic_resettodr();
 
 /*
  * proc0: nothing to do, back to sleep
Index: kern/kern_time.c
===
RCS file: /cvs/src/sys/kern/kern_time.c,v
retrieving revision 1.95
diff -u -p -r1.95 kern_time.c
--- kern/kern_time.c1 Nov 2015 19:03:33 -   1.95
+++ kern/kern_time.c8 Nov 2015 16:46:46 -
@@ -41,6 +41,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -792,3 +793,21 @@ ppsratecheck(struct timeval *lasttime, i
return (rv);
 }
 
+
+#define RESETTODR_PERIOD   1800
+
+void periodic_resettodr(void *);
+struct timeout resettodr_to = TIMEOUT_INITIALIZER(periodic_resettodr, NULL);
+
+void
+periodic_resettodr(void *arg __unused)
+{
+   resettodr();
+   timeout_add_sec(_to, RESETTODR_PERIOD);
+}
+
+void
+start_periodic_resettodr(void)
+{
+   timeout_add_sec(_to, RESETTODR_PERIOD);
+}
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: Periodically sync RTC

2015-11-08 Thread Ted Unangst

Christian Weisgerber wrote:
> I would like to sync the system time periodically back to the RTC.
> 
> Currently we update the RTC
> (1) when the time is set with clock_settime() or settimeofday(), which
> never happens for a typical ntpd setup,
> (2) before suspend,
> (3) when the system is properly shut down.
> 
> This means if a machine has been running for a few months and it
> loses power, it may come back up with the time, say, 200 seconds
> off because of RTC drift, and then your SixXS tunnel won't come up
> and you can no longer reach your home network on the last day of
> u2k15.  For example.

I like this, and would also add that the problem is even worse for those
systems too cheap to have an RTC. Saving the filesystem time periodically
would also be a wonder.

Re: Periodically sync RTC

2015-11-08 Thread David Gwynne

OK

I don't care if there is better machinery or numbers, this is far better
than the current situation. We lost power in a DC a few months back and we
had machines come up with clocks off by values between 30 seconds and 45
minutes.
On 9 Nov 2015 8:04 am, "Christian Weisgerber"  wrote:

> I would like to sync the system time periodically back to the RTC.
>
> Currently we update the RTC
> (1) when the time is set with clock_settime() or settimeofday(), which
> never happens for a typical ntpd setup,
> (2) before suspend,
> (3) when the system is properly shut down.
>
> This means if a machine has been running for a few months and it
> loses power, it may come back up with the time, say, 200 seconds
> off because of RTC drift, and then your SixXS tunnel won't come up
> and you can no longer reach your home network on the last day of
> u2k15.  For example.
>
> FreeBSD uses a period of 30 minutes.  I have no idea what a good
> number would be so I went with that.  (Maybe a prime number of
> seconds?)  FreeBSD also has a sysctl knob to change the period,
> which is silly.
>
> The patch below, inspired by FreeBSD, "seems to work for me", but
> I don't really know what I'm doing and if it's okay to just use a
> timeout(9) like that.  Do I need a task?  Any locking?  I'm also
> uncertain where to put the hook to kick off the initial timeout_add().
>
>
> Index: kern/init_main.c
> ===
> RCS file: /cvs/src/sys/kern/init_main.c,v
> retrieving revision 1.246
> diff -u -p -r1.246 init_main.c
> --- kern/init_main.c8 Nov 2015 20:45:57 -   1.246
> +++ kern/init_main.c8 Nov 2015 21:06:47 -
> @@ -118,6 +118,8 @@ struct  sigacts sigacts0;
>  struct process *initprocess;
>  struct proc *reaperproc;
>
> +void   start_periodic_resettodr(void);
> +
>  extern struct user *proc0paddr;
>
>  struct vnode *rootvp, *swapdev_vp;
> @@ -550,6 +552,8 @@ main(void *framep)
>  #if !(defined(__m88k__) && defined(MULTIPROCESSOR))/* XXX */
> pool_gc_pages(NULL);
>  #endif
> +
> +   start_periodic_resettodr();
>
>  /*
>   * proc0: nothing to do, back to sleep
> Index: kern/kern_time.c
> ===
> RCS file: /cvs/src/sys/kern/kern_time.c,v
> retrieving revision 1.95
> diff -u -p -r1.95 kern_time.c
> --- kern/kern_time.c1 Nov 2015 19:03:33 -   1.95
> +++ kern/kern_time.c8 Nov 2015 16:46:46 -
> @@ -41,6 +41,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  #include 
> @@ -792,3 +793,21 @@ ppsratecheck(struct timeval *lasttime, i
> return (rv);
>  }
>
> +
> +#define RESETTODR_PERIOD   1800
> +
> +void periodic_resettodr(void *);
> +struct timeout resettodr_to = TIMEOUT_INITIALIZER(periodic_resettodr,
> NULL);
> +
> +void
> +periodic_resettodr(void *arg __unused)
> +{
> +   resettodr();
> +   timeout_add_sec(_to, RESETTODR_PERIOD);
> +}
> +
> +void
> +start_periodic_resettodr(void)
> +{
> +   timeout_add_sec(_to, RESETTODR_PERIOD);
> +}
> --
> Christian "naddy" Weisgerber  na...@mips.inka.de
>
>

Re: Periodically sync RTC

Periodically sync RTC

Re: Periodically sync RTC

Periodically sync RTC

Re: Periodically sync RTC

Re: Periodically sync RTC

6 matches

Site Navigation

Mail list logo

Footer information