Re: [PATCH 1/4] softirq: implement IRQ flood detection mechanism

From: Thomas Gleixner
Date: Tue Aug 27 2019 - 19:10:31 EST


On Wed, 28 Aug 2019, Ming Lei wrote:
> On Tue, Aug 27, 2019 at 04:42:02PM +0200, Thomas Gleixner wrote:
> > On Tue, 27 Aug 2019, Ming Lei wrote:
> > > +
> > > + 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.
>
> OK, will change to __smp_processor_id().

You do not need smp_processor_id() as all.

> > Also how is that supposed to work when sched_clock is jiffies based?
>
> Good catch, looks ktime_get_ns() is needed.

And what is ktime_get_ns() returning when the only available clocksource is
jiffies?

> >
> > > + 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.
>
> I will convert the above into add/subtract/shift only.

No. Talk to Daniel. There is not going to be yet another ad hoc thingy just
to make block happy.

And aside of that why does block not have a "NAPI" mode which was
explicitely designed to avoid all that?

Thanks,

tglx