From 98a5231fe5497c526849f2d84b1a9bbcbdfd2dbc Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Sat, 27 Jul 2013 17:45:28 +0200 Subject: [PATCH] Introduce vnode op supports_operation(), fix devfs_io() devfs_io() can't fall back to calling vfs_synchronous_io(), if the device driver doesn't support handling requests asynchronously. The presence of the io() hook leads the VFS (do_iterative_fd_io()) to believe that asynchronous handling is supported and set a finished-callback on the request which calls the io() hook to start the next chunk. Thus, instead of iterating through the request in a loop the iteration happens recursively. For sufficiently fragmented requests the stack may overflow (ticket #9900). * Introduce a new vnode operation supports_operation(). It can be called by the VFS to determine whether a present hook is actually currently supported for a given vnode. * devfs: implement the new hook and remove the fallback handling in devfs_io(). * vfs_request_io.cpp: use the new hook to determine whether the io() hook is really supported. --- docs/user/drivers/fs_interface.dox | 35 ++++++++ headers/os/drivers/fs_interface.h | 9 ++ src/system/kernel/device_manager/devfs.cpp | 95 +++++++++++----------- src/system/kernel/fs/vfs_request_io.cpp | 15 +++- 4 files changed, 105 insertions(+), 49 deletions(-) diff --git a/docs/user/drivers/fs_interface.dox b/docs/user/drivers/fs_interface.dox index edfe0573fe..5fcbfdcf1f 100644 --- a/docs/user/drivers/fs_interface.dox +++ b/docs/user/drivers/fs_interface.dox @@ -92,6 +92,16 @@ */ +/*! + \enum BVnodeOperation + Enumeration type for specifying vnode operations. +*/ + +/*! + \var BVnodeOperation B_VNODE_OPERATION_IO + Refers to the fs_vnode_ops::io operation. +*/ + ///// file_system_module_info ///// @@ -1698,6 +1708,31 @@ \brief TODO: Document! */ +/* TODO: test_lock(), acquire_lock(), release_lock() */ + +/*! + \fn bool (*fs_vnode_ops::supports_operation)(fs_volume* volume, + fs_vnode* vnode, BVnodeOperation operation) + \brief Returns whether the specified vnode operation is supported for the + specified vnode. + + For most vnode operations the FS can simply indicate that it doesn't support + it by setting the respective attribute in the fs_vnode_ops structure to + \c NULL. For certain nodes the support of an operation may change over the + life time of the node. While in most cases the implementation of the + operation can then simply return an error (\c B_NOT_SUPPORTED), for some + operations, however, the VFS has to know beforehand whether to call it at + all or resort to a different handling. It calls this hook only for + operations that aren't set to \c NULL in the fs_vnode_ops structure. If this + hook is not present, the VFS behaves as if it was present always returned + \c true. + + \param volume The volume object. + \param vnode The node object. + \param operation The enum value identifying the operation. + \return \c true, if the specified operation is currently supported, \c false + otherwise. +*/ //! @} diff --git a/headers/os/drivers/fs_interface.h b/headers/os/drivers/fs_interface.h index 1b3f0be8f7..b3b4847d02 100644 --- a/headers/os/drivers/fs_interface.h +++ b/headers/os/drivers/fs_interface.h @@ -45,6 +45,11 @@ struct file_io_vec { #define B_VNODE_DONT_CREATE_SPECIAL_SUB_NODE 0x02 +enum BVnodeOperation { + B_VNODE_OPERATION_IO = 0 +}; + + #ifdef __cplusplus extern "C" { #endif @@ -250,6 +255,10 @@ struct fs_vnode_ops { const struct flock* lock, bool wait); status_t (*release_lock)(fs_volume* volume, fs_vnode* vnode, void* cookie, const struct flock* lock); + + /* supported vnode operations info */ + bool (*supports_operation)(fs_volume* volume, fs_vnode* vnode, + enum BVnodeOperation operation); }; struct file_system_module_info { diff --git a/src/system/kernel/device_manager/devfs.cpp b/src/system/kernel/device_manager/devfs.cpp index ca77751768..ebd0af3ecb 100644 --- a/src/system/kernel/device_manager/devfs.cpp +++ b/src/system/kernel/device_manager/devfs.cpp @@ -118,11 +118,6 @@ struct devfs_cookie { void* device_cookie; }; -struct synchronous_io_cookie { - BaseDevice* device; - void* cookie; -}; - // directory iteration states enum { ITERATION_STATE_DOT = 0, @@ -772,22 +767,6 @@ get_device_name(struct devfs_vnode* vnode, char* buffer, size_t size) } -static status_t -device_read(void* _cookie, off_t offset, void* buffer, size_t* length) -{ - synchronous_io_cookie* cookie = (synchronous_io_cookie*)_cookie; - return cookie->device->Read(cookie->cookie, offset, buffer, length); -} - - -static status_t -device_write(void* _cookie, off_t offset, void* buffer, size_t* length) -{ - synchronous_io_cookie* cookie = (synchronous_io_cookie*)_cookie; - return cookie->device->Write(cookie->cookie, offset, buffer, length); -} - - static int dump_node(int argc, char** argv) { @@ -1782,36 +1761,14 @@ devfs_io(fs_volume* volume, fs_vnode* _vnode, void* _cookie, devfs_vnode* vnode = (devfs_vnode*)_vnode->private_node; devfs_cookie* cookie = (devfs_cookie*)_cookie; - bool isWrite = request->IsWrite(); - if (!S_ISCHR(vnode->stream.type) - || (((isWrite && !vnode->stream.u.dev.device->HasWrite()) - || (!isWrite && !vnode->stream.u.dev.device->HasRead())) - && !vnode->stream.u.dev.device->HasIO()) + || !vnode->stream.u.dev.device->HasIO() || cookie == NULL) { request->SetStatusAndNotify(B_NOT_ALLOWED); return B_NOT_ALLOWED; } - if (vnode->stream.u.dev.partition != NULL) { - if (request->Offset() + (off_t)request->Length() - > vnode->stream.u.dev.partition->info.size) { - request->SetStatusAndNotify(B_BAD_VALUE); - return B_BAD_VALUE; - } - translate_partition_access(vnode->stream.u.dev.partition, request); - } - - if (vnode->stream.u.dev.device->HasIO()) - return vnode->stream.u.dev.device->IO(cookie->device_cookie, request); - - synchronous_io_cookie synchronousCookie = { - vnode->stream.u.dev.device, - cookie->device_cookie - }; - - return vfs_synchronous_io(request, - request->IsWrite() ? &device_write : &device_read, &synchronousCookie); + return vnode->stream.u.dev.device->IO(cookie->device_cookie, request); } @@ -1901,6 +1858,22 @@ devfs_write_stat(fs_volume* _volume, fs_vnode* _vnode, const struct stat* stat, } +bool devfs_supports_operation(fs_volume* _volume, fs_vnode* _vnode, + BVnodeOperation operation) +{ + struct devfs_vnode* vnode = (struct devfs_vnode*)_vnode->private_node; + + switch (operation) { + case B_VNODE_OPERATION_IO: + return S_ISCHR(vnode->stream.type) + && vnode->stream.u.dev.device->HasIO(); + + default: + return false; + } +} + + static status_t devfs_std_ops(int32 op, ...) { @@ -1995,8 +1968,36 @@ fs_vnode_ops kVnodeOps = { &devfs_read_dir, &devfs_rewind_dir, - // attributes operations are not supported - NULL, + /* attribute directory */ + NULL, // open_attr_dir() + NULL, // close_attr_dir() + NULL, // free_attr_dir_cookie() + NULL, // read_attr_dir() + NULL, // rewind_attr_dir() + + /* attribute */ + NULL, // create_attr() + NULL, // open_attr() + NULL, // close_attr() + NULL, // free_attr_cookie() + NULL, // read_attr() + NULL, // write_attr() + NULL, // read_attr_stat() + NULL, // write_attr_stat() + NULL, // rename_attr() + NULL, // remove_attr() + + /* support for node and FS layers */ + NULL, // create_special_node() + NULL, // get_super_vnode() + + /* lock operations */ + NULL, // test_lock() + NULL, // acquire_lock() + NULL, // release_lock() + + /* supported vnode operations info */ + &devfs_supports_operation, }; } // namespace diff --git a/src/system/kernel/fs/vfs_request_io.cpp b/src/system/kernel/fs/vfs_request_io.cpp index 4a7984d94a..dd2d1cbba5 100644 --- a/src/system/kernel/fs/vfs_request_io.cpp +++ b/src/system/kernel/fs/vfs_request_io.cpp @@ -133,6 +133,17 @@ private: }; +static bool +is_vnode_io_operation_supported(struct vnode* vnode) +{ + if (!HAS_FS_CALL(vnode, io)) + return false; + if (!HAS_FS_CALL(vnode, supports_operation)) + return true; + return FS_CALL(vnode, supports_operation, B_VNODE_OPERATION_IO); +} + + static status_t do_iterative_fd_io_iterate(void* _cookie, io_request* request, bool* _partialTransfer) @@ -360,7 +371,7 @@ status_t vfs_vnode_io(struct vnode* vnode, void* cookie, io_request* request) { status_t result = B_ERROR; - if (!HAS_FS_CALL(vnode, io) + if (!is_vnode_io_operation_supported(vnode) || (result = FS_CALL(vnode, io, cookie, request)) == B_UNSUPPORTED) { // no io() call -- fall back to synchronous I/O VnodeIO io(request->IsWrite(), vnode, cookie); @@ -470,7 +481,7 @@ do_iterative_fd_io(int fd, io_request* request, iterative_io_get_vecs getVecs, CObjectDeleter descriptorPutter(descriptor, put_fd); - if (!HAS_FS_CALL(vnode, io)) { + if (!is_vnode_io_operation_supported(vnode)) { // no io() call -- fall back to synchronous I/O return do_synchronous_iterative_vnode_io(vnode, descriptor->cookie, request, getVecs, finished, cookie);