From: Fotis Xenakis <[email protected]> Committer: Waldemar Kozaczuk <[email protected]> Branch: master
virtio-fs: refactor driver / fs Since in virtio-fs the filesystem is very tightly coupled with the driver, this tries to make clear the dependence of the first on the second, as well as simplify. This includes: - The definition of fuse_request is moved from the fs to the driver, since it is part of the interface it provides. Also, it is enhanced with methods, somewhat promoting it to a "proper" class. - fuse_strategy, as a redirection to the driver is removed and instead the dependence on the driver is made explicit. - Last, virtio::fs::fs_req is removed and fuse_request is used in its place, since it offered no value with fuse_request now defined in the driver. Signed-off-by: Fotis Xenakis <[email protected]> Message-Id: <vi1pr03mb4383c1d8e60219c273114e4aa6...@vi1pr03mb4383.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 @@ -28,25 +28,23 @@ using namespace memory; -void fuse_req_wait(fuse_request* req) -{ - WITH_LOCK(req->req_mutex) { - req->req_wait.wait(req->req_mutex); - } -} +using fuse_request = virtio::fs::fuse_request; namespace virtio { -static int fuse_make_request(void* driver, fuse_request* req) +// Wait for the request to be marked as completed. +void fs::fuse_request::wait() { - auto fs_driver = static_cast<fs*>(driver); - return fs_driver->make_request(req); + WITH_LOCK(req_mutex) { + req_wait.wait(req_mutex); + } } -static void fuse_req_done(fuse_request* req) +// Mark the request as completed. +void fs::fuse_request::done() { - WITH_LOCK(req->req_mutex) { - req->req_wait.wake_one(req->req_mutex); + WITH_LOCK(req_mutex) { + req_wait.wake_one(req_mutex); } } @@ -87,7 +85,7 @@ static struct devops fs_devops { struct driver fs_driver = { "virtio_fs", &fs_devops, - sizeof(struct fuse_strategy), + sizeof(fs*), }; bool fs::ack_irq() @@ -161,10 +159,7 @@ fs::fs(virtio_device& virtio_dev) dev_name += std::to_string(_disk_idx++); struct device* dev = device_create(&fs_driver, dev_name.c_str(), D_BLK); // TODO Should it be really D_BLK? - auto* strategy = static_cast<fuse_strategy*>(dev->private_data); - strategy->drv = this; - strategy->make_request = fuse_make_request; - + dev->private_data = this; debugf("virtio-fs: Add device instance %d as [%s]\n", _id, dev_name.c_str()); } @@ -201,13 +196,12 @@ void fs::req_done() while (true) { virtio_driver::wait_for_queue(queue, &vring::used_ring_not_empty); - fs_req* req; + fuse_request* req; u32 len; - while ((req = static_cast<fs_req*>(queue->get_buf_elem(&len))) != + while ((req = static_cast<fuse_request*>(queue->get_buf_elem(&len))) != nullptr) { - fuse_req_done(req->fuse_req); - delete req; + req->done(); queue->get_buf_finalize(); } @@ -231,11 +225,7 @@ int fs::make_request(fuse_request* req) fuse_req_enqueue_input(queue, req); fuse_req_enqueue_output(queue, req); - auto* fs_request = new (std::nothrow) fs_req(req); - if (!fs_request) { - return ENOMEM; - } - queue->add_buf_wait(fs_request); + queue->add_buf_wait(req); queue->kick(); return 0; 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,7 @@ #include <osv/waitqueue.hh> #include "drivers/virtio.hh" #include "drivers/virtio-device.hh" -#include "fs/virtiofs/virtiofs_i.hh" +#include "fs/virtiofs/fuse_kernel.h" namespace virtio { @@ -23,6 +23,24 @@ enum { class fs : public virtio_driver { public: + struct fuse_request { + struct fuse_in_header in_header; + struct fuse_out_header out_header; + + void* input_args_data; + size_t input_args_size; + + void* output_args_data; + size_t output_args_size; + + void wait(); + void done(); + + private: + mutex_t req_mutex; + waitqueue req_wait; + }; + struct fs_config { char tag[36]; u32 num_queues; @@ -59,13 +77,6 @@ public: static hw_driver* probe(hw_device* dev); private: - struct fs_req { - fs_req(fuse_request* f) : fuse_req(f) {}; - ~fs_req() {}; - - fuse_request* fuse_req; - }; - std::string _driver_name; fs_config _config; dax_window _dax; diff --git a/fs/virtiofs/virtiofs_i.hh b/fs/virtiofs/virtiofs_i.hh --- a/fs/virtiofs/virtiofs_i.hh +++ b/fs/virtiofs/virtiofs_i.hh @@ -11,30 +11,10 @@ #include "fuse_kernel.h" #include <osv/mutex.h> #include <osv/waitqueue.hh> +#include "drivers/virtio-fs.hh" -struct fuse_request { - struct fuse_in_header in_header; - struct fuse_out_header out_header; - - void* input_args_data; - size_t input_args_size; - - void* output_args_data; - size_t output_args_size; - - mutex_t req_mutex; - waitqueue req_wait; -}; - -struct fuse_strategy { - void* drv; - int (*make_request)(void*, fuse_request*); -}; - -int fuse_req_send_and_receive_reply(fuse_strategy* strategy, uint32_t opcode, +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); -void fuse_req_wait(fuse_request* req); - #endif 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 @@ -14,9 +14,11 @@ #include "virtiofs_i.hh" #include "drivers/virtio-fs.hh" +using fuse_request = virtio::fs::fuse_request; + static std::atomic<uint64_t> fuse_unique_id(1); -int fuse_req_send_and_receive_reply(fuse_strategy* strategy, uint32_t opcode, +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) { @@ -36,9 +38,9 @@ int fuse_req_send_and_receive_reply(fuse_strategy* strategy, uint32_t opcode, req->output_args_data = output_args_data; req->output_args_size = output_args_size; - assert(strategy->drv); - strategy->make_request(strategy->drv, req.get()); - fuse_req_wait(req.get()); + assert(drv); + drv->make_request(req.get()); + req->wait(); int error = -req->out_header.error; @@ -88,8 +90,8 @@ static int virtiofs_mount(struct mount* mp, const char* dev, int flags, in_args->max_readahead = PAGE_SIZE; in_args->flags |= FUSE_MAP_ALIGNMENT; - auto* strategy = static_cast<fuse_strategy*>(device->private_data); - error = fuse_req_send_and_receive_reply(strategy, FUSE_INIT, FUSE_ROOT_ID, + auto* drv = static_cast<virtio::fs*>(device->private_data); + error = fuse_req_send_and_receive_reply(drv, FUSE_INIT, FUSE_ROOT_ID, in_args.get(), sizeof(*in_args), out_args.get(), sizeof(*out_args)); if (error) { kprintf("[virtiofs] Failed to initialize fuse filesystem!\n"); @@ -101,7 +103,6 @@ static int virtiofs_mount(struct mount* mp, const char* dev, int flags, "minor: %d\n", out_args->major, out_args->minor); if (out_args->flags & FUSE_MAP_ALIGNMENT) { - auto* drv = static_cast<virtio::fs*>(strategy->drv); drv->set_map_alignment(out_args->map_alignment); } @@ -114,7 +115,7 @@ static int virtiofs_mount(struct mount* mp, const char* dev, int flags, virtiofs_set_vnode(mp->m_root->d_vnode, root_node); - mp->m_data = strategy; + mp->m_data = drv; mp->m_dev = device; return 0; diff --git a/fs/virtiofs/virtiofs_vnops.cc b/fs/virtiofs/virtiofs_vnops.cc --- a/fs/virtiofs/virtiofs_vnops.cc +++ b/fs/virtiofs/virtiofs_vnops.cc @@ -28,7 +28,6 @@ #include "virtiofs.hh" #include "virtiofs_i.hh" -#include "drivers/virtio-fs.hh" static constexpr uint32_t OPEN_FLAGS = O_RDONLY; @@ -60,8 +59,8 @@ static int virtiofs_lookup(struct vnode* vnode, char* name, struct vnode** vpp) } strcpy(in_args.get(), name); - auto* strategy = static_cast<fuse_strategy*>(vnode->v_mount->m_data); - auto error = fuse_req_send_and_receive_reply(strategy, FUSE_LOOKUP, + auto* drv = static_cast<virtio::fs*>(vnode->v_mount->m_data); + auto error = fuse_req_send_and_receive_reply(drv, FUSE_LOOKUP, inode->nodeid, in_args.get(), in_args_len, out_args.get(), sizeof(*out_args)); if (error) { @@ -111,8 +110,8 @@ static int virtiofs_open(struct file* fp) } in_args->flags = OPEN_FLAGS; - auto* strategy = static_cast<fuse_strategy*>(vnode->v_mount->m_data); - auto error = fuse_req_send_and_receive_reply(strategy, FUSE_OPEN, + auto* drv = static_cast<virtio::fs*>(vnode->v_mount->m_data); + auto error = fuse_req_send_and_receive_reply(drv, FUSE_OPEN, inode->nodeid, in_args.get(), sizeof(*in_args), out_args.get(), sizeof(*out_args)); if (error) { @@ -146,8 +145,8 @@ static int virtiofs_close(struct vnode* vnode, struct file* fp) in_args->fh = f_data->file_handle; in_args->flags = OPEN_FLAGS; // need to be same as in FUSE_OPEN - auto* strategy = static_cast<fuse_strategy*>(vnode->v_mount->m_data); - auto error = fuse_req_send_and_receive_reply(strategy, FUSE_RELEASE, + auto* drv = static_cast<virtio::fs*>(vnode->v_mount->m_data); + auto error = fuse_req_send_and_receive_reply(drv, FUSE_RELEASE, inode->nodeid, in_args.get(), sizeof(*in_args), nullptr, 0); if (error) { kprintf("[virtiofs] inode %lld, close failed\n", inode->nodeid); @@ -173,8 +172,8 @@ static int virtiofs_readlink(struct vnode* vnode, struct uio* uio) return ENOMEM; } - auto* strategy = static_cast<fuse_strategy*>(vnode->v_mount->m_data); - auto error = fuse_req_send_and_receive_reply(strategy, FUSE_READLINK, + auto* drv = static_cast<virtio::fs*>(vnode->v_mount->m_data); + auto error = fuse_req_send_and_receive_reply(drv, FUSE_READLINK, inode->nodeid, nullptr, 0, link_path.get(), PATH_MAX); if (error) { kprintf("[virtiofs] inode %lld, readlink failed\n", inode->nodeid); @@ -188,10 +187,9 @@ static int virtiofs_readlink(struct vnode* vnode, struct uio* uio) // Read @read_amt bytes from @inode, using the DAX window. static int virtiofs_read_direct(virtiofs_inode& inode, u64 file_handle, - u64 read_amt, fuse_strategy& strategy, struct uio& uio) + u64 read_amt, virtio::fs& drv, struct uio& uio) { - auto* drv = static_cast<virtio::fs*>(strategy.drv); - auto* dax = drv->get_dax(); + auto* dax = drv.get_dax(); // Enter the critical path: setup mapping -> read -> remove mapping std::lock_guard<mutex> guard {dax->lock}; @@ -215,7 +213,7 @@ static int virtiofs_read_direct(virtiofs_inode& inode, u64 file_handle, uint64_t moffset = 0; in_args->moffset = moffset; - auto map_align = drv->get_map_alignment(); + auto map_align = drv.get_map_alignment(); if (map_align < 0) { kprintf("[virtiofs] inode %lld, map alignment not set\n", inode.nodeid); return ENOTSUP; @@ -239,7 +237,7 @@ static int virtiofs_read_direct(virtiofs_inode& inode, u64 file_handle, virtiofs_debug("inode %lld, setting up mapping (foffset=%lld, len=%lld, " "moffset=%lld)\n", inode.nodeid, in_args->foffset, in_args->len, in_args->moffset); - auto error = fuse_req_send_and_receive_reply(&strategy, FUSE_SETUPMAPPING, + auto error = fuse_req_send_and_receive_reply(&drv, FUSE_SETUPMAPPING, inode.nodeid, in_args.get(), sizeof(*in_args), nullptr, 0); if (error) { kprintf("[virtiofs] inode %lld, mapping setup failed\n", inode.nodeid); @@ -277,7 +275,7 @@ static int virtiofs_read_direct(virtiofs_inode& inode, u64 file_handle, virtiofs_debug("inode %lld, removing mapping (moffset=%lld, len=%lld)\n", inode.nodeid, r_one->moffset, r_one->len); - error = fuse_req_send_and_receive_reply(&strategy, FUSE_REMOVEMAPPING, + error = fuse_req_send_and_receive_reply(&drv, FUSE_REMOVEMAPPING, inode.nodeid, r_in_args.get(), r_in_args_size, nullptr, 0); if (error) { kprintf("[virtiofs] inode %lld, mapping removal failed\n", @@ -290,7 +288,7 @@ static int virtiofs_read_direct(virtiofs_inode& inode, u64 file_handle, // Read @read_amt bytes from @inode, using the fallback FUSE_READ mechanism. static int virtiofs_read_fallback(virtiofs_inode& inode, u64 file_handle, - u32 read_amt, u32 flags, fuse_strategy& strategy, struct uio& uio) + u32 read_amt, u32 flags, virtio::fs& drv, struct uio& uio) { std::unique_ptr<fuse_read_in> in_args {new (std::nothrow) fuse_read_in()}; std::unique_ptr<void, std::function<void(void*)>> buf { @@ -306,7 +304,7 @@ static int virtiofs_read_fallback(virtiofs_inode& inode, u64 file_handle, virtiofs_debug("inode %lld, reading %lld bytes at offset %lld\n", inode.nodeid, read_amt, uio.uio_offset); - auto error = fuse_req_send_and_receive_reply(&strategy, FUSE_READ, + auto error = fuse_req_send_and_receive_reply(&drv, FUSE_READ, inode.nodeid, in_args.get(), sizeof(*in_args), buf.get(), read_amt); if (error) { kprintf("[virtiofs] inode %lld, read failed\n", inode.nodeid); @@ -345,24 +343,23 @@ static int virtiofs_read(struct vnode* vnode, struct file* fp, struct uio* uio, auto* inode = static_cast<virtiofs_inode*>(vnode->v_data); auto* file_data = static_cast<virtiofs_file_data*>(fp->f_data); - auto* strategy = static_cast<fuse_strategy*>(vnode->v_mount->m_data); + auto* drv = static_cast<virtio::fs*>(vnode->v_mount->m_data); // Total read amount is what they requested, or what is left auto read_amt = std::min<uint64_t>(uio->uio_resid, inode->attr.size - uio->uio_offset); - auto* drv = static_cast<virtio::fs*>(strategy->drv); if (drv->get_dax()) { // Try to read from DAX if (!virtiofs_read_direct(*inode, file_data->file_handle, read_amt, - *strategy, *uio)) { + *drv, *uio)) { return 0; } } // DAX unavailable or failed, use fallback return virtiofs_read_fallback(*inode, file_data->file_handle, read_amt, - ioflag, *strategy, *uio); + ioflag, *drv, *uio); } static int virtiofs_readdir(struct vnode* vnode, struct file* fp, -- 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/000000000000cd7aea05a6d643c6%40google.com.
