Re: TSC not updating after resume: Bug or Feature?

From: Pavel Machek
Date: Thu Dec 25 2008 - 13:32:28 EST


Hi!
> > >
> > > Can please you revert the last patch and apply the following ? Does
> > > the WARN_ON trigger ?
> > >
>
> > Yes, it does:
>
> > [ 159.768005] [<c01389a2>] getnstimeofday+0x21/0xcd
> > [ 159.768005] [<c0135e26>] ktime_get_ts+0x1d/0x3f
> > [ 159.768005] [<c0135e57>] ktime_get+0xf/0x2b
> > [ 159.768005] [<c013736b>] sched_clock_tick+0x46/0x83
> > [ 159.768005] [<c01373ad>] sched_clock_idle_wakeup_event+0x5/0xa
> > [ 159.768005] [<c0108670>] set_cyc2ns_scale+0x3f/0x5e
> > [ 159.768005] [<c01088af>] time_cpufreq_notifier+0xf9/0x103
> > [ 159.768005] [<c0136782>] notifier_call_chain+0x2a/0x52
> > [ 159.768005] [<c0136866>] __srcu_notifier_call_chain+0x35/0x4a
> > [ 159.768005] [<c0136884>] srcu_notifier_call_chain+0x9/0xc
> > [ 159.768005] [<c031cbca>] cpufreq_resume+0xf3/0x112
> > [ 159.768005] [<c028c3f3>] __sysdev_resume+0x24/0x34
> > [ 159.768005] [<c028c421>] sysdev_resume+0x1e/0x50
>
> Thanks for testing. It's exaclty the code path I described :)
>
> So my code analysis holds and your test confirms my suspicion that
> Shaggy's patch just unearthed some other weirdness in the
> suspend/resume code.
>
> Can you please apply the following hack^Wpatch and retest ? It
> restores Shaggys patch, but prevents the sched_clock_tick() call when
> timekeeping is not resumed. The WARN_ON should not longer trigger
> except there is some other code path which fiddles with that as well.
>
> If I'm not completely nuts then this should solve your suspend/resume
> problem really instead of papering over the root cause.

Should we move timekeeping resume before cpufreq resume, instead of this?

> The patch should work on top of 2.6.27.10 as well, so whatever is
> easier to verify for you is fine.
>
> Thanks,
>
> tglx
> ---
> diff --git a/kernel/sched_clock.c b/kernel/sched_clock.c
> index e8ab096..dd97801 100644
> --- a/kernel/sched_clock.c
> +++ b/kernel/sched_clock.c
> @@ -124,7 +124,7 @@ static u64 __update_sched_clock(struct sched_clock_data *scd, u64 now)
>
> clock = scd->tick_gtod + delta;
> min_clock = wrap_max(scd->tick_gtod, scd->clock);
> - max_clock = scd->tick_gtod + TICK_NSEC;
> + max_clock = wrap_max(scd->clock, scd->tick_gtod + TICK_NSEC);
>
> clock = wrap_max(clock, min_clock);
> clock = wrap_min(clock, max_clock);
> @@ -222,11 +222,16 @@ void sched_clock_idle_sleep_event(void)
> }
> EXPORT_SYMBOL_GPL(sched_clock_idle_sleep_event);
>
> +extern int timekeeping_suspended;
> +
> /*
> * We just idled delta nanoseconds (called with irqs disabled):
> */
> void sched_clock_idle_wakeup_event(u64 delta_ns)
> {
> + if (timekeeping_suspended)
> + return;
> +
> sched_clock_tick();
> touch_softlockup_watchdog();
> }
> diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c
> index fa05e88..50ba3d0 100644
> --- a/kernel/time/timekeeping.c
> +++ b/kernel/time/timekeeping.c
> @@ -46,6 +46,9 @@ struct timespec xtime __attribute__ ((aligned (16)));
> struct timespec wall_to_monotonic __attribute__ ((aligned (16)));
> static unsigned long total_sleep_time; /* seconds */
>
> +/* flag for if timekeeping is suspended */
> +int timekeeping_suspended;
> +
> static struct timespec xtime_cache __attribute__ ((aligned (16)));
> void update_xtime_cache(u64 nsec)
> {
> @@ -92,6 +95,8 @@ void getnstimeofday(struct timespec *ts)
> unsigned long seq;
> s64 nsecs;
>
> + WARN_ON(timekeeping_suspended);
> +
> do {
> seq = read_seqbegin(&xtime_lock);
>
> @@ -299,8 +304,6 @@ void __init timekeeping_init(void)
> write_sequnlock_irqrestore(&xtime_lock, flags);
> }
>
> -/* flag for if timekeeping is suspended */
> -static int timekeeping_suspended;
> /* time in seconds when suspend began */
> static unsigned long timekeeping_suspend_time;

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/