From: Fotis Xenakis <[email protected]> Committer: Waldemar Kozaczuk <[email protected]> Branch: master
virtio-fs: fix lost wakeup in driver Previously, virtio::fs::fuse_request used a waitqueue for waiting until the request was processed by the device. After the request was dispatched, the calling thread would wait on that waitqueue, woken by the driver's request callback thread, when the request was processed. This left room for a lost wakeup, where the request would be processed before the interested thread would wait() on it (happened during testing with fio). This is now fixed, using a waiter instead of a waitqueue. Signed-off-by: Fotis Xenakis <[email protected]> Message-Id: <am0pr03mb62922e4ebb042309920e6d61a6...@am0pr03mb6292.eurprd03.prod.outlook.com> --- diff --git a/drivers/virtio-fs.cc b/drivers/virtio-fs.cc --- a/drivers/virtio-fs.cc +++ b/drivers/virtio-fs.cc @@ -24,22 +24,6 @@ using fuse_request = virtio::fs::fuse_request; namespace virtio { -// Wait for the request to be marked as completed. -void fs::fuse_request::wait() -{ - WITH_LOCK(req_mutex) { - req_wait.wait(req_mutex); - } -} - -// Mark the request as completed. -void fs::fuse_request::done() -{ - WITH_LOCK(req_mutex) { - req_wait.wake_one(req_mutex); - } -} - static void fuse_req_enqueue_input(vring& queue, fuse_request& req) { // Header goes first diff --git a/drivers/virtio-fs.hh b/drivers/virtio-fs.hh --- a/drivers/virtio-fs.hh +++ b/drivers/virtio-fs.hh @@ -12,7 +12,8 @@ #include <osv/mmio.hh> #include <osv/mutex.h> -#include <osv/waitqueue.hh> +#include <osv/sched.hh> +#include <osv/wait_record.hh> #include "drivers/device.hh" #include "drivers/driver.hh" @@ -29,6 +30,8 @@ enum { class fs : public virtio_driver { public: + // A fuse_request encapsulates a low-level virtio-fs device request. This is + // a single-use object. struct fuse_request { struct fuse_in_header in_header; struct fuse_out_header out_header; @@ -39,12 +42,19 @@ public: void* output_args_data; size_t output_args_size; - void wait(); - void done(); + // Constructs a fuse_request, determining that wait() on it will be + // called by @t. + fuse_request(sched::thread* t): processed{t} {} + + // Waits for the request to be marked as completed. Should only be + // called once, from the thread specified in the constructor. + void wait() { processed.wait(); } + // Marks the request as completed. Should only be called once. + void done() { processed.wake(); } private: - mutex_t req_mutex; - waitqueue req_wait; + // Signifies whether the request has been processed by the device + waiter processed; }; struct fs_config { diff --git a/fs/virtiofs/virtiofs_vfsops.cc b/fs/virtiofs/virtiofs_vfsops.cc --- a/fs/virtiofs/virtiofs_vfsops.cc +++ b/fs/virtiofs/virtiofs_vfsops.cc @@ -15,6 +15,7 @@ #include <osv/debug.h> #include <osv/device.h> #include <osv/mutex.h> +#include <osv/sched.hh> #include "drivers/virtio-fs.hh" #include "virtiofs.hh" @@ -35,7 +36,8 @@ int fuse_req_send_and_receive_reply(virtio::fs* drv, uint32_t opcode, uint64_t nodeid, void* input_args_data, size_t input_args_size, void* output_args_data, size_t output_args_size) { - std::unique_ptr<fuse_request> req {new (std::nothrow) fuse_request()}; + std::unique_ptr<fuse_request> req { + new (std::nothrow) fuse_request(sched::thread::current())}; if (!req) { return ENOMEM; } -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/0000000000007c403605aa5a0cd6%40google.com.
