Re: [PATCH] Add MCE resume under ia32
On Čt 25-08-05 10:36:31, Shaohua Li wrote:
> On Wed, 2005-08-24 at 10:50 +0200, Pavel Machek wrote:
> > Hi!
> >
> > > > > diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
> > > > > --- linux-2.6.13-rc6/arch/i386/power/cpu.c~mcheck_resume
> > > > > 2005-08-23 09:32:13.054008584 +0800
> > > > > +++ linux-2.6.13-rc6-root/arch/i386/power/cpu.c 2005-08-23
> > > > > 09:41:54.992540480 +0800
> > > > > @@ -104,6 +104,8 @@ static void fix_processor_context(void)
> > > > >
> > > > > }
> > > > >
> > > > > +extern void mcheck_init(struct cpuinfo_x86 *c);
> > > > > +
> > > > > void __restore_processor_state(struct saved_context *ctxt)
> > > > > {
> > > > > /*
> > > >
> > > >
> > > > this should go to some header file and most importantly
> > > If you agree my other points, I'll do this.
> Ok, updated one.
> Reinitialize MCE to avoid MCE non-fatal warning after resume.
ACK.
Pavel
--
if you have sharp zaurus hardware you don't need... you know my address
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Čt 25-08-05 10:36:31, Shaohua Li wrote:
On Wed, 2005-08-24 at 10:50 +0200, Pavel Machek wrote:
Hi!
diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
--- linux-2.6.13-rc6/arch/i386/power/cpu.c~mcheck_resume
2005-08-23 09:32:13.054008584 +0800
+++ linux-2.6.13-rc6-root/arch/i386/power/cpu.c 2005-08-23
09:41:54.992540480 +0800
@@ -104,6 +104,8 @@ static void fix_processor_context(void)
}
+extern void mcheck_init(struct cpuinfo_x86 *c);
+
void __restore_processor_state(struct saved_context *ctxt)
{
/*
this should go to some header file and most importantly
If you agree my other points, I'll do this.
Ok, updated one.
Reinitialize MCE to avoid MCE non-fatal warning after resume.
ACK.
Pavel
--
if you have sharp zaurus hardware you don't need... you know my address
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Wed, 2005-08-24 at 10:50 +0200, Pavel Machek wrote:
> Hi!
>
> > > > diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
> > > > --- linux-2.6.13-rc6/arch/i386/power/cpu.c~mcheck_resume
> > > > 2005-08-23 09:32:13.054008584 +0800
> > > > +++ linux-2.6.13-rc6-root/arch/i386/power/cpu.c 2005-08-23
> > > > 09:41:54.992540480 +0800
> > > > @@ -104,6 +104,8 @@ static void fix_processor_context(void)
> > > >
> > > > }
> > > >
> > > > +extern void mcheck_init(struct cpuinfo_x86 *c);
> > > > +
> > > > void __restore_processor_state(struct saved_context *ctxt)
> > > > {
> > > > /*
> > >
> > >
> > > this should go to some header file and most importantly
> > If you agree my other points, I'll do this.
Ok, updated one.
Reinitialize MCE to avoid MCE non-fatal warning after resume.
Signed-off-by: Shaohua Li<[EMAIL PROTECTED]>
---
linux-2.6.13-rc6-root/arch/i386/kernel/cpu/common.c |5 +
linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/k7.c |2 +-
linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/mce.c |4 ++--
linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p4.c |4 ++--
linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p5.c |2 +-
linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p6.c |2 +-
linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/winchip.c |2 +-
linux-2.6.13-rc6-root/arch/i386/power/cpu.c |1 +
linux-2.6.13-rc6-root/include/asm-i386/processor.h |6 ++
9 files changed, 16 insertions(+), 12 deletions(-)
diff -puN arch/i386/kernel/cpu/mcheck/k7.c~mcheck_resume
arch/i386/kernel/cpu/mcheck/k7.c
--- linux-2.6.13-rc6/arch/i386/kernel/cpu/mcheck/k7.c~mcheck_resume
2005-08-24 17:00:32.0 +0800
+++ linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/k7.c 2005-08-24
17:00:32.0 +0800
@@ -69,7 +69,7 @@ static fastcall void k7_machine_check(st
/* AMD K7 machine check is Intel like */
-void __devinit amd_mcheck_init(struct cpuinfo_x86 *c)
+void amd_mcheck_init(struct cpuinfo_x86 *c)
{
u32 l, h;
int i;
diff -puN arch/i386/kernel/cpu/mcheck/mce.c~mcheck_resume
arch/i386/kernel/cpu/mcheck/mce.c
--- linux-2.6.13-rc6/arch/i386/kernel/cpu/mcheck/mce.c~mcheck_resume
2005-08-24 17:00:32.0 +0800
+++ linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/mce.c 2005-08-24
17:00:32.0 +0800
@@ -16,7 +16,7 @@
#include "mce.h"
-int mce_disabled __devinitdata = 0;
+int mce_disabled = 0;
int nr_mce_banks;
EXPORT_SYMBOL_GPL(nr_mce_banks); /* non-fatal.o */
@@ -31,7 +31,7 @@ static fastcall void unexpected_machine_
void fastcall (*machine_check_vector)(struct pt_regs *, long error_code) =
unexpected_machine_check;
/* This has to be run for each processor */
-void __devinit mcheck_init(struct cpuinfo_x86 *c)
+void mcheck_init(struct cpuinfo_x86 *c)
{
if (mce_disabled==1)
return;
diff -puN arch/i386/kernel/cpu/mcheck/p4.c~mcheck_resume
arch/i386/kernel/cpu/mcheck/p4.c
--- linux-2.6.13-rc6/arch/i386/kernel/cpu/mcheck/p4.c~mcheck_resume
2005-08-24 17:00:32.0 +0800
+++ linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p4.c 2005-08-24
17:00:32.0 +0800
@@ -78,7 +78,7 @@ fastcall void smp_thermal_interrupt(stru
}
/* P4/Xeon Thermal regulation detect and init */
-static void __devinit intel_init_thermal(struct cpuinfo_x86 *c)
+static void intel_init_thermal(struct cpuinfo_x86 *c)
{
u32 l, h;
unsigned int cpu = smp_processor_id();
@@ -232,7 +232,7 @@ static fastcall void intel_machine_check
}
-void __devinit intel_p4_mcheck_init(struct cpuinfo_x86 *c)
+void intel_p4_mcheck_init(struct cpuinfo_x86 *c)
{
u32 l, h;
int i;
diff -puN arch/i386/kernel/cpu/mcheck/p5.c~mcheck_resume
arch/i386/kernel/cpu/mcheck/p5.c
--- linux-2.6.13-rc6/arch/i386/kernel/cpu/mcheck/p5.c~mcheck_resume
2005-08-24 17:00:32.0 +0800
+++ linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p5.c 2005-08-24
17:00:32.0 +0800
@@ -29,7 +29,7 @@ static fastcall void pentium_machine_che
}
/* Set up machine check reporting for processors with Intel style MCE */
-void __devinit intel_p5_mcheck_init(struct cpuinfo_x86 *c)
+void intel_p5_mcheck_init(struct cpuinfo_x86 *c)
{
u32 l, h;
diff -puN arch/i386/kernel/cpu/mcheck/p6.c~mcheck_resume
arch/i386/kernel/cpu/mcheck/p6.c
--- linux-2.6.13-rc6/arch/i386/kernel/cpu/mcheck/p6.c~mcheck_resume
2005-08-24 17:00:32.0 +0800
+++ linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p6.c 2005-08-24
17:00:32.0 +0800
@@ -80,7 +80,7 @@ static fastcall void intel_machine_check
}
/* Set up machine check reporting for processors with Intel style MCE */
-void __devinit intel_p6_mcheck_init(struct cpuinfo_x86 *c)
+void intel_p6_mcheck_init(struct cpuinfo_x86 *c)
{
u32 l, h;
int i;
diff -puN
Re: [PATCH] Add MCE resume under ia32
Hi! > > > The boot code already initialized MCE for APs, it isn't required to > > > initialize again. The MCE entries are cpuhotplug friendly, so for > > > suspend/resume. > > > > Ok so you're saying the only change needed is to remove > > the on_each_cpu() in the resume method? Fine I can do that. > Yep, only BP needs it. But I'm not sure if we should do the same (add > the sysdev class) in ia32, considering only BP needs it. Adding sysdev class is nicer than #ifdef in the code, I'd say. Pavel -- if you have sharp zaurus hardware you don't need... you know my address - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
Hi!
> > > diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
> > > --- linux-2.6.13-rc6/arch/i386/power/cpu.c~mcheck_resume 2005-08-23
> > > 09:32:13.054008584 +0800
> > > +++ linux-2.6.13-rc6-root/arch/i386/power/cpu.c 2005-08-23
> > > 09:41:54.992540480 +0800
> > > @@ -104,6 +104,8 @@ static void fix_processor_context(void)
> > >
> > > }
> > >
> > > +extern void mcheck_init(struct cpuinfo_x86 *c);
> > > +
> > > void __restore_processor_state(struct saved_context *ctxt)
> > > {
> > > /*
> >
> >
> > this should go to some header file and most importantly
> If you agree my other points, I'll do this.
Ok.
> > > @@ -138,6 +140,9 @@ void __restore_processor_state(struct sa
> > > fix_processor_context();
> > > do_fpu_end();
> > > mtrr_ap_init();
> > > +#ifdef CONFIG_X86_MCE
> > > + mcheck_init(_cpu_data);
> > > +#endif
> > > }
> >
> > c) can't we register MCEs like some kind of system device so that this
> > kind of hooks is not neccessary?
> Like x86-64 does, right? In this way, we must register a device for each
> cpu. But APs directly call mcheck_init in resume time (cpuhotplug
> framework). Only BP requires to call the resume method, so I think
> restore_processor_state calls it might be cleaner.
Ahha, ok.
Pavel
--
if you have sharp zaurus hardware you don't need... you know my address
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
Hi! The boot code already initialized MCE for APs, it isn't required to initialize again. The MCE entries are cpuhotplug friendly, so for suspend/resume. Ok so you're saying the only change needed is to remove the on_each_cpu() in the resume method? Fine I can do that. Yep, only BP needs it. But I'm not sure if we should do the same (add the sysdev class) in ia32, considering only BP needs it. Adding sysdev class is nicer than #ifdef in the code, I'd say. Pavel -- if you have sharp zaurus hardware you don't need... you know my address - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Wed, 2005-08-24 at 10:50 +0200, Pavel Machek wrote:
Hi!
diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
--- linux-2.6.13-rc6/arch/i386/power/cpu.c~mcheck_resume
2005-08-23 09:32:13.054008584 +0800
+++ linux-2.6.13-rc6-root/arch/i386/power/cpu.c 2005-08-23
09:41:54.992540480 +0800
@@ -104,6 +104,8 @@ static void fix_processor_context(void)
}
+extern void mcheck_init(struct cpuinfo_x86 *c);
+
void __restore_processor_state(struct saved_context *ctxt)
{
/*
this should go to some header file and most importantly
If you agree my other points, I'll do this.
Ok, updated one.
Reinitialize MCE to avoid MCE non-fatal warning after resume.
Signed-off-by: Shaohua Li[EMAIL PROTECTED]
---
linux-2.6.13-rc6-root/arch/i386/kernel/cpu/common.c |5 +
linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/k7.c |2 +-
linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/mce.c |4 ++--
linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p4.c |4 ++--
linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p5.c |2 +-
linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p6.c |2 +-
linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/winchip.c |2 +-
linux-2.6.13-rc6-root/arch/i386/power/cpu.c |1 +
linux-2.6.13-rc6-root/include/asm-i386/processor.h |6 ++
9 files changed, 16 insertions(+), 12 deletions(-)
diff -puN arch/i386/kernel/cpu/mcheck/k7.c~mcheck_resume
arch/i386/kernel/cpu/mcheck/k7.c
--- linux-2.6.13-rc6/arch/i386/kernel/cpu/mcheck/k7.c~mcheck_resume
2005-08-24 17:00:32.0 +0800
+++ linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/k7.c 2005-08-24
17:00:32.0 +0800
@@ -69,7 +69,7 @@ static fastcall void k7_machine_check(st
/* AMD K7 machine check is Intel like */
-void __devinit amd_mcheck_init(struct cpuinfo_x86 *c)
+void amd_mcheck_init(struct cpuinfo_x86 *c)
{
u32 l, h;
int i;
diff -puN arch/i386/kernel/cpu/mcheck/mce.c~mcheck_resume
arch/i386/kernel/cpu/mcheck/mce.c
--- linux-2.6.13-rc6/arch/i386/kernel/cpu/mcheck/mce.c~mcheck_resume
2005-08-24 17:00:32.0 +0800
+++ linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/mce.c 2005-08-24
17:00:32.0 +0800
@@ -16,7 +16,7 @@
#include mce.h
-int mce_disabled __devinitdata = 0;
+int mce_disabled = 0;
int nr_mce_banks;
EXPORT_SYMBOL_GPL(nr_mce_banks); /* non-fatal.o */
@@ -31,7 +31,7 @@ static fastcall void unexpected_machine_
void fastcall (*machine_check_vector)(struct pt_regs *, long error_code) =
unexpected_machine_check;
/* This has to be run for each processor */
-void __devinit mcheck_init(struct cpuinfo_x86 *c)
+void mcheck_init(struct cpuinfo_x86 *c)
{
if (mce_disabled==1)
return;
diff -puN arch/i386/kernel/cpu/mcheck/p4.c~mcheck_resume
arch/i386/kernel/cpu/mcheck/p4.c
--- linux-2.6.13-rc6/arch/i386/kernel/cpu/mcheck/p4.c~mcheck_resume
2005-08-24 17:00:32.0 +0800
+++ linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p4.c 2005-08-24
17:00:32.0 +0800
@@ -78,7 +78,7 @@ fastcall void smp_thermal_interrupt(stru
}
/* P4/Xeon Thermal regulation detect and init */
-static void __devinit intel_init_thermal(struct cpuinfo_x86 *c)
+static void intel_init_thermal(struct cpuinfo_x86 *c)
{
u32 l, h;
unsigned int cpu = smp_processor_id();
@@ -232,7 +232,7 @@ static fastcall void intel_machine_check
}
-void __devinit intel_p4_mcheck_init(struct cpuinfo_x86 *c)
+void intel_p4_mcheck_init(struct cpuinfo_x86 *c)
{
u32 l, h;
int i;
diff -puN arch/i386/kernel/cpu/mcheck/p5.c~mcheck_resume
arch/i386/kernel/cpu/mcheck/p5.c
--- linux-2.6.13-rc6/arch/i386/kernel/cpu/mcheck/p5.c~mcheck_resume
2005-08-24 17:00:32.0 +0800
+++ linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p5.c 2005-08-24
17:00:32.0 +0800
@@ -29,7 +29,7 @@ static fastcall void pentium_machine_che
}
/* Set up machine check reporting for processors with Intel style MCE */
-void __devinit intel_p5_mcheck_init(struct cpuinfo_x86 *c)
+void intel_p5_mcheck_init(struct cpuinfo_x86 *c)
{
u32 l, h;
diff -puN arch/i386/kernel/cpu/mcheck/p6.c~mcheck_resume
arch/i386/kernel/cpu/mcheck/p6.c
--- linux-2.6.13-rc6/arch/i386/kernel/cpu/mcheck/p6.c~mcheck_resume
2005-08-24 17:00:32.0 +0800
+++ linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p6.c 2005-08-24
17:00:32.0 +0800
@@ -80,7 +80,7 @@ static fastcall void intel_machine_check
}
/* Set up machine check reporting for processors with Intel style MCE */
-void __devinit intel_p6_mcheck_init(struct cpuinfo_x86 *c)
+void intel_p6_mcheck_init(struct cpuinfo_x86 *c)
{
u32 l, h;
int i;
diff -puN arch/i386/kernel/cpu/mcheck/winchip.c~mcheck_resume
arch/i386/kernel/cpu/mcheck/winchip.c
---
Re: [PATCH] Add MCE resume under ia32
Hi!
diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
--- linux-2.6.13-rc6/arch/i386/power/cpu.c~mcheck_resume 2005-08-23
09:32:13.054008584 +0800
+++ linux-2.6.13-rc6-root/arch/i386/power/cpu.c 2005-08-23
09:41:54.992540480 +0800
@@ -104,6 +104,8 @@ static void fix_processor_context(void)
}
+extern void mcheck_init(struct cpuinfo_x86 *c);
+
void __restore_processor_state(struct saved_context *ctxt)
{
/*
this should go to some header file and most importantly
If you agree my other points, I'll do this.
Ok.
@@ -138,6 +140,9 @@ void __restore_processor_state(struct sa
fix_processor_context();
do_fpu_end();
mtrr_ap_init();
+#ifdef CONFIG_X86_MCE
+ mcheck_init(boot_cpu_data);
+#endif
}
c) can't we register MCEs like some kind of system device so that this
kind of hooks is not neccessary?
Like x86-64 does, right? In this way, we must register a device for each
cpu. But APs directly call mcheck_init in resume time (cpuhotplug
framework). Only BP requires to call the resume method, so I think
restore_processor_state calls it might be cleaner.
Ahha, ok.
Pavel
--
if you have sharp zaurus hardware you don't need... you know my address
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Wed, 2005-08-24 at 06:26 +0200, Andi Kleen wrote: > On Wednesday 24 August 2005 06:16, Shaohua Li wrote: > > > The boot code already initialized MCE for APs, it isn't required to > > initialize again. The MCE entries are cpuhotplug friendly, so for > > suspend/resume. > > Ok so you're saying the only change needed is to remove > the on_each_cpu() in the resume method? Fine I can do that. Yep, only BP needs it. But I'm not sure if we should do the same (add the sysdev class) in ia32, considering only BP needs it. Thanks, Shaohua - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Wednesday 24 August 2005 06:16, Shaohua Li wrote: > The boot code already initialized MCE for APs, it isn't required to > initialize again. The MCE entries are cpuhotplug friendly, so for > suspend/resume. Ok so you're saying the only change needed is to remove the on_each_cpu() in the resume method? Fine I can do that. -Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Wed, 2005-08-24 at 05:59 +0200, Andi Kleen wrote: > [adding discuss to cc] > > On Wednesday 24 August 2005 05:47, Shaohua Li wrote: > > On Wed, 2005-08-24 at 05:12 +0200, Andi Kleen wrote: > > > On Wednesday 24 August 2005 03:59, Shaohua Li wrote: > > > > On Wed, 2005-08-24 at 03:52 +0200, Andi Kleen wrote: > > > > > Shaohua Li <[EMAIL PROTECTED]> writes: > > > > > > x86-64 has resume support. It uses 'on_each_cpu' in resume method, > > > > > > which is known broken. We'd better fix it. > > > > > > > > > > What is broken with it? > > > > > > > > It's a sysdev. The resume method is invoked with interrupt disabled. > > > > > > But only local interrupt disabled, no? > > > > > > Hmm - didn't we have a WARN_ON(irqs_disabled()) in smp_call_function(). > > > > > > Anyways, it'll probably still work for now because the system should > > > be synchronized at this point. > > > > We are using cpu hotplug framework for MP suspend/resume. When sysdev's > > resume is calling, APs actually aren't up. So it actually can't work. > > Ok, that's a new problem. > > There were recently some patches to add individual MCE entries > for each CPU to sysfs. They are only used for set up right now, > but perhaps they can be linked somehow to the cpu sysfs devices > and get suspend/resume events from there. The boot code already initialized MCE for APs, it isn't required to initialize again. The MCE entries are cpuhotplug friendly, so for suspend/resume. Thanks, Shaohua - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
[adding discuss to cc] On Wednesday 24 August 2005 05:47, Shaohua Li wrote: > On Wed, 2005-08-24 at 05:12 +0200, Andi Kleen wrote: > > On Wednesday 24 August 2005 03:59, Shaohua Li wrote: > > > On Wed, 2005-08-24 at 03:52 +0200, Andi Kleen wrote: > > > > Shaohua Li <[EMAIL PROTECTED]> writes: > > > > > x86-64 has resume support. It uses 'on_each_cpu' in resume method, > > > > > which is known broken. We'd better fix it. > > > > > > > > What is broken with it? > > > > > > It's a sysdev. The resume method is invoked with interrupt disabled. > > > > But only local interrupt disabled, no? > > > > Hmm - didn't we have a WARN_ON(irqs_disabled()) in smp_call_function(). > > > > Anyways, it'll probably still work for now because the system should > > be synchronized at this point. > > We are using cpu hotplug framework for MP suspend/resume. When sysdev's > resume is calling, APs actually aren't up. So it actually can't work. Ok, that's a new problem. There were recently some patches to add individual MCE entries for each CPU to sysfs. They are only used for set up right now, but perhaps they can be linked somehow to the cpu sysfs devices and get suspend/resume events from there. -Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Wed, 2005-08-24 at 05:12 +0200, Andi Kleen wrote: > On Wednesday 24 August 2005 03:59, Shaohua Li wrote: > > On Wed, 2005-08-24 at 03:52 +0200, Andi Kleen wrote: > > > Shaohua Li <[EMAIL PROTECTED]> writes: > > > > x86-64 has resume support. It uses 'on_each_cpu' in resume method, > > > > which is known broken. We'd better fix it. > > > > > > What is broken with it? > > > > It's a sysdev. The resume method is invoked with interrupt disabled. > > But only local interrupt disabled, no? > > Hmm - didn't we have a WARN_ON(irqs_disabled()) in smp_call_function(). > > Anyways, it'll probably still work for now because the system should > be synchronized at this point. We are using cpu hotplug framework for MP suspend/resume. When sysdev's resume is calling, APs actually aren't up. So it actually can't work. Thanks, Shaohua - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Wednesday 24 August 2005 03:59, Shaohua Li wrote: > On Wed, 2005-08-24 at 03:52 +0200, Andi Kleen wrote: > > Shaohua Li <[EMAIL PROTECTED]> writes: > > > x86-64 has resume support. It uses 'on_each_cpu' in resume method, > > > which is known broken. We'd better fix it. > > > > What is broken with it? > > It's a sysdev. The resume method is invoked with interrupt disabled. But only local interrupt disabled, no? Hmm - didn't we have a WARN_ON(irqs_disabled()) in smp_call_function(). Anyways, it'll probably still work for now because the system should be synchronized at this point. -Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Wed, 2005-08-24 at 03:52 +0200, Andi Kleen wrote: > Shaohua Li <[EMAIL PROTECTED]> writes: > > > x86-64 has resume support. It uses 'on_each_cpu' in resume method, which > > is known broken. We'd better fix it. > > What is broken with it? It's a sysdev. The resume method is invoked with interrupt disabled. Thanks, Shaohua - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
Shaohua Li <[EMAIL PROTECTED]> writes: > x86-64 has resume support. It uses 'on_each_cpu' in resume method, which > is known broken. We'd better fix it. What is broken with it? -Andi - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Tue, 2005-08-23 at 12:32 +0200, Pavel Machek wrote:
> > It's widely seen a MCE non-fatal error reported after resume. It seems
> > MCE resume is lacked under ia32. This patch tries to fix the gap.
>
> Well, you patch seems like missing piece of puzzle, but:
>
> a) we probably want to do it for x86-64, too, and
x86-64 has resume support. It uses 'on_each_cpu' in resume method, which
is known broken. We'd better fix it.
>
> > diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
> > --- linux-2.6.13-rc6/arch/i386/power/cpu.c~mcheck_resume2005-08-23
> > 09:32:13.054008584 +0800
> > +++ linux-2.6.13-rc6-root/arch/i386/power/cpu.c 2005-08-23
> > 09:41:54.992540480 +0800
> > @@ -104,6 +104,8 @@ static void fix_processor_context(void)
> >
> > }
> >
> > +extern void mcheck_init(struct cpuinfo_x86 *c);
> > +
> > void __restore_processor_state(struct saved_context *ctxt)
> > {
> > /*
>
>
> this should go to some header file and most importantly
If you agree my other points, I'll do this.
>
> > @@ -138,6 +140,9 @@ void __restore_processor_state(struct sa
> > fix_processor_context();
> > do_fpu_end();
> > mtrr_ap_init();
> > +#ifdef CONFIG_X86_MCE
> > + mcheck_init(_cpu_data);
> > +#endif
> > }
>
> c) can't we register MCEs like some kind of system device so that this
> kind of hooks is not neccessary?
Like x86-64 does, right? In this way, we must register a device for each
cpu. But APs directly call mcheck_init in resume time (cpuhotplug
framework). Only BP requires to call the resume method, so I think
restore_processor_state calls it might be cleaner.
Thanks,
Shaohua
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
Hi!
> It's widely seen a MCE non-fatal error reported after resume. It seems
> MCE resume is lacked under ia32. This patch tries to fix the gap.
Well, you patch seems like missing piece of puzzle, but:
a) we probably want to do it for x86-64, too, and
b)
> diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
> --- linux-2.6.13-rc6/arch/i386/power/cpu.c~mcheck_resume 2005-08-23
> 09:32:13.054008584 +0800
> +++ linux-2.6.13-rc6-root/arch/i386/power/cpu.c 2005-08-23
> 09:41:54.992540480 +0800
> @@ -104,6 +104,8 @@ static void fix_processor_context(void)
>
> }
>
> +extern void mcheck_init(struct cpuinfo_x86 *c);
> +
> void __restore_processor_state(struct saved_context *ctxt)
> {
> /*
this should go to some header file and most importantly
> @@ -138,6 +140,9 @@ void __restore_processor_state(struct sa
> fix_processor_context();
> do_fpu_end();
> mtrr_ap_init();
> +#ifdef CONFIG_X86_MCE
> + mcheck_init(_cpu_data);
> +#endif
> }
c) can't we register MCEs like some kind of system device so that this
kind of hooks is not neccessary?
Pavel
--
if you have sharp zaurus hardware you don't need... you know my address
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
Hi!
It's widely seen a MCE non-fatal error reported after resume. It seems
MCE resume is lacked under ia32. This patch tries to fix the gap.
Well, you patch seems like missing piece of puzzle, but:
a) we probably want to do it for x86-64, too, and
b)
diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
--- linux-2.6.13-rc6/arch/i386/power/cpu.c~mcheck_resume 2005-08-23
09:32:13.054008584 +0800
+++ linux-2.6.13-rc6-root/arch/i386/power/cpu.c 2005-08-23
09:41:54.992540480 +0800
@@ -104,6 +104,8 @@ static void fix_processor_context(void)
}
+extern void mcheck_init(struct cpuinfo_x86 *c);
+
void __restore_processor_state(struct saved_context *ctxt)
{
/*
this should go to some header file and most importantly
@@ -138,6 +140,9 @@ void __restore_processor_state(struct sa
fix_processor_context();
do_fpu_end();
mtrr_ap_init();
+#ifdef CONFIG_X86_MCE
+ mcheck_init(boot_cpu_data);
+#endif
}
c) can't we register MCEs like some kind of system device so that this
kind of hooks is not neccessary?
Pavel
--
if you have sharp zaurus hardware you don't need... you know my address
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Tue, 2005-08-23 at 12:32 +0200, Pavel Machek wrote:
It's widely seen a MCE non-fatal error reported after resume. It seems
MCE resume is lacked under ia32. This patch tries to fix the gap.
Well, you patch seems like missing piece of puzzle, but:
a) we probably want to do it for x86-64, too, and
x86-64 has resume support. It uses 'on_each_cpu' in resume method, which
is known broken. We'd better fix it.
diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
--- linux-2.6.13-rc6/arch/i386/power/cpu.c~mcheck_resume2005-08-23
09:32:13.054008584 +0800
+++ linux-2.6.13-rc6-root/arch/i386/power/cpu.c 2005-08-23
09:41:54.992540480 +0800
@@ -104,6 +104,8 @@ static void fix_processor_context(void)
}
+extern void mcheck_init(struct cpuinfo_x86 *c);
+
void __restore_processor_state(struct saved_context *ctxt)
{
/*
this should go to some header file and most importantly
If you agree my other points, I'll do this.
@@ -138,6 +140,9 @@ void __restore_processor_state(struct sa
fix_processor_context();
do_fpu_end();
mtrr_ap_init();
+#ifdef CONFIG_X86_MCE
+ mcheck_init(boot_cpu_data);
+#endif
}
c) can't we register MCEs like some kind of system device so that this
kind of hooks is not neccessary?
Like x86-64 does, right? In this way, we must register a device for each
cpu. But APs directly call mcheck_init in resume time (cpuhotplug
framework). Only BP requires to call the resume method, so I think
restore_processor_state calls it might be cleaner.
Thanks,
Shaohua
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
Shaohua Li [EMAIL PROTECTED] writes: x86-64 has resume support. It uses 'on_each_cpu' in resume method, which is known broken. We'd better fix it. What is broken with it? -Andi - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Wed, 2005-08-24 at 03:52 +0200, Andi Kleen wrote: Shaohua Li [EMAIL PROTECTED] writes: x86-64 has resume support. It uses 'on_each_cpu' in resume method, which is known broken. We'd better fix it. What is broken with it? It's a sysdev. The resume method is invoked with interrupt disabled. Thanks, Shaohua - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Wednesday 24 August 2005 03:59, Shaohua Li wrote: On Wed, 2005-08-24 at 03:52 +0200, Andi Kleen wrote: Shaohua Li [EMAIL PROTECTED] writes: x86-64 has resume support. It uses 'on_each_cpu' in resume method, which is known broken. We'd better fix it. What is broken with it? It's a sysdev. The resume method is invoked with interrupt disabled. But only local interrupt disabled, no? Hmm - didn't we have a WARN_ON(irqs_disabled()) in smp_call_function(). Anyways, it'll probably still work for now because the system should be synchronized at this point. -Andi - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Wed, 2005-08-24 at 05:12 +0200, Andi Kleen wrote: On Wednesday 24 August 2005 03:59, Shaohua Li wrote: On Wed, 2005-08-24 at 03:52 +0200, Andi Kleen wrote: Shaohua Li [EMAIL PROTECTED] writes: x86-64 has resume support. It uses 'on_each_cpu' in resume method, which is known broken. We'd better fix it. What is broken with it? It's a sysdev. The resume method is invoked with interrupt disabled. But only local interrupt disabled, no? Hmm - didn't we have a WARN_ON(irqs_disabled()) in smp_call_function(). Anyways, it'll probably still work for now because the system should be synchronized at this point. We are using cpu hotplug framework for MP suspend/resume. When sysdev's resume is calling, APs actually aren't up. So it actually can't work. Thanks, Shaohua - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
[adding discuss to cc] On Wednesday 24 August 2005 05:47, Shaohua Li wrote: On Wed, 2005-08-24 at 05:12 +0200, Andi Kleen wrote: On Wednesday 24 August 2005 03:59, Shaohua Li wrote: On Wed, 2005-08-24 at 03:52 +0200, Andi Kleen wrote: Shaohua Li [EMAIL PROTECTED] writes: x86-64 has resume support. It uses 'on_each_cpu' in resume method, which is known broken. We'd better fix it. What is broken with it? It's a sysdev. The resume method is invoked with interrupt disabled. But only local interrupt disabled, no? Hmm - didn't we have a WARN_ON(irqs_disabled()) in smp_call_function(). Anyways, it'll probably still work for now because the system should be synchronized at this point. We are using cpu hotplug framework for MP suspend/resume. When sysdev's resume is calling, APs actually aren't up. So it actually can't work. Ok, that's a new problem. There were recently some patches to add individual MCE entries for each CPU to sysfs. They are only used for set up right now, but perhaps they can be linked somehow to the cpu sysfs devices and get suspend/resume events from there. -Andi - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Wed, 2005-08-24 at 05:59 +0200, Andi Kleen wrote: [adding discuss to cc] On Wednesday 24 August 2005 05:47, Shaohua Li wrote: On Wed, 2005-08-24 at 05:12 +0200, Andi Kleen wrote: On Wednesday 24 August 2005 03:59, Shaohua Li wrote: On Wed, 2005-08-24 at 03:52 +0200, Andi Kleen wrote: Shaohua Li [EMAIL PROTECTED] writes: x86-64 has resume support. It uses 'on_each_cpu' in resume method, which is known broken. We'd better fix it. What is broken with it? It's a sysdev. The resume method is invoked with interrupt disabled. But only local interrupt disabled, no? Hmm - didn't we have a WARN_ON(irqs_disabled()) in smp_call_function(). Anyways, it'll probably still work for now because the system should be synchronized at this point. We are using cpu hotplug framework for MP suspend/resume. When sysdev's resume is calling, APs actually aren't up. So it actually can't work. Ok, that's a new problem. There were recently some patches to add individual MCE entries for each CPU to sysfs. They are only used for set up right now, but perhaps they can be linked somehow to the cpu sysfs devices and get suspend/resume events from there. The boot code already initialized MCE for APs, it isn't required to initialize again. The MCE entries are cpuhotplug friendly, so for suspend/resume. Thanks, Shaohua - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Wednesday 24 August 2005 06:16, Shaohua Li wrote: The boot code already initialized MCE for APs, it isn't required to initialize again. The MCE entries are cpuhotplug friendly, so for suspend/resume. Ok so you're saying the only change needed is to remove the on_each_cpu() in the resume method? Fine I can do that. -Andi - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add MCE resume under ia32
On Wed, 2005-08-24 at 06:26 +0200, Andi Kleen wrote: On Wednesday 24 August 2005 06:16, Shaohua Li wrote: The boot code already initialized MCE for APs, it isn't required to initialize again. The MCE entries are cpuhotplug friendly, so for suspend/resume. Ok so you're saying the only change needed is to remove the on_each_cpu() in the resume method? Fine I can do that. Yep, only BP needs it. But I'm not sure if we should do the same (add the sysdev class) in ia32, considering only BP needs it. Thanks, Shaohua - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
