Messages in this thread | | | Date | Tue, 29 Dec 2020 15:30:54 +0100 | From | Frederic Weisbecker <> | Subject | Re: [PATCH 4/5] irqtime: Move irqtime entry accounting after irq offset incrementation |
| |
On Tue, Dec 29, 2020 at 02:12:31PM +0000, Qais Yousef wrote: > On 12/29/20 14:41, Frederic Weisbecker wrote: > > > > -void vtime_account_irq(struct task_struct *tsk) > > > > +void vtime_account_irq(struct task_struct *tsk, unsigned int offset) > > > > { > > > > - if (hardirq_count()) { > > > > + unsigned int pc = preempt_count() - offset; > > > > + > > > > + if (pc & HARDIRQ_OFFSET) { > > > > > > Shouldn't this be HARDIRQ_MASK like above? > > > > In the rare cases of nested hardirqs happening with broken drivers, Only the outer hardirq > > does matter. All the time spent in the inner hardirqs is included in the outer > > one. > > Ah I see. The original code was doing hardirq_count(), which apparently wasn't > right either. > > Shouldn't it be pc == HARDIRQ_OFFSET then? All odd nest counts will trigger > this otherwise, and IIUC we want this to trigger once on first entry only.
Right but we must also handle hardirqs interrupting either preempt disabled sections or softirq servicing/disabled section.
3 stacking hardirqs should be rare enough that we don't really care. In the worst case we are going to account the third IRQ seperately. Not a correctness issue, just a rare unoptimized case.
> > Thanks > > -- > Qais Yousef
| |