* Re: [Fwd: Re: via-pmu runs device_power_down in atomic context] [not found] <1148531830.13249.237.camel@localhost.localdomain> @ 2006-05-25 4:59 ` Andrew Morton 2006-05-25 5:19 ` Benjamin Herrenschmidt 2006-05-25 14:12 ` Alan Stern 0 siblings, 2 replies; 13+ messages in thread From: Andrew Morton @ 2006-05-25 4:59 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Johannes Berg, Alan Stern, cpufreq Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > > -------- Forwarded Message -------- > From: Benjamin Herrenschmidt <benh@kernel.crashing.org> > To: Johannes Berg <johannes@sipsolutions.net> > Cc: linuxppc-dev list <linuxppc-dev@ozlabs.org>, Michael Hanselmann > <linux-kernel@hansmi.ch>, cpufreq@lists.linux.org.uk > Subject: Re: via-pmu runs device_power_down in atomic context > Date: Thu, 25 May 2006 12:28:15 +1000 > > On Wed, 2006-05-24 at 10:01 +0200, Johannes Berg wrote: > > Hey, > > > > Everytime I suspend my powerbook, I see the following trace: > > > > [10655.887546] BUG: sleeping function called from invalid context at include/linux/rwsem.h:43 > > [10655.887558] in_atomic():0, irqs_disabled():1 > > [10655.887562] Call Trace: > > [10655.887565] [C581BD20] [C00081E8] show_stack+0x50/0x190 (unreliable) > > [10655.887582] [C581BD50] [C0023BB0] __might_sleep+0xcc/0xe8 > > [10655.887592] [C581BD60] [C0038290] blocking_notifier_call_chain+0x2c/0xc0 > > [10655.887606] [C581BD80] [C01E90C0] cpufreq_suspend+0x130/0x148 > > [10655.887616] [C581BDB0] [C019D9E8] sysdev_suspend+0x10c/0x300 > > [10655.887627] [C581BDF0] [C01A3888] device_power_down+0x74/0xac > > [10655.887636] [C581BE10] [C01B1264] pmac_suspend_devices+0x98/0x188 > > [10655.887643] [C581BE30] [C01B18F0] pmu_ioctl+0x59c/0xbc0 > > [10655.887649] [C581BED0] [C008E898] do_ioctl+0x80/0x84 > > [10655.887660] [C581BEE0] [C008E928] vfs_ioctl+0x8c/0x48c > > [10655.887666] [C581BF10] [C008ED68] sys_ioctl+0x40/0x74 > > [10655.887673] [C581BF40] [C000F3A4] ret_from_syscall+0x0/0x38 > > > > The might_sleep() comes from down_read() and this happens because > > blocking_notifier_call_chain calls it, it is also commented to run in > > process context so this is all proper. > > device_power_down should be called with interrupts off, thus the PMU > driver is fine. It's a misnamed function, it calls the sysdev's suspend > and those should be called with irq off. I think the problem is more due > to some cpufreq or notifier change that somebody done to recent kernels > and that added some might_sleep.... I wonder why. > > Andrew, what's up there ? What is this new > "blocking_notifier_call_chain" thing ? notifiers use to not use > semaphores and not be blocking... at least powermac implementation of > cpufreq relies on that. notifiers used to be racy too - we just waddled across them without any locking. Alan made a best-effort conversion of callers, and there have been a few problems. Here, pmac has gone and unilaterally decided that device_power_down() is atomic, even though device_power_down() _already_ calls suspend_device(), which does down(). So I'd say you've gone and found a via-pmu bug here. A way of shutting up the warning would be to use an atomic notifier, but it'll still be buggy. Better would be to teach pmac_suspend_devices() not to assume things which aren't true ;) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Fwd: Re: via-pmu runs device_power_down in atomic context] 2006-05-25 4:59 ` [Fwd: Re: via-pmu runs device_power_down in atomic context] Andrew Morton @ 2006-05-25 5:19 ` Benjamin Herrenschmidt 2006-05-25 5:36 ` Andrew Morton 2006-05-25 5:44 ` Benjamin Herrenschmidt 2006-05-25 14:12 ` Alan Stern 1 sibling, 2 replies; 13+ messages in thread From: Benjamin Herrenschmidt @ 2006-05-25 5:19 UTC (permalink / raw) To: Andrew Morton; +Cc: linuxppc-dev, Johannes Berg, Alan Stern, cpufreq > Here, pmac has gone and unilaterally decided that device_power_down() is > atomic, even though device_power_down() _already_ calls suspend_device(), > which does down(). So I'd say you've gone and found a via-pmu bug here. No. Look at the implementation (and the comment) in device_power_down(). It's designed to be called with irqs off... Of course, somebody changed the locking in there and it's indeed ending up calling suspend_device() for devices on the irq_off list which calls down ... bad bad... that's another bug in the drivers/power/* to add to an already long list. Fortunately, very few (if any) devics rely on this irq_off list. But sysdev's do. > A way of shutting up the warning would be to use an atomic notifier, but > it'll still be buggy. Better would be to teach pmac_suspend_devices() not > to assume things which aren't true ;) No. If we call device_power_down with interrupts enabled, very bad things will happen. This powermac code is very carefully crafted to do things in a strict order and it's along those lines that the callbacks in the device model were initially defined. Now, people who don't understand shit about how to make power management reliable may have broken things around, but the powermac implementation is right there. Ben. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Fwd: Re: via-pmu runs device_power_down in atomic context] 2006-05-25 5:19 ` Benjamin Herrenschmidt @ 2006-05-25 5:36 ` Andrew Morton 2006-05-25 5:46 ` Benjamin Herrenschmidt 2006-05-25 5:44 ` Benjamin Herrenschmidt 1 sibling, 1 reply; 13+ messages in thread From: Andrew Morton @ 2006-05-25 5:36 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, johannes, stern, cpufreq Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > > > > Here, pmac has gone and unilaterally decided that device_power_down() is > > atomic, even though device_power_down() _already_ calls suspend_device(), > > which does down(). So I'd say you've gone and found a via-pmu bug here. > > No. Look at the implementation (and the comment) in device_power_down(). > It's designed to be called with irqs off... Of course, somebody changed > the locking in there and it's indeed ending up calling suspend_device() > for devices on the irq_off list which calls down ... bad bad... that's > another bug in the drivers/power/* to add to an already long list. > Fortunately, very few (if any) devics rely on this irq_off list. But > sysdev's do. uh-huh. > > A way of shutting up the warning would be to use an atomic notifier, but > > it'll still be buggy. Better would be to teach pmac_suspend_devices() not > > to assume things which aren't true ;) > > No. If we call device_power_down with interrupts enabled, very bad > things will happen. This powermac code is very carefully crafted to do > things in a strict order and it's along those lines that the callbacks > in the device model were initially defined. Now, people who don't > understand shit about how to make power management reliable may have > broken things around, but the powermac implementation is right there. > This requirement to keep interrupts off in there breaks things again and again and again and again. And this time: again. It looks like we (again) have to live with it in which case conversion to an atomic notifier is probably needed. That may break other things though. Alan would know. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Fwd: Re: via-pmu runs device_power_down in atomic context] 2006-05-25 5:36 ` Andrew Morton @ 2006-05-25 5:46 ` Benjamin Herrenschmidt 2006-05-25 5:53 ` Benjamin Herrenschmidt 0 siblings, 1 reply; 13+ messages in thread From: Benjamin Herrenschmidt @ 2006-05-25 5:46 UTC (permalink / raw) To: Andrew Morton; +Cc: linuxppc-dev, johannes, stern, cpufreq > This requirement to keep interrupts off in there breaks things again and > again and again and again. And this time: again. Where else ? If you look at this function it's _designed_ for interrupts off ! the driver suspend have the option of deferring their suspend() callback until after irqs have been turned off (needed for legacy stuff afaik but rarely used) and the sysdev's are very low level things whose suspend and resume callbacks should be called after that point as well. Ben. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Fwd: Re: via-pmu runs device_power_down in atomic context] 2006-05-25 5:46 ` Benjamin Herrenschmidt @ 2006-05-25 5:53 ` Benjamin Herrenschmidt 0 siblings, 0 replies; 13+ messages in thread From: Benjamin Herrenschmidt @ 2006-05-25 5:53 UTC (permalink / raw) To: Andrew Morton; +Cc: linuxppc-dev, johannes, stern, cpufreq On Thu, 2006-05-25 at 15:46 +1000, Benjamin Herrenschmidt wrote: > > This requirement to keep interrupts off in there breaks things again and > > again and again and again. And this time: again. > > Where else ? > > If you look at this function it's _designed_ for interrupts off ! the > driver suspend have the option of deferring their suspend() callback > until after irqs have been turned off (needed for legacy stuff afaik but > rarely used) and the sysdev's are very low level things whose suspend > and resume callbacks should be called after that point as well. BTW. The root of the problem with cpufreq is it's upside down design :) Well, not all of it, but the fact that it registers a sysdev. It's not the "midlayer" (ugh) business to register devices and hook things like PM events to pass them down to actual drivers. It should be the cpufreq drivers themselves that attach to the device model in a way or another and "instanciate" the ability to control the cpu frequency, passing along their struct device. The driver itself should pick the right type of "device" to attach to (sysdev's are just weird beast and were a bad idea in the first place since they aren't even struct device). But then, iirc, that's because we also did the cpu stuff in sysfs upside down too ... :) Now, wether the cpufreq notifier might end up calling things that will sleep or not ... hrm... that's an interesting issue. Part of the problem is that because cpufreq is a sysdev, it will be called so late in the suspend process, pretty much everything else is asleep. So I'm quite confident that things attaching to the cpufreq notifiers other than bits of cpufreq itself are likely to break anyway. Is it documented anywhere that registering a cpufreq notifier might cause it to be called in atomic context or very later in the suspend process (possibly after the interrupt controller hs been put down) ? Yes it's messy, no I don't have a miracle solution, but I think here the proper way to fix it in the long run is for cpufreq not to be a sysdev or anything like that and to stop trying to do the suspend/resume thing for the drivers. Drivers are in charge, they get to create the device of whatever type it is and get the suspend/resume events whenever they are sent. cpufreq can then provide maybe "helpers" to help work out what to do at suspend and/or resume time but with the knowledge that for some drivers maybe, this will happen in atomic context. Ben. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Fwd: Re: via-pmu runs device_power_down in atomic context] 2006-05-25 5:19 ` Benjamin Herrenschmidt 2006-05-25 5:36 ` Andrew Morton @ 2006-05-25 5:44 ` Benjamin Herrenschmidt 1 sibling, 0 replies; 13+ messages in thread From: Benjamin Herrenschmidt @ 2006-05-25 5:44 UTC (permalink / raw) To: Andrew Morton; +Cc: linuxppc-dev, Johannes Berg, Alan Stern, cpufreq > No. If we call device_power_down with interrupts enabled, very bad > things will happen. This powermac code is very carefully crafted to do > things in a strict order and it's along those lines that the callbacks > in the device model were initially defined. Now, people who don't > understand shit about how to make power management reliable may have > broken things around, but the powermac implementation is right there. To be more precise, sysdev suspend is supposed to happen with irq offs (it's specifically designed to handle legacy things, interrupt controllers, ec...). Thus cpufreq suspend/resume need to assume that it's being called in that context or be made something else than a sysdev... Regarding the possible down() if we walk through the irq_off device list, well, that's indeed annoying, we probably need to pass a "no_lock" argument. Never hit that one since as I told you, there are really few if not no drivers using this facility of deferring suspend to after interrupts are disabled. Also, the down there is harmless as in no normal circumstances should it ever turn into a schedule() (there should be no contention possible that late in the suspend process). Ben. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Fwd: Re: via-pmu runs device_power_down in atomic context] 2006-05-25 4:59 ` [Fwd: Re: via-pmu runs device_power_down in atomic context] Andrew Morton 2006-05-25 5:19 ` Benjamin Herrenschmidt @ 2006-05-25 14:12 ` Alan Stern 2006-05-25 14:44 ` Andrew Morton 1 sibling, 1 reply; 13+ messages in thread From: Alan Stern @ 2006-05-25 14:12 UTC (permalink / raw) To: Andrew Morton; +Cc: cpufreq, Johannes Berg, linuxppc-dev On Wed, 24 May 2006, Andrew Morton wrote: > > device_power_down should be called with interrupts off, thus the PMU > > driver is fine. It's a misnamed function, it calls the sysdev's suspend > > and those should be called with irq off. I think the problem is more due > > to some cpufreq or notifier change that somebody done to recent kernels > > and that added some might_sleep.... I wonder why. > > > > Andrew, what's up there ? What is this new > > "blocking_notifier_call_chain" thing ? notifiers use to not use > > semaphores and not be blocking... at least powermac implementation of > > cpufreq relies on that. > > notifiers used to be racy too - we just waddled across them without any > locking. > > Alan made a best-effort conversion of callers, and there have been a few > problems. > > Here, pmac has gone and unilaterally decided that device_power_down() is > atomic, even though device_power_down() _already_ calls suspend_device(), > which does down(). So I'd say you've gone and found a via-pmu bug here. > > A way of shutting up the warning would be to use an atomic notifier, but > it'll still be buggy. Better would be to teach pmac_suspend_devices() not > to assume things which aren't true ;) Someone else had a problem with the cpufreq conversion earlier. I posted a message on the cpufreq mailing list but nobody ever responded to it. This may or may not be related to that earlier problem. In essence, the problem seemed to be that the cpufreq notifier chain is sometimes expected to be blocking and sometimes expected to be atomic, based on the "val" code passed to notifier_call_chain. The cleanest solution would be to split the single notifier chain into two chains, one always blocking and the other always atomic. Somebody who knows more about cpufreq than I do will have to make that change. Alan Stern ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Fwd: Re: via-pmu runs device_power_down in atomic context] 2006-05-25 14:12 ` Alan Stern @ 2006-05-25 14:44 ` Andrew Morton 2006-05-25 16:44 ` Jon Loeliger 0 siblings, 1 reply; 13+ messages in thread From: Andrew Morton @ 2006-05-25 14:44 UTC (permalink / raw) To: Alan Stern; +Cc: cpufreq, johannes, linuxppc-dev Alan Stern <stern@rowland.harvard.edu> wrote: > > On Wed, 24 May 2006, Andrew Morton wrote: > > > > device_power_down should be called with interrupts off, thus the PMU > > > driver is fine. It's a misnamed function, it calls the sysdev's suspend > > > and those should be called with irq off. I think the problem is more due > > > to some cpufreq or notifier change that somebody done to recent kernels > > > and that added some might_sleep.... I wonder why. > > > > > > Andrew, what's up there ? What is this new > > > "blocking_notifier_call_chain" thing ? notifiers use to not use > > > semaphores and not be blocking... at least powermac implementation of > > > cpufreq relies on that. > > > > notifiers used to be racy too - we just waddled across them without any > > locking. > > > > Alan made a best-effort conversion of callers, and there have been a few > > problems. > > > > Here, pmac has gone and unilaterally decided that device_power_down() is > > atomic, even though device_power_down() _already_ calls suspend_device(), > > which does down(). So I'd say you've gone and found a via-pmu bug here. > > > > A way of shutting up the warning would be to use an atomic notifier, but > > it'll still be buggy. Better would be to teach pmac_suspend_devices() not > > to assume things which aren't true ;) > > Someone else had a problem with the cpufreq conversion earlier. I posted > a message on the cpufreq mailing list but nobody ever responded to it. > This may or may not be related to that earlier problem. > > In essence, the problem seemed to be that the cpufreq notifier chain is > sometimes expected to be blocking and sometimes expected to be atomic, > based on the "val" code passed to notifier_call_chain. The cleanest > solution would be to split the single notifier chain into two chains, > one always blocking and the other always atomic. > > Somebody who knows more about cpufreq than I do will have to make that > change. > I wouldn't describe the cpufreq project as a teeming hive of frenetic activity, and we need something pronto. We could go back to a raw_notifier and be as buggy as we used to be. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Fwd: Re: via-pmu runs device_power_down in atomic context] 2006-05-25 14:44 ` Andrew Morton @ 2006-05-25 16:44 ` Jon Loeliger 2006-05-25 17:53 ` [PATCH] Make cpufreq_transition_notifier a raw notifier Alan Stern 0 siblings, 1 reply; 13+ messages in thread From: Jon Loeliger @ 2006-05-25 16:44 UTC (permalink / raw) To: Andrew Morton; +Cc: cpufreq, johannes, Alan Stern, linuxppc-dev@ozlabs.org On Thu, 2006-05-25 at 09:44, Andrew Morton wrote: > > In essence, the problem seemed to be that the cpufreq notifier chain is > > sometimes expected to be blocking and sometimes expected to be atomic, > > based on the "val" code passed to notifier_call_chain. The cleanest > > solution would be to split the single notifier chain into two chains, > > one always blocking and the other always atomic. > > > > Somebody who knows more about cpufreq than I do will have to make that > > change. > > > > I wouldn't describe the cpufreq project as a teeming hive of frenetic > activity, and we need something pronto. > > We could go back to a raw_notifier and be as buggy as we used to be. It _is_ actively being pursued today for a cleaned up implementation. If there are issues or requirements here, we should really pass them on to the linux-pm list. Thanks, jdl ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] Make cpufreq_transition_notifier a raw notifier 2006-05-25 16:44 ` Jon Loeliger @ 2006-05-25 17:53 ` Alan Stern 2006-05-25 18:41 ` Johannes Berg 2006-05-25 23:18 ` Benjamin Herrenschmidt 0 siblings, 2 replies; 13+ messages in thread From: Alan Stern @ 2006-05-25 17:53 UTC (permalink / raw) To: Andrew Morton; +Cc: linuxppc-dev@ozlabs.org, johannes, cpufreq The cpufreq code has problems with atomic vs. blocking notifier calls and enabling vs. disabling interrupts (show up on pmac). As a temporary band-aid, this patch (as697) makes the cpufreq_transition_notifier_list into a raw notifier. Signed-off-by: Alan Stern <stern@rowland.harvard.edu> --- On Thu, 25 May 2006, Jon Loeliger wrote: > On Thu, 2006-05-25 at 09:44, Andrew Morton wrote: > > > > In essence, the problem seemed to be that the cpufreq notifier chain is > > > sometimes expected to be blocking and sometimes expected to be atomic, > > > based on the "val" code passed to notifier_call_chain. The cleanest > > > solution would be to split the single notifier chain into two chains, > > > one always blocking and the other always atomic. > > > > > > Somebody who knows more about cpufreq than I do will have to make that > > > change. > > > > > > > I wouldn't describe the cpufreq project as a teeming hive of frenetic > > activity, and we need something pronto. > > > > We could go back to a raw_notifier and be as buggy as we used to be. > > It _is_ actively being pursued today for a cleaned up > implementation. If there are issues or requirements > here, we should really pass them on to the linux-pm list. > > Thanks, > jdl Here's the patch Andrew asked for. I have no idea whether it will solve any of these problems, and I'm certain that in the long run we shouldn't keep it. Perhaps it will at least help stabilize 2.6.17. It's up to you guys to see if the patch helps at all and to decide whether or not it should be applied. Alan Stern Index: usb-2.6/drivers/cpufreq/cpufreq.c =================================================================== --- usb-2.6.orig/drivers/cpufreq/cpufreq.c +++ usb-2.6/drivers/cpufreq/cpufreq.c @@ -50,10 +50,9 @@ static void handle_update(void *data); * validation process for a new CPU frequency policy; the * "transition" list for kernel code that needs to handle * changes to devices when the CPU clock speed changes. - * The mutex locks both lists. */ static BLOCKING_NOTIFIER_HEAD(cpufreq_policy_notifier_list); -static BLOCKING_NOTIFIER_HEAD(cpufreq_transition_notifier_list); +static RAW_NOTIFIER_HEAD(cpufreq_transition_notifier_list); static LIST_HEAD(cpufreq_governor_list); @@ -263,14 +262,14 @@ void cpufreq_notify_transition(struct cp freqs->old = policy->cur; } } - blocking_notifier_call_chain(&cpufreq_transition_notifier_list, + raw_notifier_call_chain(&cpufreq_transition_notifier_list, CPUFREQ_PRECHANGE, freqs); adjust_jiffies(CPUFREQ_PRECHANGE, freqs); break; case CPUFREQ_POSTCHANGE: adjust_jiffies(CPUFREQ_POSTCHANGE, freqs); - blocking_notifier_call_chain(&cpufreq_transition_notifier_list, + raw_notifier_call_chain(&cpufreq_transition_notifier_list, CPUFREQ_POSTCHANGE, freqs); if (likely(policy) && likely(policy->cpu == freqs->cpu)) policy->cur = freqs->new; @@ -1014,7 +1013,7 @@ static int cpufreq_suspend(struct sys_de freqs.old = cpu_policy->cur; freqs.new = cur_freq; - blocking_notifier_call_chain(&cpufreq_transition_notifier_list, + raw_notifier_call_chain(&cpufreq_transition_notifier_list, CPUFREQ_SUSPENDCHANGE, &freqs); adjust_jiffies(CPUFREQ_SUSPENDCHANGE, &freqs); @@ -1095,7 +1094,7 @@ static int cpufreq_resume(struct sys_dev freqs.old = cpu_policy->cur; freqs.new = cur_freq; - blocking_notifier_call_chain( + raw_notifier_call_chain( &cpufreq_transition_notifier_list, CPUFREQ_RESUMECHANGE, &freqs); adjust_jiffies(CPUFREQ_RESUMECHANGE, &freqs); @@ -1141,7 +1140,7 @@ int cpufreq_register_notifier(struct not switch (list) { case CPUFREQ_TRANSITION_NOTIFIER: - ret = blocking_notifier_chain_register( + ret = raw_notifier_chain_register( &cpufreq_transition_notifier_list, nb); break; case CPUFREQ_POLICY_NOTIFIER: @@ -1173,7 +1172,7 @@ int cpufreq_unregister_notifier(struct n switch (list) { case CPUFREQ_TRANSITION_NOTIFIER: - ret = blocking_notifier_chain_unregister( + ret = raw_notifier_chain_unregister( &cpufreq_transition_notifier_list, nb); break; case CPUFREQ_POLICY_NOTIFIER: ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Make cpufreq_transition_notifier a raw notifier 2006-05-25 17:53 ` [PATCH] Make cpufreq_transition_notifier a raw notifier Alan Stern @ 2006-05-25 18:41 ` Johannes Berg 2006-05-25 19:14 ` Alan Stern 2006-05-25 23:18 ` Benjamin Herrenschmidt 1 sibling, 1 reply; 13+ messages in thread From: Johannes Berg @ 2006-05-25 18:41 UTC (permalink / raw) To: Alan Stern; +Cc: Andrew Morton, linuxppc-dev@ozlabs.org, cpufreq [-- Attachment #1: Type: text/plain, Size: 602 bytes --] On Thu, 2006-05-25 at 13:53 -0400, Alan Stern wrote: > Here's the patch Andrew asked for. I have no idea whether it will solve > any of these problems, and I'm certain that in the long run we shouldn't > keep it. Perhaps it will at least help stabilize 2.6.17. > > It's up to you guys to see if the patch helps at all and to decide whether > or not it should be applied. Maybe I should note that I don't actually have seen any problems because of this, the kernel just warns about it. I guess that's because the semaphore won't ever actually be contended at that point. johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 793 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Make cpufreq_transition_notifier a raw notifier 2006-05-25 18:41 ` Johannes Berg @ 2006-05-25 19:14 ` Alan Stern 0 siblings, 0 replies; 13+ messages in thread From: Alan Stern @ 2006-05-25 19:14 UTC (permalink / raw) To: Johannes Berg; +Cc: Andrew Morton, linuxppc-dev@ozlabs.org, cpufreq On Thu, 25 May 2006, Johannes Berg wrote: > On Thu, 2006-05-25 at 13:53 -0400, Alan Stern wrote: > > > Here's the patch Andrew asked for. I have no idea whether it will solve > > any of these problems, and I'm certain that in the long run we shouldn't > > keep it. Perhaps it will at least help stabilize 2.6.17. > > > > It's up to you guys to see if the patch helps at all and to decide whether > > or not it should be applied. > > Maybe I should note that I don't actually have seen any problems because > of this, the kernel just warns about it. I guess that's because the > semaphore won't ever actually be contended at that point. The problem isn't contention for the semaphore. The problem is that down_read() will enable interrupts even when the semaphore is not in contention, but parts of the cpufreq notifier chain need to execute with interrupts disabled. Alan Stern ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Make cpufreq_transition_notifier a raw notifier 2006-05-25 17:53 ` [PATCH] Make cpufreq_transition_notifier a raw notifier Alan Stern 2006-05-25 18:41 ` Johannes Berg @ 2006-05-25 23:18 ` Benjamin Herrenschmidt 1 sibling, 0 replies; 13+ messages in thread From: Benjamin Herrenschmidt @ 2006-05-25 23:18 UTC (permalink / raw) To: Alan Stern; +Cc: Andrew Morton, linuxppc-dev@ozlabs.org, johannes, cpufreq On Thu, 2006-05-25 at 13:53 -0400, Alan Stern wrote: > The cpufreq code has problems with atomic vs. blocking notifier calls and > enabling vs. disabling interrupts (show up on pmac). As a temporary > band-aid, this patch (as697) makes the cpufreq_transition_notifier_list > into a raw notifier. Thanks Alan. That should fix our warning for now, though we'll have to sync with brodo to get cpufreq right. As I explained separately, I think part of the problem is bcs cpufreq is a sysdev, it shouldn't have to be, at least not necessarily. I'm worried that even if they work in atomic context, some drivers who hook on the cpufreq events might still be buggy when notified so late in the sleep process. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2006-05-25 23:18 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1148531830.13249.237.camel@localhost.localdomain>
2006-05-25 4:59 ` [Fwd: Re: via-pmu runs device_power_down in atomic context] Andrew Morton
2006-05-25 5:19 ` Benjamin Herrenschmidt
2006-05-25 5:36 ` Andrew Morton
2006-05-25 5:46 ` Benjamin Herrenschmidt
2006-05-25 5:53 ` Benjamin Herrenschmidt
2006-05-25 5:44 ` Benjamin Herrenschmidt
2006-05-25 14:12 ` Alan Stern
2006-05-25 14:44 ` Andrew Morton
2006-05-25 16:44 ` Jon Loeliger
2006-05-25 17:53 ` [PATCH] Make cpufreq_transition_notifier a raw notifier Alan Stern
2006-05-25 18:41 ` Johannes Berg
2006-05-25 19:14 ` Alan Stern
2006-05-25 23:18 ` Benjamin Herrenschmidt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).