Hi On Jan,Sunday 25 2009, at 9:19 PM, Andrew Doran wrote:
The code belongs in specfs because all of the concurrency issues around device nodes are managed there and in vfs. Nothing should be calling the cdev or bdev functions directly other than specfs (I know there is existingcode that does this). Also:
I have looked at my patch again and add device module autoloading to specfs
spec_open routine.
Fixed I'm autoloading modules between VOP_UNLOCK and vn_lock in spec_open,- It is preferable to _not_ hold vnode locks when autoloading a module.Ideally this means dropping the vnode's lock before calling module_autoload.
but VOP_UNLOCK is used only with char device, therefore I'm not sure if block device vnode is locked, too or not.
- I think that pooka pointed out the race condition with module loading in your code. The module can be gone by the time you try to use it. You canwork around this by testing module_gen. 1) cache module_gen 2) try to autoload device driver 3) if it fails, and module_gen != cached copy, restart.
With current patch if I autoload module and switch to another process which will unload this module. I will try to open device which fail with ENXIO and whole process will start again. But this module_gen approach is not optimal I think, because module_gen is changed even when different module is loaded/unloaded and therefore I can
repeat whole process with already loaded module.
- Note that we do not yet have code to prevent removal of in-use devsw entries. A starting point would be to track opens by major number in specfs but there are a lot of issues to consider.
Can you better describe these issues ? I think that this is way to go if we want to properly fix this type of race conditions.
Attachment:
specfs.diff
Description: Binary data
Regards Adam.