Re: [PATCH v3] pm_qos: make update_request callable from interrupt context

[Florian Mickler <florian@mickler.org>]

On Mon, 7 Jun 2010 21:13:41 -0700
mark gross <640e9920@gmail.com> wrote:

> On Mon, Jun 07, 2010 at 12:19:41PM -0400, James Bottomley wrote:
> > On Mon, 2010-06-07 at 17:34 +0200, florian@mickler.org wrote:
> > > We use the spinlocked notifier chain variant (struct
> > > atomic_notifier_head) and add an __might_sleep() to the chain for
> > > constraints which have non-atomic notifiers. This way we catch all
> > > interrupt-context-update-sites at runtime.
> > 
> > Actually, I'm afraid we can't really call blocking notifiers through the
> > atomic chain because we might end up with a contested chain call and a
> > huge busy wait in the spinlock (especially if one of the notifiers is
> > sleeping).

Argh. True. The spinlock is held while calling the notifiers. 

> > 
> > I think the pm_qos_object still needs the two notifier chains ... it's
> > just that when set up, one must either fill an atomic or a blocking
> > chain (leaving the other NULL).  We use the NULL to check to decide what
> > chain to add notifiers to, and if the blocking chain is null, we refuse
> > to add blocking notifiers (with a BUG). If the blocking chain is
> > non-null, we register the might_sleep() notifier (actually, given the
> > argument mismatch, you'll have to wrapper that).
> > 
> > James
> Can't we just requiere that all notifier callbacks be atomic context
> safe and not fart around with 2 classes of notifiers?
> 
> --mgross

I think that would be conceptual right. 

Under the assumption, that the qos-infrastructure is used by the
drivers to guarantee their functioning, we can not allow a race to
occur. 

( Individual listeners of course are free to spawn or schedule work if
they think it is safe to do so and have all races possible considered. )

I'll audit the current listeners this evening and respin this patch
without the might_sleep()'s and adapted comments.
What do you think?

Cheers,
Flo