Skip to contentSkip to conversation

Commit ac3ebafa81a makes NHM EX/EP machines hung out since 3.9-rc1

DISCUSSION Brown, Len 6 participants 11 messages Mar 26, 2013 – Apr 2, 2013 1 message not in the archive ACPI

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): 20130313022759.GA120840@bee

Brown, Len

author
#1 lore raw
Quoted 57 lines

-----Original Message-----
From: Xie, ChanglongX
Sent: Tuesday, March 12, 2013 10:28 PM
To: Brown, Len
Cc: Daniel Lezcano; [email protected]; [email protected]
Subject: Commit ac3ebafa81a makes NHM EX/EP machines hung out since 3.9- rc1

Hi Len,

FYI, since 3.9-rc1 our three NHM EP/EX LKP(linux kernel
performance) test servers
except SNB/IVB/WSM hung up unexpectly.

We did git bisect for about 8 times on all servers, it said that
the first bad commit is ac3ebafa.

commit ac3ebafa81af76d65e4fb45c6388f08e90ddcc6d
Author: Daniel Lezcano <[email protected]>
Date: Mon Feb 4 22:44:43 2013 +0000

ACPI / idle: remove usage of the statedata

Len Brown sent a patch to remove this field in the intel_idle
driver.
The other user of this field is the davinci cpuidle driver and a patch has been sent to remove the usage of it.
This patch removes the last user of this field.

Signed-off-by Daniel Lezcano <[email protected]>
Signed-off-by Len Brown <[email protected]>

More, all machines hung with following similar info. Would you
please have a look at it?

	178748 ^[%Gmodprobe (1602) used greatest stack depth: 5504 bytes
left^M
	2178749 modprobe (1606) used greatest stack depth: 5472 bytes

left^M

	2178751 Task dump for CPU 1:^M
	2178752 swapper/1       R  running task     6736     0      1
0x00000000^M
	2178753  ffff8801e8029dc8 ffffffff8101cf96 ffff8801e8029e28
ffffffff813d294b^M
	2178754  0000000000000f99 0000000000000003 00000000003cf654
0000000025c17d03^M
	2178755  ffff8801e8029e38 ffff8801e74fc000 00000002590dc5c4
ffffffff8163cdb0^M
	2178756 Call Trace:^M
	2178757  [<ffffffff8101cf96>] ?
acpi_processor_ffh_cstate_enter+0x2d/0x2f^M
	2178758  [<ffffffff813d294b>] acpi_idle_enter_bm+0x1b1/0x236^M
	2178759  [<ffffffff8163cdb0>] ? disable_cpuidle+0x10/0x10^M
Quoted 8 lines
	2178760  [<ffffffff8163cdc2>] cpuidle_enter+0x12/0x14^M
	2178761  [<ffffffff8163d286>] cpuidle_wrap_enter+0x2f/0x6d^M
	2178762  [<ffffffff8163d2d4>] cpuidle_enter_tk+0x10/0x12^M
	2178763  [<ffffffff8163cdd6>] cpuidle_enter_state+0x12/0x3a^M
	2178764  [<ffffffff8163d4a7>] cpuidle_idle_call+0xe8/0x161^M
	2178765  [<ffffffff81008d99>] cpu_idle+0x5e/0xa4^M
	2178766  [<ffffffff8174c6c1>] start_secondary+0x1a9/0x1ad^M
	2178767 Task dump for CPU 2:^M

Hmm, is it possible that the nth CPU is trying to use an idle state before acpi_cstate[] is initialized?

Does bootin with maxcpus=1 work fine?

how about booting with processor.max_cstate=1?

-Len

Quoted 8 lines

Best regards
Changlong Xie

---
Linux Kernel Performance (LKP) project Open Source Technology
Center
Intel CorporationIntel Corporation

#2 lore raw
Quoted 76 lines

-----Original Message-----
From: Brown, Len
Sent: Tuesday, March 26, 2013 9:44 AM
To: Xie, ChanglongX
Cc: Daniel Lezcano; [email protected]; [email protected]
Subject: RE: Commit ac3ebafa81a makes NHM EX/EP machines hung out since 3.9-rc1

Quoted 54 lines

-----Original Message-----
From: Xie, ChanglongX
Sent: Tuesday, March 12, 2013 10:28 PM
To: Brown, Len
Cc: Daniel Lezcano; [email protected]; [email protected]
Subject: Commit ac3ebafa81a makes NHM EX/EP machines hung out since 3.9-
rc1

Hi Len,

FYI, since 3.9-rc1 our three NHM EP/EX LKP(linux kernel
performance) test servers
except SNB/IVB/WSM hung up unexpectly.

We did git bisect for about 8 times on all servers, it said that the
first bad commit is ac3ebafa.

commit ac3ebafa81af76d65e4fb45c6388f08e90ddcc6d
Author: Daniel Lezcano <[email protected]>
Date: Mon Feb 4 22:44:43 2013 +0000

ACPI / idle: remove usage of the statedata

Len Brown sent a patch to remove this field in the intel_idle driver. The other user of this field is the davinci cpuidle driver and a patch has been sent to remove the usage of it.
This patch removes the last user of this field.

Signed-off-by Daniel Lezcano <[email protected]>
Signed-off-by Len Brown <[email protected]>

More, all machines hung with following similar info. Would you please
have a look at it?

	178748 ^[%Gmodprobe (1602) used greatest stack depth: 5504 bytes
left^M
	2178749 modprobe (1606) used greatest stack depth: 5472 bytes left^M

	2178751 Task dump for CPU 1:^M
	2178752 swapper/1       R  running task     6736     0      1
0x00000000^M
	2178753  ffff8801e8029dc8 ffffffff8101cf96 ffff8801e8029e28
ffffffff813d294b^M
	2178754  0000000000000f99 0000000000000003 00000000003cf654
0000000025c17d03^M
	2178755  ffff8801e8029e38 ffff8801e74fc000 00000002590dc5c4
ffffffff8163cdb0^M
	2178756 Call Trace:^M
	2178757  [<ffffffff8101cf96>] ?
acpi_processor_ffh_cstate_enter+0x2d/0x2f^M
	2178758  [<ffffffff813d294b>] acpi_idle_enter_bm+0x1b1/0x236^M
	2178759  [<ffffffff8163cdb0>] ? disable_cpuidle+0x10/0x10^M
Quoted 8 lines
	2178760  [<ffffffff8163cdc2>] cpuidle_enter+0x12/0x14^M
	2178761  [<ffffffff8163d286>] cpuidle_wrap_enter+0x2f/0x6d^M
	2178762  [<ffffffff8163d2d4>] cpuidle_enter_tk+0x10/0x12^M
	2178763  [<ffffffff8163cdd6>] cpuidle_enter_state+0x12/0x3a^M
	2178764  [<ffffffff8163d4a7>] cpuidle_idle_call+0xe8/0x161^M
	2178765  [<ffffffff81008d99>] cpu_idle+0x5e/0xa4^M
	2178766  [<ffffffff8174c6c1>] start_secondary+0x1a9/0x1ad^M
	2178767 Task dump for CPU 2:^M
Quoted 6 lines

Hmm, is it possible that the nth CPU is trying to use an idle state before acpi_cstate[] is initialized?

Does bootin with maxcpus=1 work fine?

how about booting with processor.max_cstate=1?

I just test it. Any one of "maxcpus=1" or "processor.max_cstate=1" or " idle=poll" Works fine.

Best Regards
Changlox

Quoted 11 lines

-Len

Quoted 8 lines

Best regards
Changlong Xie

---
Linux Kernel Performance (LKP) project Open Source Technology
Center
Intel CorporationIntel Corporation

#3 lore raw
In reply to a message not in the archive
Quoted 5 lines · Changlong Xie

Hi Len,

FYI, since 3.9-rc1 our three NHM EP/EX LKP(linux kernel performance) test servers except SNB/IVB/WSM hung up unexpectly.

the following draft patch can fix the panic, so it proved we still need the percpu cstate.

but look at the struct acpi_processor_cx {

        u8 valid;
        u8 type;
        u32 address;
        u8 entry_method;
        u8 index;
        u32 latency;
        u8 bm_sts_skip;
        char desc[ACPI_CX_DESC_LEN];
};

I have printed all members except the last one. all of them are same on all cpu. It's interesting.

-----------

Quoted 1 line

From 5a4fc23fdf5202f8555a25a03afc4d06ac168032 Mon Sep 17 00:00:00 2001

From: Alex Shi <[email protected]>
Date: Tue, 26 Mar 2013 22:57:47 +0800
Subject: [PATCH] acpi: abc

---
 drivers/acpi/processor_idle.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)
drivers/acpi/processor_idle.c +10 -6
index fc95308..61373ab 100644
@@ -66,7 +66,7 @@ module_param(latency_factor, uint, 0644);
 
 static DEFINE_PER_CPU(struct cpuidle_device *, acpi_cpuidle_device);
 
-static struct acpi_processor_cx *acpi_cstate[CPUIDLE_STATE_MAX];
+static DEFINE_PER_CPU (struct acpi_processor_cx *[CPUIDLE_STATE_MAX], acpi_cstate);
 
 static int disabled_by_idle_boot_param(void)
 {

@@ -722,9 +722,10 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev,
 		struct cpuidle_driver *drv, int index)
 {
 	struct acpi_processor *pr;
-	struct acpi_processor_cx *cx = acpi_cstate[index];
+	struct acpi_processor_cx *cx;
 
 	pr = __this_cpu_read(processors);
+ 	cx = per_cpu(acpi_cstate[index], pr->id);
 
 	if (unlikely(!pr))
 		return -EINVAL;

@@ -745,7 +746,8 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev,
  */
 static int acpi_idle_play_dead(struct cpuidle_device *dev, int index)
 {
-	struct acpi_processor_cx *cx = acpi_cstate[index];
+	struct acpi_processor_cx *cx;
+ 	cx = per_cpu(acpi_cstate[index], dev->cpu);
 
 	ACPI_FLUSH_CPU_CACHE();
 

@@ -775,9 +777,10 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
 		struct cpuidle_driver *drv, int index)
 {
 	struct acpi_processor *pr;
-	struct acpi_processor_cx *cx = acpi_cstate[index];
+	struct acpi_processor_cx *cx;
 
 	pr = __this_cpu_read(processors);
+ 	cx = per_cpu(acpi_cstate[index], pr->id);
 
 	if (unlikely(!pr))
 		return -EINVAL;

@@ -833,9 +836,10 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
 		struct cpuidle_driver *drv, int index)
 {
 	struct acpi_processor *pr;
-	struct acpi_processor_cx *cx = acpi_cstate[index];
+	struct acpi_processor_cx *cx;
 
 	pr = __this_cpu_read(processors);
+ 	cx = per_cpu(acpi_cstate[index], pr->id);
 
 	if (unlikely(!pr))
 		return -EINVAL;

@@ -960,7 +964,7 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
 		    !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
 			continue;
 #endif
-		acpi_cstate[count] = cx;
+		per_cpu(acpi_cstate[count], dev->cpu) = cx;
 
 		count++;
 		if (count == CPUIDLE_STATE_MAX)
Signature · 1 line
1.7.5.4
#4 lore raw
Quoted 8 lines · Alex Shi
Quoted 5 lines · Changlong Xie

Hi Len,

FYI, since 3.9-rc1 our three NHM EP/EX LKP(linux kernel performance) test servers except SNB/IVB/WSM hung up unexpectly.

the following draft patch can fix the panic, so it proved we still need the percpu cstate.

Alex,

I don't get the connection between the patch and "we still need the percpu cstate" ?

Could you give more informations about how to reproduce the problem ?

How many processors has your system ?

Could you provide a small test program to trigger the issue ?

When does it appear ? At boot time ? Under load ? At idle time ? etc ...

What is the result of your investigation which lead you to propose the patch below ?

Thanks
-- Daniel

Quoted 95 lines

but look at the struct acpi_processor_cx {

        u8 valid;
        u8 type;
        u32 address;
        u8 entry_method;
        u8 index;
        u32 latency;
        u8 bm_sts_skip;
        char desc[ACPI_CX_DESC_LEN];
};

I have printed all members except the last one. all of them are same on all cpu. It's interesting.

-----------

From 5a4fc23fdf5202f8555a25a03afc4d06ac168032 Mon Sep 17 00:00:00 2001
From: Alex Shi <[email protected]>
Date: Tue, 26 Mar 2013 22:57:47 +0800
Subject: [PATCH] acpi: abc

---
 drivers/acpi/processor_idle.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)
drivers/acpi/processor_idle.c +10 -6
index fc95308..61373ab 100644
@@ -66,7 +66,7 @@ module_param(latency_factor, uint, 0644);
 
 static DEFINE_PER_CPU(struct cpuidle_device *, acpi_cpuidle_device);
 
-static struct acpi_processor_cx *acpi_cstate[CPUIDLE_STATE_MAX];
+static DEFINE_PER_CPU (struct acpi_processor_cx *[CPUIDLE_STATE_MAX], acpi_cstate);
 
 static int disabled_by_idle_boot_param(void)
 {

@@ -722,9 +722,10 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev,
 		struct cpuidle_driver *drv, int index)
 {
 	struct acpi_processor *pr;
-	struct acpi_processor_cx *cx = acpi_cstate[index];
+	struct acpi_processor_cx *cx;
 
 	pr = __this_cpu_read(processors);
+ 	cx = per_cpu(acpi_cstate[index], pr->id);
 
 	if (unlikely(!pr))
 		return -EINVAL;

@@ -745,7 +746,8 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev,
  */
 static int acpi_idle_play_dead(struct cpuidle_device *dev, int index)
 {
-	struct acpi_processor_cx *cx = acpi_cstate[index];
+	struct acpi_processor_cx *cx;
+ 	cx = per_cpu(acpi_cstate[index], dev->cpu);
 
 	ACPI_FLUSH_CPU_CACHE();
 

@@ -775,9 +777,10 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
 		struct cpuidle_driver *drv, int index)
 {
 	struct acpi_processor *pr;
-	struct acpi_processor_cx *cx = acpi_cstate[index];
+	struct acpi_processor_cx *cx;
 
 	pr = __this_cpu_read(processors);
+ 	cx = per_cpu(acpi_cstate[index], pr->id);
 
 	if (unlikely(!pr))
 		return -EINVAL;

@@ -833,9 +836,10 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
 		struct cpuidle_driver *drv, int index)
 {
 	struct acpi_processor *pr;
-	struct acpi_processor_cx *cx = acpi_cstate[index];
+	struct acpi_processor_cx *cx;
 
 	pr = __this_cpu_read(processors);
+ 	cx = per_cpu(acpi_cstate[index], pr->id);
 
 	if (unlikely(!pr))
 		return -EINVAL;

@@ -960,7 +964,7 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
 		    !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
 			continue;
 #endif
-		acpi_cstate[count] = cx;
+		per_cpu(acpi_cstate[count], dev->cpu) = cx;
 
 		count++;
 		if (count == CPUIDLE_STATE_MAX)
Signature · 10 lines
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#5 lore raw
In reply to a message not in the archive
Quoted 10 lines · Changlong Xie

Hi Len,

FYI, since 3.9-rc1 our three NHM EP/EX LKP(linux kernel performance) test servers except SNB/IVB/WSM hung up unexpectly.

We did git bisect for about 8 times on all servers, it said that the first bad commit is ac3ebafa.

commit ac3ebafa81af76d65e4fb45c6388f08e90ddcc6d
Author: Daniel Lezcano <[email protected]>
Date: Mon Feb 4 22:44:43 2013 +0000

fixing patch for review:

-----------------------

Quoted 1 line

From 78a74aea386b0969909c2e4ae388024ce71fdb18 Mon Sep 17 00:00:00 2001

From: Alex Shi <[email protected]>
Date: Tue, 26 Mar 2013 22:57:47 +0800
Subject: [PATCH] cpuidle/acpi: recover percpu acpi processor cstate

Commit: ac3ebafa81af76d6 "ACPI / idle: remove usage of the statedata" change the percpu processor cstate to a unify unique cstate in acpi idle. That cause all our NHM box boot hang or panic.

2178751 Task dump for CPU 1:^M
2178752 swapper/1 R running task 6736 0 1
0x00000000^M

	2178753  ffff8801e8029dc8 ffffffff8101cf96 ffff8801e8029e28
ffffffff813d294b^M
	2178754  0000000000000f99 0000000000000003 00000000003cf654
0000000025c17d03^M
	2178755  ffff8801e8029e38 ffff8801e74fc000 00000002590dc5c4
ffffffff8163cdb0^M
	2178756 Call Trace:^M
	2178757  [<ffffffff8101cf96>] ? acpi_processor_ffh_cstate_enter+0x2d/0x2f^M
	2178758  [<ffffffff813d294b>] acpi_idle_enter_bm+0x1b1/0x236^M
	2178759  [<ffffffff8163cdb0>] ? disable_cpuidle+0x10/0x10^M
	2178760  [<ffffffff8163cdc2>] cpuidle_enter+0x12/0x14^M
	2178761  [<ffffffff8163d286>] cpuidle_wrap_enter+0x2f/0x6d^M
	2178762  [<ffffffff8163d2d4>] cpuidle_enter_tk+0x10/0x12^M
	2178763  [<ffffffff8163cdd6>] cpuidle_enter_state+0x12/0x3a^M
	2178764  [<ffffffff8163d4a7>] cpuidle_idle_call+0xe8/0x161^M
	2178765  [<ffffffff81008d99>] cpu_idle+0x5e/0xa4^M
	2178766  [<ffffffff8174c6c1>] start_secondary+0x1a9/0x1ad^M
	2178767 Task dump for CPU 2:^M

In fact, the acpi idle bases on percpu cstate difference assumption, the infrastructure use many percpu structures to implement self. Just unique acpi_processor_cx is far far not enough.

This patch just is a quick fix by introducing back the percpu cstates. And keep driver_data away.

If someone really want to unify the acpi cstates, please make sure whole software infrastructure changed and get the grant from hardware, include many kinds of BIOS setting.

Signed-off-by Alex Shi <[email protected]>
---
 drivers/acpi/processor_idle.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
drivers/acpi/processor_idle.c +7 -6
index fc95308..ee255c6 100644
@@ -66,7 +66,8 @@ module_param(latency_factor, uint, 0644);
 
 static DEFINE_PER_CPU(struct cpuidle_device *, acpi_cpuidle_device);
 
-static struct acpi_processor_cx *acpi_cstate[CPUIDLE_STATE_MAX];
+static DEFINE_PER_CPU(struct acpi_processor_cx * [CPUIDLE_STATE_MAX],
+								acpi_cstate);
 
 static int disabled_by_idle_boot_param(void)
 {

@@ -722,7 +723,7 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev,
 		struct cpuidle_driver *drv, int index)
 {
 	struct acpi_processor *pr;
-	struct acpi_processor_cx *cx = acpi_cstate[index];
+	struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);
 
 	pr = __this_cpu_read(processors);
 

@@ -745,7 +746,7 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev,
  */
 static int acpi_idle_play_dead(struct cpuidle_device *dev, int index)
 {
-	struct acpi_processor_cx *cx = acpi_cstate[index];
+	struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);
 
 	ACPI_FLUSH_CPU_CACHE();
 

@@ -775,7 +776,7 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
 		struct cpuidle_driver *drv, int index)
 {
 	struct acpi_processor *pr;
-	struct acpi_processor_cx *cx = acpi_cstate[index];
+	struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);
 
 	pr = __this_cpu_read(processors);
 

@@ -833,7 +834,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
 		struct cpuidle_driver *drv, int index)
 {
 	struct acpi_processor *pr;
-	struct acpi_processor_cx *cx = acpi_cstate[index];
+	struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);
 
 	pr = __this_cpu_read(processors);
 

@@ -960,7 +961,7 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
 		    !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
 			continue;
 #endif
-		acpi_cstate[count] = cx;
+		per_cpu(acpi_cstate[count], dev->cpu) = cx;
 
 		count++;
 		if (count == CPUIDLE_STATE_MAX)
Signature · 1 line
1.7.12
#6 lore raw
Quoted 51 lines · Alex Shi
Quoted 10 lines · Changlong Xie

Hi Len,

FYI, since 3.9-rc1 our three NHM EP/EX LKP(linux kernel performance) test servers except SNB/IVB/WSM hung up unexpectly.

We did git bisect for about 8 times on all servers, it said that the first bad commit is ac3ebafa.

commit ac3ebafa81af76d65e4fb45c6388f08e90ddcc6d
Author: Daniel Lezcano <[email protected]>
Date: Mon Feb 4 22:44:43 2013 +0000

fixing patch for review:

-----------------------
From 78a74aea386b0969909c2e4ae388024ce71fdb18 Mon Sep 17 00:00:00 2001
From: Alex Shi <[email protected]>
Date: Tue, 26 Mar 2013 22:57:47 +0800
Subject: [PATCH] cpuidle/acpi: recover percpu acpi processor cstate

Commit: ac3ebafa81af76d6 "ACPI / idle: remove usage of the statedata" change the percpu processor cstate to a unify unique cstate in acpi idle. That cause all our NHM box boot hang or panic.

2178751 Task dump for CPU 1:^M
2178752 swapper/1 R running task 6736 0 1
0x00000000^M

	2178753  ffff8801e8029dc8 ffffffff8101cf96 ffff8801e8029e28
ffffffff813d294b^M
	2178754  0000000000000f99 0000000000000003 00000000003cf654
0000000025c17d03^M
	2178755  ffff8801e8029e38 ffff8801e74fc000 00000002590dc5c4
ffffffff8163cdb0^M
	2178756 Call Trace:^M
	2178757  [<ffffffff8101cf96>] ? acpi_processor_ffh_cstate_enter+0x2d/0x2f^M
	2178758  [<ffffffff813d294b>] acpi_idle_enter_bm+0x1b1/0x236^M
	2178759  [<ffffffff8163cdb0>] ? disable_cpuidle+0x10/0x10^M
	2178760  [<ffffffff8163cdc2>] cpuidle_enter+0x12/0x14^M
	2178761  [<ffffffff8163d286>] cpuidle_wrap_enter+0x2f/0x6d^M
	2178762  [<ffffffff8163d2d4>] cpuidle_enter_tk+0x10/0x12^M
	2178763  [<ffffffff8163cdd6>] cpuidle_enter_state+0x12/0x3a^M
	2178764  [<ffffffff8163d4a7>] cpuidle_idle_call+0xe8/0x161^M
	2178765  [<ffffffff81008d99>] cpu_idle+0x5e/0xa4^M
	2178766  [<ffffffff8174c6c1>] start_secondary+0x1a9/0x1ad^M
	2178767 Task dump for CPU 2:^M

In fact, the acpi idle bases on percpu cstate difference assumption, the infrastructure use many percpu structures to implement self. Just unique acpi_processor_cx is far far not enough.

Quoted 6 lines

This patch just is a quick fix by introducing back the percpu cstates. And keep driver_data away.

If someone really want to unify the acpi cstates, please make sure whole software infrastructure changed and get the grant from hardware, include many kinds of BIOS setting.

Hi Alex,

could you elaborate a bit the explanation. I don't see where is the analysis of the bug. Why do we have this stack trace ?

Thanks
-- Daniel

Signature · 10 lines
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#7 lore raw
Quoted 4 lines · Daniel Lezcano

Hi Alex,

could you elaborate a bit the explanation. I don't see where is the analysis of the bug. Why do we have this stack trace ?

I also don't know much details of the mwait setting for cpu idle, and can not reproduce the ops on my box. Most of time the nhm box is just hang.

Maybe BIOS setting have bugs and your change is just expose them.

but it is clear inappropriate to change percpu cstate to a unified unique value, while left lots of percpu struct service for this. A example of x86, you can look into arch/x86/kernel/acpi/cstate.c and other acpi head files.

Signature · 2 lines
Thanks
    Alex
Tested-by
#8 lore raw
In reply to Alex Shi
Quoted 114 lines · Alex Shi
Quoted 10 lines · Changlong Xie

Hi Len,

FYI, since 3.9-rc1 our three NHM EP/EX LKP(linux kernel performance) test servers except SNB/IVB/WSM hung up unexpectly.

We did git bisect for about 8 times on all servers, it said that the first bad commit is ac3ebafa.

commit ac3ebafa81af76d65e4fb45c6388f08e90ddcc6d
Author: Daniel Lezcano <[email protected]>
Date: Mon Feb 4 22:44:43 2013 +0000

fixing patch for review:

-----------------------
From 78a74aea386b0969909c2e4ae388024ce71fdb18 Mon Sep 17 00:00:00 2001
From: Alex Shi <[email protected]>
Date: Tue, 26 Mar 2013 22:57:47 +0800
Subject: [PATCH] cpuidle/acpi: recover percpu acpi processor cstate

Commit: ac3ebafa81af76d6 "ACPI / idle: remove usage of the statedata" change the percpu processor cstate to a unify unique cstate in acpi idle. That cause all our NHM box boot hang or panic.

2178751 Task dump for CPU 1:^M
2178752 swapper/1 R running task 6736 0 1
0x00000000^M

        2178753  ffff8801e8029dc8 ffffffff8101cf96 ffff8801e8029e28
ffffffff813d294b^M
        2178754  0000000000000f99 0000000000000003 00000000003cf654
0000000025c17d03^M
        2178755  ffff8801e8029e38 ffff8801e74fc000 00000002590dc5c4
ffffffff8163cdb0^M
        2178756 Call Trace:^M
        2178757  [<ffffffff8101cf96>] ? acpi_processor_ffh_cstate_enter+0x2d/0x2f^M
        2178758  [<ffffffff813d294b>] acpi_idle_enter_bm+0x1b1/0x236^M
        2178759  [<ffffffff8163cdb0>] ? disable_cpuidle+0x10/0x10^M
        2178760  [<ffffffff8163cdc2>] cpuidle_enter+0x12/0x14^M
        2178761  [<ffffffff8163d286>] cpuidle_wrap_enter+0x2f/0x6d^M
        2178762  [<ffffffff8163d2d4>] cpuidle_enter_tk+0x10/0x12^M
        2178763  [<ffffffff8163cdd6>] cpuidle_enter_state+0x12/0x3a^M
        2178764  [<ffffffff8163d4a7>] cpuidle_idle_call+0xe8/0x161^M
        2178765  [<ffffffff81008d99>] cpu_idle+0x5e/0xa4^M
        2178766  [<ffffffff8174c6c1>] start_secondary+0x1a9/0x1ad^M
        2178767 Task dump for CPU 2:^M

In fact, the acpi idle bases on percpu cstate difference assumption, the infrastructure use many percpu structures to implement self. Just unique acpi_processor_cx is far far not enough.

This patch just is a quick fix by introducing back the percpu cstates. And keep driver_data away.

If someone really want to unify the acpi cstates, please make sure whole software infrastructure changed and get the grant from hardware, include many kinds of BIOS setting.

Signed-off-by Alex Shi <[email protected]>
---
 drivers/acpi/processor_idle.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
drivers/acpi/processor_idle.c +6 -5
index fc95308..ee255c6 100644
@@ -66,7 +66,8 @@ module_param(latency_factor, uint, 0644);
 
 static DEFINE_PER_CPU(struct cpuidle_device *, acpi_cpuidle_device);
 
-static struct acpi_processor_cx *acpi_cstate[CPUIDLE_STATE_MAX];
+static DEFINE_PER_CPU(struct acpi_processor_cx * [CPUIDLE_STATE_MAX],
+                                                               acpi_cstate);
 
 static int disabled_by_idle_boot_param(void)
 {

@@ -722,7 +723,7 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev,
                struct cpuidle_driver *drv, int index)
 {
        struct acpi_processor *pr;
-       struct acpi_processor_cx *cx = acpi_cstate[index];
+       struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);
 
        pr = __this_cpu_read(processors);
 

@@ -745,7 +746,7 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev,
  */
 static int acpi_idle_play_dead(struct cpuidle_device *dev, int index)
 {
-       struct acpi_processor_cx *cx = acpi_cstate[index];
+       struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);
 
        ACPI_FLUSH_CPU_CACHE();
 

@@ -775,7 +776,7 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
                struct cpuidle_driver *drv, int index)
 {
        struct acpi_processor *pr;
-       struct acpi_processor_cx *cx = acpi_cstate[index];
+       struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);
 
        pr = __this_cpu_read(processors);
 

@@ -833,7 +834,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
                struct cpuidle_driver *drv, int index)
 {
        struct acpi_processor *pr;
-       struct acpi_processor_cx *cx = acpi_cstate[index];
+       struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);
 
        pr = __this_cpu_read(processors);

drivers/acpi/processor_idle.c static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,

Quoted 9 lines
drivers/acpi/processor_idle.c +1 -2
@@ -960,7 +961,7 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
                    !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
                        continue;
 #endif
-               acpi_cstate[count] = cx;
+               per_cpu(acpi_cstate[count], dev->cpu) = cx;
 
                count++;
                if (count == CPUIDLE_STATE_MAX)
--

It fixes the problem on my 8 sockets Nehalem-EX that will hang around ip_auto_config()

Tested-by Yinghai Lu <[email protected]>

Rafael,
Can you please push this one to Linus tree ? As the offending patch was via your tree to upstream.

Thanks

Yinghai

#9 lore raw
Quoted 128 lines · Yinghai Lu
Quoted 114 lines · Alex Shi
Quoted 10 lines · Changlong Xie

Hi Len,

FYI, since 3.9-rc1 our three NHM EP/EX LKP(linux kernel performance) test servers except SNB/IVB/WSM hung up unexpectly.

We did git bisect for about 8 times on all servers, it said that the first bad commit is ac3ebafa.

commit ac3ebafa81af76d65e4fb45c6388f08e90ddcc6d
Author: Daniel Lezcano <[email protected]>
Date: Mon Feb 4 22:44:43 2013 +0000

fixing patch for review:

-----------------------
From 78a74aea386b0969909c2e4ae388024ce71fdb18 Mon Sep 17 00:00:00 2001
From: Alex Shi <[email protected]>
Date: Tue, 26 Mar 2013 22:57:47 +0800
Subject: [PATCH] cpuidle/acpi: recover percpu acpi processor cstate

Commit: ac3ebafa81af76d6 "ACPI / idle: remove usage of the statedata" change the percpu processor cstate to a unify unique cstate in acpi idle. That cause all our NHM box boot hang or panic.

2178751 Task dump for CPU 1:^M
2178752 swapper/1 R running task 6736 0 1
0x00000000^M

        2178753  ffff8801e8029dc8 ffffffff8101cf96 ffff8801e8029e28
ffffffff813d294b^M
        2178754  0000000000000f99 0000000000000003 00000000003cf654
0000000025c17d03^M
        2178755  ffff8801e8029e38 ffff8801e74fc000 00000002590dc5c4
ffffffff8163cdb0^M
        2178756 Call Trace:^M
        2178757  [<ffffffff8101cf96>] ? acpi_processor_ffh_cstate_enter+0x2d/0x2f^M
        2178758  [<ffffffff813d294b>] acpi_idle_enter_bm+0x1b1/0x236^M
        2178759  [<ffffffff8163cdb0>] ? disable_cpuidle+0x10/0x10^M
        2178760  [<ffffffff8163cdc2>] cpuidle_enter+0x12/0x14^M
        2178761  [<ffffffff8163d286>] cpuidle_wrap_enter+0x2f/0x6d^M
        2178762  [<ffffffff8163d2d4>] cpuidle_enter_tk+0x10/0x12^M
        2178763  [<ffffffff8163cdd6>] cpuidle_enter_state+0x12/0x3a^M
        2178764  [<ffffffff8163d4a7>] cpuidle_idle_call+0xe8/0x161^M
        2178765  [<ffffffff81008d99>] cpu_idle+0x5e/0xa4^M
        2178766  [<ffffffff8174c6c1>] start_secondary+0x1a9/0x1ad^M
        2178767 Task dump for CPU 2:^M

In fact, the acpi idle bases on percpu cstate difference assumption, the infrastructure use many percpu structures to implement self. Just unique acpi_processor_cx is far far not enough.

This patch just is a quick fix by introducing back the percpu cstates. And keep driver_data away.

If someone really want to unify the acpi cstates, please make sure whole software infrastructure changed and get the grant from hardware, include many kinds of BIOS setting.

Signed-off-by Alex Shi <[email protected]>
---
 drivers/acpi/processor_idle.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
drivers/acpi/processor_idle.c +6 -5
index fc95308..ee255c6 100644
@@ -66,7 +66,8 @@ module_param(latency_factor, uint, 0644);
 
 static DEFINE_PER_CPU(struct cpuidle_device *, acpi_cpuidle_device);
 
-static struct acpi_processor_cx *acpi_cstate[CPUIDLE_STATE_MAX];
+static DEFINE_PER_CPU(struct acpi_processor_cx * [CPUIDLE_STATE_MAX],
+                                                               acpi_cstate);
 
 static int disabled_by_idle_boot_param(void)
 {

@@ -722,7 +723,7 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev,
                struct cpuidle_driver *drv, int index)
 {
        struct acpi_processor *pr;
-       struct acpi_processor_cx *cx = acpi_cstate[index];
+       struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);
 
        pr = __this_cpu_read(processors);
 

@@ -745,7 +746,7 @@ static int acpi_idle_enter_c1(struct cpuidle_device *dev,
  */
 static int acpi_idle_play_dead(struct cpuidle_device *dev, int index)
 {
-       struct acpi_processor_cx *cx = acpi_cstate[index];
+       struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);
 
        ACPI_FLUSH_CPU_CACHE();
 

@@ -775,7 +776,7 @@ static int acpi_idle_enter_simple(struct cpuidle_device *dev,
                struct cpuidle_driver *drv, int index)
 {
        struct acpi_processor *pr;
-       struct acpi_processor_cx *cx = acpi_cstate[index];
+       struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);
 
        pr = __this_cpu_read(processors);
 

@@ -833,7 +834,7 @@ static int acpi_idle_enter_bm(struct cpuidle_device *dev,
                struct cpuidle_driver *drv, int index)
 {
        struct acpi_processor *pr;
-       struct acpi_processor_cx *cx = acpi_cstate[index];
+       struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu);
 
        pr = __this_cpu_read(processors);

drivers/acpi/processor_idle.c static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,

Quoted 9 lines
drivers/acpi/processor_idle.c +1 -2
@@ -960,7 +961,7 @@ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr,
                    !(acpi_gbl_FADT.flags & ACPI_FADT_C2_MP_SUPPORTED))
                        continue;
 #endif
-               acpi_cstate[count] = cx;
+               per_cpu(acpi_cstate[count], dev->cpu) = cx;
 
                count++;
                if (count == CPUIDLE_STATE_MAX)
--

It fixes the problem on my 8 sockets Nehalem-EX that will hang around ip_auto_config()

Quoted 5 lines
Tested-by Yinghai Lu <[email protected]>

Rafael,
Can you please push this one to Linus tree ? As the offending patch was via your tree to upstream.

I will if there's no better way to address this issue before -rc6.

Thanks,
Rafael

Signature · 2 lines
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
#10 lore raw
Quoted 10 lines · Rafael J. Wysocki
Quoted 9 lines
Quoted 9 lines

It fixes the problem on my 8 sockets Nehalem-EX that will hang around ip_auto_config()

Tested-by Yinghai Lu <[email protected]>

Rafael,
Can you please push this one to Linus tree ? As the offending patch was via your tree to upstream.

I will if there's no better way to address this issue before -rc6.

If it can be pushed to upstream, please add:

Reported-by LKP project <[email protected]>
Reported-by Xie ChanglongX <[email protected]>
Signature · 1 line
Thanks Alex
#11 lore raw
Quoted 10 lines · Alex Shi
Quoted 9 lines · Rafael J. Wysocki
Quoted 9 lines
Quoted 9 lines

It fixes the problem on my 8 sockets Nehalem-EX that will hang around ip_auto_config()

Tested-by Yinghai Lu <[email protected]>

Rafael,
Can you please push this one to Linus tree ? As the offending patch was via your tree to upstream.

Quoted 6 lines
Quoted 1 line

I will if there's no better way to address this issue before -rc6.

If it can be pushed to upstream, please add:

Reported-by LKP project <[email protected]>
Reported-by Xie ChanglongX <[email protected]>

I have applied the patch, but I've modified the changelog. Please check the linux-next branch of my tree and see if the commit is what you meant.

Thanks,
Rafael

Signature · 2 lines
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.