linux-kernel - Re: TSC not updating after resume: Bug or Feature?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20081224140900.GA1624@ucw.cz>
Date:	Wed, 24 Dec 2008 15:09:01 +0100
From:	Pavel Machek <pavel@...e.cz>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Fabio Comolli <fabio.comolli@...il.com>,
	Ingo Molnar <mingo@...e.hu>, "Rafael J. Wysocki" <rjw@...k.pl>,
	Peter Zijlstra <peterz@...radead.org>,
	Steven Rostedt <srostedt@...hat.com>, dsaxena@...xity.net,
	LKML <linux-kernel@...r.kernel.org>,
	Dave Kleikamp <shaggy@...ux.vnet.ibm.com>,
	toralf.foerster@....de
Subject: Re: TSC not updating after resume: Bug or Feature?

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/