Re: [PATCH] powerpc: mitigate impact of decrementer reset

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Thu, Nov 13, 2014 at 01:42:12PM +1100, Michael Ellerman wrote:
> On Mon, 2014-11-10 at 14:58 -0600, Paul Clarke wrote:
> > On 11/10/2014 04:08 AM, Benjamin Herrenschmidt wrote:
> > > On Tue, 2014-10-07 at 14:13 -0500, Paul Clarke wrote:
> > >> This patch short-circuits the reset of the decrementer, exiting after
> > >> the decrementer reset, but before the housekeeping tasks if the only
> > >> need for the interrupt is simply to reset it.  After this patch,
> > >> the latency spike was measured at about 150 nanoseconds.
> > >
> > > Doesn't this break the irq_work stuff ? We trigger it with a set_dec(1);
> > > and your patch will probably cause it to be skipped...
> > 
> > You're right.
> 
> Yeah, thanks Ben, that would have been bad.
> 
> So we'll need to come up with a different approach.
> 
> > I'm confused by the division between timer_interrupt() and 
> > __timer_interrupt().  The former is called with interrupts disabled (and 
> > enables them), but also calls irq_enter()/irq_exit().  Why are those 
> > calls not in __timer_interrupt()?  (If they were, the short-circuit 
> > logic might be a bit easier to put directly in __timer_interrupt(), 
> > which would eliminate any duplicate code.)
> > 
> > It looks like __timer_interrupt is only called directly by the broadcast 
> > timer IPI handler.  (Why is __timer_interrupt not static?)  Does this 
> > path not need irq_enter/irq_exit?
> 
> I think I answered most of this in the other mail I just sent, but let me know
> if not.
> 
> And __timer_interrupt() is static, if you have a new enough kernel :)

If I am understanding this correctly, it underscores the need for more
bits in the decrementer register.  :-/

							Thanx, Paul