On 12/29/20 15:30, Frederic Weisbecker wrote: > 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.
I admit I need to wrap my head around some more details to fully comprehend that, but that's my own confusion to clear out :-) Thanks for your answer. Cheers -- Qais Yousef