On Fri, Jul 08, 2016 at 11:56:49AM -0400, Rik van Riel wrote: > On Fri, 2016-07-08 at 16:34 +0200, Paolo Bonzini wrote: > > 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...
Good catch Paolo! > > I think you are right! > > __do_softirq() calls account_irq_enter_time() with irqs > already disabled, and also has irqs disabled when it > calls account_irq_exit_time() > > This appears to be true for both ksoftirqd and softirq > from irq context. Indeed! And irq_enter()/irq_exit() have irqs disabled requirements. The other users are __irq_enter() and __irq_exit() called by lockdep selftests which takes care about it too. I just did a boot test with a WARN_ON_ONCE(!irqs_disabled()) on account_irq_enter_time() and it triggered no issue. > > This could simplify my patch series a lot :) Definetly! ;-) Would you mind resending it? Thanks.
