Re: clockevents: fix resume logic

Re: clockevents: fix resume logic

[Andrew Morton]

On Sun, 22 Jul 2007 01:59:11 GMT Linux Kernel Mailing List <linux-kernel@vger.kernel.org> wrote:

> Gitweb:     http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6
> Commit:     18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6
> Parent:     93da56efcf8c6a111f0349f6b7651172d4745ca0
> Author:     Thomas Gleixner <tglx@linutronix.de>
> AuthorDate: Sat Jul 21 04:37:34 2007 -0700
> Committer:  Linus Torvalds <torvalds@woody.linux-foundation.org>
> CommitDate: Sat Jul 21 17:49:15 2007 -0700
> 
>     clockevents: fix resume logic
>     
>     We need to make sure, that the clockevent devices are resumed, before
>     the tick is resumed. The current resume logic does not guarantee this.
>     
>     Add CLOCK_EVT_MODE_RESUME and call the set mode functions of the clock
>     event devices before resuming the tick / oneshot functionality.
>     
>     Fixup the existing users.
>     
>     Thanks to Nigel Cunningham for tracking down a long standing thinko,
>     which affected the jinxed VAIO.
>     

This patch broke the jinxed vaio.

Which is a bit odd, considering that I must have tested it at the time. 
But I bisected it right down to this commit, and the below revert patch
fixed it up.

The symptoms are that with this patch applied, resume-from-RAM will get
stuck partway through doing its stuff.  If I then repeatedly hit a key on
the keyboard, resume will struggle through and complete.  The system time
is then a few seconds behind the time which `hwclock' says, so it looks
like we're also not restoring the time correctly.

Also, a `reboot -f' get stuck in the same way.  No progress until I start
hitting a key.


With the below revert patch against current-Linus-mainline, resume-from-RAM
does work correctly.  However suspend-to-disk is still busted, in the same
way: I need to repeatedly hit a key to make progress.



From: Andrew Morton <akpm@linux-foundation.org>

Revert:

commit 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Sat Jul 21 04:37:34 2007 -0700

    clockevents: fix resume logic
    
    We need to make sure, that the clockevent devices are resumed, before
    the tick is resumed. The current resume logic does not guarantee this.
    
    Add CLOCK_EVT_MODE_RESUME and call the set mode functions of the clock
    event devices before resuming the tick / oneshot functionality.
    
    Fixup the existing users.
    
    Thanks to Nigel Cunningham for tracking down a long standing thinko,
    which affected the jinxed VAIO.


It causes the following on the Vaio:

- resume-from-ram gets stuck and requires repeated hitting of a key for it
  to make progress

- `reboot -f' fails in the same way, with the same fix

- the system time after resume is wrong: it is behind the hwclock time by a
  period which appears to be equal to the time spent in suspend


Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: john stultz <johnstul@us.ibm.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 arch/arm/mach-davinci/time.c      |    2 
 arch/arm/mach-imx/time.c          |    1 
 arch/arm/mach-ixp4xx/common.c     |    2 
 arch/arm/mach-omap1/time.c        |    1 
 arch/arm/plat-omap/timer32k.c     |    2 
 arch/i386/kernel/apic.c           |    3 -
 arch/i386/kernel/hpet.c           |   71 ++++++++++++++++++++++++++--
 arch/i386/kernel/i8253.c          |   29 +++++------
 arch/i386/kernel/vmiclock.c       |    1 
 arch/i386/xen/time.c              |    3 -
 arch/sh/kernel/timers/timer-tmu.c |    1 
 arch/sparc64/kernel/time.c        |    1 
 drivers/lguest/lguest.c           |    2 
 include/linux/clockchips.h        |    1 
 kernel/time/tick-broadcast.c      |    6 --
 kernel/time/tick-common.c         |   16 ++----
 16 files changed, 88 insertions(+), 54 deletions(-)

diff -puN arch/arm/mach-davinci/time.c~a arch/arm/mach-davinci/time.c
--- a/arch/arm/mach-davinci/time.c~a
+++ a/arch/arm/mach-davinci/time.c
@@ -285,8 +285,6 @@ static void davinci_set_mode(enum clock_
 	case CLOCK_EVT_MODE_SHUTDOWN:
 		t->opts = TIMER_OPTS_DISABLED;
 		break;
-	case CLOCK_EVT_MODE_RESUME:
-		break;
 	}
 }
 
diff -puN arch/arm/mach-imx/time.c~a arch/arm/mach-imx/time.c
--- a/arch/arm/mach-imx/time.c~a
+++ a/arch/arm/mach-imx/time.c
@@ -159,7 +159,6 @@ static void imx_set_mode(enum clock_even
 		break;
 	case CLOCK_EVT_MODE_SHUTDOWN:
 	case CLOCK_EVT_MODE_UNUSED:
-	case CLOCK_EVT_MODE_RESUME:
 		/* Left event sources disabled, no more interrupts appears */
 		break;
 	}
diff -puN arch/arm/mach-ixp4xx/common.c~a arch/arm/mach-ixp4xx/common.c
--- a/arch/arm/mach-ixp4xx/common.c~a
+++ a/arch/arm/mach-ixp4xx/common.c
@@ -459,8 +459,6 @@ static void ixp4xx_set_mode(enum clock_e
 	default:
 		osrt = opts = 0;
 		break;
-	case CLOCK_EVT_MODE_RESUME:
-		break;
 	}
 
 	*IXP4XX_OSRT1 = osrt | opts;
diff -puN arch/arm/mach-omap1/time.c~a arch/arm/mach-omap1/time.c
--- a/arch/arm/mach-omap1/time.c~a
+++ a/arch/arm/mach-omap1/time.c
@@ -156,7 +156,6 @@ static void omap_mpu_set_mode(enum clock
 		break;
 	case CLOCK_EVT_MODE_UNUSED:
 	case CLOCK_EVT_MODE_SHUTDOWN:
-	case CLOCK_EVT_MODE_RESUME:
 		break;
 	}
 }
diff -puN arch/arm/plat-omap/timer32k.c~a arch/arm/plat-omap/timer32k.c
--- a/arch/arm/plat-omap/timer32k.c~a
+++ a/arch/arm/plat-omap/timer32k.c
@@ -157,8 +157,6 @@ static void omap_32k_timer_set_mode(enum
 	case CLOCK_EVT_MODE_UNUSED:
 	case CLOCK_EVT_MODE_SHUTDOWN:
 		break;
-	case CLOCK_EVT_MODE_RESUME:
-		break;
 	}
 }
 
diff -puN arch/i386/kernel/apic.c~a arch/i386/kernel/apic.c
--- a/arch/i386/kernel/apic.c~a
+++ a/arch/i386/kernel/apic.c
@@ -264,9 +264,6 @@ static void lapic_timer_setup(enum clock
 		v |= (APIC_LVT_MASKED | LOCAL_TIMER_VECTOR);
 		apic_write_around(APIC_LVTT, v);
 		break;
-	case CLOCK_EVT_MODE_RESUME:
-		/* Nothing to do here */
-		break;
 	}
 
 	local_irq_restore(flags);
diff -puN arch/i386/kernel/hpet.c~a arch/i386/kernel/hpet.c
--- a/arch/i386/kernel/hpet.c~a
+++ a/arch/i386/kernel/hpet.c
@@ -188,10 +188,6 @@ static void hpet_set_mode(enum clock_eve
 		cfg &= ~HPET_TN_ENABLE;
 		hpet_writel(cfg, HPET_T0_CFG);
 		break;
-
-	case CLOCK_EVT_MODE_RESUME:
-		hpet_enable_int();
-		break;
 	}
 }
 
@@ -222,7 +218,6 @@ static struct clocksource clocksource_hp
 	.mask		= HPET_MASK,
 	.shift		= HPET_SHIFT,
 	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
-	.resume		= hpet_start_counter,
 };
 
 /*
@@ -319,6 +314,7 @@ int __init hpet_enable(void)
 
 	clocksource_register(&clocksource_hpet);
 
+
 	if (id & HPET_ID_LEGSUP) {
 		hpet_enable_int();
 		hpet_reserve_platform_timers(id);
@@ -551,3 +547,68 @@ irqreturn_t hpet_rtc_interrupt(int irq, 
 	return IRQ_HANDLED;
 }
 #endif
+
+
+/*
+ * Suspend/resume part
+ */
+
+#ifdef CONFIG_PM
+
+static int hpet_suspend(struct sys_device *sys_device, pm_message_t state)
+{
+	unsigned long cfg = hpet_readl(HPET_CFG);
+
+	cfg &= ~(HPET_CFG_ENABLE|HPET_CFG_LEGACY);
+	hpet_writel(cfg, HPET_CFG);
+
+	return 0;
+}
+
+static int hpet_resume(struct sys_device *sys_device)
+{
+	unsigned int id;
+
+	hpet_start_counter();
+
+	id = hpet_readl(HPET_ID);
+
+	if (id & HPET_ID_LEGSUP)
+		hpet_enable_int();
+
+	return 0;
+}
+
+static struct sysdev_class hpet_class = {
+	set_kset_name("hpet"),
+	.suspend	= hpet_suspend,
+	.resume		= hpet_resume,
+};
+
+static struct sys_device hpet_device = {
+	.id		= 0,
+	.cls		= &hpet_class,
+};
+
+
+static __init int hpet_register_sysfs(void)
+{
+	int err;
+
+	if (!is_hpet_capable())
+		return 0;
+
+	err = sysdev_class_register(&hpet_class);
+
+	if (!err) {
+		err = sysdev_register(&hpet_device);
+		if (err)
+			sysdev_class_unregister(&hpet_class);
+	}
+
+	return err;
+}
+
+device_initcall(hpet_register_sysfs);
+
+#endif
diff -puN arch/i386/kernel/i8253.c~a arch/i386/kernel/i8253.c
--- a/arch/i386/kernel/i8253.c~a
+++ a/arch/i386/kernel/i8253.c
@@ -3,11 +3,11 @@
  *
  */
 #include <linux/clockchips.h>
-#include <linux/init.h>
-#include <linux/interrupt.h>
+#include <linux/spinlock.h>
 #include <linux/jiffies.h>
+#include <linux/sysdev.h>
 #include <linux/module.h>
-#include <linux/spinlock.h>
+#include <linux/init.h>
 
 #include <asm/smp.h>
 #include <asm/delay.h>
@@ -40,27 +40,26 @@ static void init_pit_timer(enum clock_ev
 	case CLOCK_EVT_MODE_PERIODIC:
 		/* binary, mode 2, LSB/MSB, ch 0 */
 		outb_p(0x34, PIT_MODE);
+		udelay(10);
 		outb_p(LATCH & 0xff , PIT_CH0);	/* LSB */
+		udelay(10);
 		outb(LATCH >> 8 , PIT_CH0);	/* MSB */
 		break;
 
+	/*
+	 * Avoid unnecessary state transitions, as it confuses
+	 * Geode / Cyrix based boxen.
+	 */
 	case CLOCK_EVT_MODE_SHUTDOWN:
+		if (evt->mode == CLOCK_EVT_MODE_UNUSED)
+			break;
 	case CLOCK_EVT_MODE_UNUSED:
-		if (evt->mode == CLOCK_EVT_MODE_PERIODIC ||
-		    evt->mode == CLOCK_EVT_MODE_ONESHOT) {
-			outb_p(0x30, PIT_MODE);
-			outb_p(0, PIT_CH0);
-			outb_p(0, PIT_CH0);
-		}
-		break;
-
+		if (evt->mode == CLOCK_EVT_MODE_SHUTDOWN)
+			break;
 	case CLOCK_EVT_MODE_ONESHOT:
 		/* One shot setup */
 		outb_p(0x38, PIT_MODE);
-		break;
-
-	case CLOCK_EVT_MODE_RESUME:
-		/* Nothing to do here */
+		udelay(10);
 		break;
 	}
 	spin_unlock_irqrestore(&i8253_lock, flags);
diff -puN arch/i386/kernel/vmiclock.c~a arch/i386/kernel/vmiclock.c
--- a/arch/i386/kernel/vmiclock.c~a
+++ a/arch/i386/kernel/vmiclock.c
@@ -143,7 +143,6 @@ static void vmi_timer_set_mode(enum cloc
 
 	switch (mode) {
 	case CLOCK_EVT_MODE_ONESHOT:
-	case CLOCK_EVT_MODE_RESUME:
 		break;
 	case CLOCK_EVT_MODE_PERIODIC:
 		cycles_per_hz = vmi_timer_ops.get_cycle_frequency();
diff -puN arch/i386/xen/time.c~a arch/i386/xen/time.c
--- a/arch/i386/xen/time.c~a
+++ a/arch/i386/xen/time.c
@@ -412,7 +412,6 @@ static void xen_timerop_set_mode(enum cl
 		break;
 
 	case CLOCK_EVT_MODE_ONESHOT:
-	case CLOCK_EVT_MODE_RESUME:
 		break;
 
 	case CLOCK_EVT_MODE_UNUSED:
@@ -475,8 +474,6 @@ static void xen_vcpuop_set_mode(enum clo
 		    HYPERVISOR_vcpu_op(VCPUOP_stop_periodic_timer, cpu, NULL))
 			BUG();
 		break;
-	case CLOCK_EVT_MODE_RESUME:
-		break;
 	}
 }
 
diff -puN arch/sh/kernel/timers/timer-tmu.c~a arch/sh/kernel/timers/timer-tmu.c
--- a/arch/sh/kernel/timers/timer-tmu.c~a
+++ a/arch/sh/kernel/timers/timer-tmu.c
@@ -80,7 +80,6 @@ static void tmu_set_mode(enum clock_even
 		break;
 	case CLOCK_EVT_MODE_UNUSED:
 	case CLOCK_EVT_MODE_SHUTDOWN:
-	case CLOCK_EVT_MODE_RESUME:
 		break;
 	}
 }
diff -puN arch/sparc64/kernel/time.c~a arch/sparc64/kernel/time.c
--- a/arch/sparc64/kernel/time.c~a
+++ a/arch/sparc64/kernel/time.c
@@ -882,7 +882,6 @@ static void sparc64_timer_setup(enum clo
 {
 	switch (mode) {
 	case CLOCK_EVT_MODE_ONESHOT:
-	case CLOCK_EVT_MODE_RESUME:
 		break;
 
 	case CLOCK_EVT_MODE_SHUTDOWN:
diff -puN drivers/lguest/lguest.c~a drivers/lguest/lguest.c
--- a/drivers/lguest/lguest.c~a
+++ a/drivers/lguest/lguest.c
@@ -730,8 +730,6 @@ static void lguest_clockevent_set_mode(e
 		break;
 	case CLOCK_EVT_MODE_PERIODIC:
 		BUG();
-	case CLOCK_EVT_MODE_RESUME:
-		break;
 	}
 }
 
diff -puN include/linux/clockchips.h~a include/linux/clockchips.h
--- a/include/linux/clockchips.h~a
+++ a/include/linux/clockchips.h
@@ -23,7 +23,6 @@ enum clock_event_mode {
 	CLOCK_EVT_MODE_SHUTDOWN,
 	CLOCK_EVT_MODE_PERIODIC,
 	CLOCK_EVT_MODE_ONESHOT,
-	CLOCK_EVT_MODE_RESUME,
 };
 
 /* Clock event notification values */
diff -puN kernel/time/tick-broadcast.c~a kernel/time/tick-broadcast.c
--- a/kernel/time/tick-broadcast.c~a
+++ a/kernel/time/tick-broadcast.c
@@ -55,7 +55,7 @@ cpumask_t *tick_get_broadcast_mask(void)
  */
 static void tick_broadcast_start_periodic(struct clock_event_device *bc)
 {
-	if (bc)
+	if (bc && bc->mode == CLOCK_EVT_MODE_SHUTDOWN)
 		tick_setup_periodic(bc, 1);
 }
 
@@ -316,7 +316,7 @@ void tick_suspend_broadcast(void)
 	spin_lock_irqsave(&tick_broadcast_lock, flags);
 
 	bc = tick_broadcast_device.evtdev;
-	if (bc)
+	if (bc && tick_broadcast_device.mode == TICKDEV_MODE_PERIODIC)
 		clockevents_set_mode(bc, CLOCK_EVT_MODE_SHUTDOWN);
 
 	spin_unlock_irqrestore(&tick_broadcast_lock, flags);
@@ -333,8 +333,6 @@ int tick_resume_broadcast(void)
 	bc = tick_broadcast_device.evtdev;
 
 	if (bc) {
-		clockevents_set_mode(bc, CLOCK_EVT_MODE_RESUME);
-
 		switch (tick_broadcast_device.mode) {
 		case TICKDEV_MODE_PERIODIC:
 			if(!cpus_empty(tick_broadcast_mask))
diff -puN kernel/time/tick-common.c~a kernel/time/tick-common.c
--- a/kernel/time/tick-common.c~a
+++ a/kernel/time/tick-common.c
@@ -318,17 +318,12 @@ static void tick_resume(void)
 {
 	struct tick_device *td = &__get_cpu_var(tick_cpu_device);
 	unsigned long flags;
-	int broadcast = tick_resume_broadcast();
 
 	spin_lock_irqsave(&tick_device_lock, flags);
-	clockevents_set_mode(td->evtdev, CLOCK_EVT_MODE_RESUME);
-
-	if (!broadcast) {
-		if (td->mode == TICKDEV_MODE_PERIODIC)
-			tick_setup_periodic(td->evtdev, 0);
-		else
-			tick_resume_oneshot();
-	}
+	if (td->mode == TICKDEV_MODE_PERIODIC)
+		tick_setup_periodic(td->evtdev, 0);
+	else
+		tick_resume_oneshot();
 	spin_unlock_irqrestore(&tick_device_lock, flags);
 }
 
@@ -365,7 +360,8 @@ static int tick_notify(struct notifier_b
 		break;
 
 	case CLOCK_EVT_NOTIFY_RESUME:
-		tick_resume();
+		if (!tick_resume_broadcast())
+			tick_resume();
 		break;
 
 	default:
_

Re: clockevents: fix resume logic

[Thomas Gleixner]

On Mon, 2007-09-10 at 14:47 -0700, Andrew Morton wrote:
> > 
> >     clockevents: fix resume logic
> >     
> >     We need to make sure, that the clockevent devices are resumed, before
> >     the tick is resumed. The current resume logic does not guarantee this.
> >     
> >     Add CLOCK_EVT_MODE_RESUME and call the set mode functions of the clock
> >     event devices before resuming the tick / oneshot functionality.
> >     
> >     Fixup the existing users.
> >     
> >     Thanks to Nigel Cunningham for tracking down a long standing thinko,
> >     which affected the jinxed VAIO.
> >     
> 
> This patch broke the jinxed vaio.
> 
> Which is a bit odd, considering that I must have tested it at the time. 
> But I bisected it right down to this commit, and the below revert patch
> fixed it up.

I just looked up, that you confirmed earlier that the patch does _not_
break the VAIO. 

I think, that some other suspend/resume/ACPI or whatever change went in
before this patch got merged into Linus tree. Sigh. That's pretty hard
to find out.

	tglx

Re: clockevents: fix resume logic

[Andrew Morton]

On Tue, 11 Sep 2007 08:37:16 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:

> On Mon, 2007-09-10 at 14:47 -0700, Andrew Morton wrote:
> > > 
> > >     clockevents: fix resume logic
> > >     
> > >     We need to make sure, that the clockevent devices are resumed, before
> > >     the tick is resumed. The current resume logic does not guarantee this.
> > >     
> > >     Add CLOCK_EVT_MODE_RESUME and call the set mode functions of the clock
> > >     event devices before resuming the tick / oneshot functionality.
> > >     
> > >     Fixup the existing users.
> > >     
> > >     Thanks to Nigel Cunningham for tracking down a long standing thinko,
> > >     which affected the jinxed VAIO.
> > >     
> > 
> > This patch broke the jinxed vaio.
> > 
> > Which is a bit odd, considering that I must have tested it at the time. 
> > But I bisected it right down to this commit, and the below revert patch
> > fixed it up.
> 
> I just looked up, that you confirmed earlier that the patch does _not_
> break the VAIO. 
> 
> I think, that some other suspend/resume/ACPI or whatever change went in
> before this patch got merged into Linus tree. Sigh. That's pretty hard
> to find out.
> 

I tried just 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 on top of 2.6.22
(threw away the lguest and xen hunks).  The same problem occurs.  2.6.22 is
OK.

I went back to the original patch which I sent to Linus and it matches
18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6.  So all I can think is that there
must have been something else in the tree which I tested which fixed the
bug which 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 introduced.  argh.

Can you think what would cause the symptoms which I described?  

Re: clockevents: fix resume logic

[Thomas Gleixner]

On Tue, 2007-09-11 at 00:00 -0700, Andrew Morton wrote:
> > > This patch broke the jinxed vaio.
> > > 
> > > Which is a bit odd, considering that I must have tested it at the time. 
> > > But I bisected it right down to this commit, and the below revert patch
> > > fixed it up.
> > 
> > I just looked up, that you confirmed earlier that the patch does _not_
> > break the VAIO. 
> > 
> > I think, that some other suspend/resume/ACPI or whatever change went in
> > before this patch got merged into Linus tree. Sigh. That's pretty hard
> > to find out.
> > 
> 
> I tried just 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 on top of 2.6.22
> (threw away the lguest and xen hunks).  The same problem occurs.  2.6.22 is
> OK.

Hmm.

> I went back to the original patch which I sent to Linus and it matches
> 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6.  So all I can think is that there
> must have been something else in the tree which I tested which fixed the
> bug which 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 introduced.  argh.
> 
> Can you think what would cause the symptoms which I described?  

It seems that time is not updated. Timer interrupt not active or some
other odd thing. I figure out what's going on when I find a box which
exposes the problem.

	tglx

Re: clockevents: fix resume logic

[Thomas Gleixner]

On Tue, 2007-09-11 at 09:23 +0200, Thomas Gleixner wrote:
> > I went back to the original patch which I sent to Linus and it matches
> > 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6.  So all I can think is that there
> > must have been something else in the tree which I tested which fixed the
> > bug which 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 introduced.  argh.
> > 
> > Can you think what would cause the symptoms which I described?  
> 
> It seems that time is not updated. Timer interrupt not active or some
> other odd thing. I figure out what's going on when I find a box which
> exposes the problem.

It get's even worse. Reverting the patch breaks _my_ VAIO. 

/me cries and looks for a job which does not involve computers

	tglx

Re: clockevents: fix resume logic

[Andrew Morton]

On Tue, 11 Sep 2007 09:34:31 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:

> On Tue, 2007-09-11 at 09:23 +0200, Thomas Gleixner wrote:
> > > I went back to the original patch which I sent to Linus and it matches
> > > 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6.  So all I can think is that there
> > > must have been something else in the tree which I tested which fixed the
> > > bug which 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 introduced.  argh.
> > > 
> > > Can you think what would cause the symptoms which I described?  
> > 
> > It seems that time is not updated. Timer interrupt not active or some
> > other odd thing. I figure out what's going on when I find a box which
> > exposes the problem.
> 
> It get's even worse. Reverting the patch breaks _my_ VAIO. 
> 
> /me cries and looks for a job which does not involve computers
> 

heh.

I've confirmed that 2.6.22-rc6+thatpatch fails, but 2.6.22-rc6 plus the
three hundred patches up to and including that patch doesn't fail.

So there's something between "origin.patch" and
"clockevents-fix-resume-logic.patch" in the 2.6.22-rc6-mm1 lineup which
fixes thatpatch, once it is applied.

So I'm doing an inverted reverse polish bisection search to find out which
patch preemptively fixes clockevents-fix-resume-logic.patch.  Try doing
that with git, suckers.


gimme twenty minutes...

Re: clockevents: fix resume logic

[Thomas Gleixner]

On Tue, 2007-09-11 at 09:23 +0200, Thomas Gleixner wrote:
> > I went back to the original patch which I sent to Linus and it matches
> > 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6.  So all I can think is that there
> > must have been something else in the tree which I tested which fixed the
> > bug which 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 introduced.  argh.
> > 
> > Can you think what would cause the symptoms which I described?  
> 
> It seems that time is not updated. Timer interrupt not active or some
> other odd thing. I figure out what's going on when I find a box which
> exposes the problem.

Can you please try the following command line options:

nolapic_timer
nohz=off
highres=off
nohz=off highres=off

	tglx

Re: clockevents: fix resume logic

[Andrew Morton]

On Tue, 11 Sep 2007 09:47:20 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:

> On Tue, 2007-09-11 at 09:23 +0200, Thomas Gleixner wrote:
> > > I went back to the original patch which I sent to Linus and it matches
> > > 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6.  So all I can think is that there
> > > must have been something else in the tree which I tested which fixed the
> > > bug which 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 introduced.  argh.
> > > 
> > > Can you think what would cause the symptoms which I described?  
> > 
> > It seems that time is not updated. Timer interrupt not active or some
> > other odd thing. I figure out what's going on when I find a box which
> > exposes the problem.

2.6.22-rc6-mm1's git-acpi.patch contains something which fixes this bug. 
It's a quarter megabyte :( Presumably clockevents-fix-resume-logic.patch
depended on something which was in that git-acpi.patch but which for some
reason didn't get merged.  Ho hum, I'll take a look, see if I can spot it.

> Can you please try the following command line options:
> 
> nolapic_timer

Works

> nohz=off

Fails


> highres=off

Fails

> nohz=off highres=off

Works

Re: clockevents: fix resume logic

[Andrew Morton]

On Tue, 11 Sep 2007 01:20:05 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 11 Sep 2007 09:47:20 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > On Tue, 2007-09-11 at 09:23 +0200, Thomas Gleixner wrote:
> > > > I went back to the original patch which I sent to Linus and it matches
> > > > 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6.  So all I can think is that there
> > > > must have been something else in the tree which I tested which fixed the
> > > > bug which 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 introduced.  argh.
> > > > 
> > > > Can you think what would cause the symptoms which I described?  
> > > 
> > > It seems that time is not updated. Timer interrupt not active or some
> > > other odd thing. I figure out what's going on when I find a box which
> > > exposes the problem.
> 
> 2.6.22-rc6-mm1's git-acpi.patch contains something which fixes this bug. 

Len's current tree fixes it too.  Maybe we just need to merge the acpi tree?

 46 files changed, 4168 insertions(+), 2224 deletions(-)

I'll let Len do that ;)

Ho hum.  I guess I could do a git-bisect on the acpi tree.  Getting a bit dull.

Re: clockevents: fix resume logic

[Thomas Gleixner]

On Tue, 2007-09-11 at 01:20 -0700, Andrew Morton wrote:
> On Tue, 11 Sep 2007 09:47:20 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > On Tue, 2007-09-11 at 09:23 +0200, Thomas Gleixner wrote:
> > > > I went back to the original patch which I sent to Linus and it matches
> > > > 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6.  So all I can think is that there
> > > > must have been something else in the tree which I tested which fixed the
> > > > bug which 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 introduced.  argh.
> > > > 
> > > > Can you think what would cause the symptoms which I described?  
> > > 
> > > It seems that time is not updated. Timer interrupt not active or some
> > > other odd thing. I figure out what's going on when I find a box which
> > > exposes the problem.
> 
> 2.6.22-rc6-mm1's git-acpi.patch contains something which fixes this bug. 
> It's a quarter megabyte :( Presumably clockevents-fix-resume-logic.patch
> depended on something which was in that git-acpi.patch but which for some
> reason didn't get merged.  Ho hum, I'll take a look, see if I can spot it.
> 
> > Can you please try the following command line options:
> > 
> > nolapic_timer
> 
> Works
> 
> > nohz=off
> 
> Fails
> 
> 
> > highres=off
> 
> Fails
> 
> > nohz=off highres=off
> 
> Works

Ah. Adding your observation of the missing bit from the acpi stuff, I
think this must be related to the broadcast functionality.

Btw, 23-rc6 does not resume the Intel Video card on one of my laptops.
Sigh !

	tglx

Re: clockevents: fix resume logic

[Thomas Gleixner]

On Tue, 2007-09-11 at 01:35 -0700, Andrew Morton wrote:
> On Tue, 11 Sep 2007 01:20:05 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Tue, 11 Sep 2007 09:47:20 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> > 
> > > On Tue, 2007-09-11 at 09:23 +0200, Thomas Gleixner wrote:
> > > > > I went back to the original patch which I sent to Linus and it matches
> > > > > 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6.  So all I can think is that there
> > > > > must have been something else in the tree which I tested which fixed the
> > > > > bug which 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 introduced.  argh.
> > > > > 
> > > > > Can you think what would cause the symptoms which I described?  
> > > > 
> > > > It seems that time is not updated. Timer interrupt not active or some
> > > > other odd thing. I figure out what's going on when I find a box which
> > > > exposes the problem.
> > 
> > 2.6.22-rc6-mm1's git-acpi.patch contains something which fixes this bug. 
> 
> Len's current tree fixes it too.  Maybe we just need to merge the acpi tree?

Did you enable CONFIG_CPU_IDLE after you applied git-acpi to rc6 ?

	tglx

Re: clockevents: fix resume logic

[Andrew Morton]

On Tue, 11 Sep 2007 11:16:04 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:

> On Tue, 2007-09-11 at 01:35 -0700, Andrew Morton wrote:
> > On Tue, 11 Sep 2007 01:20:05 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > > On Tue, 11 Sep 2007 09:47:20 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> > > 
> > > > On Tue, 2007-09-11 at 09:23 +0200, Thomas Gleixner wrote:
> > > > > > I went back to the original patch which I sent to Linus and it matches
> > > > > > 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6.  So all I can think is that there
> > > > > > must have been something else in the tree which I tested which fixed the
> > > > > > bug which 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 introduced.  argh.
> > > > > > 
> > > > > > Can you think what would cause the symptoms which I described?  
> > > > > 
> > > > > It seems that time is not updated. Timer interrupt not active or some
> > > > > other odd thing. I figure out what's going on when I find a box which
> > > > > exposes the problem.
> > > 
> > > 2.6.22-rc6-mm1's git-acpi.patch contains something which fixes this bug. 
> > 
> > Len's current tree fixes it too.  Maybe we just need to merge the acpi tree?
> 
> Did you enable CONFIG_CPU_IDLE after you applied git-acpi to rc6 ?
> 

Nope.

Anyway, I've had it.  git seems a bit weird when it's trying to bisect down
to a point where code started to work correctly, rather than finding when
it failed (I just reversed "good" and "bad").  The acpi tree is littered
with build errors at various points and I just don't know which patch fixed
it - git-bisect seems to keep turning up junk.

Ho hum.  Maybe tomorrow I'll have a go at turning the acpi tree into a
series of flat patches and do it sanely.

Re: clockevents: fix resume logic

[Andrew Morton]

On Tue, 11 Sep 2007 13:23:55 +0200 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> On Tuesday, 11 September 2007 12:31, Andrew Morton wrote:
> > On Tue, 11 Sep 2007 11:16:04 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> > 
> > > On Tue, 2007-09-11 at 01:35 -0700, Andrew Morton wrote:
> > > > On Tue, 11 Sep 2007 01:20:05 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> > > > 
> > > > > On Tue, 11 Sep 2007 09:47:20 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> > > > > 
> > > > > > On Tue, 2007-09-11 at 09:23 +0200, Thomas Gleixner wrote:
> > > > > > > > I went back to the original patch which I sent to Linus and it matches
> > > > > > > > 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6.  So all I can think is that there
> > > > > > > > must have been something else in the tree which I tested which fixed the
> > > > > > > > bug which 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 introduced.  argh.
> > > > > > > > 
> > > > > > > > Can you think what would cause the symptoms which I described?  
> > > > > > > 
> > > > > > > It seems that time is not updated. Timer interrupt not active or some
> > > > > > > other odd thing. I figure out what's going on when I find a box which
> > > > > > > exposes the problem.
> > > > > 
> > > > > 2.6.22-rc6-mm1's git-acpi.patch contains something which fixes this bug. 
> > > > 
> > > > Len's current tree fixes it too.
> 
> Do you mean this one:
> 
> http://git.kernel.org/?p=linux/kernel/git/lenb/linux-acpi-2.6.git

Nope.

Here's the algorthm: go to the latest -mm tree and look at the first line
of broken-out/git-acpi.patch:

GIT f94aac9883f9b02700270cf286577a9bccf98f47 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git#test

that gives the URL, the branch and the top-level commit.  All the -mm git
trees have that first line.



Anyway, I was able to extract all the diffs and generate a patch series. 
The below patch fixes current mainline on the Vaio.

It doesn't compile (natch), so there's a second hackpatch at the end which
I used to test it.



commit ef5f15a8b79123a047285ec2e3899108661df779
Author: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Date:   Thu Feb 22 13:54:03 2007 -0800

    cpuidle take2: Hookup ACPI C-states driver with cpuidle
    
    Hookup ACPI C-states onto generic cpuidle infrastructure.
    
    drivers/acpi/procesor_idle.c is now a ACPI C-states driver that registers as
    a driver in cpuidle infrastructure and the policy part is removed from
    drivers/acpi/processor_idle.c. We use governor in cpuidle instead.
    
    Signed-off-by: Shaohua Li <shaohua.li@intel.com>
    Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
    Signed-off-by: Adam Belay <abelay@novell.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index f7de02a..7352254 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -44,6 +44,7 @@ #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #include <linux/dmi.h>
 #include <linux/moduleparam.h>
+#include <linux/cpuidle.h>
 
 #include <asm/io.h>
 #include <asm/system.h>
@@ -1022,11 +1023,13 @@ #endif
 
 	acpi_processor_ppc_init();
 
+	cpuidle_register_driver(&acpi_idle_driver);
 	return 0;
 }
 
 static void __exit acpi_processor_exit(void)
 {
+	cpuidle_unregister_driver(&acpi_idle_driver);
 
 	acpi_processor_ppc_exit();
 
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 80ffc78..0916f5e 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -40,6 +40,7 @@ #include <linux/moduleparam.h>
 #include <linux/sched.h>	/* need_resched() */
 #include <linux/latency.h>
 #include <linux/clockchips.h>
+#include <linux/cpuidle.h>
 
 /*
  * Include the apic definitions for x86 to have the APIC timer related defines
@@ -62,25 +63,15 @@ #define ACPI_PROCESSOR_CLASS            
 #define _COMPONENT              ACPI_PROCESSOR_COMPONENT
 ACPI_MODULE_NAME("processor_idle");
 #define ACPI_PROCESSOR_FILE_POWER	"power"
-#define US_TO_PM_TIMER_TICKS(t)		((t * (PM_TIMER_FREQUENCY/1000)) / 1000)
-#define C2_OVERHEAD			4	/* 1us (3.579 ticks per us) */
-#define C3_OVERHEAD			4	/* 1us (3.579 ticks per us) */
-static void (*pm_idle_save) (void) __read_mostly;
+#define PM_TIMER_TICKS_TO_US(p)		(((p) * 1000)/(PM_TIMER_FREQUENCY/1000))
+#define C2_OVERHEAD			1	/* 1us */
+#define C3_OVERHEAD			1	/* 1us */
+
 module_param(max_cstate, uint, 0644);
 
 static unsigned int nocst __read_mostly;
 module_param(nocst, uint, 0000);
 
-/*
- * bm_history -- bit-mask with a bit per jiffy of bus-master activity
- * 1000 HZ: 0xFFFFFFFF: 32 jiffies = 32ms
- * 800 HZ: 0xFFFFFFFF: 32 jiffies = 40ms
- * 100 HZ: 0x0000000F: 4 jiffies = 40ms
- * reduce history for more aggressive entry into C3
- */
-static unsigned int bm_history __read_mostly =
-    (HZ >= 800 ? 0xFFFFFFFF : ((1U << (HZ / 25)) - 1));
-module_param(bm_history, uint, 0644);
 /* --------------------------------------------------------------------------
                                 Power Management
    -------------------------------------------------------------------------- */
@@ -166,88 +157,6 @@ static struct dmi_system_id __cpuinitdat
 	{},
 };
 
-static inline u32 ticks_elapsed(u32 t1, u32 t2)
-{
-	if (t2 >= t1)
-		return (t2 - t1);
-	else if (!(acpi_gbl_FADT.flags & ACPI_FADT_32BIT_TIMER))
-		return (((0x00FFFFFF - t1) + t2) & 0x00FFFFFF);
-	else
-		return ((0xFFFFFFFF - t1) + t2);
-}
-
-static void
-acpi_processor_power_activate(struct acpi_processor *pr,
-			      struct acpi_processor_cx *new)
-{
-	struct acpi_processor_cx *old;
-
-	if (!pr || !new)
-		return;
-
-	old = pr->power.state;
-
-	if (old)
-		old->promotion.count = 0;
-	new->demotion.count = 0;
-
-	/* Cleanup from old state. */
-	if (old) {
-		switch (old->type) {
-		case ACPI_STATE_C3:
-			/* Disable bus master reload */
-			if (new->type != ACPI_STATE_C3 && pr->flags.bm_check)
-				acpi_set_register(ACPI_BITREG_BUS_MASTER_RLD, 0);
-			break;
-		}
-	}
-
-	/* Prepare to use new state. */
-	switch (new->type) {
-	case ACPI_STATE_C3:
-		/* Enable bus master reload */
-		if (old->type != ACPI_STATE_C3 && pr->flags.bm_check)
-			acpi_set_register(ACPI_BITREG_BUS_MASTER_RLD, 1);
-		break;
-	}
-
-	pr->power.state = new;
-
-	return;
-}
-
-static void acpi_safe_halt(void)
-{
-	current_thread_info()->status &= ~TS_POLLING;
-	/*
-	 * TS_POLLING-cleared state must be visible before we
-	 * test NEED_RESCHED:
-	 */
-	smp_mb();
-	if (!need_resched())
-		safe_halt();
-	current_thread_info()->status |= TS_POLLING;
-}
-
-static atomic_t c3_cpu_count;
-
-/* Common C-state entry for C2, C3, .. */
-static void acpi_cstate_enter(struct acpi_processor_cx *cstate)
-{
-	if (cstate->space_id == ACPI_CSTATE_FFH) {
-		/* Call into architectural FFH based C-state */
-		acpi_processor_ffh_cstate_enter(cstate);
-	} else {
-		int unused;
-		/* IO port based C-state */
-		inb(cstate->address);
-		/* Dummy wait op - must do something useless after P_LVL2 read
-		   because chipsets cannot guarantee that STPCLK# signal
-		   gets asserted in time to freeze execution properly. */
-		unused = inl(acpi_gbl_FADT.xpm_timer_block.address);
-	}
-}
-
 #ifdef ARCH_APICTIMER_STOPS_ON_C3
 
 /*
@@ -324,377 +233,6 @@ static void acpi_state_timer_broadcast(s
 
 #endif
 
-static void acpi_processor_idle(void)
-{
-	struct acpi_processor *pr = NULL;
-	struct acpi_processor_cx *cx = NULL;
-	struct acpi_processor_cx *next_state = NULL;
-	int sleep_ticks = 0;
-	u32 t1, t2 = 0;
-
-	/*
-	 * Interrupts must be disabled during bus mastering calculations and
-	 * for C2/C3 transitions.
-	 */
-	local_irq_disable();
-
-	pr = processors[smp_processor_id()];
-	if (!pr) {
-		local_irq_enable();
-		return;
-	}
-
-	/*
-	 * Check whether we truly need to go idle, or should
-	 * reschedule:
-	 */
-	if (unlikely(need_resched())) {
-		local_irq_enable();
-		return;
-	}
-
-	cx = pr->power.state;
-	if (!cx) {
-		if (pm_idle_save)
-			pm_idle_save();
-		else
-			acpi_safe_halt();
-		return;
-	}
-
-	/*
-	 * Check BM Activity
-	 * -----------------
-	 * Check for bus mastering activity (if required), record, and check
-	 * for demotion.
-	 */
-	if (pr->flags.bm_check) {
-		u32 bm_status = 0;
-		unsigned long diff = jiffies - pr->power.bm_check_timestamp;
-
-		if (diff > 31)
-			diff = 31;
-
-		pr->power.bm_activity <<= diff;
-
-		acpi_get_register(ACPI_BITREG_BUS_MASTER_STATUS, &bm_status);
-		if (bm_status) {
-			pr->power.bm_activity |= 0x1;
-			acpi_set_register(ACPI_BITREG_BUS_MASTER_STATUS, 1);
-		}
-		/*
-		 * PIIX4 Erratum #18: Note that BM_STS doesn't always reflect
-		 * the true state of bus mastering activity; forcing us to
-		 * manually check the BMIDEA bit of each IDE channel.
-		 */
-		else if (errata.piix4.bmisx) {
-			if ((inb_p(errata.piix4.bmisx + 0x02) & 0x01)
-			    || (inb_p(errata.piix4.bmisx + 0x0A) & 0x01))
-				pr->power.bm_activity |= 0x1;
-		}
-
-		pr->power.bm_check_timestamp = jiffies;
-
-		/*
-		 * If bus mastering is or was active this jiffy, demote
-		 * to avoid a faulty transition.  Note that the processor
-		 * won't enter a low-power state during this call (to this
-		 * function) but should upon the next.
-		 *
-		 * TBD: A better policy might be to fallback to the demotion
-		 *      state (use it for this quantum only) istead of
-		 *      demoting -- and rely on duration as our sole demotion
-		 *      qualification.  This may, however, introduce DMA
-		 *      issues (e.g. floppy DMA transfer overrun/underrun).
-		 */
-		if ((pr->power.bm_activity & 0x1) &&
-		    cx->demotion.threshold.bm) {
-			local_irq_enable();
-			next_state = cx->demotion.state;
-			goto end;
-		}
-	}
-
-#ifdef CONFIG_HOTPLUG_CPU
-	/*
-	 * Check for P_LVL2_UP flag before entering C2 and above on
-	 * an SMP system. We do it here instead of doing it at _CST/P_LVL
-	 * detection phase, to work cleanly with logical CPU hotplug.
-	 */
-	if ((cx->type != ACPI_STATE_C1) && (num_online_cpus() > 1) && 
-	    !pr->flags.has_cst && !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
-		cx = &pr->power.states[ACPI_STATE_C1];
-#endif
-
-	/*
-	 * Sleep:
-	 * ------
-	 * Invoke the current Cx state to put the processor to sleep.
-	 */
-	if (cx->type == ACPI_STATE_C2 || cx->type == ACPI_STATE_C3) {
-		current_thread_info()->status &= ~TS_POLLING;
-		/*
-		 * TS_POLLING-cleared state must be visible before we
-		 * test NEED_RESCHED:
-		 */
-		smp_mb();
-		if (need_resched()) {
-			current_thread_info()->status |= TS_POLLING;
-			local_irq_enable();
-			return;
-		}
-	}
-
-	switch (cx->type) {
-
-	case ACPI_STATE_C1:
-		/*
-		 * Invoke C1.
-		 * Use the appropriate idle routine, the one that would
-		 * be used without acpi C-states.
-		 */
-		if (pm_idle_save)
-			pm_idle_save();
-		else
-			acpi_safe_halt();
-
-		/*
-		 * TBD: Can't get time duration while in C1, as resumes
-		 *      go to an ISR rather than here.  Need to instrument
-		 *      base interrupt handler.
-		 */
-		sleep_ticks = 0xFFFFFFFF;
-		break;
-
-	case ACPI_STATE_C2:
-		/* Get start time (ticks) */
-		t1 = inl(acpi_gbl_FADT.xpm_timer_block.address);
-		/* Invoke C2 */
-		acpi_state_timer_broadcast(pr, cx, 1);
-		acpi_cstate_enter(cx);
-		/* Get end time (ticks) */
-		t2 = inl(acpi_gbl_FADT.xpm_timer_block.address);
-
-#ifdef CONFIG_GENERIC_TIME
-		/* TSC halts in C2, so notify users */
-		mark_tsc_unstable("possible TSC halt in C2");
-#endif
-		/* Re-enable interrupts */
-		local_irq_enable();
-		current_thread_info()->status |= TS_POLLING;
-		/* Compute time (ticks) that we were actually asleep */
-		sleep_ticks =
-		    ticks_elapsed(t1, t2) - cx->latency_ticks - C2_OVERHEAD;
-		acpi_state_timer_broadcast(pr, cx, 0);
-		break;
-
-	case ACPI_STATE_C3:
-
-		if (pr->flags.bm_check) {
-			if (atomic_inc_return(&c3_cpu_count) ==
-			    num_online_cpus()) {
-				/*
-				 * All CPUs are trying to go to C3
-				 * Disable bus master arbitration
-				 */
-				acpi_set_register(ACPI_BITREG_ARB_DISABLE, 1);
-			}
-		} else {
-			/* SMP with no shared cache... Invalidate cache  */
-			ACPI_FLUSH_CPU_CACHE();
-		}
-
-		/* Get start time (ticks) */
-		t1 = inl(acpi_gbl_FADT.xpm_timer_block.address);
-		/* Invoke C3 */
-		acpi_state_timer_broadcast(pr, cx, 1);
-		acpi_cstate_enter(cx);
-		/* Get end time (ticks) */
-		t2 = inl(acpi_gbl_FADT.xpm_timer_block.address);
-		if (pr->flags.bm_check) {
-			/* Enable bus master arbitration */
-			atomic_dec(&c3_cpu_count);
-			acpi_set_register(ACPI_BITREG_ARB_DISABLE, 0);
-		}
-
-#ifdef CONFIG_GENERIC_TIME
-		/* TSC halts in C3, so notify users */
-		mark_tsc_unstable("TSC halts in C3");
-#endif
-		/* Re-enable interrupts */
-		local_irq_enable();
-		current_thread_info()->status |= TS_POLLING;
-		/* Compute time (ticks) that we were actually asleep */
-		sleep_ticks =
-		    ticks_elapsed(t1, t2) - cx->latency_ticks - C3_OVERHEAD;
-		acpi_state_timer_broadcast(pr, cx, 0);
-		break;
-
-	default:
-		local_irq_enable();
-		return;
-	}
-	cx->usage++;
-	if ((cx->type != ACPI_STATE_C1) && (sleep_ticks > 0))
-		cx->time += sleep_ticks;
-
-	next_state = pr->power.state;
-
-#ifdef CONFIG_HOTPLUG_CPU
-	/* Don't do promotion/demotion */
-	if ((cx->type == ACPI_STATE_C1) && (num_online_cpus() > 1) &&
-	    !pr->flags.has_cst && !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED)) {
-		next_state = cx;
-		goto end;
-	}
-#endif
-
-	/*
-	 * Promotion?
-	 * ----------
-	 * Track the number of longs (time asleep is greater than threshold)
-	 * and promote when the count threshold is reached.  Note that bus
-	 * mastering activity may prevent promotions.
-	 * Do not promote above max_cstate.
-	 */
-	if (cx->promotion.state &&
-	    ((cx->promotion.state - pr->power.states) <= max_cstate)) {
-		if (sleep_ticks > cx->promotion.threshold.ticks &&
-		  cx->promotion.state->latency <= system_latency_constraint()) {
-			cx->promotion.count++;
-			cx->demotion.count = 0;
-			if (cx->promotion.count >=
-			    cx->promotion.threshold.count) {
-				if (pr->flags.bm_check) {
-					if (!
-					    (pr->power.bm_activity & cx->
-					     promotion.threshold.bm)) {
-						next_state =
-						    cx->promotion.state;
-						goto end;
-					}
-				} else {
-					next_state = cx->promotion.state;
-					goto end;
-				}
-			}
-		}
-	}
-
-	/*
-	 * Demotion?
-	 * ---------
-	 * Track the number of shorts (time asleep is less than time threshold)
-	 * and demote when the usage threshold is reached.
-	 */
-	if (cx->demotion.state) {
-		if (sleep_ticks < cx->demotion.threshold.ticks) {
-			cx->demotion.count++;
-			cx->promotion.count = 0;
-			if (cx->demotion.count >= cx->demotion.threshold.count) {
-				next_state = cx->demotion.state;
-				goto end;
-			}
-		}
-	}
-
-      end:
-	/*
-	 * Demote if current state exceeds max_cstate
-	 * or if the latency of the current state is unacceptable
-	 */
-	if ((pr->power.state - pr->power.states) > max_cstate ||
-		pr->power.state->latency > system_latency_constraint()) {
-		if (cx->demotion.state)
-			next_state = cx->demotion.state;
-	}
-
-	/*
-	 * New Cx State?
-	 * -------------
-	 * If we're going to start using a new Cx state we must clean up
-	 * from the previous and prepare to use the new.
-	 */
-	if (next_state != pr->power.state)
-		acpi_processor_power_activate(pr, next_state);
-}
-
-static int acpi_processor_set_power_policy(struct acpi_processor *pr)
-{
-	unsigned int i;
-	unsigned int state_is_set = 0;
-	struct acpi_processor_cx *lower = NULL;
-	struct acpi_processor_cx *higher = NULL;
-	struct acpi_processor_cx *cx;
-
-
-	if (!pr)
-		return -EINVAL;
-
-	/*
-	 * This function sets the default Cx state policy (OS idle handler).
-	 * Our scheme is to promote quickly to C2 but more conservatively
-	 * to C3.  We're favoring C2  for its characteristics of low latency
-	 * (quick response), good power savings, and ability to allow bus
-	 * mastering activity.  Note that the Cx state policy is completely
-	 * customizable and can be altered dynamically.
-	 */
-
-	/* startup state */
-	for (i = 1; i < ACPI_PROCESSOR_MAX_POWER; i++) {
-		cx = &pr->power.states[i];
-		if (!cx->valid)
-			continue;
-
-		if (!state_is_set)
-			pr->power.state = cx;
-		state_is_set++;
-		break;
-	}
-
-	if (!state_is_set)
-		return -ENODEV;
-
-	/* demotion */
-	for (i = 1; i < ACPI_PROCESSOR_MAX_POWER; i++) {
-		cx = &pr->power.states[i];
-		if (!cx->valid)
-			continue;
-
-		if (lower) {
-			cx->demotion.state = lower;
-			cx->demotion.threshold.ticks = cx->latency_ticks;
-			cx->demotion.threshold.count = 1;
-			if (cx->type == ACPI_STATE_C3)
-				cx->demotion.threshold.bm = bm_history;
-		}
-
-		lower = cx;
-	}
-
-	/* promotion */
-	for (i = (ACPI_PROCESSOR_MAX_POWER - 1); i > 0; i--) {
-		cx = &pr->power.states[i];
-		if (!cx->valid)
-			continue;
-
-		if (higher) {
-			cx->promotion.state = higher;
-			cx->promotion.threshold.ticks = cx->latency_ticks;
-			if (cx->type >= ACPI_STATE_C2)
-				cx->promotion.threshold.count = 4;
-			else
-				cx->promotion.threshold.count = 10;
-			if (higher->type == ACPI_STATE_C3)
-				cx->promotion.threshold.bm = bm_history;
-		}
-
-		higher = cx;
-	}
-
-	return 0;
-}
-
 static int acpi_processor_get_power_info_fadt(struct acpi_processor *pr)
 {
 
@@ -912,7 +450,7 @@ static void acpi_processor_power_verify_
 	 * Normalize the C2 latency to expidite policy
 	 */
 	cx->valid = 1;
-	cx->latency_ticks = US_TO_PM_TIMER_TICKS(cx->latency);
+	cx->latency_ticks = cx->latency;
 
 	return;
 }
@@ -986,7 +524,7 @@ static void acpi_processor_power_verify_
 	 * use this in our C3 policy
 	 */
 	cx->valid = 1;
-	cx->latency_ticks = US_TO_PM_TIMER_TICKS(cx->latency);
+	cx->latency_ticks = cx->latency;
 
 	return;
 }
@@ -1052,18 +590,6 @@ static int acpi_processor_get_power_info
 	pr->power.count = acpi_processor_power_verify(pr);
 
 	/*
-	 * Set Default Policy
-	 * ------------------
-	 * Now that we know which states are supported, set the default
-	 * policy.  Note that this policy can be changed dynamically
-	 * (e.g. encourage deeper sleeps to conserve battery life when
-	 * not on AC).
-	 */
-	result = acpi_processor_set_power_policy(pr);
-	if (result)
-		return result;
-
-	/*
 	 * if one state of type C2 or C3 is available, mark this
 	 * CPU as being "idle manageable"
 	 */
@@ -1080,9 +606,6 @@ static int acpi_processor_get_power_info
 
 int acpi_processor_cst_has_changed(struct acpi_processor *pr)
 {
-	int result = 0;
-
-
 	if (!pr)
 		return -EINVAL;
 
@@ -1093,16 +616,8 @@ int acpi_processor_cst_has_changed(struc
 	if (!pr->flags.power_setup_done)
 		return -ENODEV;
 
-	/* Fall back to the default idle loop */
-	pm_idle = pm_idle_save;
-	synchronize_sched();	/* Relies on interrupts forcing exit from idle. */
-
-	pr->flags.power = 0;
-	result = acpi_processor_get_power_info(pr);
-	if ((pr->flags.power == 1) && (pr->flags.power_setup_done))
-		pm_idle = acpi_processor_idle;
-
-	return result;
+	acpi_processor_get_power_info(pr);
+	return cpuidle_force_redetect(&per_cpu(cpuidle_devices, pr->id));
 }
 
 /* proc interface */
@@ -1188,30 +703,6 @@ static const struct file_operations acpi
 	.release = single_release,
 };
 
-#ifdef CONFIG_SMP
-static void smp_callback(void *v)
-{
-	/* we already woke the CPU up, nothing more to do */
-}
-
-/*
- * This function gets called when a part of the kernel has a new latency
- * requirement.  This means we need to get all processors out of their C-state,
- * and then recalculate a new suitable C-state. Just do a cross-cpu IPI; that
- * wakes them all right up.
- */
-static int acpi_processor_latency_notify(struct notifier_block *b,
-		unsigned long l, void *v)
-{
-	smp_call_function(smp_callback, NULL, 0, 1);
-	return NOTIFY_OK;
-}
-
-static struct notifier_block acpi_processor_latency_notifier = {
-	.notifier_call = acpi_processor_latency_notify,
-};
-#endif
-
 int __cpuinit acpi_processor_power_init(struct acpi_processor *pr,
 			      struct acpi_device *device)
 {
@@ -1228,9 +719,6 @@ int __cpuinit acpi_processor_power_init(
 			       "ACPI: processor limited to max C-state %d\n",
 			       max_cstate);
 		first_run++;
-#ifdef CONFIG_SMP
-		register_latency_notifier(&acpi_processor_latency_notifier);
-#endif
 	}
 
 	if (!pr)
@@ -1247,6 +735,7 @@ #endif
 
 	acpi_processor_get_power_info(pr);
 
+
 	/*
 	 * Install the idle handler if processor power management is supported.
 	 * Note that we use previously set idle handler will be used on
@@ -1259,11 +748,6 @@ #endif
 				printk(" C%d[C%d]", i,
 				       pr->power.states[i].type);
 		printk(")\n");
-
-		if (pr->id == 0) {
-			pm_idle_save = pm_idle;
-			pm_idle = acpi_processor_idle;
-		}
 	}
 
 	/* 'power' [R] */
@@ -1291,21 +775,332 @@ int acpi_processor_power_exit(struct acp
 	if (acpi_device_dir(device))
 		remove_proc_entry(ACPI_PROCESSOR_FILE_POWER,
 				  acpi_device_dir(device));
+	return 0;
+}
+
+/**
+ * ticks_elapsed - a helper function that determines how many ticks (in US)
+ *		   have elapsed between two PM Timer timestamps
+ * @t1: the start time
+ * @t2: the end time
+ */
+static inline u32 ticks_elapsed(u32 t1, u32 t2)
+{
+	if (t2 >= t1)
+		return PM_TIMER_TICKS_TO_US(t2 - t1);
+	else if (!(acpi_gbl_FADT.flags & ACPI_FADT_32BIT_TIMER))
+		return PM_TIMER_TICKS_TO_US(((0x00FFFFFF - t1) + t2) & 0x00FFFFFF);
+	else
+		return PM_TIMER_TICKS_TO_US((0xFFFFFFFF - t1) + t2);
+}
 
-	/* Unregister the idle handler when processor #0 is removed. */
-	if (pr->id == 0) {
-		pm_idle = pm_idle_save;
+/**
+ * acpi_idle_update_bm_rld - updates the BM_RLD bit depending on target state
+ * @pr: the processor
+ * @target: the new target state
+ */
+static inline void acpi_idle_update_bm_rld(struct acpi_processor *pr,
+					   struct acpi_processor_cx *target)
+{
+	if (pr->flags.bm_rld_set && target->type != ACPI_STATE_C3) {
+		acpi_set_register(ACPI_BITREG_BUS_MASTER_RLD, 0);
+		pr->flags.bm_rld_set = 0;
+	}
 
-		/*
-		 * We are about to unload the current idle thread pm callback
-		 * (pm_idle), Wait for all processors to update cached/local
-		 * copies of pm_idle before proceeding.
-		 */
-		cpu_idle_wait();
-#ifdef CONFIG_SMP
-		unregister_latency_notifier(&acpi_processor_latency_notifier);
+	if (!pr->flags.bm_rld_set && target->type == ACPI_STATE_C3) {
+		acpi_set_register(ACPI_BITREG_BUS_MASTER_RLD, 1);
+		pr->flags.bm_rld_set = 1;
+	}
+}
+
+/**
+ * acpi_idle_do_entry - a helper function that does C2 and C3 type entry
+ * @cx: cstate data
+ */
+static inline void acpi_idle_do_entry(struct acpi_processor_cx *cx)
+{
+	if (cx->space_id == ACPI_CSTATE_FFH) {
+		/* Call into architectural FFH based C-state */
+		acpi_processor_ffh_cstate_enter(cx);
+	} else {
+		int unused;
+		/* IO port based C-state */
+		inb(cx->address);
+		/* Dummy wait op - must do something useless after P_LVL2 read
+		   because chipsets cannot guarantee that STPCLK# signal
+		   gets asserted in time to freeze execution properly. */
+		unused = inl(acpi_gbl_FADT.xpm_timer_block.address);
+	}
+}
+
+/**
+ * acpi_idle_enter_c1 - enters an ACPI C1 state-type
+ * @dev: the target CPU
+ * @state: the state data
+ *
+ * This is equivalent to the HALT instruction.
+ */
+static int acpi_idle_enter_c1(struct cpuidle_device *dev,
+			      struct cpuidle_state *state)
+{
+	struct acpi_processor *pr;
+	struct acpi_processor_cx *cx = cpuidle_get_statedata(state);
+	pr = processors[smp_processor_id()];
+
+	if (unlikely(!pr))
+		return 0;
+
+	if (pr->flags.bm_check)
+		acpi_idle_update_bm_rld(pr, cx);
+
+	current_thread_info()->status &= ~TS_POLLING;
+	/*
+	 * TS_POLLING-cleared state must be visible before we test
+	 * NEED_RESCHED:
+	 */
+	smp_mb();
+	if (!need_resched())
+		safe_halt();
+	current_thread_info()->status |= TS_POLLING;
+
+	cx->usage++;
+
+	return 0;
+}
+
+/**
+ * acpi_idle_enter_c2 - enters an ACPI C2 state-type
+ * @dev: the target CPU
+ * @state: the state data
+ */
+static int acpi_idle_enter_c2(struct cpuidle_device *dev,
+			      struct cpuidle_state *state)
+{
+	struct acpi_processor *pr;
+	struct acpi_processor_cx *cx = cpuidle_get_statedata(state);
+	u32 t1, t2;
+	pr = processors[smp_processor_id()];
+
+	if (unlikely(!pr))
+		return 0;
+
+	if (pr->flags.bm_check)
+		acpi_idle_update_bm_rld(pr, cx);
+
+	local_irq_disable();
+	current_thread_info()->status &= ~TS_POLLING;
+	/*
+	 * TS_POLLING-cleared state must be visible before we test
+	 * NEED_RESCHED:
+	 */
+	smp_mb();
+
+	if (unlikely(need_resched())) {
+		current_thread_info()->status |= TS_POLLING;
+		local_irq_enable();
+		return 0;
+	}
+
+	t1 = inl(acpi_gbl_FADT.xpm_timer_block.address);
+	acpi_state_timer_broadcast(pr, cx, 1);
+	acpi_idle_do_entry(cx);
+	t2 = inl(acpi_gbl_FADT.xpm_timer_block.address);
+
+#ifdef CONFIG_GENERIC_TIME
+	/* TSC halts in C2, so notify users */
+	mark_tsc_unstable("possible TSC halt in C2");
 #endif
+
+	local_irq_enable();
+	current_thread_info()->status |= TS_POLLING;
+
+	cx->usage++;
+
+	acpi_state_timer_broadcast(pr, cx, 0);
+	return ticks_elapsed(t1, t2);
+}
+
+static int c3_cpu_count;
+static DEFINE_SPINLOCK(c3_lock);
+
+/**
+ * acpi_idle_enter_c3 - enters an ACPI C3 state-type
+ * @dev: the target CPU
+ * @state: the state data
+ *
+ * Similar to C2 entry, except special bus master handling is needed.
+ */
+static int acpi_idle_enter_c3(struct cpuidle_device *dev,
+			      struct cpuidle_state *state)
+{
+	struct acpi_processor *pr;
+	struct acpi_processor_cx *cx = cpuidle_get_statedata(state);
+	u32 t1, t2;
+	pr = processors[smp_processor_id()];
+
+	if (unlikely(!pr))
+		return 0;
+
+	if (pr->flags.bm_check)
+		acpi_idle_update_bm_rld(pr, cx);
+
+	local_irq_disable();
+	current_thread_info()->status &= ~TS_POLLING;
+	/*
+	 * TS_POLLING-cleared state must be visible before we test
+	 * NEED_RESCHED:
+	 */
+	smp_mb();
+
+	if (unlikely(need_resched())) {
+		current_thread_info()->status |= TS_POLLING;
+		local_irq_enable();
+		return 0;
+	}
+
+	/* disable bus master */
+	if (pr->flags.bm_check) {
+		spin_lock(&c3_lock);
+			c3_cpu_count++;
+		if (c3_cpu_count == num_online_cpus()) {
+			/*
+			 * All CPUs are trying to go to C3
+			 * Disable bus master arbitration
+			 */
+			acpi_set_register(ACPI_BITREG_ARB_DISABLE, 1);
+		}
+		spin_unlock(&c3_lock);
+	} else {
+		/* SMP with no shared cache... Invalidate cache  */
+		ACPI_FLUSH_CPU_CACHE();
+	}
+
+	/* Get start time (ticks) */
+	t1 = inl(acpi_gbl_FADT.xpm_timer_block.address);
+	acpi_state_timer_broadcast(pr, cx, 1);
+	acpi_idle_do_entry(cx);
+	t2 = inl(acpi_gbl_FADT.xpm_timer_block.address);
+
+	if (pr->flags.bm_check) {
+		spin_lock(&c3_lock);
+		/* Enable bus master arbitration */
+		if (c3_cpu_count == num_online_cpus())
+			acpi_set_register(ACPI_BITREG_ARB_DISABLE, 0);
+		c3_cpu_count--;
+		spin_unlock(&c3_lock);
 	}
 
+#ifdef CONFIG_GENERIC_TIME
+	/* TSC halts in C3, so notify users */
+	mark_tsc_unstable("TSC halts in C3");
+#endif
+
+	local_irq_enable();
+	current_thread_info()->status |= TS_POLLING;
+
+	cx->usage++;
+
+	acpi_state_timer_broadcast(pr, cx, 0);
+	return ticks_elapsed(t1, t2);
+}
+
+/**
+ * acpi_idle_bm_check - checks if bus master activity was detected
+ */
+static int acpi_idle_bm_check(void)
+{
+	u32 bm_status = 0;
+
+	acpi_get_register(ACPI_BITREG_BUS_MASTER_STATUS, &bm_status);
+	if (bm_status)
+		acpi_set_register(ACPI_BITREG_BUS_MASTER_STATUS, 1);
+	/*
+	 * PIIX4 Erratum #18: Note that BM_STS doesn't always reflect
+	 * the true state of bus mastering activity; forcing us to
+	 * manually check the BMIDEA bit of each IDE channel.
+	 */
+	else if (errata.piix4.bmisx) {
+		if ((inb_p(errata.piix4.bmisx + 0x02) & 0x01)
+		    || (inb_p(errata.piix4.bmisx + 0x0A) & 0x01))
+			bm_status = 1;
+	}
+	return bm_status;
+}
+
+/**
+ * acpi_idle_init - attaches the driver to a CPU
+ * @dev: the CPU
+ */
+static int acpi_idle_init(struct cpuidle_device *dev)
+{
+	int cpu = dev->cpu;
+	int i, count = 0;
+	struct acpi_processor_cx *cx;
+	struct cpuidle_state *state;
+
+	struct acpi_processor *pr = processors[cpu];
+
+	if (!pr->flags.power_setup_done)
+		return -EINVAL;
+
+	if (pr->flags.power == 0) {
+		return -EINVAL;
+	}
+
+	for (i = 1; i < ACPI_PROCESSOR_MAX_POWER; i++) {
+		cx = &pr->power.states[i];
+		state = &dev->states[count];
+
+		if (!cx->valid)
+			continue;
+
+#ifdef CONFIG_HOTPLUG_CPU
+		if ((cx->type != ACPI_STATE_C1) && (num_online_cpus() > 1) &&
+		    !pr->flags.has_cst &&
+		    !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
+			continue;
+#endif
+		cpuidle_set_statedata(state, cx);
+
+		state->exit_latency = cx->latency;
+		state->target_residency = cx->latency * 6;
+		state->power_usage = cx->power;
+
+		state->flags = 0;
+		switch (cx->type) {
+			case ACPI_STATE_C1:
+			state->flags |= CPUIDLE_FLAG_SHALLOW;
+			state->enter = acpi_idle_enter_c1;
+			break;
+
+			case ACPI_STATE_C2:
+			state->flags |= CPUIDLE_FLAG_BALANCED;
+			state->flags |= CPUIDLE_FLAG_TIME_VALID;
+			state->enter = acpi_idle_enter_c2;
+			break;
+
+			case ACPI_STATE_C3:
+			state->flags |= CPUIDLE_FLAG_DEEP;
+			state->flags |= CPUIDLE_FLAG_TIME_VALID;
+			state->flags |= CPUIDLE_FLAG_CHECK_BM;
+			state->enter = acpi_idle_enter_c3;
+			break;
+		}
+
+		count++;
+	}
+
+	if (!count)
+		return -EINVAL;
+
+	dev->state_count = count;
 	return 0;
 }
+
+struct cpuidle_driver acpi_idle_driver = {
+	.name =		"acpi_idle",
+	.init =		acpi_idle_init,
+	.redetect =	acpi_idle_init,
+	.bm_check =	acpi_idle_bm_check,
+	.owner =	THIS_MODULE,
+};
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index b4b0ffd..dcd6aca 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -161,6 +161,7 @@ struct acpi_processor_flags {
 	u8 bm_check:1;
 	u8 has_cst:1;
 	u8 power_setup_done:1;
+	u8 bm_rld_set:1;
 };
 
 struct acpi_processor {
@@ -279,6 +280,7 @@ int acpi_processor_power_init(struct acp
 int acpi_processor_cst_has_changed(struct acpi_processor *pr);
 int acpi_processor_power_exit(struct acpi_processor *pr,
 			      struct acpi_device *device);
+extern struct cpuidle_driver acpi_idle_driver;
 
 /* in processor_thermal.c */
 int acpi_processor_get_limit_info(struct acpi_processor *pr);









hackpatch:

 acpi/processor.h              |    0 
 drivers/acpi/processor_core.c |    4 ++--
 drivers/acpi/processor_idle.c |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff -puN drivers/acpi/processor_core.c~ef5f15a8b79123a047285ec2e3899108661df779-fix drivers/acpi/processor_core.c
--- a/drivers/acpi/processor_core.c~ef5f15a8b79123a047285ec2e3899108661df779-fix
+++ a/drivers/acpi/processor_core.c
@@ -1039,13 +1039,13 @@ static int __init acpi_processor_init(vo
 
 	acpi_processor_ppc_init();
 
-	cpuidle_register_driver(&acpi_idle_driver);
+//	cpuidle_register_driver(&acpi_idle_driver);
 	return 0;
 }
 
 static void __exit acpi_processor_exit(void)
 {
-	cpuidle_unregister_driver(&acpi_idle_driver);
+//	cpuidle_unregister_driver(&acpi_idle_driver);
 
 	acpi_processor_ppc_exit();
 
diff -puN drivers/acpi/processor_idle.c~ef5f15a8b79123a047285ec2e3899108661df779-fix drivers/acpi/processor_idle.c
--- a/drivers/acpi/processor_idle.c~ef5f15a8b79123a047285ec2e3899108661df779-fix
+++ a/drivers/acpi/processor_idle.c
@@ -633,7 +633,7 @@ int acpi_processor_cst_has_changed(struc
 		return -ENODEV;
 
 	acpi_processor_get_power_info(pr);
-	return cpuidle_force_redetect(&per_cpu(cpuidle_devices, pr->id));
+	return 0;
 }
 
 /* proc interface */
diff -puN include/acpi/processor.h~ef5f15a8b79123a047285ec2e3899108661df779-fix include/acpi/processor.h
_

Re: clockevents: fix resume logic

[Rafael J. Wysocki]

On Tuesday, 11 September 2007 12:31, Andrew Morton wrote:
> On Tue, 11 Sep 2007 11:16:04 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > On Tue, 2007-09-11 at 01:35 -0700, Andrew Morton wrote:
> > > On Tue, 11 Sep 2007 01:20:05 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> > > 
> > > > On Tue, 11 Sep 2007 09:47:20 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> > > > 
> > > > > On Tue, 2007-09-11 at 09:23 +0200, Thomas Gleixner wrote:
> > > > > > > I went back to the original patch which I sent to Linus and it matches
> > > > > > > 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6.  So all I can think is that there
> > > > > > > must have been something else in the tree which I tested which fixed the
> > > > > > > bug which 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 introduced.  argh.
> > > > > > > 
> > > > > > > Can you think what would cause the symptoms which I described?  
> > > > > > 
> > > > > > It seems that time is not updated. Timer interrupt not active or some
> > > > > > other odd thing. I figure out what's going on when I find a box which
> > > > > > exposes the problem.
> > > > 
> > > > 2.6.22-rc6-mm1's git-acpi.patch contains something which fixes this bug. 
> > > 
> > > Len's current tree fixes it too.

Do you mean this one:

http://git.kernel.org/?p=linux/kernel/git/lenb/linux-acpi-2.6.git

Re: clockevents: fix resume logic

[Rafael J. Wysocki]

On Tuesday, 11 September 2007 13:22, Andrew Morton wrote:
> On Tue, 11 Sep 2007 13:23:55 +0200 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > On Tuesday, 11 September 2007 12:31, Andrew Morton wrote:
> > > On Tue, 11 Sep 2007 11:16:04 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> > > 
> > > > On Tue, 2007-09-11 at 01:35 -0700, Andrew Morton wrote:
> > > > > On Tue, 11 Sep 2007 01:20:05 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> > > > > 
> > > > > > On Tue, 11 Sep 2007 09:47:20 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> > > > > > 
> > > > > > > On Tue, 2007-09-11 at 09:23 +0200, Thomas Gleixner wrote:
> > > > > > > > > I went back to the original patch which I sent to Linus and it matches
> > > > > > > > > 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6.  So all I can think is that there
> > > > > > > > > must have been something else in the tree which I tested which fixed the
> > > > > > > > > bug which 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 introduced.  argh.
> > > > > > > > > 
> > > > > > > > > Can you think what would cause the symptoms which I described?  
> > > > > > > > 
> > > > > > > > It seems that time is not updated. Timer interrupt not active or some
> > > > > > > > other odd thing. I figure out what's going on when I find a box which
> > > > > > > > exposes the problem.
> > > > > > 
> > > > > > 2.6.22-rc6-mm1's git-acpi.patch contains something which fixes this bug. 
> > > > > 
> > > > > Len's current tree fixes it too.
> > 
> > Do you mean this one:
> > 
> > http://git.kernel.org/?p=linux/kernel/git/lenb/linux-acpi-2.6.git
> 
> Nope.
> 
> Here's the algorthm: go to the latest -mm tree and look at the first line
> of broken-out/git-acpi.patch:
> 
> GIT f94aac9883f9b02700270cf286577a9bccf98f47 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git#test
> 
> that gives the URL, the branch and the top-level commit.  All the -mm git
> trees have that first line.
> 
> 
> 
> Anyway, I was able to extract all the diffs and generate a patch series. 
> The below patch fixes current mainline on the Vaio.

It evidently assumes cpuidle to be present, which is not in the mainline.

It seems to me that the total effect of this one and the hackpatch is that
the C states are not handled any more.

Re: clockevents: fix resume logic

[Andrew Morton]

On Tue, 11 Sep 2007 14:09:12 +0200 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> On Tuesday, 11 September 2007 13:22, Andrew Morton wrote:
> > On Tue, 11 Sep 2007 13:23:55 +0200 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > 
> > > On Tuesday, 11 September 2007 12:31, Andrew Morton wrote:
> > > > On Tue, 11 Sep 2007 11:16:04 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> > > > 
> > > > > On Tue, 2007-09-11 at 01:35 -0700, Andrew Morton wrote:
> > > > > > On Tue, 11 Sep 2007 01:20:05 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> > > > > > 
> > > > > > > On Tue, 11 Sep 2007 09:47:20 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> > > > > > > 
> > > > > > > > On Tue, 2007-09-11 at 09:23 +0200, Thomas Gleixner wrote:
> > > > > > > > > > I went back to the original patch which I sent to Linus and it matches
> > > > > > > > > > 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6.  So all I can think is that there
> > > > > > > > > > must have been something else in the tree which I tested which fixed the
> > > > > > > > > > bug which 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 introduced.  argh.
> > > > > > > > > > 
> > > > > > > > > > Can you think what would cause the symptoms which I described?  
> > > > > > > > > 
> > > > > > > > > It seems that time is not updated. Timer interrupt not active or some
> > > > > > > > > other odd thing. I figure out what's going on when I find a box which
> > > > > > > > > exposes the problem.
> > > > > > > 
> > > > > > > 2.6.22-rc6-mm1's git-acpi.patch contains something which fixes this bug. 
> > > > > > 
> > > > > > Len's current tree fixes it too.
> > > 
> > > Do you mean this one:
> > > 
> > > http://git.kernel.org/?p=linux/kernel/git/lenb/linux-acpi-2.6.git
> > 
> > Nope.
> > 
> > Here's the algorthm: go to the latest -mm tree and look at the first line
> > of broken-out/git-acpi.patch:
> > 
> > GIT f94aac9883f9b02700270cf286577a9bccf98f47 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git#test
> > 
> > that gives the URL, the branch and the top-level commit.  All the -mm git
> > trees have that first line.
> > 
> > 
> > 
> > Anyway, I was able to extract all the diffs and generate a patch series. 
> > The below patch fixes current mainline on the Vaio.
> 
> It evidently assumes cpuidle to be present, which is not in the mainline.

Bear in mind that the cpuidle patch fixes resume-from-ram when cpuidle is
disabled in config.

> It seems to me that the total effect of this one and the hackpatch is that
> the C states are not handled any more.

hm.

dmesg without the cpuidle patch: http://userweb.kernel.org/~akpm/dmesg-bad.txt
dmesg with the cpuidle patch: http://userweb.kernel.org/~akpm/dmesg-good.txt
difference: http://userweb.kernel.org/~akpm/dmesg-diff.txt

there doesn't seem to be a lot of difference in the time handling, except
there are large changes in when things happen in the bootup sequence.

Re: clockevents: fix resume logic

[Thomas Gleixner]

On Tue, 2007-09-11 at 11:25 -0700, Andrew Morton wrote:
> > It evidently assumes cpuidle to be present, which is not in the mainline.
> 
> Bear in mind that the cpuidle patch fixes resume-from-ram when cpuidle is
> disabled in config.
> 
> > It seems to me that the total effect of this one and the hackpatch is that
> > the C states are not handled any more.
> 
> hm.
> 
> dmesg without the cpuidle patch: http://userweb.kernel.org/~akpm/dmesg-bad.txt
> dmesg with the cpuidle patch: http://userweb.kernel.org/~akpm/dmesg-good.txt
> difference: http://userweb.kernel.org/~akpm/dmesg-diff.txt
> 
> there doesn't seem to be a lot of difference in the time handling, except
> there are large changes in when things happen in the bootup sequence.

The question is whether the system goes into C2 with the patch applied.

Can you please provide the output of /proc/acpi/processor/CPU0/power for
both the bad and the good one ?

	tglx

Re: clockevents: fix resume logic

[Andrew Morton]

On Tue, 11 Sep 2007 20:38:03 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:

> On Tue, 2007-09-11 at 11:25 -0700, Andrew Morton wrote:
> > > It evidently assumes cpuidle to be present, which is not in the mainline.
> > 
> > Bear in mind that the cpuidle patch fixes resume-from-ram when cpuidle is
> > disabled in config.
> > 
> > > It seems to me that the total effect of this one and the hackpatch is that
> > > the C states are not handled any more.
> > 
> > hm.
> > 
> > dmesg without the cpuidle patch: http://userweb.kernel.org/~akpm/dmesg-bad.txt
> > dmesg with the cpuidle patch: http://userweb.kernel.org/~akpm/dmesg-good.txt
> > difference: http://userweb.kernel.org/~akpm/dmesg-diff.txt
> > 
> > there doesn't seem to be a lot of difference in the time handling, except
> > there are large changes in when things happen in the bootup sequence.
> 
> The question is whether the system goes into C2 with the patch applied.
> 
> Can you please provide the output of /proc/acpi/processor/CPU0/power for
> both the bad and the good one ?
> 

good:

sony:/home/akpm> cat /proc/acpi/processor/CPU0/power
active state:            C0
max_cstate:              C8
bus master activity:     00000000
maximum allowed latency: 8000 usec
states:
    C1:                  type[C1] promotion[--] demotion[--] latency[001] usage[00000000] duration[00000000000000000000]
    C2:                  type[C2] promotion[--] demotion[--] latency[001] usage[00000000] duration[00000000000000000000]

bad:

sony:/home/akpm> cat /proc/acpi/processor/CPU0/power
active state:            C2
max_cstate:              C8
bus master activity:     00000000
maximum allowed latency: 8000 usec
states:
    C1:                  type[C1] promotion[C2] demotion[--] latency[001] usage[00000010] duration[00000000000000000000]
   *C2:                  type[C2] promotion[--] demotion[C1] latency[001] usage[00008316] duration[00000000000170717293]

Re: clockevents: fix resume logic

[Rafael J. Wysocki]

On Tuesday, 11 September 2007 20:25, Andrew Morton wrote:
> On Tue, 11 Sep 2007 14:09:12 +0200 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > On Tuesday, 11 September 2007 13:22, Andrew Morton wrote:
> > > On Tue, 11 Sep 2007 13:23:55 +0200 "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > > 
> > > > On Tuesday, 11 September 2007 12:31, Andrew Morton wrote:
> > > > > On Tue, 11 Sep 2007 11:16:04 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> > > > > 
> > > > > > On Tue, 2007-09-11 at 01:35 -0700, Andrew Morton wrote:
> > > > > > > On Tue, 11 Sep 2007 01:20:05 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> > > > > > > 
> > > > > > > > On Tue, 11 Sep 2007 09:47:20 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> > > > > > > > 
> > > > > > > > > On Tue, 2007-09-11 at 09:23 +0200, Thomas Gleixner wrote:
> > > > > > > > > > > I went back to the original patch which I sent to Linus and it matches
> > > > > > > > > > > 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6.  So all I can think is that there
> > > > > > > > > > > must have been something else in the tree which I tested which fixed the
> > > > > > > > > > > bug which 18de5bc4c1f1f1fa5e14f354a7603bd6e9d4e3b6 introduced.  argh.
> > > > > > > > > > > 
> > > > > > > > > > > Can you think what would cause the symptoms which I described?  
> > > > > > > > > > 
> > > > > > > > > > It seems that time is not updated. Timer interrupt not active or some
> > > > > > > > > > other odd thing. I figure out what's going on when I find a box which
> > > > > > > > > > exposes the problem.
> > > > > > > > 
> > > > > > > > 2.6.22-rc6-mm1's git-acpi.patch contains something which fixes this bug. 
> > > > > > > 
> > > > > > > Len's current tree fixes it too.
> > > > 
> > > > Do you mean this one:
> > > > 
> > > > http://git.kernel.org/?p=linux/kernel/git/lenb/linux-acpi-2.6.git
> > > 
> > > Nope.
> > > 
> > > Here's the algorthm: go to the latest -mm tree and look at the first line
> > > of broken-out/git-acpi.patch:
> > > 
> > > GIT f94aac9883f9b02700270cf286577a9bccf98f47 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git#test
> > > 
> > > that gives the URL, the branch and the top-level commit.  All the -mm git
> > > trees have that first line.
> > > 
> > > 
> > > 
> > > Anyway, I was able to extract all the diffs and generate a patch series. 
> > > The below patch fixes current mainline on the Vaio.
> > 
> > It evidently assumes cpuidle to be present, which is not in the mainline.
> 
> Bear in mind that the cpuidle patch fixes resume-from-ram when cpuidle is
> disabled in config.
> 
> > It seems to me that the total effect of this one and the hackpatch is that
> > the C states are not handled any more.
> 
> hm.
> 
> dmesg without the cpuidle patch: http://userweb.kernel.org/~akpm/dmesg-bad.txt
> dmesg with the cpuidle patch: http://userweb.kernel.org/~akpm/dmesg-good.txt
> difference: http://userweb.kernel.org/~akpm/dmesg-diff.txt
> 
> there doesn't seem to be a lot of difference in the time handling, except
> there are large changes in when things happen in the bootup sequence.

Hm, these things look like they may be related to the suspend/resume problems:

+Marking TSC unstable due to: possible TSC halt in C2.

+Clocksource tsc unstable (delta = -458965792 ns)

Re: clockevents: fix resume logic

[Andrew Morton]

On Tue, 11 Sep 2007 21:35:15 +0200
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> > dmesg without the cpuidle patch: http://userweb.kernel.org/~akpm/dmesg-bad.txt
> > dmesg with the cpuidle patch: http://userweb.kernel.org/~akpm/dmesg-good.txt
> > difference: http://userweb.kernel.org/~akpm/dmesg-diff.txt
> > 
> > there doesn't seem to be a lot of difference in the time handling, except
> > there are large changes in when things happen in the bootup sequence.
> 
> Hm, these things look like they may be related to the suspend/resume problems:
> 
> +Marking TSC unstable due to: possible TSC halt in C2.
> 
> +Clocksource tsc unstable (delta = -458965792 ns)

sort-of.  But look:

akpm:/home/akpm> grep -i tsc dmesg-diff.txt 
+Marking TSC unstable due to: possible TSC halt in C2.
+Clocksource tsc unstable (delta = -458965792 ns)
-Marking TSC unstable due to: cpufreq changes.
-Clocksource tsc unstable (delta = -150503296 ns)

the good and bad kernels seem to be doing quite similar things with the
time management.  But they're doing it at quite different places in the
boot process.

Re: clockevents: fix resume logic

[Arjan van de Ven]

On Tue, 11 Sep 2007 11:25:31 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> dmesg without the cpuidle patch:
> http://userweb.kernel.org/~akpm/dmesg-bad.txt dmesg with the cpuidle
> patch: http://userweb.kernel.org/~akpm/dmesg-good.txt difference:
> http://userweb.kernel.org/~akpm/dmesg-diff.txt
> 
> there doesn't seem to be a lot of difference in the time handling,
> except there are large changes in when things happen in the bootup
> sequence.

powertop can show you which C-states are actually used; it could be a
theory that in the working case the deeper C-states aren't used (which
then means lapic keeps working etc)...
powertop would show this change ....

Re: clockevents: fix resume logic

[Thomas Gleixner]

On Tue, 2007-09-11 at 11:44 -0700, Andrew Morton wrote:
> > > there doesn't seem to be a lot of difference in the time handling, except
> > > there are large changes in when things happen in the bootup sequence.
> > 
> > The question is whether the system goes into C2 with the patch applied.
> > 
> > Can you please provide the output of /proc/acpi/processor/CPU0/power for
> > both the bad and the good one ?
> > 
> 
> good:
> 
> sony:/home/akpm> cat /proc/acpi/processor/CPU0/power
> active state:            C0
> max_cstate:              C8
> bus master activity:     00000000
> maximum allowed latency: 8000 usec
> states:
>     C1:                  type[C1] promotion[--] demotion[--] latency[001] usage[00000000] duration[00000000000000000000]
>     C2:                  type[C2] promotion[--] demotion[--] latency[001] usage[00000000] duration[00000000000000000000]
> 
> bad:
> 
> sony:/home/akpm> cat /proc/acpi/processor/CPU0/power
> active state:            C2
> max_cstate:              C8
> bus master activity:     00000000
> maximum allowed latency: 8000 usec
> states:
>     C1:                  type[C1] promotion[C2] demotion[--] latency[001] usage[00000010] duration[00000000000000000000]
>    *C2:                  type[C2] promotion[--] demotion[C1] latency[001] usage[00008316] duration[00000000000170717293]

Ok, here we are. The bad one uses C2 which stops the local apic on the
VAIO. I suspect we end up in the suspend/resume with going into C2
without the broadcast active.

Can you try to get the output of SysRq-Q during the "it needs help from
keyboard" period ?

	tglx

Re: clockevents: fix resume logic

[Thomas Gleixner]

On Tue, 2007-09-11 at 21:52 +0200, Thomas Gleixner wrote:
> >     C1:                  type[C1] promotion[C2] demotion[--] latency[001] usage[00000010] duration[00000000000000000000]
> >    *C2:                  type[C2] promotion[--] demotion[C1] latency[001] usage[00008316] duration[00000000000170717293]
> 
> Ok, here we are. The bad one uses C2 which stops the local apic on the
> VAIO. I suspect we end up in the suspend/resume with going into C2
> without the broadcast active.
> 
> Can you try to get the output of SysRq-Q during the "it needs help from
> keyboard" period ?

Summary of the oddities we are seing:

1.) disabling local apic timer makes the problem go away
2.) disabling nohz and highres makes the problem go away
3.) adding the cpuidle patches from the acpi tree makes the problem go
away

The obvious conclusion is, that in all other cases we run into a state,
where the local apic timer is not working.

1) we do not use it
2) it is used in periodic mode
3) the cpu does not enter C2 (which turns the lapic timer off on the
VAIO)

While 1) and 3) are understandable, the reason why 2) is working is a
mystery because the periodic mode is affected by the C state as well.

Andrew, can you please provide the output of /proc/timer_list when you
boot the kernel with "nohz=off highres=off", but honestly I do not
expect a lot of enlightenment from it.

The Sysrq-Q output from the point where the box is stuck without
keystrokes might give us more information.

	tglx

Re: clockevents: fix resume logic

[Andrew Morton]

On Tue, 11 Sep 2007 21:52:01 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:

> On Tue, 2007-09-11 at 11:44 -0700, Andrew Morton wrote:
> > > > there doesn't seem to be a lot of difference in the time handling, except
> > > > there are large changes in when things happen in the bootup sequence.
> > > 
> > > The question is whether the system goes into C2 with the patch applied.
> > > 
> > > Can you please provide the output of /proc/acpi/processor/CPU0/power for
> > > both the bad and the good one ?
> > > 
> > 
> > good:
> > 
> > sony:/home/akpm> cat /proc/acpi/processor/CPU0/power
> > active state:            C0
> > max_cstate:              C8
> > bus master activity:     00000000
> > maximum allowed latency: 8000 usec
> > states:
> >     C1:                  type[C1] promotion[--] demotion[--] latency[001] usage[00000000] duration[00000000000000000000]
> >     C2:                  type[C2] promotion[--] demotion[--] latency[001] usage[00000000] duration[00000000000000000000]
> > 
> > bad:
> > 
> > sony:/home/akpm> cat /proc/acpi/processor/CPU0/power
> > active state:            C2
> > max_cstate:              C8
> > bus master activity:     00000000
> > maximum allowed latency: 8000 usec
> > states:
> >     C1:                  type[C1] promotion[C2] demotion[--] latency[001] usage[00000010] duration[00000000000000000000]
> >    *C2:                  type[C2] promotion[--] demotion[C1] latency[001] usage[00008316] duration[00000000000170717293]
> 
> Ok, here we are. The bad one uses C2 which stops the local apic on the
> VAIO. I suspect we end up in the suspend/resume with going into C2
> without the broadcast active.
> 
> Can you try to get the output of SysRq-Q during the "it needs help from
> keyboard" period ?
> 

That's a bit tricky because hitting the keyboard is what unsticks things. 
And the video is black after resume-from-RAM (has always been thus) and we
broke netconsole-over-e100-during-resume back in 2.6.21 or thereabouts.

<struggles a bit>

Here: http://userweb.kernel.org/~akpm/x.txt

Re: clockevents: fix resume logic

[Andrew Morton]

On Tue, 11 Sep 2007 23:49:47 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:

> On Tue, 2007-09-11 at 21:52 +0200, Thomas Gleixner wrote:
> > >     C1:                  type[C1] promotion[C2] demotion[--] latency[001] usage[00000010] duration[00000000000000000000]
> > >    *C2:                  type[C2] promotion[--] demotion[C1] latency[001] usage[00008316] duration[00000000000170717293]
> > 
> > Ok, here we are. The bad one uses C2 which stops the local apic on the
> > VAIO. I suspect we end up in the suspend/resume with going into C2
> > without the broadcast active.
> > 
> > Can you try to get the output of SysRq-Q during the "it needs help from
> > keyboard" period ?
> 
> Summary of the oddities we are seing:
> 
> 1.) disabling local apic timer makes the problem go away
> 2.) disabling nohz and highres makes the problem go away
> 3.) adding the cpuidle patches from the acpi tree makes the problem go
> away

Only the fist cpuidle patch, actually.  I'd have though this was a big hint?

> The obvious conclusion is, that in all other cases we run into a state,
> where the local apic timer is not working.
> 
> 1) we do not use it
> 2) it is used in periodic mode
> 3) the cpu does not enter C2 (which turns the lapic timer off on the
> VAIO)
> 
> While 1) and 3) are understandable, the reason why 2) is working is a
> mystery because the periodic mode is affected by the C state as well.
> 
> Andrew, can you please provide the output of /proc/timer_list when you
> boot the kernel with "nohz=off highres=off", but honestly I do not
> expect a lot of enlightenment from it.

Timer List Version: v0.3
HRTIMER_MAX_CLOCK_BASES: 2
now at 91231842576 nsecs

cpu: 0
 clock 0:
  .index:      0
  .resolution: 4000250 nsecs
  .get_time:   ktime_get_real
  .offset:     0 nsecs
active timers:
 clock 1:
  .index:      1
  .resolution: 4000250 nsecs
  .get_time:   ktime_get
  .offset:     0 nsecs
active timers:
 #0: <f67d5eb0>, hrtimer_wakeup, S:01, do_nanosleep, cpuspeed/1685
 # expires at 91415336182 nsecs [in 183493606 nsecs]
 #1: <f67d5eb0>, it_real_fn, S:01, do_setitimer, ntpd/2182
 # expires at 91859626754 nsecs [in 627784178 nsecs]
 #2: <f67d5eb0>, hrtimer_wakeup, S:01, do_nanosleep, hald-addon-stor/2321
 # expires at 92102929614 nsecs [in 871087038 nsecs]
 #3: <f67d5eb0>, it_real_fn, S:01, do_setitimer, automount/2115
 # expires at 105148251993 nsecs [in 13916409417 nsecs]
 #4: <f67d5eb0>, hrtimer_wakeup, S:01, do_nanosleep, crond/2228
 # expires at 111446499472 nsecs [in 20214656896 nsecs]
 #5: <f67d5eb0>, it_real_fn, S:01, do_setitimer, syslogd/1886
 # expires at 114439083561 nsecs [in 23207240985 nsecs]
 #6: <f67d5eb0>, hrtimer_wakeup, S:01, do_nanosleep, atd/2280
 # expires at 343127174592 nsecs [in 251895332016 nsecs]
 #7: <f67d5eb0>, it_real_fn, S:01, do_setitimer, sendmail/2917
 # expires at 3659006407026 nsecs [in 3567774564450 nsecs]
 #8: <f67d5eb0>, it_real_fn, S:01, do_setitimer, sendmail/2927
 # expires at 3659252445429 nsecs [in 3568020602853 nsecs]
 #9: <f67d5eb0>, it_real_fn, S:01, do_setitimer, anacron/2272
 # expires at 3941983748056 nsecs [in 3850751905480 nsecs]
  .expires_next   : 9223372036854775807 nsecs
  .hres_active    : 0
  .nr_events      : 0
  .nohz_mode      : 0
  .idle_tick      : 0 nsecs
  .tick_stopped   : 0
  .idle_jiffies   : 0
  .idle_calls     : 0
  .idle_sleeps    : 0
  .idle_entrytime : 0 nsecs
  .idle_sleeptime : 0 nsecs
  .last_jiffies   : 0
  .next_jiffies   : 0
  .idle_expires   : 0 nsecs
jiffies: 4294915081


Tick Device: mode:     0
Clock Event Device: pit
 max_delta_ns:   27461866
 min_delta_ns:   12571
 mult:           5124677
 shift:          32
 mode:           2
 next_event:     9223372036854775807 nsecs
 set_next_event: pit_next_event
 set_mode:       init_pit_timer
 event_handler:  tick_handle_periodic_broadcast
tick_broadcast_mask: 00000001
tick_broadcast_oneshot_mask: 00000000


Tick Device: mode:     0
Clock Event Device: lapic
 max_delta_ns:   1009158122
 min_delta_ns:   1804
 mult:           35701831
 shift:          32
 mode:           1
 next_event:     0 nsecs
 set_next_event: lapic_next_event
 set_mode:       lapic_timer_setup
 event_handler:  tick_handle_periodic

Re: clockevents: fix resume logic

[Thomas Gleixner]

On Wed, 2007-09-12 at 02:16 -0700, Andrew Morton wrote:
> On Tue, 11 Sep 2007 23:49:47 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > On Tue, 2007-09-11 at 21:52 +0200, Thomas Gleixner wrote:
> > > >     C1:                  type[C1] promotion[C2] demotion[--] latency[001] usage[00000010] duration[00000000000000000000]
> > > >    *C2:                  type[C2] promotion[--] demotion[C1] latency[001] usage[00008316] duration[00000000000170717293]
> > > 
> > > Ok, here we are. The bad one uses C2 which stops the local apic on the
> > > VAIO. I suspect we end up in the suspend/resume with going into C2
> > > without the broadcast active.
> > > 
> > > Can you try to get the output of SysRq-Q during the "it needs help from
> > > keyboard" period ?
> > 
> > Summary of the oddities we are seing:
> > 
> > 1.) disabling local apic timer makes the problem go away
> > 2.) disabling nohz and highres makes the problem go away
> > 3.) adding the cpuidle patches from the acpi tree makes the problem go
> > away
> 
> Only the fist cpuidle patch, actually.  I'd have though this was a big hint?

Yes, it prevents the cpu to enter C2, which keeps the local APIC alive.

> > The obvious conclusion is, that in all other cases we run into a state,
> > where the local apic timer is not working.
> > 
> > 1) we do not use it
> > 2) it is used in periodic mode
> > 3) the cpu does not enter C2 (which turns the lapic timer off on the
> > VAIO)
> > 
> > While 1) and 3) are understandable, the reason why 2) is working is a
> > mystery because the periodic mode is affected by the C state as well.
> > 
> > Andrew, can you please provide the output of /proc/timer_list when you
> > boot the kernel with "nohz=off highres=off", but honestly I do not
> > expect a lot of enlightenment from it.

> Tick Device: mode:     0
> Clock Event Device: pit
>  max_delta_ns:   27461866
>  min_delta_ns:   12571
>  mult:           5124677
>  shift:          32
>  mode:           2
>  next_event:     9223372036854775807 nsecs
>  set_next_event: pit_next_event
>  set_mode:       init_pit_timer
>  event_handler:  tick_handle_periodic_broadcast
> tick_broadcast_mask: 00000001
> tick_broadcast_oneshot_mask: 00000000

Ok, I got my brain together and read through the code. The periodic mode
switches to broadcast right away when the acpi code discovers that there
is a lapic stops C-State available.

Does the test hack below fix the problem for nohz/highres enabled
kernels ?

	tglx

--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -382,6 +382,8 @@ static int tick_broadcast_set_event(ktime_t expires, int force)
 
 int tick_resume_broadcast_oneshot(struct clock_event_device *bc)
 {
+	cpu_set(smp_processor_id(), tick_broadcast_oneshot_mask);
+
 	clockevents_set_mode(bc, CLOCK_EVT_MODE_ONESHOT);
 
 	if(!cpus_empty(tick_broadcast_oneshot_mask))

Re: clockevents: fix resume logic

[Andrew Morton]

On Wed, 12 Sep 2007 18:57:42 +0200 Thomas Gleixner <tglx@linutronix.de> wrote:

> Does the test hack below fix the problem for nohz/highres enabled
> kernels ?
> 
> 	tglx
> 
> --- a/kernel/time/tick-broadcast.c
> +++ b/kernel/time/tick-broadcast.c
> @@ -382,6 +382,8 @@ static int tick_broadcast_set_event(ktime_t expires, int force)
>  
>  int tick_resume_broadcast_oneshot(struct clock_event_device *bc)
>  {
> +	cpu_set(smp_processor_id(), tick_broadcast_oneshot_mask);
> +
>  	clockevents_set_mode(bc, CLOCK_EVT_MODE_ONESHOT);
>  
>  	if(!cpus_empty(tick_broadcast_oneshot_mask))

yup

Re: clockevents: fix resume logic

[Pavel Machek]

Hi!

> > Ok, here we are. The bad one uses C2 which stops the local apic on the
> > VAIO. I suspect we end up in the suspend/resume with going into C2
> > without the broadcast active.
> > 
> > Can you try to get the output of SysRq-Q during the "it needs help from
> > keyboard" period ?
> > 
> 
> That's a bit tricky because hitting the keyboard is what unsticks things. 
> And the video is black after resume-from-RAM (has always been thus) and we

Ok, can we try to fix the video issue for you? That should make the
development easier... I assume you tried s2ram from suspend.sf.net,
and no combination of switches helped?
							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: clockevents: fix resume logic

[Thomas Gleixner]

On Mon, 2007-09-17 at 18:37 +0000, Pavel Machek wrote:
> > That's a bit tricky because hitting the keyboard is what unsticks things. 
> > And the video is black after resume-from-RAM (has always been thus) and we
> 
> Ok, can we try to fix the video issue for you? That should make the
> development easier... I assume you tried s2ram from suspend.sf.net,
> and no combination of switches helped?

I have the same issue. Blank screen after suspend to ram. Hibernate
works.

Do you have a debug patch or something ?

	tglx

Re: clockevents: fix resume logic

[Rafael J. Wysocki]

On Saturday, 22 September 2007 10:50, Thomas Gleixner wrote:
> On Mon, 2007-09-17 at 18:37 +0000, Pavel Machek wrote:
> > > That's a bit tricky because hitting the keyboard is what unsticks things. 
> > > And the video is black after resume-from-RAM (has always been thus) and we
> > 
> > Ok, can we try to fix the video issue for you? That should make the
> > development easier... I assume you tried s2ram from suspend.sf.net,
> > and no combination of switches helped?
> 
> I have the same issue. Blank screen after suspend to ram. Hibernate
> works.
> 
> Do you have a debug patch or something ?

IIRC, Andrew said that the X server could restore the graphics after a resume
from RAM on his Vaio.

Greetings,
Rafael

Re: clockevents: fix resume logic

[Pavel Machek]

> On Mon, 2007-09-17 at 18:37 +0000, Pavel Machek wrote:
> > > That's a bit tricky because hitting the keyboard is what unsticks things. 
> > > And the video is black after resume-from-RAM (has always been thus) and we
> > 
> > Ok, can we try to fix the video issue for you? That should make the
> > development easier... I assume you tried s2ram from suspend.sf.net,
> > and no combination of switches helped?
> 
> I have the same issue. Blank screen after suspend to ram. Hibernate
> works.

So, you have tried s2ram with all the different options, and none of
them can restore the screen? (s2ram is available at suspend.sf.net.)
i386 emulator is needed to restore the screen on many machines, that's
why we can't do it from kernel :-(.

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html