Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752586AbYLYScP (ORCPT ); Thu, 25 Dec 2008 13:32:15 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751978AbYLYSb7 (ORCPT ); Thu, 25 Dec 2008 13:31:59 -0500 Received: from gprs189-60.eurotel.cz ([160.218.189.60]:52495 "EHLO gprs189-60.eurotel.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751935AbYLYSb6 (ORCPT ); Thu, 25 Dec 2008 13:31:58 -0500 Date: Wed, 24 Dec 2008 15:09:01 +0100 From: Pavel Machek To: Thomas Gleixner Cc: Fabio Comolli , Ingo Molnar , "Rafael J. Wysocki" , Peter Zijlstra , Steven Rostedt , dsaxena@plexity.net, LKML , Dave Kleikamp , toralf.foerster@gmx.de Subject: Re: TSC not updating after resume: Bug or Feature? Message-ID: <20081224140900.GA1624@ucw.cz> References: <20081222150021.GA13839@elte.hu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4165 Lines: 115 Hi! > > > > > > Can please you revert the last patch and apply the following ? Does > > > the WARN_ON trigger ? > > > > > > Yes, it does: > > > [ 159.768005] [] getnstimeofday+0x21/0xcd > > [ 159.768005] [] ktime_get_ts+0x1d/0x3f > > [ 159.768005] [] ktime_get+0xf/0x2b > > [ 159.768005] [] sched_clock_tick+0x46/0x83 > > [ 159.768005] [] sched_clock_idle_wakeup_event+0x5/0xa > > [ 159.768005] [] set_cyc2ns_scale+0x3f/0x5e > > [ 159.768005] [] time_cpufreq_notifier+0xf9/0x103 > > [ 159.768005] [] notifier_call_chain+0x2a/0x52 > > [ 159.768005] [] __srcu_notifier_call_chain+0x35/0x4a > > [ 159.768005] [] srcu_notifier_call_chain+0x9/0xc > > [ 159.768005] [] cpufreq_resume+0xf3/0x112 > > [ 159.768005] [] __sysdev_resume+0x24/0x34 > > [ 159.768005] [] 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/