On Tue, Jun 21, 2016 at 06:23:34PM -0400, Rik van Riel wrote:
> > > cpu = smp_processor_id();
> > Per this smp_processor_id() usage, preemption is disabled.
>
> This code is called from the timer code. Surely preemption
> is already disabled?
That's what I said.
> > >
> > > + /*
> > > + * Softirq context may get interrupted by hardirq context,
> > > + * on the same CPU. At softirq entry time the amount of
> > > time
> > > + * spent in hardirq context is stored. At softirq exit
> > > time,
> > > + * the time spent in hardirq context during the softirq is
> > > + * subtracted.
> > > + */
> > > + prev_hardirq = __this_cpu_read(prev_hardirq_time);
> > > + prev_softirq_start = __this_cpu_read(softirq_start_time);
> > > +
> > > + if (irqtype == HARDIRQ_OFFSET) {
> > > + delta = sched_clock_cpu(cpu) -
> > > __this_cpu_read(hardirq_start_time);
> > > + __this_cpu_add(hardirq_start_time, delta);
> > > + } else do {
> > > + u64 now = sched_clock_cpu(cpu);
> > > + hardirq_time = READ_ONCE(per_cpu(cpu_hardirq_time,
> > > cpu));
> > Which makes this per_cpu(,cpu) usage somewhat curious. What's wrong
> > with
> > __this_cpu_read() ?
>
> Is __this_cpu_read() as fast as per_cpu(,cpu) on all
> architectures?
Can't be slower. Don't get the argument though; you've used __this_cpu
stuff all over the place, and here you use a per_cpu() for no reason.
> > >
> > > +
> > > + delta = now - prev_softirq_start;
> > > + if (in_serving_softirq()) {
> > > + /*
> > > + * Leaving softirq context. Avoid double
> > > counting by
> > > + * subtracting hardirq time from this
> > > interval.
> > > + */
> > > + s64 hi_delta = hardirq_time -
> > > prev_hardirq;
> > > + delta -= hi_delta;
> > > + } else {
> > > + /* Entering softirq context. Note start
> > > times. */
> > > + __this_cpu_write(softirq_start_time, now);
> > > + __this_cpu_write(prev_hardirq_time,
> > > hardirq_time);
> > > + }
> > > + /*
> > > + * If a hardirq happened during this calculation,
> > > it may not
> > > + * have gotten a consistent snapshot. Try again.
> > > + */
> > > + } while (hardirq_time !=
> > > READ_ONCE(per_cpu(cpu_hardirq_time, cpu)));
> > That whole thing is somewhat hard to read; but its far too late for
> > me
> > to suggest anything more readable :/
>
> I only had 2 1/2 hours of sleep last night, so I will not
> try to rewrite it now, but I will see if there is anything
> I can do to make it more readable tomorrow.
>
> If you have any ideas before then, please let me know :)
Heh, step away from the computer ... ;-)
