[PATCH] time: add an offline-cpu check in add_timer_on()

[PATCH] time: add an offline-cpu check in add_timer_on()

[Miao Xie]

Hi everyone,
   I think we need a check in the function add_timer_on() to avoid adding a
timer into the timer list of an offline cpu.

This patch fixes this problem in add_timer_on().

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>

---
  kernel/timer.c |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/timer.c b/kernel/timer.c
index a05817c..6189561 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -451,6 +451,7 @@ void add_timer_on(struct timer_list *timer, int cpu)
  	timer_stats_timer_set_start_info(timer);
  	BUG_ON(timer_pending(timer) || !timer->function);
  	spin_lock_irqsave(&base->lock, flags);
+	BUG_ON(cpu_is_offline(cpu));
  	timer_set_base(timer, base);
  	internal_add_timer(base, timer);
  	spin_unlock_irqrestore(&base->lock, flags);
-- 
1.5.3.7

Re: [PATCH] time: add an offline-cpu check in add_timer_on()

[Andi Kleen]

Miao Xie <miaox@cn.fujitsu.com> writes:

> Hi everyone,
>   I think we need a check in the function add_timer_on() to avoid adding a
> timer into the timer list of an offline cpu.
>
> This patch fixes this problem in add_timer_on().

No it doesn't -- it will just crash if a caller does that. If it's a real 
problem then you either need to fix the callers or change add_timer_on()
to transparently put it onto another CPU.

I don't think it's a real problem though because CPU offlining does
a stop machine and that should take care of most such races.

-Andi

Re: [PATCH] time: add an offline-cpu check in add_timer_on()

[Thomas Gleixner]

On Fri, 14 Dec 2007, Miao Xie wrote:

> Hi everyone,
>   I think we need a check in the function add_timer_on() to avoid adding a
> timer into the timer list of an offline cpu.
> 
> This patch fixes this problem in add_timer_on().

No, the patch is not fixing the problem. It crashes the machine.

Where did you run into this problem ?

Thanks,

	tglx

Re: [PATCH] time: add an offline-cpu check in add_timer_on()

[Thomas Gleixner]

On Fri, 14 Dec 2007, Andi Kleen wrote:

> Miao Xie <miaox@cn.fujitsu.com> writes:
> 
> > Hi everyone,
> >   I think we need a check in the function add_timer_on() to avoid adding a
> > timer into the timer list of an offline cpu.
> >
> > This patch fixes this problem in add_timer_on().
> 
> No it doesn't -- it will just crash if a caller does that. If it's a real 
> problem then you either need to fix the callers or change add_timer_on()
> to transparently put it onto another CPU.
> 
> I don't think it's a real problem though because CPU offlining does
> a stop machine and that should take care of most such races.

No. CPU offlining does not prevent code to try inserting a timer on a
offlined CPU. If there is a real problem with this we need to fix the
calling code. We should not insert it somewhere else magically.

Thanks,

	 tglx