On Friday, 20 April 2007 21:51, Johannes Berg wrote: > On Fri, 2007-04-20 at 12:44 -0700, Andrew Morton wrote: > > > > - local_irq_restore(flags); > > > + arch_s2ram_enable_irqs(&flags); > > > + BUG_ON(irqs_disabled()); > > > return error; > > > } > > > > The second BUG_ON() looks a bit fishy to me. If someone calls > > suspend_enter() with local interrupts disabled, it will trigger. > > Huh, hm, I don't think it's valid to do that. I'm not sure why the code > does irq_save/restore instead of just disable/enable. Well, I can't say why exactly suspend_enter() usues irq_save/restore, but it looks like that's not needed. For the suspend to disk we call local_irq_disable/enable() in the corresponding places and, for example, acpi_pm_enter apparently uses irq_save/restore() itself too. > The only users of this function are in the power management core code (main.c > and user.c) and they certainly always call it with interrupts enabled, the > entry point for other users is pm_suspend() and that surely cannot be called > with interrupts disabled. Rafael? That's correct, plus pm_suspend() also uses enter_state(). In fact suspend_enter() has been made extern so that we can call it from kernel/power/user.c and no one else is supposed to use it. _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
