Skip to content

Commit 67fcc08

Browse files
foxengwkozaczuk
authored andcommitted
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: <VI1PR03MB4383C1D8E60219C273114E4AA6BB0@VI1PR03MB4383.eurprd03.prod.outlook.com>
1 parent 7a2eaf2 commit 67fcc08

File tree

5 files changed

+64
-85
lines changed

5 files changed

+64
-85
lines changed

drivers/virtio-fs.cc

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -28,25 +28,23 @@
2828

2929
using namespace memory;
3030

31-
void fuse_req_wait(fuse_request* req)
32-
{
33-
WITH_LOCK(req->req_mutex) {
34-
req->req_wait.wait(req->req_mutex);
35-
}
36-
}
31+
using fuse_request = virtio::fs::fuse_request;
3732

3833
namespace virtio {
3934

40-
static int fuse_make_request(void* driver, fuse_request* req)
35+
// Wait for the request to be marked as completed.
36+
void fs::fuse_request::wait()
4137
{
42-
auto fs_driver = static_cast<fs*>(driver);
43-
return fs_driver->make_request(req);
38+
WITH_LOCK(req_mutex) {
39+
req_wait.wait(req_mutex);
40+
}
4441
}
4542

46-
static void fuse_req_done(fuse_request* req)
43+
// Mark the request as completed.
44+
void fs::fuse_request::done()
4745
{
48-
WITH_LOCK(req->req_mutex) {
49-
req->req_wait.wake_one(req->req_mutex);
46+
WITH_LOCK(req_mutex) {
47+
req_wait.wake_one(req_mutex);
5048
}
5149
}
5250

@@ -87,7 +85,7 @@ static struct devops fs_devops {
8785
struct driver fs_driver = {
8886
"virtio_fs",
8987
&fs_devops,
90-
sizeof(struct fuse_strategy),
88+
sizeof(fs*),
9189
};
9290

9391
bool fs::ack_irq()
@@ -161,10 +159,7 @@ fs::fs(virtio_device& virtio_dev)
161159
dev_name += std::to_string(_disk_idx++);
162160

163161
struct device* dev = device_create(&fs_driver, dev_name.c_str(), D_BLK); // TODO Should it be really D_BLK?
164-
auto* strategy = static_cast<fuse_strategy*>(dev->private_data);
165-
strategy->drv = this;
166-
strategy->make_request = fuse_make_request;
167-
162+
dev->private_data = this;
168163
debugf("virtio-fs: Add device instance %d as [%s]\n", _id,
169164
dev_name.c_str());
170165
}
@@ -201,13 +196,12 @@ void fs::req_done()
201196
while (true) {
202197
virtio_driver::wait_for_queue(queue, &vring::used_ring_not_empty);
203198

204-
fs_req* req;
199+
fuse_request* req;
205200
u32 len;
206-
while ((req = static_cast<fs_req*>(queue->get_buf_elem(&len))) !=
201+
while ((req = static_cast<fuse_request*>(queue->get_buf_elem(&len))) !=
207202
nullptr) {
208203

209-
fuse_req_done(req->fuse_req);
210-
delete req;
204+
req->done();
211205
queue->get_buf_finalize();
212206
}
213207

@@ -231,11 +225,7 @@ int fs::make_request(fuse_request* req)
231225
fuse_req_enqueue_input(queue, req);
232226
fuse_req_enqueue_output(queue, req);
233227

234-
auto* fs_request = new (std::nothrow) fs_req(req);
235-
if (!fs_request) {
236-
return ENOMEM;
237-
}
238-
queue->add_buf_wait(fs_request);
228+
queue->add_buf_wait(req);
239229
queue->kick();
240230

241231
return 0;

drivers/virtio-fs.hh

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
#include <osv/waitqueue.hh>
1313
#include "drivers/virtio.hh"
1414
#include "drivers/virtio-device.hh"
15-
#include "fs/virtiofs/virtiofs_i.hh"
15+
#include "fs/virtiofs/fuse_kernel.h"
1616

1717
namespace virtio {
1818

@@ -23,6 +23,24 @@ enum {
2323

2424
class fs : public virtio_driver {
2525
public:
26+
struct fuse_request {
27+
struct fuse_in_header in_header;
28+
struct fuse_out_header out_header;
29+
30+
void* input_args_data;
31+
size_t input_args_size;
32+
33+
void* output_args_data;
34+
size_t output_args_size;
35+
36+
void wait();
37+
void done();
38+
39+
private:
40+
mutex_t req_mutex;
41+
waitqueue req_wait;
42+
};
43+
2644
struct fs_config {
2745
char tag[36];
2846
u32 num_queues;
@@ -59,13 +77,6 @@ public:
5977
static hw_driver* probe(hw_device* dev);
6078

6179
private:
62-
struct fs_req {
63-
fs_req(fuse_request* f) : fuse_req(f) {};
64-
~fs_req() {};
65-
66-
fuse_request* fuse_req;
67-
};
68-
6980
std::string _driver_name;
7081
fs_config _config;
7182
dax_window _dax;

fs/virtiofs/virtiofs_i.hh

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,30 +11,10 @@
1111
#include "fuse_kernel.h"
1212
#include <osv/mutex.h>
1313
#include <osv/waitqueue.hh>
14+
#include "drivers/virtio-fs.hh"
1415

15-
struct fuse_request {
16-
struct fuse_in_header in_header;
17-
struct fuse_out_header out_header;
18-
19-
void* input_args_data;
20-
size_t input_args_size;
21-
22-
void* output_args_data;
23-
size_t output_args_size;
24-
25-
mutex_t req_mutex;
26-
waitqueue req_wait;
27-
};
28-
29-
struct fuse_strategy {
30-
void* drv;
31-
int (*make_request)(void*, fuse_request*);
32-
};
33-
34-
int fuse_req_send_and_receive_reply(fuse_strategy* strategy, uint32_t opcode,
16+
int fuse_req_send_and_receive_reply(virtio::fs* drv, uint32_t opcode,
3517
uint64_t nodeid, void* input_args_data, size_t input_args_size,
3618
void* output_args_data, size_t output_args_size);
3719

38-
void fuse_req_wait(fuse_request* req);
39-
4020
#endif

fs/virtiofs/virtiofs_vfsops.cc

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
#include "virtiofs_i.hh"
1515
#include "drivers/virtio-fs.hh"
1616

17+
using fuse_request = virtio::fs::fuse_request;
18+
1719
static std::atomic<uint64_t> fuse_unique_id(1);
1820

19-
int fuse_req_send_and_receive_reply(fuse_strategy* strategy, uint32_t opcode,
21+
int fuse_req_send_and_receive_reply(virtio::fs* drv, uint32_t opcode,
2022
uint64_t nodeid, void* input_args_data, size_t input_args_size,
2123
void* output_args_data, size_t output_args_size)
2224
{
@@ -36,9 +38,9 @@ int fuse_req_send_and_receive_reply(fuse_strategy* strategy, uint32_t opcode,
3638
req->output_args_data = output_args_data;
3739
req->output_args_size = output_args_size;
3840

39-
assert(strategy->drv);
40-
strategy->make_request(strategy->drv, req.get());
41-
fuse_req_wait(req.get());
41+
assert(drv);
42+
drv->make_request(req.get());
43+
req->wait();
4244

4345
int error = -req->out_header.error;
4446

@@ -88,8 +90,8 @@ static int virtiofs_mount(struct mount* mp, const char* dev, int flags,
8890
in_args->max_readahead = PAGE_SIZE;
8991
in_args->flags |= FUSE_MAP_ALIGNMENT;
9092

91-
auto* strategy = static_cast<fuse_strategy*>(device->private_data);
92-
error = fuse_req_send_and_receive_reply(strategy, FUSE_INIT, FUSE_ROOT_ID,
93+
auto* drv = static_cast<virtio::fs*>(device->private_data);
94+
error = fuse_req_send_and_receive_reply(drv, FUSE_INIT, FUSE_ROOT_ID,
9395
in_args.get(), sizeof(*in_args), out_args.get(), sizeof(*out_args));
9496
if (error) {
9597
kprintf("[virtiofs] Failed to initialize fuse filesystem!\n");
@@ -101,7 +103,6 @@ static int virtiofs_mount(struct mount* mp, const char* dev, int flags,
101103
"minor: %d\n", out_args->major, out_args->minor);
102104

103105
if (out_args->flags & FUSE_MAP_ALIGNMENT) {
104-
auto* drv = static_cast<virtio::fs*>(strategy->drv);
105106
drv->set_map_alignment(out_args->map_alignment);
106107
}
107108

@@ -114,7 +115,7 @@ static int virtiofs_mount(struct mount* mp, const char* dev, int flags,
114115

115116
virtiofs_set_vnode(mp->m_root->d_vnode, root_node);
116117

117-
mp->m_data = strategy;
118+
mp->m_data = drv;
118119
mp->m_dev = device;
119120

120121
return 0;

fs/virtiofs/virtiofs_vnops.cc

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828

2929
#include "virtiofs.hh"
3030
#include "virtiofs_i.hh"
31-
#include "drivers/virtio-fs.hh"
3231

3332
static constexpr uint32_t OPEN_FLAGS = O_RDONLY;
3433

@@ -60,8 +59,8 @@ static int virtiofs_lookup(struct vnode* vnode, char* name, struct vnode** vpp)
6059
}
6160
strcpy(in_args.get(), name);
6261

63-
auto* strategy = static_cast<fuse_strategy*>(vnode->v_mount->m_data);
64-
auto error = fuse_req_send_and_receive_reply(strategy, FUSE_LOOKUP,
62+
auto* drv = static_cast<virtio::fs*>(vnode->v_mount->m_data);
63+
auto error = fuse_req_send_and_receive_reply(drv, FUSE_LOOKUP,
6564
inode->nodeid, in_args.get(), in_args_len, out_args.get(),
6665
sizeof(*out_args));
6766
if (error) {
@@ -111,8 +110,8 @@ static int virtiofs_open(struct file* fp)
111110
}
112111
in_args->flags = OPEN_FLAGS;
113112

114-
auto* strategy = static_cast<fuse_strategy*>(vnode->v_mount->m_data);
115-
auto error = fuse_req_send_and_receive_reply(strategy, FUSE_OPEN,
113+
auto* drv = static_cast<virtio::fs*>(vnode->v_mount->m_data);
114+
auto error = fuse_req_send_and_receive_reply(drv, FUSE_OPEN,
116115
inode->nodeid, in_args.get(), sizeof(*in_args), out_args.get(),
117116
sizeof(*out_args));
118117
if (error) {
@@ -146,8 +145,8 @@ static int virtiofs_close(struct vnode* vnode, struct file* fp)
146145
in_args->fh = f_data->file_handle;
147146
in_args->flags = OPEN_FLAGS; // need to be same as in FUSE_OPEN
148147

149-
auto* strategy = static_cast<fuse_strategy*>(vnode->v_mount->m_data);
150-
auto error = fuse_req_send_and_receive_reply(strategy, FUSE_RELEASE,
148+
auto* drv = static_cast<virtio::fs*>(vnode->v_mount->m_data);
149+
auto error = fuse_req_send_and_receive_reply(drv, FUSE_RELEASE,
151150
inode->nodeid, in_args.get(), sizeof(*in_args), nullptr, 0);
152151
if (error) {
153152
kprintf("[virtiofs] inode %lld, close failed\n", inode->nodeid);
@@ -173,8 +172,8 @@ static int virtiofs_readlink(struct vnode* vnode, struct uio* uio)
173172
return ENOMEM;
174173
}
175174

176-
auto* strategy = static_cast<fuse_strategy*>(vnode->v_mount->m_data);
177-
auto error = fuse_req_send_and_receive_reply(strategy, FUSE_READLINK,
175+
auto* drv = static_cast<virtio::fs*>(vnode->v_mount->m_data);
176+
auto error = fuse_req_send_and_receive_reply(drv, FUSE_READLINK,
178177
inode->nodeid, nullptr, 0, link_path.get(), PATH_MAX);
179178
if (error) {
180179
kprintf("[virtiofs] inode %lld, readlink failed\n", inode->nodeid);
@@ -188,10 +187,9 @@ static int virtiofs_readlink(struct vnode* vnode, struct uio* uio)
188187

189188
// Read @read_amt bytes from @inode, using the DAX window.
190189
static int virtiofs_read_direct(virtiofs_inode& inode, u64 file_handle,
191-
u64 read_amt, fuse_strategy& strategy, struct uio& uio)
190+
u64 read_amt, virtio::fs& drv, struct uio& uio)
192191
{
193-
auto* drv = static_cast<virtio::fs*>(strategy.drv);
194-
auto* dax = drv->get_dax();
192+
auto* dax = drv.get_dax();
195193
// Enter the critical path: setup mapping -> read -> remove mapping
196194
std::lock_guard<mutex> guard {dax->lock};
197195

@@ -215,7 +213,7 @@ static int virtiofs_read_direct(virtiofs_inode& inode, u64 file_handle,
215213
uint64_t moffset = 0;
216214
in_args->moffset = moffset;
217215

218-
auto map_align = drv->get_map_alignment();
216+
auto map_align = drv.get_map_alignment();
219217
if (map_align < 0) {
220218
kprintf("[virtiofs] inode %lld, map alignment not set\n", inode.nodeid);
221219
return ENOTSUP;
@@ -239,7 +237,7 @@ static int virtiofs_read_direct(virtiofs_inode& inode, u64 file_handle,
239237
virtiofs_debug("inode %lld, setting up mapping (foffset=%lld, len=%lld, "
240238
"moffset=%lld)\n", inode.nodeid, in_args->foffset,
241239
in_args->len, in_args->moffset);
242-
auto error = fuse_req_send_and_receive_reply(&strategy, FUSE_SETUPMAPPING,
240+
auto error = fuse_req_send_and_receive_reply(&drv, FUSE_SETUPMAPPING,
243241
inode.nodeid, in_args.get(), sizeof(*in_args), nullptr, 0);
244242
if (error) {
245243
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,
277275

278276
virtiofs_debug("inode %lld, removing mapping (moffset=%lld, len=%lld)\n",
279277
inode.nodeid, r_one->moffset, r_one->len);
280-
error = fuse_req_send_and_receive_reply(&strategy, FUSE_REMOVEMAPPING,
278+
error = fuse_req_send_and_receive_reply(&drv, FUSE_REMOVEMAPPING,
281279
inode.nodeid, r_in_args.get(), r_in_args_size, nullptr, 0);
282280
if (error) {
283281
kprintf("[virtiofs] inode %lld, mapping removal failed\n",
@@ -290,7 +288,7 @@ static int virtiofs_read_direct(virtiofs_inode& inode, u64 file_handle,
290288

291289
// Read @read_amt bytes from @inode, using the fallback FUSE_READ mechanism.
292290
static int virtiofs_read_fallback(virtiofs_inode& inode, u64 file_handle,
293-
u32 read_amt, u32 flags, fuse_strategy& strategy, struct uio& uio)
291+
u32 read_amt, u32 flags, virtio::fs& drv, struct uio& uio)
294292
{
295293
std::unique_ptr<fuse_read_in> in_args {new (std::nothrow) fuse_read_in()};
296294
std::unique_ptr<void, std::function<void(void*)>> buf {
@@ -306,7 +304,7 @@ static int virtiofs_read_fallback(virtiofs_inode& inode, u64 file_handle,
306304

307305
virtiofs_debug("inode %lld, reading %lld bytes at offset %lld\n",
308306
inode.nodeid, read_amt, uio.uio_offset);
309-
auto error = fuse_req_send_and_receive_reply(&strategy, FUSE_READ,
307+
auto error = fuse_req_send_and_receive_reply(&drv, FUSE_READ,
310308
inode.nodeid, in_args.get(), sizeof(*in_args), buf.get(), read_amt);
311309
if (error) {
312310
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,
345343

346344
auto* inode = static_cast<virtiofs_inode*>(vnode->v_data);
347345
auto* file_data = static_cast<virtiofs_file_data*>(fp->f_data);
348-
auto* strategy = static_cast<fuse_strategy*>(vnode->v_mount->m_data);
346+
auto* drv = static_cast<virtio::fs*>(vnode->v_mount->m_data);
349347

350348
// Total read amount is what they requested, or what is left
351349
auto read_amt = std::min<uint64_t>(uio->uio_resid,
352350
inode->attr.size - uio->uio_offset);
353351

354-
auto* drv = static_cast<virtio::fs*>(strategy->drv);
355352
if (drv->get_dax()) {
356353
// Try to read from DAX
357354
if (!virtiofs_read_direct(*inode, file_data->file_handle, read_amt,
358-
*strategy, *uio)) {
355+
*drv, *uio)) {
359356

360357
return 0;
361358
}
362359
}
363360
// DAX unavailable or failed, use fallback
364361
return virtiofs_read_fallback(*inode, file_data->file_handle, read_amt,
365-
ioflag, *strategy, *uio);
362+
ioflag, *drv, *uio);
366363
}
367364

368365
static int virtiofs_readdir(struct vnode* vnode, struct file* fp,

0 commit comments

Comments
 (0)