On Mon, 7 Jun 2010 21:13:41 -0700 mark gross <640e9920@xxxxxxxxx> wrote: > On Mon, Jun 07, 2010 at 12:19:41PM -0400, James Bottomley wrote: > > On Mon, 2010-06-07 at 17:34 +0200, florian@xxxxxxxxxxx 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 _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
