[PATCH][2.6-mm] radeonfb as module

[PATCH][2.6-mm] radeonfb as module

[Svetoslav Slavtchev]

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

acquire_console_sem is exported, 
but release_console_sem is not

this seems like a bug for me,
as if one acquire console_sem, he should be able to relase it

svetljo

PS.
missing symbol in (IIRC) drivers/video/aty/radeon_pm.c

-- 
NEU FÜR ALLE - GMX MediaCenter - für Fotos, Musik, Dateien...
Fotoalbum, File Sharing, MMS, Multimedia-Gruß, GMX FotoService

Jetzt kostenlos anmelden unter http://www.gmx.net

+++ GMX - die erste Adresse für Mail, Message, More! +++

[-- Attachment #2: missed_export-release_console_sem.patch --]
[-- Type: application/octet-stream, Size: 388 bytes --]

--- linux-2.6.0-test7/kernel/printk.c.orig	2003-10-21 09:45:32.294956018 +0200
+++ linux-2.6.0-test7/kernel/printk.c	2003-10-21 09:43:59.952049306 +0200
@@ -566,6 +566,7 @@
 	if (wake_klogd && !oops_in_progress && waitqueue_active(&log_wait))
 		wake_up_interruptible(&log_wait);
 }
+EXPORT_SYMBOL(release_console_sem);
 
 /** console_conditional_schedule - yield the CPU if required
  *

Re: [PATCH][2.6-mm] radeonfb as module

[James Simmons]

> acquire_console_sem is exported, 
> but release_console_sem is not
> 
> this seems like a bug for me,
> as if one acquire console_sem, he should be able to relase it


True that release_console_sem should be exported as well. But that lock 
shouldn't be in a driver. In fbcon.c yes but not the driver. My next pacth 
will fix that.

Re: [PATCH][2.6-mm] radeonfb as module

[Svetoslav Slavtchev]

> 
> > acquire_console_sem is exported, 
> > but release_console_sem is not
> > 
> > this seems like a bug for me,
> > as if one acquire console_sem, he should be able to relase it
> 
> 
> True that release_console_sem should be exported as well. But that lock 
> shouldn't be in a driver. In fbcon.c yes but not the driver. My next pacth
> 
> will fix that.
> 
Hi James,
i've some related questions :-)

are you going to merge new radeonfb driver in fbdev bk tree ?
is the driver in ppc bk tree newer? (i have such impression)


svetljo

-- 
NEU FÜR ALLE - GMX MediaCenter - für Fotos, Musik, Dateien...
Fotoalbum, File Sharing, MMS, Multimedia-Gruß, GMX FotoService

Jetzt kostenlos anmelden unter http://www.gmx.net

+++ GMX - die erste Adresse für Mail, Message, More! +++

Re: [PATCH][2.6-mm] radeonfb as module

[James Simmons]

> Hi James,
> i've some related questions :-)
> 
> are you going to merge new radeonfb driver in fbdev bk tree ?
> is the driver in ppc bk tree newer? (i have such impression)
I will end up merging Ben's latest driver. A few things have to be done 
tho to do that (power management code). I just added that today and I will 
be also restoring fbset mode changing support again.

Re: [PATCH][2.6-mm] radeonfb as module

[Benjamin Herrenschmidt]

On Tue, 2003-10-21 at 19:28, James Simmons wrote:
> > acquire_console_sem is exported, 
> > but release_console_sem is not
> > 
> > this seems like a bug for me,
> > as if one acquire console_sem, he should be able to relase it
> 
> 
> True that release_console_sem should be exported as well. But that lock 
> shouldn't be in a driver. In fbcon.c yes but not the driver. My next pacth 
> will fix that.

Well... please leave it the way I did it for now. There are subtle
races if you don't do. In a more general manner, there are a lot of
races in fbdev regarding lack of console_sem. For example, we should
take it on any mode change ioctl etc...

The problem is that, except for eventual local spinlocks, the console
sem is the only way to protect the low level drivers against concurrent
accesses, and those _do_ need some protection or you can have
accelerated printk racing against a mode change for example, etc...

It's even worse if mode change can sleep. Some updates to fbdev I'm
currently working on will require the ability to sleep from the
set_par() function. I would even to sleep in blank(), but that's
not possible, I think, at the moment (isn't it called by a timer
in the console code ?) so I'm doing something more complicated
using timers.

So almost all calls to the fbdev should be done with the console
sem held (even blank imho, if you can't get it using a trylock,
delay it by firing the timer).

Regarding the fb_client callbacks, to avoid lock nesting, I decided
the only sane way is to have the _caller_ take the console sem.
That's important as those can be called from things like set_par()
which will (if the above is fixed properly) be called with the
console sem already held. So the act of taking the console sem should
be moved up the chain as much as possible. In the case of calls issued
from outside the fbdev world, like PM callbacks, the driver can only
take the console sem itself to protect against concurrent set_par
among others.

Now, if you want to "hide" the console sem, maybe export an fbdev_lock/
unlock API whose implementation takes the console sem... That would
allow maybe to move to a per-fbdev locking in the future provided we
can deal with the "printk" problem (that is deferring actual display
while still letting fbcon draw into the console buffer).

Ben.