Messages in this thread | | | Date | Tue, 27 Aug 2019 16:42:02 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH 1/4] softirq: implement IRQ flood detection mechanism |
| |
On Tue, 27 Aug 2019, Ming Lei wrote: > +/* > + * Update average irq interval with the Exponential Weighted Moving > + * Average(EWMA) > + */ > +static void irq_update_interval(void) > +{ > +#define IRQ_INTERVAL_EWMA_WEIGHT 128 > +#define IRQ_INTERVAL_EWMA_PREV_FACTOR 127 > +#define IRQ_INTERVAL_EWMA_CURR_FACTOR (IRQ_INTERVAL_EWMA_WEIGHT - \ > + IRQ_INTERVAL_EWMA_PREV_FACTOR)
Please do not stick defines into a function body. That's horrible.
> + > + int cpu = raw_smp_processor_id(); > + struct irq_interval *inter = per_cpu_ptr(&avg_irq_interval, cpu); > + u64 delta = sched_clock_cpu(cpu) - inter->last_irq_end;
Why are you doing that raw_smp_processor_id() dance? The call site has interrupts and preemption disabled.
Also how is that supposed to work when sched_clock is jiffies based?
> + inter->avg = (inter->avg * IRQ_INTERVAL_EWMA_PREV_FACTOR + > + delta * IRQ_INTERVAL_EWMA_CURR_FACTOR) / > + IRQ_INTERVAL_EWMA_WEIGHT;
We definitely are not going to have a 64bit multiplication and division on every interrupt. Asided of that this breaks 32bit builds all over the place.
Thanks,
tglx
| |