CPU_IDLE prevents resuming from STR [was: Re: 2.6.21-rc6-mm1]

CPU_IDLE prevents resuming from STR [was: Re: 2.6.21-rc6-mm1]

[Mattia Dongili]

On Sun, Apr 08, 2007 at 02:35:59PM -0700, Andrew Morton wrote:
...
>  git-acpi.patch

after bisecting I can finally say what breaks resume from STR here:

tadaaaaa: CPU_IDLE.
I first spotted the git-acpi.patch then reapplied it and disabled
CPU_IDLE, now my laptop resumes.

Any useful information I should add?

$ cat /sys/devices/system/cpu/cpuidle/*
acpi_idle 
no governors
acpi_idle
no governor

$ cat /proc/cpuinfo
processor	: 0
vendor_id	: GenuineIntel
cpu family	: 6
model		: 15
model name	: Intel(R) Core(TM)2 CPU         T5600  @ 1.83GHz
stepping	: 6
cpu MHz		: 1000.000
cache size	: 2048 KB
physical id	: 0
siblings	: 2
core id		: 0
cpu cores	: 2
fdiv_bug	: no
hlt_bug		: no
f00f_bug	: no
coma_bug	: no
fpu		: yes
fpu_exception	: yes
cpuid level	: 10
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe nx lm constant_tsc pni monitor ds_cpl vmx est tm2 ssse3 cx16 xtpr lahf_lm
bogomips	: 3671.24
clflush size	: 64

processor	: 1
vendor_id	: GenuineIntel
cpu family	: 6
model		: 15
model name	: Intel(R) Core(TM)2 CPU         T5600  @ 1.83GHz
stepping	: 6
cpu MHz		: 1000.000
cache size	: 2048 KB
physical id	: 0
siblings	: 2
core id		: 1
cpu cores	: 2
fdiv_bug	: no
hlt_bug		: no
f00f_bug	: no
coma_bug	: no
fpu		: yes
fpu_exception	: yes
cpuid level	: 10
wp		: yes
flags		: fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe nx lm constant_tsc pni monitor ds_cpl vmx est tm2 ssse3 cx16 xtpr lahf_lm
bogomips	: 15805.85
clflush size	: 64

-- 
mattia
:wq!

Re: CPU_IDLE prevents resuming from STR [was: Re: 2.6.21-rc6-mm1]

[Shaohua Li]

On Sat, 2007-04-14 at 01:45 +0200, Mattia Dongili wrote:
> On Sun, Apr 08, 2007 at 02:35:59PM -0700, Andrew Morton wrote:
> ...
> >  git-acpi.patch
> 
> after bisecting I can finally say what breaks resume from STR here:
> 
> tadaaaaa: CPU_IDLE.
> I first spotted the git-acpi.patch then reapplied it and disabled
> CPU_IDLE, now my laptop resumes.
> 
> Any useful information I should add?
> 
> $ cat /sys/devices/system/cpu/cpuidle/*
> acpi_idle 
> no governors
> acpi_idle
> no governor
please check if the patch at
http://marc.info/?l=linux-acpi&m=117523651630038&w=2 fixed the issue

Thanks,
Shaohua

Re: CPU_IDLE prevents resuming from STR [was: Re: 2.6.21-rc6-mm1]

[Joshua Wise]

On Mon, 16 Apr 2007, Shaohua Li wrote:
> On Sat, 2007-04-14 at 01:45 +0200, Mattia Dongili wrote:
>> ...
> please check if the patch at
> http://marc.info/?l=linux-acpi&m=117523651630038&w=2 fixed the issue

I have the same system as Mattia, and when I applied this patch and turned
CPU_IDLE back on, I got a panic on boot. Unfortunately, the EIP scrolled off
screen, so I can't get a line number.

(I had the same STR breakage as him; STR did not work with CPU_IDLE turned
on, and it did work with CPU_IDLE turned off.)

I'm running +rc6+mm(April 11) on a Sony VAIO SZ.

joshua

Re: CPU_IDLE prevents resuming from STR [was: Re: 2.6.21-rc6-mm1]

[Shaohua Li]

On Mon, 2007-04-16 at 22:50 -0400, Joshua Wise wrote:
> On Mon, 16 Apr 2007, Shaohua Li wrote:
> > On Sat, 2007-04-14 at 01:45 +0200, Mattia Dongili wrote:
> >> ...
> > please check if the patch at
> > http://marc.info/?l=linux-acpi&m=117523651630038&w=2 fixed the issue
> 
> I have the same system as Mattia, and when I applied this patch and turned
> CPU_IDLE back on, I got a panic on boot. Unfortunately, the EIP scrolled off
> screen, so I can't get a line number.
> 
> (I had the same STR breakage as him; STR did not work with CPU_IDLE turned
> on, and it did work with CPU_IDLE turned off.)
> 
> I'm running +rc6+mm(April 11) on a Sony VAIO SZ.
Is it possible you can get the log from a serial? I thought at least you
can see some log info in the screen, if you haven't serial, please write
it down. The boot panic surprise me, as it works here.

Thanks,
Shaohua

Re: CPU_IDLE prevents resuming from STR [was: Re: 2.6.21-rc6-mm1]

[Shaohua Li]

On Mon, 2007-04-16 at 22:50 -0400, Joshua Wise wrote:
> On Mon, 16 Apr 2007, Shaohua Li wrote:
> > On Sat, 2007-04-14 at 01:45 +0200, Mattia Dongili wrote:
> >> ...
> > please check if the patch at
> > http://marc.info/?l=linux-acpi&m=117523651630038&w=2 fixed the issue
> 
> I have the same system as Mattia, and when I applied this patch and turned
> CPU_IDLE back on, I got a panic on boot. Unfortunately, the EIP scrolled off
> screen, so I can't get a line number.
> 
> (I had the same STR breakage as him; STR did not work with CPU_IDLE turned
> on, and it did work with CPU_IDLE turned off.)
> 
> I'm running +rc6+mm(April 11) on a Sony VAIO SZ.
Looks there is init order issue of sysfs files. The new refreshed patch
should fix your bug.

Signed-off-by: Shaohua Li <shaohua.li@intel.com>

Index: 21-rc6-mm1/drivers/acpi/processor_idle.c
===================================================================
--- 21-rc6-mm1.orig/drivers/acpi/processor_idle.c	2007-04-17 13:41:29.000000000 +0800
+++ 21-rc6-mm1/drivers/acpi/processor_idle.c	2007-04-17 14:03:56.000000000 +0800
@@ -624,7 +624,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 cpuidle_force_redetect(per_cpu(cpuidle_devices, pr->id));
 }
 
 /* proc interface */
Index: 21-rc6-mm1/drivers/cpuidle/cpuidle.c
===================================================================
--- 21-rc6-mm1.orig/drivers/cpuidle/cpuidle.c	2007-04-17 13:41:29.000000000 +0800
+++ 21-rc6-mm1/drivers/cpuidle/cpuidle.c	2007-04-17 14:42:17.000000000 +0800
@@ -18,7 +18,7 @@
 
 #include "cpuidle.h"
 
-DEFINE_PER_CPU(struct cpuidle_device, cpuidle_devices);
+DEFINE_PER_CPU(struct cpuidle_device *, cpuidle_devices);
 EXPORT_PER_CPU_SYMBOL_GPL(cpuidle_devices);
 
 DEFINE_MUTEX(cpuidle_lock);
@@ -34,13 +34,13 @@ static void (*pm_idle_old)(void);
  */
 static void cpuidle_idle_call(void)
 {
-	struct cpuidle_device *dev = &__get_cpu_var(cpuidle_devices);
+	struct cpuidle_device *dev = __get_cpu_var(cpuidle_devices);
 
 	struct cpuidle_state *target_state;
 	int next_state;
 
 	/* check if the device is ready */
-	if (dev->status != CPUIDLE_STATUS_DOIDLE) {
+	if (!dev || dev->status != CPUIDLE_STATUS_DOIDLE) {
 		if (pm_idle_old)
 			pm_idle_old();
 		return;
@@ -117,19 +117,32 @@ static int cpuidle_add_device(struct sys
 	int cpu = sys_dev->id;
 	struct cpuidle_device *dev;
 
-	dev = &per_cpu(cpuidle_devices, cpu);
+	dev = per_cpu(cpuidle_devices, cpu);
 
-	dev->cpu = cpu;
 	mutex_lock(&cpuidle_lock);
 	if (cpu_is_offline(cpu)) {
 		mutex_unlock(&cpuidle_lock);
 		return 0;
 	}
 
+	if (!dev) {
+		dev = kzalloc(sizeof(struct cpuidle_device), GFP_KERNEL);
+		if (!dev) {
+			mutex_unlock(&cpuidle_lock);
+			return -ENOMEM;
+		}
+		init_completion(&dev->kobj_unregister);
+		per_cpu(cpuidle_devices, cpu) = dev;
+	}
+	dev->cpu = cpu;
+
 	if (dev->status & CPUIDLE_STATUS_DETECTED) {
 		mutex_unlock(&cpuidle_lock);
 		return 0;
 	}
+
+	cpuidle_add_sysfs(sys_dev);
+
 	if (cpuidle_curr_driver) {
 		if (cpuidle_attach_driver(dev))
 			goto err_ret;
@@ -146,7 +159,6 @@ static int cpuidle_add_device(struct sys
 		cpuidle_install_idle_handler();
 
 	list_add(&dev->device_list, &cpuidle_detected_devices);
-	cpuidle_add_sysfs(sys_dev);
 	dev->status |= CPUIDLE_STATUS_DETECTED;
 
 err_ret:
@@ -165,7 +177,7 @@ static int __cpuidle_remove_device(struc
 {
 	struct cpuidle_device *dev;
 
-	dev = &per_cpu(cpuidle_devices, sys_dev->id);
+	dev = per_cpu(cpuidle_devices, sys_dev->id);
 
 	if (!(dev->status & CPUIDLE_STATUS_DETECTED)) {
 		return 0;
@@ -178,6 +190,9 @@ static int __cpuidle_remove_device(struc
 		cpuidle_detach_driver(dev);
 	cpuidle_remove_sysfs(sys_dev);
 	list_del(&dev->device_list);
+	wait_for_completion(&dev->kobj_unregister);
+	per_cpu(cpuidle_devices, sys_dev->id) = NULL;
+	kfree(dev);
 
 	return 0;
 }
Index: 21-rc6-mm1/drivers/cpuidle/sysfs.c
===================================================================
--- 21-rc6-mm1.orig/drivers/cpuidle/sysfs.c	2007-04-17 13:41:29.000000000 +0800
+++ 21-rc6-mm1/drivers/cpuidle/sysfs.c	2007-04-17 14:03:56.000000000 +0800
@@ -210,8 +210,16 @@ static struct sysfs_ops cpuidle_sysfs_op
 	.store = cpuidle_store,
 };
 
+static void cpuidle_sysfs_release(struct kobject *kobj)
+{
+	struct cpuidle_device *dev = kobj_to_cpuidledev(kobj);
+
+	complete(&dev->kobj_unregister);
+}
+
 static struct kobj_type ktype_cpuidle = {
 	.sysfs_ops = &cpuidle_sysfs_ops,
+	.release = cpuidle_sysfs_release,
 };
 
 struct cpuidle_state_attr {
@@ -246,7 +254,8 @@ static struct attribute *cpuidle_state_d
 	NULL
 };
 
-#define kobj_to_state(k) container_of(k, struct cpuidle_state, kobj)
+#define kobj_to_state_obj(k) container_of(k, struct cpuidle_state_kobj, kobj)
+#define kobj_to_state(k) (kobj_to_state_obj(k)->state)
 #define attr_to_stateattr(a) container_of(a, struct cpuidle_state_attr, attr)
 static ssize_t cpuidle_state_show(struct kobject * kobj,
 	struct attribute * attr ,char * buf)
@@ -265,11 +274,27 @@ static struct sysfs_ops cpuidle_state_sy
 	.show = cpuidle_state_show,
 };
 
+static void cpuidle_state_sysfs_release(struct kobject *kobj)
+{
+	struct cpuidle_state_kobj *state_obj = kobj_to_state_obj(kobj);
+
+	complete(&state_obj->kobj_unregister);
+}
+
 static struct kobj_type ktype_state_cpuidle = {
 	.sysfs_ops = &cpuidle_state_sysfs_ops,
 	.default_attrs = cpuidle_state_default_attrs,
+	.release = cpuidle_state_sysfs_release,
 };
 
+static void inline cpuidle_free_state_kobj(struct cpuidle_device *device, int i)
+{
+	kobject_unregister(&device->kobjs[i]->kobj);
+	wait_for_completion(&device->kobjs[i]->kobj_unregister);
+	kfree(device->kobjs[i]);
+	device->kobjs[i] = NULL;
+}
+
 /**
  * cpuidle_add_driver_sysfs - adds driver-specific sysfs attributes
  * @device: the target device
@@ -277,24 +302,32 @@ static struct kobj_type ktype_state_cpui
 int cpuidle_add_driver_sysfs(struct cpuidle_device *device)
 {
 	int i, ret;
-	struct cpuidle_state *state;
+	struct cpuidle_state_kobj *kobj;
 
 	/* state statistics */
 	for (i = 0; i < device->state_count; i++) {
-		state = &device->states[i];
-		state->kobj.parent = &device->kobj;
-		state->kobj.ktype = &ktype_state_cpuidle;
-		kobject_set_name(&state->kobj, "state%d", i);
-		ret = kobject_register(&state->kobj);
-		if (ret)
+		kobj = kzalloc(sizeof(struct cpuidle_state_kobj), GFP_KERNEL);
+		if (!kobj)
+			goto error_state;
+		kobj->state = &device->states[i];
+		init_completion(&kobj->kobj_unregister);
+
+		kobj->kobj.parent = &device->kobj;
+		kobj->kobj.ktype = &ktype_state_cpuidle;
+		kobject_set_name(&kobj->kobj, "state%d", i);
+		ret = kobject_register(&kobj->kobj);
+		if (ret) {
+			kfree(kobj);
 			goto error_state;
+		}
+		device->kobjs[i] = kobj;
 	}
 
 	return 0;
 
 error_state:
 	for (i = i - 1; i >= 0; i--)
-		kobject_unregister(&device->states[i].kobj);
+		cpuidle_free_state_kobj(device, i);
 	return ret;
 }
 
@@ -307,7 +340,7 @@ void cpuidle_remove_driver_sysfs(struct 
 	int i;
 
 	for (i = 0; i < device->state_count; i++)
-		kobject_unregister(&device->states[i].kobj);
+		cpuidle_free_state_kobj(device, i);
 }
 
 /**
@@ -319,7 +352,7 @@ int cpuidle_add_sysfs(struct sys_device 
 	int cpu = sysdev->id;
 	struct cpuidle_device *dev;
 
-	dev = &per_cpu(cpuidle_devices, cpu);
+	dev = per_cpu(cpuidle_devices, cpu);
 	dev->kobj.parent = &sysdev->kobj;
 	dev->kobj.ktype = &ktype_cpuidle;
 	kobject_set_name(&dev->kobj, "%s", "cpuidle");
@@ -335,6 +368,6 @@ void cpuidle_remove_sysfs(struct sys_dev
 	int cpu = sysdev->id;
 	struct cpuidle_device *dev;
 
-	dev = &per_cpu(cpuidle_devices, cpu);
+	dev = per_cpu(cpuidle_devices, cpu);
 	kobject_unregister(&dev->kobj);
 }
Index: 21-rc6-mm1/include/linux/cpuidle.h
===================================================================
--- 21-rc6-mm1.orig/include/linux/cpuidle.h	2007-04-17 13:41:29.000000000 +0800
+++ 21-rc6-mm1/include/linux/cpuidle.h	2007-04-17 14:03:56.000000000 +0800
@@ -41,8 +41,6 @@ struct cpuidle_state {
 
 	int (*enter)	(struct cpuidle_device *dev,
 			 struct cpuidle_state *state);
-
-	struct kobject	kobj;
 };
 
 /* Idle State Flags */
@@ -74,6 +72,12 @@ cpuidle_set_statedata(struct cpuidle_sta
 	state->driver_data = data;
 }
 
+struct cpuidle_state_kobj {
+	struct cpuidle_state *state;
+	struct completion kobj_unregister;
+	struct kobject kobj;
+};
+
 struct cpuidle_device {
 	unsigned int		status;
 	int			cpu;
@@ -81,6 +85,7 @@ struct cpuidle_device {
 	int			last_residency;
 	int			state_count;
 	struct cpuidle_state	states[CPUIDLE_STATE_MAX];
+	struct cpuidle_state_kobj *kobjs[CPUIDLE_STATE_MAX];
 	struct cpuidle_state	*last_state;
 
 	struct list_head 	device_list;
@@ -89,9 +94,7 @@ struct cpuidle_device {
 	void			*governor_data;
 };
 
-#define to_cpuidle_device(n) container_of(n, struct cpuidle_device, kobj);
-
-DECLARE_PER_CPU(struct cpuidle_device, cpuidle_devices);
+DECLARE_PER_CPU(struct cpuidle_device *, cpuidle_devices);
 
 /* Device Status Flags */
 #define CPUIDLE_STATUS_DETECTED		 (0x1)

Re: CPU_IDLE prevents resuming from STR [was: Re: 2.6.21-rc6-mm1]

[Joshua Wise]

On Tue, 17 Apr 2007, Shaohua Li wrote:
> Looks there is init order issue of sysfs files. The new refreshed patch
> should fix your bug.

Yes, that did fix the hang on resume from STR -- that now works fine.

However:
joshua@rebirth:/sys/devices/system/cpu/cpuidle$ cat available_drivers current_driver

<NULL>
joshua@rebirth:/sys/devices/system/cpu/cpuidle$ cat available_governors current_governor
ladder
ladder

Is this correct? For reference, my config is http://joshuawise.com/config.gz
-- I didn't see any options for cpuidle drivers to access ACPI states...

joshua

Re: CPU_IDLE prevents resuming from STR [was: Re: 2.6.21-rc6-mm1]

[Shaohua Li]

On Wed, 2007-04-18 at 19:00 -0400, Joshua Wise wrote:
> On Tue, 17 Apr 2007, Shaohua Li wrote:
> > Looks there is init order issue of sysfs files. The new refreshed patch
> > should fix your bug.
> 
> Yes, that did fix the hang on resume from STR -- that now works fine.
> 
> However:
> joshua@rebirth:/sys/devices/system/cpu/cpuidle$ cat available_drivers current_driver
> 
> <NULL>
> joshua@rebirth:/sys/devices/system/cpu/cpuidle$ cat available_governors current_governor
> ladder
> ladder
it's correct and looks you didn't compile the acpi processor module.

Thanks,
Shaohua

Re: 2.6.21-rc6-mm1 USB related boot hang - bisection result

[Andrew Morton]

On Fri, 27 Apr 2007 00:39:19 +0200 (CEST) Jiri Kosina <jikos@jikos.cz> wrote:

> On Fri, 27 Apr 2007, Helge Hafting wrote:
> 
> > 2.6.21-rc6 boots up fine.  Both rc6 and rc7 has a different problem - 
> > the machine tends to hang after some minutes work in X.  That hang is 
> > unusual in that moving the mouse still move the X cursor, but everything 
> > else stops and sysrq fails me. But that is another story.
> [...]
> > The (first) "hanging" patch in 2.6.21-rc6-mm1 is: git-acpi.patch

linux-acpi: we have a problem.

> Hi Helge,
> 
> thanks for the effort. If you take stock rc6-mm1 and revert just 
> git-acpi.patch, doesn the machine behave correctly?
> 

It would be easier and would produce a clearer result to test just

	2.6.21-rc7
+	2.6.21-rc7-mm2's origin.patch
+	2.6.21-rc7-mm2's acpi.patch

from
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.21-rc7/2.6.21-rc7-mm2/broken-out/

Re: 2.6.21-rc6-mm1 USB related boot hang - bisection result

[Andrew Morton]

On Fri, 27 Apr 2007 23:04:58 +0200
Helge Hafting <helgehaf@aitel.hist.no> wrote:

> On Fri, Apr 27, 2007 at 12:39:19AM +0200, Jiri Kosina wrote:
> > On Fri, 27 Apr 2007, Helge Hafting wrote:
> > 
> > > 2.6.21-rc6 boots up fine.  Both rc6 and rc7 has a different problem - 
> > > the machine tends to hang after some minutes work in X.  That hang is 
> > > unusual in that moving the mouse still move the X cursor, but everything 
> > > else stops and sysrq fails me. But that is another story.
> > [...]
> > > The (first) "hanging" patch in 2.6.21-rc6-mm1 is: git-acpi.patch
> > 
> > Hi Helge,
> > 
> > thanks for the effort. If you take stock rc6-mm1 and revert just 
> > git-acpi.patch, doesn the machine behave correctly?
> 
> Just compiled & booted such a kernel - it came up fine!
> So it looks like USB is fine then, and the problem is in
> that ACPI patch.
> 

OK, thanks.  Len&co: we've established that 2.6.21-rc6-mm1's git-acpi.patch
causes this:

> 2.6.21-rc6 boots up fine.  Both rc6 and rc7 has a different problem - the
> machine tends to hang after some minutes work in X.  That hang is
> unusual in that moving the mouse still move the X cursor, but
> everything else stops and sysrq fails me. But that is another story.
> 
> rc6 boots, rc6-mm1 hangs at the "usbcore registered hiddev" message.
> Bisection:
> 1, 2, 3: the three first hangs at "usbcore registered hiddev"
> 4, 5, 6: the next three hangs at a message about ACPI  PCI[A]->IRQ17
> I decided to keep bisecting these hangers as "bad", I don't really know
> if this could be the same thing or completely different issues.  If they are
> different, then one problem will mask the other anyway, so
> calling every hanging kernel "bad" will at least find the first broken 
> patch.
> 7: boots up ok!
> 8,9,10: hangs at the aboce mentioned ACPI message
> The (first) "hanging" patch in 2.6.21-rc6-mm1 is: git-acpi.patch
> 
>