2012-06-06 21:53:54

by Thomas Gleixner

Subject: [patch 3/5] x86: mce: Split timer init

Split timer init function into the init and the start part, so the
start part can replace the open coded version in CPU_DOWN_FAILED.

Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/kernel/cpu/mcheck/mce.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)

Index: tip/arch/x86/kernel/cpu/mcheck/mce.c
===================================================================
--- tip.orig/arch/x86/kernel/cpu/mcheck/mce.c
+++ tip/arch/x86/kernel/cpu/mcheck/mce.c
@@ -1554,23 +1554,28 @@ static void __mcheck_cpu_init_vendor(str
}
}

-static void __mcheck_cpu_init_timer(void)
+static void mce_start_timer(unsigned int cpu, struct timer_list *t)
{
- struct timer_list *t = &__get_cpu_var(mce_timer);
unsigned long iv = check_interval * HZ;

- setup_timer(t, mce_timer_fn, smp_processor_id());
+ __this_cpu_write(mce_next_interval, iv);

- if (mce_ignore_ce)
+ if (mce_ignore_ce || !iv)
return;

- __this_cpu_write(mce_next_interval, iv);
- if (!iv)
- return;
t->expires = round_jiffies(jiffies + iv);
add_timer_on(t, smp_processor_id());
}

+static void __mcheck_cpu_init_timer(void)
+{
+ struct timer_list *t = &__get_cpu_var(mce_timer);
+ unsigned int cpu = smp_processor_id();
+
+ setup_timer(t, mce_timer_fn, cpu);
+ mce_start_timer(cpu, t);
+}
+
/* Handle unconfigured int18 (should never happen) */
static void unexpected_machine_check(struct pt_regs *regs, long error_code)
{
@@ -2275,12 +2280,8 @@ mce_cpu_callback(struct notifier_block *
break;
case CPU_DOWN_FAILED:
case CPU_DOWN_FAILED_FROZEN:
- if (!mce_ignore_ce && check_interval) {
- t->expires = round_jiffies(jiffies +
- per_cpu(mce_next_interval, cpu));
- add_timer_on(t, cpu);
- }
smp_call_function_single(cpu, mce_reenable_cpu, &action, 1);
+ mce_start_timer(cpu, t);
break;
case CPU_POST_DEAD:
/* intentionally ignoring frozen here */

2012-06-07 15:18:05

by tip-bot for Borislav Petkov

Subject: Re: [patch 3/5] x86: mce: Split timer init

On Wed, Jun 06, 2012 at 09:53:23PM +0000, Thomas Gleixner wrote:
> Split timer init function into the init and the start part, so the
> start part can replace the open coded version in CPU_DOWN_FAILED.
>
> Signed-off-by: Thomas Gleixner <[email protected]>

Ok, it looks good to me and it has been lightly tested. In light of last
time's review debacle, however, I'd only very tentatively say:

Acked-by: Borislav Petkov <[email protected]>

--
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

2012-06-20 03:36:17

by Hidetoshi Seto

Subject: Re: [patch 3/5] x86: mce: Split timer init

(2012/06/07 6:53), Thomas Gleixner wrote:
> --- tip.orig/arch/x86/kernel/cpu/mcheck/mce.c
> +++ tip/arch/x86/kernel/cpu/mcheck/mce.c
> @@ -1554,23 +1554,28 @@ static void __mcheck_cpu_init_vendor(str
> }
> }
>
> -static void __mcheck_cpu_init_timer(void)
> +static void mce_start_timer(unsigned int cpu, struct timer_list *t)
> {
> - struct timer_list *t = &__get_cpu_var(mce_timer);
> unsigned long iv = check_interval * HZ;
>
> - setup_timer(t, mce_timer_fn, smp_processor_id());
> + __this_cpu_write(mce_next_interval, iv);
>
> - if (mce_ignore_ce)
> + if (mce_ignore_ce || !iv)
> return;
>
> - __this_cpu_write(mce_next_interval, iv);
> - if (!iv)
> - return;
> t->expires = round_jiffies(jiffies + iv);
> add_timer_on(t, smp_processor_id());

add_timer_on(t, cpu) ?

If so, using __this_cpu_write() here is wrong too.

> }
>

Thanks,
H.Seto