On 11/01/2019 00:10, Boris Ostrovsky wrote: > On 1/10/19 12:17 PM, Boris Ostrovsky wrote: >> On 1/10/19 11:14 AM, Juergen Gross wrote: >>> On 10/01/2019 16:34, Boris Ostrovsky wrote: >>>> On 1/10/19 5:07 AM, Juergen Gross wrote: >>>>> >>>>> +void xen_clocksource_suspend(void) >>>>> +{ >>>>> + xen_clock_value_saved = xen_clocksource_read() - xen_sched_clock_offset; >>>> xen_clock_value_saved = xen_sched_clock() maybe? >>> I wanted xen_clocksource_suspend() and xen_clocksource_resume() to >>> be symmetrical to each other. >> OK. >> >> Reviewed-by: Boris Ostrovsky <boris.ostrov...@oracle.com> >> >>> In case you are feeling strong about that, I'm not. :-) So in case >>> you insist on it I can change it. Or you can do so while committing. > > > I did some basic testing and noticed this (at loglevel=8): > > [ 64.336488] Freezing user space processes ... (elapsed 0.001 seconds) > done. > [ 64.337805] OOM killer disabled. > [ 64.337814] Freezing remaining freezable tasks ... (elapsed 0.000 > seconds) done. > [ 64.339066] suspending xenstore... > [ 85.888340] xen:grant_table: Grant tables using version 1 layout > [ 64.359729] OOM killer enabled. > [ 64.359736] Restarting tasks ... done. > > > Which made me think that perhaps we should do suspend/restore of the > clocksource as close as possible to HYPERVISOR_suspend() call, e.g. in > xen_arch_pre_suspend()/xen_arch_post_suspend(): > > diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c > index 45fc9caf3880..80ecba3fcc8c 100644 > --- a/arch/x86/xen/suspend.c > +++ b/arch/x86/xen/suspend.c > @@ -22,6 +22,7 @@ static DEFINE_PER_CPU(u64, spec_ctrl); > > void xen_arch_pre_suspend(void) > { > + xen_clocksource_suspend(); > xen_save_time_memory_area(); > > if (xen_pv_domain()) > @@ -36,6 +37,7 @@ void xen_arch_post_suspend(int cancelled) > xen_hvm_post_suspend(cancelled); > > xen_restore_time_memory_area(); > + xen_clocksource_resume(); > } > > static void xen_vcpu_notify_restore(void *data) > > > This still has a window of incorrect clock value (you can see it, for > example, when xen_hvm_post_suspend() does pr_info("Xen HVM callback > vector for event delivery is enabled\n")), but it's smaller than before. > In particular, we will make time right before dpm_resume_start() call.
You are right, this is better. Sending out V2 soon. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel