This mainly includes changes allowing the actual DAX window operations to be replaced with stubs. To achieve that we: 1. Introduce a new class virtiofs::dax_window_impl, encapsulating the DAX window and its operations (previously those were members of dax_manager). 2. Introduce a stub version of the DAX window, virtiofs::dax_window_stub. 3. Turn the dax_manager class into a template, with the DAX window flavour (normal or stub) as its parameter. This was deemed cleaner than going with run-time polymorphism. 4. Make all previously private members of dax_manager protected, allowing them to be accessed by the test code.
Signed-off-by: Fotis Xenakis <[email protected]> --- fs/virtiofs/virtiofs.hh | 9 +--- fs/virtiofs/virtiofs_dax.cc | 55 +++++++++++++-------- fs/virtiofs/virtiofs_dax.hh | 90 ++++++++++++++++++++++++++-------- fs/virtiofs/virtiofs_vfsops.cc | 5 +- 4 files changed, 108 insertions(+), 51 deletions(-) diff --git a/fs/virtiofs/virtiofs.hh b/fs/virtiofs/virtiofs.hh index 8a3b06f2..b474b718 100644 --- a/fs/virtiofs/virtiofs.hh +++ b/fs/virtiofs/virtiofs.hh @@ -16,6 +16,7 @@ #include "drivers/virtio-fs.hh" #include "fuse_kernel.h" +#include "virtiofs_dax.hh" #define VIRTIOFS_DEBUG_ENABLED 1 @@ -25,15 +26,9 @@ #define virtiofs_debug(...) #endif -// Necessary pre-declaration because virtiofs::dax depends on virtiofs_inode, -// declared below. -namespace virtiofs { -class dax_manager; -} - struct virtiofs_mount_data { virtio::fs* drv; - std::shared_ptr<virtiofs::dax_manager> dax_mgr; + std::shared_ptr<virtiofs::dax_manager_impl> dax_mgr; }; struct virtiofs_inode { diff --git a/fs/virtiofs/virtiofs_dax.cc b/fs/virtiofs/virtiofs_dax.cc index 38ccdc39..eceff3e0 100644 --- a/fs/virtiofs/virtiofs_dax.cc +++ b/fs/virtiofs/virtiofs_dax.cc @@ -8,6 +8,7 @@ #include <algorithm> #include <mutex> +#include <api/assert.h> #include <osv/debug.h> #include <osv/uio.h> @@ -18,8 +19,9 @@ namespace virtiofs { -int dax_manager::read(virtiofs_inode& inode, uint64_t file_handle, u64 read_amt, - struct uio& uio, bool aggressive) +template<typename W> +int dax_manager<W>::read(virtiofs_inode& inode, uint64_t file_handle, + u64 read_amt, struct uio& uio, bool aggressive) { std::lock_guard<mutex> guard {_lock}; @@ -63,7 +65,7 @@ int dax_manager::read(virtiofs_inode& inode, uint64_t file_handle, u64 read_amt, } out: - auto req_data = _window->addr + (mp.mstart * _chunk_size) + coffset; + auto req_data = _window.data() + (mp.mstart * _chunk_size) + coffset; auto read_amt_act = std::min<size_t>(read_amt, (mp.nchunks * _chunk_size) - coffset); // NOTE: It shouldn't be necessary to use the mmio* interface (i.e. volatile @@ -76,7 +78,8 @@ out: return error; } -int dax_manager::map(uint64_t nodeid, uint64_t file_handle, chunk nchunks, +template<typename W> +int dax_manager<W>::map(uint64_t nodeid, uint64_t file_handle, chunk nchunks, chunk fstart, mapping_part& mapped, bool evict) { // If necessary, unmap just enough chunks @@ -99,7 +102,8 @@ int dax_manager::map(uint64_t nodeid, uint64_t file_handle, chunk nchunks, // Map new chunks auto mstart = _window_chunks - empty; - auto error = map_ll(nodeid, file_handle, to_map, fstart, mstart); + auto error = _window.map(nodeid, file_handle, to_map * _chunk_size, + fstart * _chunk_size, mstart * _chunk_size); if (error) { return error; } @@ -119,7 +123,8 @@ int dax_manager::map(uint64_t nodeid, uint64_t file_handle, chunk nchunks, return 0; } -int dax_manager::unmap(chunk nchunks, mapping_part& unmapped, bool deep) +template<typename W> +int dax_manager<W>::unmap(chunk nchunks, mapping_part& unmapped, bool deep) { // Determine necessary changes chunk to_unmap = 0; @@ -148,7 +153,8 @@ int dax_manager::unmap(chunk nchunks, mapping_part& unmapped, bool deep) // Apply changes if (deep) { auto mstart = first_empty() - to_unmap; - auto error = unmap_ll(to_unmap, mstart); + auto error = _window.unmap(to_unmap * _chunk_size, + mstart * _chunk_size); if (error) { return error; } @@ -163,10 +169,10 @@ int dax_manager::unmap(chunk nchunks, mapping_part& unmapped, bool deep) return 0; } -int dax_manager::map_ll(uint64_t nodeid, uint64_t file_handle, chunk nchunks, - chunk fstart, chunk mstart) +int dax_window_impl::map(uint64_t nodeid, uint64_t fh, uint64_t len, + uint64_t fstart, uint64_t mstart) { - assert(mstart + nchunks <= _window_chunks); + assert(mstart + len <= _window->len); // NOTE: There are restrictions on the arguments to FUSE_SETUPMAPPING, from // the spec: "Alignment constraints for FUSE_SETUPMAPPING and @@ -177,18 +183,18 @@ int dax_manager::map_ll(uint64_t nodeid, uint64_t file_handle, chunk nchunks, // - moffset: multiple of map_alignment from FUSE_INIT // In practice, map_alignment is the host's page size, because foffset and // moffset are passed to mmap() on the host. These are satisfied by - // _chunk_size being a multiple of map_alignment. + // the caller (chunk size being a multiple of map alignment in dax_manager). std::unique_ptr<fuse_setupmapping_in> in_args { new (std::nothrow) fuse_setupmapping_in()}; if (!in_args) { return ENOMEM; } - in_args->fh = file_handle; - in_args->foffset = fstart * _chunk_size; - in_args->len = nchunks * _chunk_size; + in_args->fh = fh; + in_args->foffset = fstart; + in_args->len = len; in_args->flags = 0; // Read-only - in_args->moffset = mstart * _chunk_size; + in_args->moffset = mstart; virtiofs_debug("inode %lld, setting up mapping (foffset=%lld, len=%lld, " "moffset=%lld)\n", nodeid, in_args->foffset, in_args->len, @@ -203,9 +209,9 @@ int dax_manager::map_ll(uint64_t nodeid, uint64_t file_handle, chunk nchunks, return 0; } -int dax_manager::unmap_ll(chunk nchunks, chunk mstart) +int dax_window_impl::unmap(uint64_t len, uint64_t mstart) { - assert(mstart + nchunks <= _window_chunks); + assert(mstart + len <= _window->len); // NOTE: FUSE_REMOVEMAPPING accepts a fuse_removemapping_in followed by // fuse_removemapping_in.count fuse_removemapping_one arguments in general. @@ -219,8 +225,8 @@ int dax_manager::unmap_ll(chunk nchunks, chunk mstart) auto r_one = new (in_args.get() + sizeof(fuse_removemapping_in)) fuse_removemapping_one(); r_in->count = 1; - r_one->moffset = mstart * _chunk_size; - r_one->len = nchunks * _chunk_size; + r_one->moffset = mstart; + r_one->len = len; // The nodeid is irrelevant for the current implementation of // FUSE_REMOVEMAPPING. If it needed to be set, would we need to make a @@ -239,7 +245,9 @@ int dax_manager::unmap_ll(chunk nchunks, chunk mstart) return 0; } -bool dax_manager::find(uint64_t nodeid, chunk fstart, mapping_part& found) const +template<typename W> +bool dax_manager<W>::find(uint64_t nodeid, chunk fstart, mapping_part& found) + const { for (auto& m : _mappings) { if (m.nodeid == nodeid && @@ -256,7 +264,8 @@ bool dax_manager::find(uint64_t nodeid, chunk fstart, mapping_part& found) const return false; } -dax_manager::chunk dax_manager::first_empty() const +template<typename W> +typename dax_manager<W>::chunk dax_manager<W>::first_empty() const { if (_mappings.empty()) { return 0; @@ -265,4 +274,8 @@ dax_manager::chunk dax_manager::first_empty() const return m.mstart + m.nchunks; } +// Explicitly instantiate the only uses of dax_manager. +template class dax_manager<dax_window_impl>; +template class dax_manager<dax_window_stub>; + } diff --git a/fs/virtiofs/virtiofs_dax.hh b/fs/virtiofs/virtiofs_dax.hh index 2b9fa341..62194e24 100644 --- a/fs/virtiofs/virtiofs_dax.hh +++ b/fs/virtiofs/virtiofs_dax.hh @@ -5,6 +5,9 @@ * BSD license as described in the LICENSE file in the top-level directory. */ +#ifndef VIRTIOFS_DAX_HH +#define VIRTIOFS_DAX_HH + #include <vector> #include <api/assert.h> @@ -12,10 +15,61 @@ #include <osv/uio.h> #include "drivers/virtio-fs.hh" -#include "virtiofs.hh" + +// Necessary pre-declaration because virtiofs_inode is declared in virtiofs.hh, +// which depends on this for defining virtiofs_mount_data. +struct virtiofs_inode; namespace virtiofs { +// A thin abstraction layer over the actual DAX window, taking care of all the +// low-level operations, interfacing with the driver. +class dax_window_impl { +public: + // Construct a new window for the DAX window associated with @drv (as + // returned by drv.get_dax()). + dax_window_impl(virtio::fs& drv) + : _drv {drv}, + _window {drv.get_dax()} {} + + // Returns the size of the window in bytes. + u64 size() const { return _window->len; } + // Returns a pointer to the underlying memory region. + mmioaddr_t data() const { return _window->addr; } + // Returns the map alignment for the window. + int map_alignment() const { return _drv.get_map_alignment(); } + // Map @len bytes of the file with @nodeid (opened as @fh), starting at + // byte @fstart of the file and byte @mstart of the window. Returns + // non-zero on failure. + int map(uint64_t nodeid, uint64_t fh, uint64_t len, uint64_t fstart, + uint64_t mstart); + // Unmap @len bytes, starting at byte @mstart of the window. Returns + // non-zero on failure. + int unmap(uint64_t len, uint64_t mstart); + +private: + virtio::fs& _drv; + const virtio::fs::dax_window* const _window; +}; + +// A stub DAX window, used for testing. Defined here to facilitate instantiation +// of dax_manager<dax_window_stub> (see virtiofs_dax.cc). +class dax_window_stub { +public: + dax_window_stub(u64 len) + : _len {len} {} + + u64 size() const { return _len; } + mmioaddr_t data() const { return nullptr; } + int map_alignment() const { return 12; /* 4KiB */ } + int map(uint64_t nodeid, uint64_t fh, uint64_t len, uint64_t fstart, + uint64_t mstart) { return 0; }; + int unmap(uint64_t len, uint64_t mstart) { return 0; }; + +private: + u64 _len; +}; + // A manager for the DAX window of a virtio-fs device. This implements a // straight-forward scheme for file mappings: // - The window is split into equally-sized chunks. Each mapping occupies an @@ -24,21 +78,19 @@ namespace virtiofs { // - When there are not enough chunks available for a new mapping, the highest // (i.e. most recently mapped) chunks occupied are evicted. Thus, chunks are // mapped in a LIFO manner (the window resembles a stack). +template<typename W> class dax_manager { public: static constexpr size_t DEFAULT_CHUNK_SIZE = 1 << 21; // 2MiB - // Construct a new manager for the DAX window associated with @drv (as - // returned by drv.get_dax()). The alignment constraint of the device (as - // reported by drv.get_map_alignment()) should be compatible with - // @chunk_size. - dax_manager(virtio::fs& drv, size_t chunk_size = DEFAULT_CHUNK_SIZE) - : _drv {drv}, - _window {drv.get_dax()}, + // Construct a new manager for @window. The @chunk_size should be compatible + // with the alignment constraint of @window. + dax_manager(W window, size_t chunk_size = DEFAULT_CHUNK_SIZE) + : _window {window}, _chunk_size {chunk_size}, - _window_chunks {_window->len / _chunk_size} { + _window_chunks {_window.size() / _chunk_size} { - assert(_chunk_size % (1ull << _drv.get_map_alignment()) == 0); + assert(_chunk_size % (1ull << _window.map_alignment()) == 0); // NOTE: If _window->len % CHUNK_SIZE > 0, that remainder (< CHUNK_SIZE) // is effectively ignored. @@ -49,7 +101,7 @@ public: int read(virtiofs_inode& inode, uint64_t file_handle, u64 read_amt, struct uio& uio, bool aggressive = false); -private: +protected: // Helper type to better distinguish referring to chunks vs bytes using chunk = size_t; @@ -80,14 +132,6 @@ private: // if @deep. Returns in @unmapped what was unmapped and non-zero on failure. // Called with _lock held (for writing). int unmap(chunk nchunks, mapping_part& unmapped, bool deep = false); - // Map @nchunks chunks of the file with @nodeid (opened as @fh), starting at - // chunk @fstart of the file and chunk @mstart of the window. Returns - // non-zero on failure. Called with _lock held (for writing). - int map_ll(uint64_t nodeid, uint64_t fh, chunk nchunks, chunk fstart, - chunk mstart); - // Unmap @nchunks chunks, starting at chunk @mstart of the window. Returns - // non-zero on failure. Called with _lock held (for writing). - int unmap_ll(chunk nchunks, chunk mstart); // Return in @found the largest contiguous existing mapping for @nodeid // starting at @fstart. If none found, returns false. Called with _lock held @@ -97,8 +141,7 @@ private: // window is full. Called with _lock held (for reading). chunk first_empty() const; - virtio::fs& _drv; - const virtio::fs::dax_window* const _window; + W _window; const size_t _chunk_size; const chunk _window_chunks; // TODO OPT: Switch to rwlock @@ -106,4 +149,9 @@ private: std::vector<mapping> _mappings; }; +using dax_manager_impl = dax_manager<dax_window_impl>; +using dax_manager_stub = dax_manager<dax_window_stub>; + } + +#endif diff --git a/fs/virtiofs/virtiofs_vfsops.cc b/fs/virtiofs/virtiofs_vfsops.cc index d44e160d..32572d7f 100644 --- a/fs/virtiofs/virtiofs_vfsops.cc +++ b/fs/virtiofs/virtiofs_vfsops.cc @@ -28,7 +28,7 @@ using fuse_request = virtio::fs::fuse_request; static std::atomic<uint64_t> fuse_unique_id(1); static struct { - std::unordered_map<virtio::fs*, std::shared_ptr<virtiofs::dax_manager>, + std::unordered_map<virtio::fs*, std::shared_ptr<virtiofs::dax_manager_impl>, virtio::fs::hasher> mgrs; mutex lock; } dax_managers; @@ -151,7 +151,8 @@ static int virtiofs_mount(struct mount* mp, const char* dev, int flags, // device is already mounted) m_data->dax_mgr = found->second; } else { - m_data->dax_mgr = std::make_shared<virtiofs::dax_manager>(*drv); + auto w {virtiofs::dax_window_impl(*drv)}; + m_data->dax_mgr = std::make_shared<virtiofs::dax_manager_impl>(w); if (!m_data->dax_mgr) { return ENOMEM; } -- 2.30.1 -- 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/VI1PR03MB3773E191D8AA7D58FAEE8FBEA6959%40VI1PR03MB3773.eurprd03.prod.outlook.com.
