Messages in this thread | | | Date | Thu, 23 Mar 2017 20:02:25 +0100 | From | Daniel Lezcano <> | Subject | Re: [PATCH V8 2/3] irq: Track the interrupt timings |
| |
On Thu, Mar 23, 2017 at 07:35:42PM +0100, Peter Zijlstra wrote: > On Thu, Mar 23, 2017 at 06:42:02PM +0100, Daniel Lezcano wrote: > > +/* > > + * The function record_irq_time is only called in one place in the > > + * interrupts handler. We want this function always inline so the code > > + * inside is embedded in the function and the static key branching > > + * code can act at the higher level. Without the explicit > > + * __always_inline we can end up with a function call and a small > > + * overhead in the hotpath for nothing. > > + */ > > +static __always_inline void record_irq_time(struct irq_desc *desc) > > +{ > > + if (static_key_enabled(&irq_timing_enabled)) { > > I think you meant to have either static_branch_likely() or > static_branch_unlikely() here. Those are runtime code patched, > static_key_enabled() generates a regular load and test condition. > > Also; if you do something like: > > if (!static_branch_likely(&irq_timing_enabled)) > return; > > you can save one level of indent.
Ok, thanks for the hint.
> > + if (desc->istate & IRQS_TIMINGS) { > > + struct irq_timings *timings = this_cpu_ptr(&irq_timings); > > + unsigned int index = timings->count & IRQ_TIMINGS_MASK; > > + > > + timings->values[index].ts = local_clock(); > > + timings->values[index].irq = irq_desc_get_irq(desc); > > + timings->count++; > > + } > > + } > > +} > > > > > +DEFINE_STATIC_KEY_FALSE(irq_timing_enabled); > > + > > +DEFINE_PER_CPU(struct irq_timings, irq_timings); > > + > > +void irq_timings_enable(void) > > +{ > > + static_branch_inc(&irq_timing_enabled); > > Do you really need counting, or do you want static_branch_enable() here?
I put counting in order to let several subsystem to use the irq timings if it is needed.
> > +} > > + > > +void irq_timings_disable(void) > > +{ > > + static_branch_dec(&irq_timing_enabled); > > idem. > > > +} > > -- > > 1.9.1 > >
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
| |