RE: [Xen-devel] [PATCH] time-xen : Reset monotonic time when sync up time from dom0 to domU

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

>>> On 13.10.10 at 05:24, "Du, Hang" <hdu@xxxxxxxxxxxx> wrote:
> Sorry for my brief description in previous mail and missing 
> is_initial_xendomain check. The kernel I submit this patch is 
> linux-2.6.18-xen-3.4.2, I submit the patch again with in_initial-xendomain 
> check.

Actually, I didn't expect you to blindly insert that check, but rather
either explain why only DomU-s need the changed behavior (as your
original description suggested), or refine the description of the
problem you're trying to solve.

> In this patch, we support the backward time changing sync to all domUs which 
> configured to use "dependent wall clock".
> Currently, without the backward time syncing, when we change the time 
> backward in Dom0, the time in DomU would be froze. 
> And this cause some commands hang and don't executed until the time catch up 
> with the domU time.
> For example:
> "rpm -q kernel-xen"
> "sleep 1"
> Monotonic time should be reset when sync up time from dom0 to domU to 
> support domU backward time syncing.

I can see your point, however ...

> diff -urN a/arch/i386/kernel/time-xen.c   b/arch/i386/kernel/time-xen.c   
> 
> --- a/arch/i386/kernel/time-xen.c   2010-10-11 10:41:06.000000000 +0800
> +++ b/arch/i386/kernel/time-xen.c   2010-10-11 10:43:32.000000000 +0800
> @@ -715,6 +715,8 @@
>     }   
>  
>     if (shadow_tv_version != HYPERVISOR_shared_info->wc_version) {
> +        if (!is_initial_xendomain() && !independent_wallclock)
> +            monotonic_reset();
>         update_wallclock();

... you definitely need to call monotonic_reset() *after* the
update_wallclock() (or else another vCPU, preempted in
do_gettimeofday() between the end of the xtime read loop
and the acquiring of the monotonic lock, would be able to
overwrite monotonic.tv with values obtained before the wall
clock update - similar to the secondary problem addressed by
c/s 1021).

Further, blindly running a reset here doesn't seem nice in
the (general) case of the time getting updated forwards.

>         schedule_clock_was_set_work = 1;
>     }

You'll also want to address Dan's concerns, and you will want to
get your patch formatted so that it would actually apply (read:
no tab -> space conversion) if you expect it to be committed
by Keir at some point.

Jan


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