[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] Re: [PATCH] [PVOPS] dom0 sync xen wallclock



On 02/09/2010 10:12 AM, Stefano Stabellini wrote:
Hi Jeremy,
this patch removes clock_was_set and adds an atomic notification chain
instead so that not only hrtimers but other parts of the kernel can be
notified of a time change as well.
In fact xen/time.c needs to be notified so that can update xen
wallclock time to keep it in sync; this is necessary otherwise other
PV guests will get a wrong wallclock time at boot.
Please let me know if my approach is suitable for upstream, I am willing
to do any change needed and to send the patch upstream myself if you
give me some directions.

I'm not sure this is the right thing to do. We have a set_wallclock pvop, which Xen currently implements as a no-op, but it should do the appropriate hypercall to set Xen's time if privileged enough.

Conceptually the Xen persistent time is the same as the platform CMOS clock, so I don't think we should update it any differently. Your patch may make sense, but it should also address the native case. At the moment it happens via sync_cmos_clock(), which is called periodically (I think) independently of whether the clock has actually been changed.

There is one big difference between the Xen clock and the CMOS clock, which is that the Xen clock is being concurrently accessed by other domains. If it is being updated periodically, then there will be discontinuities in time which may affect other domains. But since there's no time-warp ABI to Xen, I don't think this can really be avoided; anyone reading periodically the Xen clock needs to be able to deal with any discontinuities. pvops kernels only inspect it at boot time, and so won't see any subsequent time adjustments anyway.

Thanks,
    J

Thanks,

Stefano

Signed-off-by: Stefano Stabellini<stefano.stabellini@xxxxxxxxxxxxx>

---

diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index 0b56fd4..a0532ec 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -13,6 +13,7 @@
  #include<linux/clockchips.h>
  #include<linux/kernel_stat.h>
  #include<linux/math64.h>
+#include<linux/notifier.h>

  #include<asm/pvclock.h>
  #include<asm/xen/hypervisor.h>
@@ -526,6 +527,43 @@ static int __init xen_setup_vsyscall_timeinfo(int cpu)
  }
  #endif        /* CONFIG_PARAVIRT_CLOCK_VSYSCALL */

+static void sync_xen_wallclock(unsigned long dummy);
+static DEFINE_TIMER(sync_xen_wallclock_timer, sync_xen_wallclock, 0, 0);
+static void sync_xen_wallclock(unsigned long dummy)
+{
+       struct xen_platform_op op;
+       struct timespec ts;
+
+       write_seqlock_irq(&xtime_lock);
+
+       set_normalized_timespec(&ts, xtime.tv_sec, xtime.tv_nsec);
+
+       op.cmd = XENPF_settime;
+       op.u.settime.secs        = ts.tv_sec;
+       op.u.settime.nsecs       = ts.tv_nsec;
+       op.u.settime.system_time = xen_clocksource_read();
+       WARN_ON(HYPERVISOR_dom0_op(&op));
+
+       write_sequnlock_irq(&xtime_lock);
+
+       /* Once per minute. */
+       mod_timer(&sync_xen_wallclock_timer, jiffies + 60*HZ);
+}
+
+int xen_update_persistent_clock(struct notifier_block *this,
+               unsigned long event, void *ptr)
+{
+       if (!xen_initial_domain())
+               return -1;
+       mod_timer(&sync_xen_wallclock_timer, jiffies + 1);
+       return 0;
+}
+
+static struct notifier_block xen_clock_was_set = {
+       .notifier_call  = xen_update_persistent_clock,
+};
+
+
  __init void xen_time_init(void)
  {
        int cpu = smp_processor_id();
@@ -549,6 +587,8 @@ __init void xen_time_init(void)
        xen_setup_runstate_info(cpu);
        xen_setup_timer(cpu);
        xen_setup_cpu_clockevents();
+       if (xen_initial_domain())
+               
atomic_notifier_chain_register(&clockset_notifier_list,&xen_clock_was_set);

  #ifdef CONFIG_PARAVIRT_CLOCK_VSYSCALL
        if (xen_setup_vsyscall_timeinfo(cpu) == 0)
diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index 5d42d55..422e8ef 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -130,7 +130,7 @@ out_resume:
        dpm_resume_end(PMSG_RESUME);

        /* Make sure timer events get retriggered on all CPUs */
-       clock_was_set();
+       atomic_notifier_call_chain(&clockset_notifier_list, 0, NULL);

  out_thaw:
  #ifdef CONFIG_PREEMPT
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 4759917..c46a4e5 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -247,7 +247,6 @@ static inline ktime_t hrtimer_expires_remaining(const 
struct hrtimer *timer)
  #ifdef CONFIG_HIGH_RES_TIMERS
  struct clock_event_device;

-extern void clock_was_set(void);
  extern void hres_timers_resume(void);
  extern void hrtimer_interrupt(struct clock_event_device *dev);

@@ -282,12 +281,6 @@ extern void hrtimer_peek_ahead_timers(void);
  # define MONOTONIC_RES_NSEC   LOW_RES_NSEC
  # define KTIME_MONOTONIC_RES  KTIME_LOW_RES

-/*
- * clock_was_set() is a NOP for non- high-resolution systems. The
- * time-sorted order guarantees that a timer does not expire early and
- * is expired in the next softirq when the clock was advanced.
- */
-static inline void clock_was_set(void) { }
  static inline void hrtimer_peek_ahead_timers(void) { }

  static inline void hres_timers_resume(void) { }
diff --git a/include/linux/time.h b/include/linux/time.h
index ea16c1a..5bb7e4a 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -131,6 +131,7 @@ static inline u32 arch_gettimeoffset(void) { return 0; }
  extern void do_gettimeofday(struct timeval *tv);
  extern int do_settimeofday(struct timespec *tv);
  extern int do_sys_settimeofday(struct timespec *tv, struct timezone *tz);
+extern struct atomic_notifier_head clockset_notifier_list;
  #define do_posix_clock_monotonic_gettime(ts) ktime_get_ts(ts)
  extern long do_utimes(int dfd, char __user *filename, struct timespec *times, 
int flags);
  struct itimerval;
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 49da79a..c170b65 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -665,7 +665,8 @@ static void retrigger_next_event(void *arg)
   * resolution timer interrupts. On UP we just disable interrupts and
   * call the high resolution interrupt code.
   */
-void clock_was_set(void)
+static int clock_was_set(struct notifier_block *this, unsigned long event,
+               void *ptr)
  {
        /* Retrigger the CPU local events everywhere */
        on_each_cpu(retrigger_next_event, NULL, 1);
@@ -772,7 +773,11 @@ static inline int hrtimer_enqueue_reprogram(struct hrtimer 
*timer,
  }
  static inline void hrtimer_init_hres(struct hrtimer_cpu_base *base) { }
  static inline void hrtimer_init_timer_hres(struct hrtimer *timer) { }
-
+static inline int clock_was_set(struct notifier_block *this, unsigned long 
event,
+               void *ptr)
+{
+       return 0;
+}
  #endif /* CONFIG_HIGH_RES_TIMERS */

  #ifdef CONFIG_TIMER_STATS
@@ -1720,11 +1725,16 @@ static struct notifier_block __cpuinitdata hrtimers_nb 
= {
        .notifier_call = hrtimer_cpu_notify,
  };

+static struct notifier_block hrt_clock_was_set = {
+       .notifier_call  = clock_was_set,
+};
+
  void __init hrtimers_init(void)
  {
        hrtimer_cpu_notify(&hrtimers_nb, (unsigned long)CPU_UP_PREPARE,
                          (void *)(long)smp_processor_id());
        register_cpu_notifier(&hrtimers_nb);
+       
atomic_notifier_chain_register(&clockset_notifier_list,&hrt_clock_was_set);
  #ifdef CONFIG_HIGH_RES_TIMERS
        open_softirq(HRTIMER_SOFTIRQ, run_hrtimer_softirq);
  #endif
diff --git a/kernel/time.c b/kernel/time.c
index 2951194..667b959 100644
--- a/kernel/time.c
+++ b/kernel/time.c
@@ -138,7 +138,7 @@ static inline void warp_clock(void)
        xtime.tv_sec += sys_tz.tz_minuteswest * 60;
        update_xtime_cache(0);
        write_sequnlock_irq(&xtime_lock);
-       clock_was_set();
+       atomic_notifier_call_chain(&clockset_notifier_list, 0, NULL);
  }

  /*
diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
index e8c77d9..284539f 100644
--- a/kernel/time/timekeeping.c
+++ b/kernel/time/timekeeping.c
@@ -18,7 +18,11 @@
  #include<linux/jiffies.h>
  #include<linux/time.h>
  #include<linux/tick.h>
+#include<linux/notifier.h>

+ATOMIC_NOTIFIER_HEAD(clockset_notifier_list);
+
+EXPORT_SYMBOL(clockset_notifier_list);

  /*
   * This read-write spinlock protects us from races in SMP while
@@ -174,8 +178,7 @@ int do_settimeofday(struct timespec *tv)

        write_sequnlock_irqrestore(&xtime_lock, flags);

-       /* signal hrtimers about time change */
-       clock_was_set();
+       atomic_notifier_call_chain(&clockset_notifier_list, 0, NULL);

        return 0;
  }



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.