Messages in this thread | | | Date | Fri, 13 Jun 2008 00:28:08 -0700 | From | Andrew Morton <> | Subject | Re: [patch 6/6] x86: add c1e aware idle function |
| |
On Fri, 13 Jun 2008 08:02:30 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 12 Jun 2008, Andrew Morton wrote: > > On Thu, 12 Jun 2008 10:29:00 -0000 > > Thomas Gleixner <tglx@linutronix.de> wrote: > > > + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu); > > > + default_idle(); > > > + local_irq_disable(); > > > + clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu); > > > + local_irq_enable(); > > > > I worked it out! It took me a while. > > > > This function is called with local irqs disabled. > > > > default_idle() is entered with local irqs disabled but returns with them > > enabled. > > > > clockevents_notify() is supposed to be called with local irqs disabled. > > > > This functions returns with local irqs enabled. > > Damn, you decoded my sekrit.
The wonders of the GPL.
> > Was I right? None of any of that is documented anywhere. But it should > > be. > > Yeah, needs a big comment. Will add one. >
Actually it need lots of little comments. One at default_idle(), one at clockevents_notify(), one at whatever-this-function-was.
Because "must be called with local irqs disabled" is as much a part of a function's interface as "must be passed a foo* and an integer and returns a bar*". Ditto "must be called under zot_lock". But people often forget these things.
I guess kerneldoc should have a standard "call environment" template. Maybe it does, dunno.
Of course, WARN_ON(!irqs_disabled()) and WARN_ON(!spin_is_locked(foo)) is robust documentation.
| |