Re: [PATCH 09/32] nohz: Move ts->idle_calls into strict idle logic

[Peter Zijlstra <a.p.zijlstra@chello.nl>]

On Tue, 2011-08-30 at 16:45 +0200, Frederic Weisbecker wrote:
> On Mon, Aug 29, 2011 at 08:33:23PM +0200, Peter Zijlstra wrote:
> > On Mon, 2011-08-29 at 20:23 +0200, Frederic Weisbecker wrote:
> > 
> > > > Well, no, on interrupt return you shouldn't do anything. If you've
> > > > stopped the tick it stays stopped until you do something that needs it,
> > > > then that action will re-enable it.
> > > 
> > > Sure, when something needs the tick in this mode, we usually
> > > receive an IPI and restart the tick from there but then
> > > tick_nohz_stop_sched_tick() handles the cases with *needs_cpu()
> > > very well on interrupt return (our IPI return) by doing a kind
> > > of "light" HZ mode by logically switching to nohz mode but
> > > with the next timer happening in HZ, assuming it's a matter
> > > of one tick and we will switch to a real nohz behaviour soon.
> > > 
> > > I don't see a good reason to duplicate that logic with a pure
> > > restart from the IPI.
> > 
> > That sounds like an optimization, and should thus be done later.
> 
> The optimization is already there upstream. I can split the logic for
> non-idle case but I'm not sure about the point of that.

care to point me to the relevant code, because I can't remember a
half-assed nohz state.. then again, maybe I didn't look hard enough.

> > > > > That said I wonder if some of the above conditions should restore a periodic
> > > > > behaviour on interrupt return...
> > > > 
> > > > I would expect the tick not to be stopped when tick_nohz_can_stop_tick()
> > > > returns false. If it returns true, then I expect anything that needs it
> > > > to re-enable it.
> > > > 
> > > 
> > > Yeah. In the case of need_resched() in idle I believe the CPU doesn't
> > > really go to sleep later so it should be fine. But for the case of
> > > softirq pending or nohz_mode, I'm not sure...
> > 
> > softirqs shouldn't be pending when you go into nohz mode..
> 
> You mean it can't happen or we don't want that to happen?

We don't want that.. going into nohz with pending softirqs means the
softirqs will be delayed for an unknown amount of time, this should not
occur.

tick_nohz_stop_sched_tick() has:

        if (unlikely(local_softirq_pending() && cpu_online(cpu))) {                          
                static int ratelimit;                                                        
                                                                                             
                if (ratelimit < 10) {                                                        
                        printk(KERN_ERR "NOHZ: local_softirq_pending %02x\n",                
                               (unsigned int) local_softirq_pending());                      
                        ratelimit++;                                                         
                }                                                                            
                goto end;                                                                    
        }

which should warn us if this ever was to occur.

> > 
> > That is, I'm really not seeing what's wrong with the very simple:
> > 
> > 
> >   if (tick_nohz_can_stop_tick())
> > 	tick_nohz_stop_tick();
> > 
> > 
> > and relying on everybody who invalidates tick_nohz_can_stop_tick(), to
> > do:
> > 
> >   tick_nohz_start_tick();
> 
> May be for the non-idle case. But for the idle case I need to ensure
> this is necessary somewhere.

How exactly do idle and non-idle differ? Its about stopping the tick,
regardless of if we're idle or not if someone needs the thing we need to
start it again.