Messages in this thread | | | Subject | Re: [PATCH 2/6] Add IRQ_TIME_ACCOUNTING, finer accounting of CPU irq time | From | Peter Zijlstra <> | Date | Sun, 19 Sep 2010 14:01:06 +0200 |
| |
On Sun, 2010-09-19 at 13:21 +0200, Peter Zijlstra wrote: > On Thu, 2010-09-16 at 18:56 -0700, Venkatesh Pallipadi wrote: > > +void account_system_vtime(struct task_struct *tsk) > > +{ > > + unsigned long flags; > > + int cpu; > > + u64 now, delta; > > + > > + if (!sched_clock_irqtime) > > + return; > > + > > + local_irq_save(flags); > > + > > + cpu = task_cpu(tsk); > > + now = sched_clock(); > > + delta = now - per_cpu(irq_start_time, cpu); > > + per_cpu(irq_start_time, cpu) = now; > > + if (hardirq_count()) > > + per_cpu(cpu_hardirq_time, cpu) += delta; > > + else if (softirq_count()) > > + per_cpu(cpu_softirq_time, cpu) += delta; > > + > > + local_irq_restore(flags); > > +} > > This seems to suggest you count time double if a hardirq hits while > we're doing softirqs, but being as this is an incomplete api its very > hard to tell indeed.
OK, so by virtue of calling the same function on _enter and _exit its not incomplete, just weird.
And it won't account time double, since it uses irq_start_time to compute deltas between invocations and will attribute that delta to only one state.
You still do have the problem with local_bh_disable() though, since you cannot distinguish between having bh disabled and processing softirq.
So a hardirq that hits while you have bh disabled will inflate your softirq time.
A possible solution is to have local_bh_{disable,enable} {add,sub} 2*SOFTIRQ_OFFSET and have the processing use SOFTIRQ_OFFSET, will need a bit of a code shuffle though.
| |