* [Regression] 2.6.27-rc2-git4: suspend and power off fails on Asus M3A32-MVP
@ 2008-08-09 21:21 Rafael J. Wysocki
2008-08-12 16:03 ` Langsdorf, Mark
0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2008-08-09 21:21 UTC (permalink / raw)
To: Mark Langsdorf
Cc: Andrew Morton, Dave Jones, Linus Torvalds, LKML,
kernel-testers-u79uwXL29TY76Z2rM5mHXA, Randy Dunlap, Frank Arnold
Hi Mark,
Unfortunately, your commit 34ae7f35a21694aa5cb8829dc5142c39d73d6ba0
("[CPUFREQ][2/2] preregister support for powernow-k8") causes a power off/
suspend/ hibernation regression on my test box with the Asus M3A32-MVP
mainboard (AMD 790 chipset) and the Phenom 9850 CPU.
Apparently, the patch causes a crash in a CPU hotplug notifier to happen, but
I'm not really sure of that, since there are a lot of stack traces resulting
from it and I'm unable to capture them over a serial console.
It is sufficient to revert commit 34ae7f35a21694aa5cb8829dc5142c39d73d6ba0 to
make things work again.
Please advise.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [Regression] 2.6.27-rc2-git4: suspend and power off fails on Asus M3A32-MVP
2008-08-09 21:21 [Regression] 2.6.27-rc2-git4: suspend and power off fails on Asus M3A32-MVP Rafael J. Wysocki
@ 2008-08-12 16:03 ` Langsdorf, Mark
[not found] ` <6453C3CB8E2B3646B0D020C112613273C5AC60-F4vEnV32AJndiMxs2dRykAC/G2K4zDHf@public.gmane.org>
0 siblings, 1 reply; 6+ messages in thread
From: Langsdorf, Mark @ 2008-08-12 16:03 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Andrew Morton, Dave Jones, Linus Torvalds, LKML, kernel-testers,
Randy Dunlap, Arnold, Frank
> Unfortunately, your commit 34ae7f35a21694aa5cb8829dc5142c39d73d6ba0
> ("[CPUFREQ][2/2] preregister support for powernow-k8") causes
> a power off/
> suspend/ hibernation regression on my test box with the Asus M3A32-MVP
> mainboard (AMD 790 chipset) and the Phenom 9850 CPU.
>
> Apparently, the patch causes a crash in a CPU hotplug
> notifier to happen, but I'm not really sure of that, since there are a
lot of stack
> traces resulting from it and I'm unable to capture them over a serial
console.
>
> It is sufficient to revert commit
> 34ae7f35a21694aa5cb8829dc5142c39d73d6ba0 to
> make things work again.
Thanks for the clear report. I'll look into it and try to fix it.
-Mark Langsdorf
Operating System Research Center
AMD
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [Regression] 2.6.27-rc2-git4: suspend and power off fails on Asus M3A32-MVP
2008-08-19 17:03 ` Rafael J. Wysocki
@ 2008-08-19 17:02 ` Langsdorf, Mark
0 siblings, 0 replies; 6+ messages in thread
From: Langsdorf, Mark @ 2008-08-19 17:02 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Andrew Morton, Dave Jones, Linus Torvalds, LKML, kernel-testers,
Randy Dunlap, Arnold, Frank
> > > It is sufficient to revert commit
> > > 34ae7f35a21694aa5cb8829dc5142c39d73d6ba0 to
> > > make things work again.
> >
> > Thanks for the clear report. I'll look into it and try to fix it.
>
> Any news?
Revert the patch for now. I'm still looking into getting
a reliable reproduction and I do not have a fix at this time.
-Mark Langsdorf
Operating System Research Center
AMD
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Regression] 2.6.27-rc2-git4: suspend and power off fails on Asus M3A32-MVP
[not found] ` <6453C3CB8E2B3646B0D020C112613273C5AC60-F4vEnV32AJndiMxs2dRykAC/G2K4zDHf@public.gmane.org>
@ 2008-08-19 17:03 ` Rafael J. Wysocki
2008-08-19 17:02 ` Langsdorf, Mark
2008-08-19 19:37 ` [PATCH] Revert "[CPUFREQ][2/2] preregister support for powernow-k8" Rafael J. Wysocki
1 sibling, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2008-08-19 17:03 UTC (permalink / raw)
To: Langsdorf, Mark
Cc: Andrew Morton, Dave Jones, Linus Torvalds, LKML,
kernel-testers-u79uwXL29TY76Z2rM5mHXA, Randy Dunlap,
Arnold, Frank
On Tuesday, 12 of August 2008, Langsdorf, Mark wrote:
>
> > Unfortunately, your commit 34ae7f35a21694aa5cb8829dc5142c39d73d6ba0
> > ("[CPUFREQ][2/2] preregister support for powernow-k8") causes
> > a power off/
> > suspend/ hibernation regression on my test box with the Asus M3A32-MVP
> > mainboard (AMD 790 chipset) and the Phenom 9850 CPU.
> >
> > Apparently, the patch causes a crash in a CPU hotplug
> > notifier to happen, but I'm not really sure of that, since there are a
> lot of stack
> > traces resulting from it and I'm unable to capture them over a serial
> console.
> >
> > It is sufficient to revert commit
> > 34ae7f35a21694aa5cb8829dc5142c39d73d6ba0 to
> > make things work again.
>
> Thanks for the clear report. I'll look into it and try to fix it.
Any news?
Rafael
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Revert "[CPUFREQ][2/2] preregister support for powernow-k8"
[not found] ` <6453C3CB8E2B3646B0D020C112613273C5AC60-F4vEnV32AJndiMxs2dRykAC/G2K4zDHf@public.gmane.org>
2008-08-19 17:03 ` Rafael J. Wysocki
@ 2008-08-19 19:37 ` Rafael J. Wysocki
2008-08-19 19:43 ` Dave Jones
1 sibling, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2008-08-19 19:37 UTC (permalink / raw)
To: Andrew Morton, Linus Torvalds
Cc: Langsdorf, Mark, Dave Jones, LKML,
kernel-testers-u79uwXL29TY76Z2rM5mHXA, Randy Dunlap,
Arnold, Frank, Torsten Kaiser
From: Rafael J. Wysocki <rjw-KKrjLPT3xs0@public.gmane.org>
Revert "[CPUFREQ][2/2] preregister support for powernow-k8"
Revert commit 34ae7f35a21694aa5cb8829dc5142c39d73d6ba0
("[CPUFREQ][2/2] preregister support for powernow-k8") that causes
regressions tracked as:
http://bugzilla.kernel.org/show_bug.cgi?id=11296 ,
http://bugzilla.kernel.org/show_bug.cgi?id=11339 .
Namely, my test box with the Asus M3A32-MVP mainboard and the AMD
Phenom 9850 CPU crashes during hibernation, suspend to RAM and ACPI
power off, apparently due to a failing CPU hotplug notifier. In
turn, on the Torsten Kaiser's test system only one CPU is correctly
handled by cpufreq. These problems go away after reverting
34ae7f35a21694aa5cb8829dc5142c39d73d6ba0, but unfortunately they are
quite difficult to debug.
Signed-off-by: Rafael J. Wysocki <rjw-KKrjLPT3xs0@public.gmane.org>
---
arch/x86/kernel/cpu/cpufreq/powernow-k8.c | 109 +++++++++---------------------
arch/x86/kernel/cpu/cpufreq/powernow-k8.h | 3
2 files changed, 37 insertions(+), 75 deletions(-)
Index: linux-2.6/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
+++ linux-2.6/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
@@ -737,63 +737,44 @@ static int find_psb_table(struct powerno
#ifdef CONFIG_X86_POWERNOW_K8_ACPI
static void powernow_k8_acpi_pst_values(struct powernow_k8_data *data, unsigned int index)
{
- if (!data->acpi_data->state_count || (cpu_family == CPU_HW_PSTATE))
+ if (!data->acpi_data.state_count || (cpu_family == CPU_HW_PSTATE))
return;
- data->irt = (data->acpi_data->states[index].control >> IRT_SHIFT) & IRT_MASK;
- data->rvo = (data->acpi_data->states[index].control >> RVO_SHIFT) & RVO_MASK;
- data->exttype = (data->acpi_data->states[index].control >> EXT_TYPE_SHIFT) & EXT_TYPE_MASK;
- data->plllock = (data->acpi_data->states[index].control >> PLL_L_SHIFT) & PLL_L_MASK;
- data->vidmvs = 1 << ((data->acpi_data->states[index].control >> MVS_SHIFT) & MVS_MASK);
- data->vstable = (data->acpi_data->states[index].control >> VST_SHIFT) & VST_MASK;
-}
-
-
-static struct acpi_processor_performance *acpi_perf_data;
-static int preregister_valid;
-
-static int powernow_k8_cpu_preinit_acpi(void)
-{
- acpi_perf_data = alloc_percpu(struct acpi_processor_performance);
- if (!acpi_perf_data)
- return -ENODEV;
-
- if (acpi_processor_preregister_performance(acpi_perf_data))
- return -ENODEV;
- else
- preregister_valid = 1;
- return 0;
+ data->irt = (data->acpi_data.states[index].control >> IRT_SHIFT) & IRT_MASK;
+ data->rvo = (data->acpi_data.states[index].control >> RVO_SHIFT) & RVO_MASK;
+ data->exttype = (data->acpi_data.states[index].control >> EXT_TYPE_SHIFT) & EXT_TYPE_MASK;
+ data->plllock = (data->acpi_data.states[index].control >> PLL_L_SHIFT) & PLL_L_MASK;
+ data->vidmvs = 1 << ((data->acpi_data.states[index].control >> MVS_SHIFT) & MVS_MASK);
+ data->vstable = (data->acpi_data.states[index].control >> VST_SHIFT) & VST_MASK;
}
static int powernow_k8_cpu_init_acpi(struct powernow_k8_data *data)
{
struct cpufreq_frequency_table *powernow_table;
int ret_val;
- int cpu = 0;
- data->acpi_data = percpu_ptr(acpi_perf_data, cpu);
- if (acpi_processor_register_performance(data->acpi_data, data->cpu)) {
+ if (acpi_processor_register_performance(&data->acpi_data, data->cpu)) {
dprintk("register performance failed: bad ACPI data\n");
return -EIO;
}
/* verify the data contained in the ACPI structures */
- if (data->acpi_data->state_count <= 1) {
+ if (data->acpi_data.state_count <= 1) {
dprintk("No ACPI P-States\n");
goto err_out;
}
- if ((data->acpi_data->control_register.space_id != ACPI_ADR_SPACE_FIXED_HARDWARE) ||
- (data->acpi_data->status_register.space_id != ACPI_ADR_SPACE_FIXED_HARDWARE)) {
+ if ((data->acpi_data.control_register.space_id != ACPI_ADR_SPACE_FIXED_HARDWARE) ||
+ (data->acpi_data.status_register.space_id != ACPI_ADR_SPACE_FIXED_HARDWARE)) {
dprintk("Invalid control/status registers (%x - %x)\n",
- data->acpi_data->control_register.space_id,
- data->acpi_data->status_register.space_id);
+ data->acpi_data.control_register.space_id,
+ data->acpi_data.status_register.space_id);
goto err_out;
}
/* fill in data->powernow_table */
powernow_table = kmalloc((sizeof(struct cpufreq_frequency_table)
- * (data->acpi_data->state_count + 1)), GFP_KERNEL);
+ * (data->acpi_data.state_count + 1)), GFP_KERNEL);
if (!powernow_table) {
dprintk("powernow_table memory alloc failure\n");
goto err_out;
@@ -806,12 +787,12 @@ static int powernow_k8_cpu_init_acpi(str
if (ret_val)
goto err_out_mem;
- powernow_table[data->acpi_data->state_count].frequency = CPUFREQ_TABLE_END;
- powernow_table[data->acpi_data->state_count].index = 0;
+ powernow_table[data->acpi_data.state_count].frequency = CPUFREQ_TABLE_END;
+ powernow_table[data->acpi_data.state_count].index = 0;
data->powernow_table = powernow_table;
/* fill in data */
- data->numps = data->acpi_data->state_count;
+ data->numps = data->acpi_data.state_count;
if (first_cpu(per_cpu(cpu_core_map, data->cpu)) == data->cpu)
print_basics(data);
powernow_k8_acpi_pst_values(data, 0);
@@ -819,31 +800,16 @@ static int powernow_k8_cpu_init_acpi(str
/* notify BIOS that we exist */
acpi_processor_notify_smm(THIS_MODULE);
- /* determine affinity, from ACPI if available */
- if (preregister_valid) {
- if ((data->acpi_data->shared_type == CPUFREQ_SHARED_TYPE_ALL) ||
- (data->acpi_data->shared_type == CPUFREQ_SHARED_TYPE_ANY))
- data->starting_core_affinity = data->acpi_data->shared_cpu_map;
- else
- data->starting_core_affinity = cpumask_of_cpu(data->cpu);
- } else {
- /* best guess from family if not */
- if (cpu_family == CPU_HW_PSTATE)
- data->starting_core_affinity = cpumask_of_cpu(data->cpu);
- else
- data->starting_core_affinity = per_cpu(cpu_core_map, data->cpu);
- }
-
return 0;
err_out_mem:
kfree(powernow_table);
err_out:
- acpi_processor_unregister_performance(data->acpi_data, data->cpu);
+ acpi_processor_unregister_performance(&data->acpi_data, data->cpu);
/* data->acpi_data.state_count informs us at ->exit() whether ACPI was used */
- data->acpi_data->state_count = 0;
+ data->acpi_data.state_count = 0;
return -ENODEV;
}
@@ -855,10 +821,10 @@ static int fill_powernow_table_pstate(st
rdmsr(MSR_PSTATE_CUR_LIMIT, hi, lo);
data->max_hw_pstate = (hi & HW_PSTATE_MAX_MASK) >> HW_PSTATE_MAX_SHIFT;
- for (i = 0; i < data->acpi_data->state_count; i++) {
+ for (i = 0; i < data->acpi_data.state_count; i++) {
u32 index;
- index = data->acpi_data->states[i].control & HW_PSTATE_MASK;
+ index = data->acpi_data.states[i].control & HW_PSTATE_MASK;
if (index > data->max_hw_pstate) {
printk(KERN_ERR PFX "invalid pstate %d - bad value %d.\n", i, index);
printk(KERN_ERR PFX "Please report to BIOS manufacturer\n");
@@ -874,7 +840,7 @@ static int fill_powernow_table_pstate(st
powernow_table[i].index = index;
- powernow_table[i].frequency = data->acpi_data->states[i].core_frequency * 1000;
+ powernow_table[i].frequency = data->acpi_data.states[i].core_frequency * 1000;
}
return 0;
}
@@ -883,16 +849,16 @@ static int fill_powernow_table_fidvid(st
{
int i;
int cntlofreq = 0;
- for (i = 0; i < data->acpi_data->state_count; i++) {
+ for (i = 0; i < data->acpi_data.state_count; i++) {
u32 fid;
u32 vid;
if (data->exttype) {
- fid = data->acpi_data->states[i].status & EXT_FID_MASK;
- vid = (data->acpi_data->states[i].status >> VID_SHIFT) & EXT_VID_MASK;
+ fid = data->acpi_data.states[i].status & EXT_FID_MASK;
+ vid = (data->acpi_data.states[i].status >> VID_SHIFT) & EXT_VID_MASK;
} else {
- fid = data->acpi_data->states[i].control & FID_MASK;
- vid = (data->acpi_data->states[i].control >> VID_SHIFT) & VID_MASK;
+ fid = data->acpi_data.states[i].control & FID_MASK;
+ vid = (data->acpi_data.states[i].control >> VID_SHIFT) & VID_MASK;
}
dprintk(" %d : fid 0x%x, vid 0x%x\n", i, fid, vid);
@@ -933,10 +899,10 @@ static int fill_powernow_table_fidvid(st
cntlofreq = i;
}
- if (powernow_table[i].frequency != (data->acpi_data->states[i].core_frequency * 1000)) {
+ if (powernow_table[i].frequency != (data->acpi_data.states[i].core_frequency * 1000)) {
printk(KERN_INFO PFX "invalid freq entries %u kHz vs. %u kHz\n",
powernow_table[i].frequency,
- (unsigned int) (data->acpi_data->states[i].core_frequency * 1000));
+ (unsigned int) (data->acpi_data.states[i].core_frequency * 1000));
powernow_table[i].frequency = CPUFREQ_ENTRY_INVALID;
continue;
}
@@ -946,12 +912,11 @@ static int fill_powernow_table_fidvid(st
static void powernow_k8_cpu_exit_acpi(struct powernow_k8_data *data)
{
- if (data->acpi_data->state_count)
- acpi_processor_unregister_performance(data->acpi_data, data->cpu);
+ if (data->acpi_data.state_count)
+ acpi_processor_unregister_performance(&data->acpi_data, data->cpu);
}
#else
-static int powernow_k8_cpu_preinit_acpi(void) { return -ENODEV; }
static int powernow_k8_cpu_init_acpi(struct powernow_k8_data *data) { return -ENODEV; }
static void powernow_k8_cpu_exit_acpi(struct powernow_k8_data *data) { return; }
static void powernow_k8_acpi_pst_values(struct powernow_k8_data *data, unsigned int index) { return; }
@@ -1136,7 +1101,7 @@ static int powernowk8_verify(struct cpuf
static int __cpuinit powernowk8_cpu_init(struct cpufreq_policy *pol)
{
struct powernow_k8_data *data;
- cpumask_t oldmask = CPU_MASK_ALL;
+ cpumask_t oldmask;
int rc;
if (!cpu_online(pol->cpu))
@@ -1209,7 +1174,10 @@ static int __cpuinit powernowk8_cpu_init
/* run on any CPU again */
set_cpus_allowed_ptr(current, &oldmask);
- pol->cpus = data->starting_core_affinity;
+ if (cpu_family == CPU_HW_PSTATE)
+ pol->cpus = cpumask_of_cpu(pol->cpu);
+ else
+ pol->cpus = per_cpu(cpu_core_map, pol->cpu);
data->available_cores = &(pol->cpus);
/* Take a crude guess here.
@@ -1332,7 +1300,6 @@ static int __cpuinit powernowk8_init(voi
}
if (supported_cpus == num_online_cpus()) {
- powernow_k8_cpu_preinit_acpi();
printk(KERN_INFO PFX "Found %d %s "
"processors (%d cpu cores) (" VERSION ")\n",
num_online_nodes(),
@@ -1349,10 +1316,6 @@ static void __exit powernowk8_exit(void)
dprintk("exit\n");
cpufreq_unregister_driver(&cpufreq_amd64_driver);
-
-#ifdef CONFIG_X86_POWERNOW_K8_ACPI
- free_percpu(acpi_perf_data);
-#endif
}
MODULE_AUTHOR("Paul Devriendt <paul.devriendt-5C7GfCeVMHo@public.gmane.org> and Mark Langsdorf <mark.langsdorf-5C7GfCeVMHo@public.gmane.org>");
Index: linux-2.6/arch/x86/kernel/cpu/cpufreq/powernow-k8.h
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/cpufreq/powernow-k8.h
+++ linux-2.6/arch/x86/kernel/cpu/cpufreq/powernow-k8.h
@@ -33,13 +33,12 @@ struct powernow_k8_data {
#ifdef CONFIG_X86_POWERNOW_K8_ACPI
/* the acpi table needs to be kept. it's only available if ACPI was
* used to determine valid frequency/vid/fid states */
- struct acpi_processor_performance *acpi_data;
+ struct acpi_processor_performance acpi_data;
#endif
/* we need to keep track of associated cores, but let cpufreq
* handle hotplug events - so just point at cpufreq pol->cpus
* structure */
cpumask_t *available_cores;
- cpumask_t starting_core_affinity;
};
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Revert "[CPUFREQ][2/2] preregister support for powernow-k8"
2008-08-19 19:37 ` [PATCH] Revert "[CPUFREQ][2/2] preregister support for powernow-k8" Rafael J. Wysocki
@ 2008-08-19 19:43 ` Dave Jones
0 siblings, 0 replies; 6+ messages in thread
From: Dave Jones @ 2008-08-19 19:43 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Andrew Morton, Linus Torvalds, Langsdorf, Mark, LKML,
kernel-testers, Randy Dunlap, Arnold, Frank, Torsten Kaiser
On Tue, Aug 19, 2008 at 09:37:54PM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
>
> Revert "[CPUFREQ][2/2] preregister support for powernow-k8"
>
> Revert commit 34ae7f35a21694aa5cb8829dc5142c39d73d6ba0
> ("[CPUFREQ][2/2] preregister support for powernow-k8") that causes
> regressions tracked as:
> http://bugzilla.kernel.org/show_bug.cgi?id=11296 ,
> http://bugzilla.kernel.org/show_bug.cgi?id=11339 .
> Namely, my test box with the Asus M3A32-MVP mainboard and the AMD
> Phenom 9850 CPU crashes during hibernation, suspend to RAM and ACPI
> power off, apparently due to a failing CPU hotplug notifier. In
> turn, on the Torsten Kaiser's test system only one CPU is correctly
> handled by cpufreq. These problems go away after reverting
> 34ae7f35a21694aa5cb8829dc5142c39d73d6ba0, but unfortunately they are
> quite difficult to debug.
>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
ACKed-by: Dave Jones <davej@redhat.com>
Dave
--
http://www.codemonkey.org.uk
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-08-19 19:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-09 21:21 [Regression] 2.6.27-rc2-git4: suspend and power off fails on Asus M3A32-MVP Rafael J. Wysocki
2008-08-12 16:03 ` Langsdorf, Mark
[not found] ` <6453C3CB8E2B3646B0D020C112613273C5AC60-F4vEnV32AJndiMxs2dRykAC/G2K4zDHf@public.gmane.org>
2008-08-19 17:03 ` Rafael J. Wysocki
2008-08-19 17:02 ` Langsdorf, Mark
2008-08-19 19:37 ` [PATCH] Revert "[CPUFREQ][2/2] preregister support for powernow-k8" Rafael J. Wysocki
2008-08-19 19:43 ` Dave Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).