lkml.org 
[lkml]   [2017]   [Mar]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2017-03-23 20:03    [W:0.065 / U:0.084 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site