From c146183167273258c790ed24e6fa6b26c8fd8bbf Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Tue, 7 Jun 2022 14:30:21 -0400 Subject: [PATCH] devfs: Do not perform synchronous requests in devfs_io. In some circumstances, we can wind up iterating on requests by recursing through this function once per "iteration" due to how vfs_synchronous_io works. This can run out of stack, as was seen in #9900 and potentially other tickets. An initial attempt was made to fix the problem in hrev45906, but it was quickly reverted because it seems to have broken I/O for all devices that do not support the "IO" hook. This solution is much simpler: the VFS layer already can handle vnodes' IO hooks returning B_UNSUPPORTED and then falling back to synchronous I/O just as if they had no IO hook at all. This should hopefully cause iteration to occur without recursion. Tested by booting off a USB drive (the usb_disk driver does not support the IO hook, so all requests must be translated.) May fix #9900. --- src/system/kernel/device_manager/devfs.cpp | 43 +++------------------- 1 file changed, 5 insertions(+), 38 deletions(-) diff --git a/src/system/kernel/device_manager/devfs.cpp b/src/system/kernel/device_manager/devfs.cpp index 0cf66faaa0..66c41a5555 100644 --- a/src/system/kernel/device_manager/devfs.cpp +++ b/src/system/kernel/device_manager/devfs.cpp @@ -149,11 +149,6 @@ struct devfs_cookie { void* device_cookie; }; -struct synchronous_io_cookie { - BaseDevice* device; - void* cookie; -}; - // directory iteration states enum { ITERATION_STATE_DOT = 0, @@ -818,22 +813,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) { @@ -1828,17 +1807,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()) - || cookie == NULL) { + if (!S_ISCHR(vnode->stream.type) || cookie == NULL) { request->SetStatusAndNotify(B_NOT_ALLOWED); return B_NOT_ALLOWED; } + if (!vnode->stream.u.dev.device->HasIO()) + return B_UNSUPPORTED; + if (vnode->stream.u.dev.partition != NULL) { if (request->Offset() + (off_t)request->Length() > vnode->stream.u.dev.partition->info.size) { @@ -1848,16 +1824,7 @@ devfs_io(fs_volume* volume, fs_vnode* _vnode, void* _cookie, 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); }