On Tue, 5 Aug 2008 07:37:55 +0200 Krzysztof Helt [off-list ref]
wrote:
quoted
From: Krzysztof Helt <redacted>
Change lock_kernel()/unlock_kernel() to local fb mutex.
Each frame buffer instance has its own mutex.
The one line try_to_load() function is unrolled to
request_module() in two places for readability.
Signed-off-by: Krzysztof Helt <redacted>
---
The only caveat is that I had to put request_module()
call outside this new mutex while it was inside the
lock_kernel() (inside fb_open()).
Why did you have to do that?
This has to be done if the per fb mutex (fb_info->lock) is used as the request_module is called before fb_info pointer exists.
I tried to avoid one big mutex for the fb layer (which would not be worse than lock_kernel now).
Alternative solution is to provide one global mutex inside the fb layer to mutex fb_open and fb_close functions and another mutex per each fb (fb_info->lock) for all other locking (so ioctls on different fb instances/cards can be concurrent).
The request_module() in fb_ioctl() is inside the lock...
Yes. This is some mistake I suppose as the fb_info pointer is used before the switch
then it is checked against null inside the switch. Either the request_module is never called there (dead code) or a core dump happens sometimes.
As I see it now, the whole fbmem locking is somehow not right as IMO the registered_fb[idx] access should be locked to avoid situation that someone is adding/removing the fb and another function grabs fb_info pointer from the registered_fb[].
quoted
If the request_module() should be called only inside this mutex
please drop the patch.
No, request_module() doesn't need lock_kernel().
The only possible problem I can see is the sole remaining lock_kernel()
in drivers/video/console/vgacon.c. The fbcon.c code used to be
serialised against vgacon_do_font_op() via lock_kernel, but with this
change vgacon_do_font_op() can now run concurrently with all the code
which you've put inside fb_info.mutex_lock. Probably not a problem, but
please check that sometime.
I will look into it and either resend this patch or another one.
Should I cc some other list or person next time?
Kind regards,
Krzysztof
----------------------------------------------------------------------
Wymien zeszyty na notebooka!
Sprawdz >>> http://link.interia.pl/f1eab
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/