Re: [v2 PATCH 4/4] timers: logic to enable timer migration.

[Gautham R Shenoy <ego@in.ibm.com>]

On Fri, Mar 06, 2009 at 03:50:02PM +0100, Oleg Nesterov wrote:
> On 03/06, Gautham R Shenoy wrote:
> >
> > On Thu, Mar 05, 2009 at 05:23:29PM +0100, Oleg Nesterov wrote:
> > > On 03/04, Arun R Bharadwaj wrote:
> > > >
> > > > +++ linux.trees.git/kernel/sched.c
> > > > @@ -4009,6 +4009,11 @@ static struct {
> > > >  	.load_balancer = ATOMIC_INIT(-1),
> > > >  };
> > > >
> > > > +inline int get_nohz_load_balancer(void)
> > >
> > > inline?
> > > 
> > > > +{
> > > > +	return atomic_read(&nohz.load_balancer);
> > > > +}
> > >
> > > Shouldn't we reset .load_balancer when this CPU is CPU_DOWN'ed ?
> > > Otherwise the timer can migrate to the dead CPU.
> >
> > In the select_nohz_load_balancer() code, we check if this CPU is in the
> > cpu_active_map. If no, then this CPU relinquishes being the idle
> > load balancer.

.load_balancer is the idle load balancer CPU which performs
load balancing on behalf of all the idle cpus in the system.
It's the only idle CPU which doesn't turn off it's ticks.

This CPU relinquishes it's role as the idle load balancer when
a) it finds a runnable task in it's runqueue, i.e before exiting idle
   state.
OR
b) all the CPUs in the system go idle.

Since it doesn't turn off it's ticks, it calls
select_nohz_load_balancer() every scheduler tick, and thus can observe
that it's no longer set in cpu_active_map within a tick or two.

Also, the cpu_down() path calls migrate_timers() in CPU_DEAD:,i.e after
the CPU has gone down. But to take the CPU down, we would have invoked
the stop_machine_run() code on that CPU, which by itself would make the
CPU relinquish it's role as the idle load balancer owing to condition (a).

Thus I believe, mod_timer() can never migrate a timer on to a DEAD CPU,
during/_after_ we invoke migrate_timers() from the CPU_DEAD callpath.

> 
> I don't understand this code, but I am not sure select_nohz_load_balancer()
> is always called on cpu_down() path before migrate_timers/migrate_hrtimers.
> 
> If this is true, then there is no problem.
> 
> > Also, the timer migration code in the CPU down path would
> > migrate any timers queued onto this CPU, right ?
> 
> Yes, but if .load_balancer is not cleared before the timer migration,
> mod_timer() can move the timer to the dead CPU after migration.
> 
> Oleg.

-- 
Thanks and Regards
gautham