powerbook/radeon PM problem

powerbook/radeon PM problem

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1599 bytes --]

Whenever I resume my powerbook I get:

[  336.307861] BUG: sleeping function called from invalid context at kernel/rwsem.c:20
[  336.307871] in_atomic():0, irqs_disabled():1
[  336.307875] Call Trace:
[  336.307879] [EC257D30] [C000901C] show_stack+0x3c/0x194 (unreliable)
[  336.307900] [EC257D60] [C00268B8] __might_sleep+0xd4/0xe8
[  336.307918] [EC257D70] [C004555C] down_read+0x24/0x5c
[  336.307930] [EC257D90] [C003A574] blocking_notifier_call_chain+0x20/0x54
[  336.307940] [EC257DB0] [C017FB60] fb_notifier_call_chain+0x24/0x34
[  336.307949] [EC257DC0] [C0180178] fb_set_suspend+0x58/0x6c
[  336.307956] [EC257DE0] [C01A79F0] radeonfb_pci_resume+0x1fc/0x3e0
[  336.307964] [EC257E00] [C01A7BF8] radeonfb_early_resume+0x24/0x40
[  336.307970] [EC257E20] [C001A5C0] pmac_call_early_video_resume+0x2c/0x3c
[  336.307985] [EC257E30] [C01E1148] pmu_ioctl+0x6f8/0xb64
[  336.307994] [EC257ED0] [C0091C84] do_ioctl+0x80/0x84
[  336.308003] [EC257EE0] [C0091D0C] vfs_ioctl+0x84/0x43c
[  336.308009] [EC257F10] [C0092104] sys_ioctl+0x40/0x74
[  336.308014] [EC257F40] [C00114B4] ret_from_syscall+0x0/0x38


Hence, there must be a bug somewhere... The documentation to
fb_set_suspend notes:
"It must be called with the console semaphore held"

So that's one of the bugs, how can that be satisfied when we're just
resuming and haven't taken that semaphore? (if we did take it, then we'd
get the warning earlier..)

Then again, is the resume code really supposed to be called with
interrupts disabled?

I'm totally unsure what the correct fix for this is.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: powerbook/radeon PM problem

[Benjamin Herrenschmidt]

On Thu, 2007-02-01 at 20:11 +0100, Johannes Berg wrote:
> Whenever I resume my powerbook I get:
> 
> [  336.307861] BUG: sleeping function called from invalid context at kernel/rwsem.c:20
> [  336.307871] in_atomic():0, irqs_disabled():1
> [  336.307875] Call Trace:
> [  336.307879] [EC257D30] [C000901C] show_stack+0x3c/0x194 (unreliable)
> [  336.307900] [EC257D60] [C00268B8] __might_sleep+0xd4/0xe8
> [  336.307918] [EC257D70] [C004555C] down_read+0x24/0x5c
> [  336.307930] [EC257D90] [C003A574] blocking_notifier_call_chain+0x20/0x54
> [  336.307940] [EC257DB0] [C017FB60] fb_notifier_call_chain+0x24/0x34
> [  336.307949] [EC257DC0] [C0180178] fb_set_suspend+0x58/0x6c
> [  336.307956] [EC257DE0] [C01A79F0] radeonfb_pci_resume+0x1fc/0x3e0
> [  336.307964] [EC257E00] [C01A7BF8] radeonfb_early_resume+0x24/0x40
> [  336.307970] [EC257E20] [C001A5C0] pmac_call_early_video_resume+0x2c/0x3c
> [  336.307985] [EC257E30] [C01E1148] pmu_ioctl+0x6f8/0xb64
> [  336.307994] [EC257ED0] [C0091C84] do_ioctl+0x80/0x84
> [  336.308003] [EC257EE0] [C0091D0C] vfs_ioctl+0x84/0x43c
> [  336.308009] [EC257F10] [C0092104] sys_ioctl+0x40/0x74
> [  336.308014] [EC257F40] [C00114B4] ret_from_syscall+0x0/0x38
> 
> 
> Hence, there must be a bug somewhere... The documentation to
> fb_set_suspend notes:
> "It must be called with the console semaphore held"

Yeah well, it's -supposed- to be taken by the resume function, but the
early resume path is a hack that runs -really- early (so you get a
screen back for debugging). I should find a way to silence those stupid
warnings...

> So that's one of the bugs, how can that be satisfied when we're just
> resuming and haven't taken that semaphore? (if we did take it, then we'd
> get the warning earlier..)
> 
> Then again, is the resume code really supposed to be called with
> interrupts disabled?

Not normally... but when I wrote that stuff, there was no semaphore
involved in that code path... In fact, the semaphore we are taking
shouldn't be held by anybody else anyway..

> I'm totally unsure what the correct fix for this is.

None other than removing the early wakeup hack :-(

Ben.

Re: powerbook/radeon PM problem

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]

On Sun, 2007-02-04 at 15:47 +1100, Benjamin Herrenschmidt wrote:

> Yeah well, it's -supposed- to be taken by the resume function, but the
> early resume path is a hack that runs -really- early (so you get a
> screen back for debugging). I should find a way to silence those stupid
> warnings...

Yes, I agree that the semaphore that the comment talks about won't ever
be taken anyway. The warning actually comes from the notifier chain that
is used there, of course.

> > I'm totally unsure what the correct fix for this is.
> 
> None other than removing the early wakeup hack :-(

Actually, looking at the code again... fb_notifier_list is a blocking
notifier list. This could be converted to a raw one without any locking,
and we could require that callers of fb_register_client,
fb_unregister_client and fb_notifier_call_chain hold the console
semaphore while calling.
Then, some callers in drivers/video/fbmem.c would need to be updated,
but since fb_set_suspend already requires the semaphore to be held it
wouldn't acquire it again, of course.

We'd still abuse fb_set_suspend here by calling it without the semaphore
held, but we know that nobody else can hold it at that time so that'd be
ok.

Just a thought.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]