On 08/07/2016 15:19, Rik van Riel wrote: > On Fri, 2016-07-08 at 14:30 +0200, Frederic Weisbecker wrote: >> On Thu, Jun 30, 2016 at 03:35:50PM -0400, r...@redhat.com wrote: >>> From: Rik van Riel <r...@redhat.com> >>> >>> Drop local_irq_save/restore from irqtime_account_irq. >>> Instead, have softirq and hardirq track their time spent >>> independently, with the softirq code subtracting hardirq >>> time that happened during the duration of the softirq run. >>> >>> The softirq code can be interrupted by hardirq code at >>> any point in time, but it can check whether it got a >>> consistent snapshot of the timekeeping variables it wants, >>> and loop around in the unlikely case that it did not. >>> >>> Signed-off-by: Rik van Riel <r...@redhat.com> >> >> So the purpose is to get rid of local_irq_save/restore()? >> Is it really worth such complication? > > local_irq_save/restore are quite slow, and look like the > largest source of overhead in irq time accounting.
I'm looking at an upstream tree, without your patches applied,
but it seems to me that irqtime_account_irq is always called with
interrupts disabled:
irqtime_account_irq
-> account_irq_enter_time
-> __irq_enter
-> HARDIRQ_ENTER [1]
-> irq_enter [3]
-> __do_softirq [1]
-> account_irq_exit_time
-> __do_softirq [1]
-> __irq_exit
-> HARDIRQ_EXIT [1]
-> irq_exit [2]
[1] = does local_irq_disable/enable
[2] = contains WARN_ON_ONCE(!irqs_disabled())
[3] = calls rcu_irq_enter(), which checks irqs_disabled()
I don't think your first two patches change this, so perhaps it's enough
to remove that local_irq-save/restore? Either this, or ENEEDWEEKEND...
Paolo
signature.asc
Description: OpenPGP digital signature
