Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1607787
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH V8 2/3] irq: Track the interrupt timings |
| Date | 2017-03-23 19:40 +0100 |
| Message-ID | <tofOp-6a9-1@gated-at.bofh.it> (permalink) |
| References | <tof22-5ye-13@gated-at.bofh.it> <tof22-5ye-11@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
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.
> + 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?
> +}
> +
> +void irq_timings_disable(void)
> +{
> + static_branch_dec(&irq_timing_enabled);
idem.
> +}
> --
> 1.9.1
>
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH V8 2/3] irq: Track the interrupt timings Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-03-23 18:50 +0100
Re: [PATCH V8 2/3] irq: Track the interrupt timings Peter Zijlstra <peterz@infradead.org> - 2017-03-23 19:40 +0100
Re: [PATCH V8 2/3] irq: Track the interrupt timings Nicolas Pitre <nicolas.pitre@linaro.org> - 2017-03-23 20:10 +0100
Re: [PATCH V8 2/3] irq: Track the interrupt timings Daniel Lezcano <daniel.lezcano@linaro.org> - 2017-03-23 20:10 +0100
Re: [PATCH V8 2/3] irq: Track the interrupt timings Nicolas Pitre <nicolas.pitre@linaro.org> - 2017-03-23 20:20 +0100
Re: [PATCH V8 2/3] irq: Track the interrupt timings Thomas Gleixner <tglx@linutronix.de> - 2017-03-23 20:40 +0100
Re: [PATCH V8 2/3] irq: Track the interrupt timings Nicolas Pitre <nicolas.pitre@linaro.org> - 2017-03-23 21:00 +0100
Re: [PATCH V8 2/3] irq: Track the interrupt timings Thomas Gleixner <tglx@linutronix.de> - 2017-03-23 22:20 +0100
csiph-web