| Message ID | 20210716104753.74377-3-jefflexu@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtiofs,fuse: support per-file DAX | expand |
On Fri, Jul 16, 2021 at 06:47:51PM +0800, Jeffle Xu wrote: > We add 'always', 'never', and 'inode' (default). '-o dax' continues to > operate the same which is equivalent to 'always'. > > By the time this patch is applied, 'inode' mode is actually equal to > 'always' mode, before the per-file DAX flag is introduced in the > following patch. > > Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com> > --- > fs/fuse/dax.c | 13 ++++++++++++- > fs/fuse/fuse_i.h | 11 +++++++++-- > fs/fuse/inode.c | 2 +- > fs/fuse/virtio_fs.c | 16 ++++++++++++++-- > 4 files changed, 36 insertions(+), 6 deletions(-) > > diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c > index c6f4e82e65f3..a478e824c2d0 100644 > --- a/fs/fuse/dax.c > +++ b/fs/fuse/dax.c > @@ -70,6 +70,9 @@ struct fuse_inode_dax { > }; > > struct fuse_conn_dax { > + /** dax mode: FUSE_DAX_MOUNT_* (always, never or per-file) **/ > + unsigned int mode; Why "/**" ? How about make it something like "enum fuse_dax_mode mode" instead? enum fuse_dax_mode dax_mode; > + > /* DAX device */ > struct dax_device *dev; > > @@ -1288,7 +1291,8 @@ static int fuse_dax_mem_range_init(struct fuse_conn_dax *fcd) > return ret; > } > > -int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev) > +int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode, > + struct dax_device *dax_dev) > { > struct fuse_conn_dax *fcd; > int err; > @@ -1301,6 +1305,7 @@ int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev) > return -ENOMEM; > > spin_lock_init(&fcd->lock); > + fcd->mode = mode; > fcd->dev = dax_dev; > err = fuse_dax_mem_range_init(fcd); > if (err) { > @@ -1339,10 +1344,16 @@ static const struct address_space_operations fuse_dax_file_aops = { > static bool fuse_should_enable_dax(struct inode *inode) > { > struct fuse_conn *fc = get_fuse_conn(inode); > + unsigned int mode; > > if (!fc->dax) > return false; > > + mode = fc->dax->mode; > + > + if (mode == FUSE_DAX_MOUNT_NEVER) > + return false; > + > return true; > } > > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h > index 07829ce78695..f29018323845 100644 > --- a/fs/fuse/fuse_i.h > +++ b/fs/fuse/fuse_i.h > @@ -487,6 +487,12 @@ struct fuse_dev { > struct list_head entry; > }; > > +enum { And this becomes. enum fuse_dax_mode { }; > + FUSE_DAX_MOUNT_INODE, > + FUSE_DAX_MOUNT_ALWAYS, > + FUSE_DAX_MOUNT_NEVER, > +}; How about getting rid of "MOUNT" and just do. FUSE_DAX_INODE, FUSE_DAX_ALWAYS, FUSE_DAX_NEVER, > + > struct fuse_fs_context { > int fd; > unsigned int rootmode; > @@ -503,7 +509,7 @@ struct fuse_fs_context { > bool no_control:1; > bool no_force_umount:1; > bool legacy_opts_show:1; > - bool dax:1; > + unsigned int dax; enum fuse_dax_mode dax_mode; > unsigned int max_read; > unsigned int blksize; > const char *subtype; > @@ -1242,7 +1248,8 @@ ssize_t fuse_dax_read_iter(struct kiocb *iocb, struct iov_iter *to); > ssize_t fuse_dax_write_iter(struct kiocb *iocb, struct iov_iter *from); > int fuse_dax_mmap(struct file *file, struct vm_area_struct *vma); > int fuse_dax_break_layouts(struct inode *inode, u64 dmap_start, u64 dmap_end); > -int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev); > +int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode, ^^ enum fuse_dax_mode dax_mode > + struct dax_device *dax_dev); > void fuse_dax_conn_free(struct fuse_conn *fc); > bool fuse_dax_inode_alloc(struct super_block *sb, struct fuse_inode *fi); > void fuse_dax_inode_init(struct inode *inode); > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c > index b9beb39a4a18..f6b46395edb2 100644 > --- a/fs/fuse/inode.c > +++ b/fs/fuse/inode.c > @@ -1434,7 +1434,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx) > sb->s_subtype = ctx->subtype; > ctx->subtype = NULL; > if (IS_ENABLED(CONFIG_FUSE_DAX)) { > - err = fuse_dax_conn_alloc(fc, ctx->dax_dev); > + err = fuse_dax_conn_alloc(fc, ctx->dax, ctx->dax_dev); > if (err) > goto err; > } > diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c > index 8f52cdaa8445..561f711d1945 100644 > --- a/fs/fuse/virtio_fs.c > +++ b/fs/fuse/virtio_fs.c > @@ -88,12 +88,21 @@ struct virtio_fs_req_work { > static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq, > struct fuse_req *req, bool in_flight); > > +static const struct constant_table dax_param_enums[] = { > + {"inode", FUSE_DAX_MOUNT_INODE }, > + {"always", FUSE_DAX_MOUNT_ALWAYS }, > + {"never", FUSE_DAX_MOUNT_NEVER }, > + {} > +}; > + > enum { > OPT_DAX, > + OPT_DAX_ENUM, > }; > > static const struct fs_parameter_spec virtio_fs_parameters[] = { > fsparam_flag("dax", OPT_DAX), > + fsparam_enum("dax", OPT_DAX_ENUM, dax_param_enums), > {} > }; > > @@ -110,7 +119,10 @@ static int virtio_fs_parse_param(struct fs_context *fc, > > switch (opt) { > case OPT_DAX: > - ctx->dax = 1; > + ctx->dax = FUSE_DAX_MOUNT_ALWAYS; > + break; > + case OPT_DAX_ENUM: > + ctx->dax = result.uint_32; Do we want to check here if result.uint_32 has one of the allowed values. FUSE_DAX_MOUNT_INODE, FUSE_DAX_MOUNT_ALWAYS or FUSE_DAX_MOUNT_NEVER. Or VFS has already taken care of that? Vivek > break; > default: > return -EINVAL; > @@ -1326,7 +1338,7 @@ static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc) > > /* virtiofs allocates and installs its own fuse devices */ > ctx->fudptr = NULL; > - if (ctx->dax) { > + if (ctx->dax != FUSE_DAX_MOUNT_NEVER) { > if (!fs->dax_dev) { > err = -EINVAL; > pr_err("virtio-fs: dax can't be enabled as filesystem" > -- > 2.27.0 >
On 7/20/21 2:02 AM, Vivek Goyal wrote: > On Fri, Jul 16, 2021 at 06:47:51PM +0800, Jeffle Xu wrote: >> We add 'always', 'never', and 'inode' (default). '-o dax' continues to >> operate the same which is equivalent to 'always'. >> >> By the time this patch is applied, 'inode' mode is actually equal to >> 'always' mode, before the per-file DAX flag is introduced in the >> following patch. >> >> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com> >> --- >> fs/fuse/dax.c | 13 ++++++++++++- >> fs/fuse/fuse_i.h | 11 +++++++++-- >> fs/fuse/inode.c | 2 +- >> fs/fuse/virtio_fs.c | 16 ++++++++++++++-- >> 4 files changed, 36 insertions(+), 6 deletions(-) >> >> diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c >> index c6f4e82e65f3..a478e824c2d0 100644 >> --- a/fs/fuse/dax.c >> +++ b/fs/fuse/dax.c >> @@ -70,6 +70,9 @@ struct fuse_inode_dax { >> }; >> >> struct fuse_conn_dax { >> + /** dax mode: FUSE_DAX_MOUNT_* (always, never or per-file) **/ >> + unsigned int mode; > > Why "/**" ? I copied this comment style from fuse in v4.19... Anyway, I will fix this. > > How about make it something like "enum fuse_dax_mode mode" instead? > > enum fuse_dax_mode dax_mode; OK. > >> + >> /* DAX device */ >> struct dax_device *dev; >> >> @@ -1288,7 +1291,8 @@ static int fuse_dax_mem_range_init(struct fuse_conn_dax *fcd) >> return ret; >> } >> >> -int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev) >> +int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode, >> + struct dax_device *dax_dev) >> { >> struct fuse_conn_dax *fcd; >> int err; >> @@ -1301,6 +1305,7 @@ int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev) >> return -ENOMEM; >> >> spin_lock_init(&fcd->lock); >> + fcd->mode = mode; >> fcd->dev = dax_dev; >> err = fuse_dax_mem_range_init(fcd); >> if (err) { >> @@ -1339,10 +1344,16 @@ static const struct address_space_operations fuse_dax_file_aops = { >> static bool fuse_should_enable_dax(struct inode *inode) >> { >> struct fuse_conn *fc = get_fuse_conn(inode); >> + unsigned int mode; >> >> if (!fc->dax) >> return false; >> >> + mode = fc->dax->mode; >> + >> + if (mode == FUSE_DAX_MOUNT_NEVER) >> + return false; >> + >> return true; >> } >> >> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h >> index 07829ce78695..f29018323845 100644 >> --- a/fs/fuse/fuse_i.h >> +++ b/fs/fuse/fuse_i.h >> @@ -487,6 +487,12 @@ struct fuse_dev { >> struct list_head entry; >> }; >> >> +enum { > And this becomes. > > enum fuse_dax_mode { > }; OK. > >> + FUSE_DAX_MOUNT_INODE, >> + FUSE_DAX_MOUNT_ALWAYS, >> + FUSE_DAX_MOUNT_NEVER, >> +}; > > How about getting rid of "MOUNT" and just do. > > FUSE_DAX_INODE, > FUSE_DAX_ALWAYS, > FUSE_DAX_NEVER, OK. > >> + >> struct fuse_fs_context { >> int fd; >> unsigned int rootmode; >> @@ -503,7 +509,7 @@ struct fuse_fs_context { >> bool no_control:1; >> bool no_force_umount:1; >> bool legacy_opts_show:1; >> - bool dax:1; >> + unsigned int dax; > > enum fuse_dax_mode dax_mode; OK. > >> unsigned int max_read; >> unsigned int blksize; >> const char *subtype; >> @@ -1242,7 +1248,8 @@ ssize_t fuse_dax_read_iter(struct kiocb *iocb, struct iov_iter *to); >> ssize_t fuse_dax_write_iter(struct kiocb *iocb, struct iov_iter *from); >> int fuse_dax_mmap(struct file *file, struct vm_area_struct *vma); >> int fuse_dax_break_layouts(struct inode *inode, u64 dmap_start, u64 dmap_end); >> -int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev); >> +int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode, > ^^ > enum fuse_dax_mode dax_mode OK. >> + struct dax_device *dax_dev); >> void fuse_dax_conn_free(struct fuse_conn *fc); >> bool fuse_dax_inode_alloc(struct super_block *sb, struct fuse_inode *fi); >> void fuse_dax_inode_init(struct inode *inode); >> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c >> index b9beb39a4a18..f6b46395edb2 100644 >> --- a/fs/fuse/inode.c >> +++ b/fs/fuse/inode.c >> @@ -1434,7 +1434,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx) >> sb->s_subtype = ctx->subtype; >> ctx->subtype = NULL; >> if (IS_ENABLED(CONFIG_FUSE_DAX)) { >> - err = fuse_dax_conn_alloc(fc, ctx->dax_dev); >> + err = fuse_dax_conn_alloc(fc, ctx->dax, ctx->dax_dev); >> if (err) >> goto err; >> } >> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c >> index 8f52cdaa8445..561f711d1945 100644 >> --- a/fs/fuse/virtio_fs.c >> +++ b/fs/fuse/virtio_fs.c >> @@ -88,12 +88,21 @@ struct virtio_fs_req_work { >> static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq, >> struct fuse_req *req, bool in_flight); >> >> +static const struct constant_table dax_param_enums[] = { >> + {"inode", FUSE_DAX_MOUNT_INODE }, >> + {"always", FUSE_DAX_MOUNT_ALWAYS }, >> + {"never", FUSE_DAX_MOUNT_NEVER }, >> + {} >> +}; >> + >> enum { >> OPT_DAX, >> + OPT_DAX_ENUM, >> }; >> >> static const struct fs_parameter_spec virtio_fs_parameters[] = { >> fsparam_flag("dax", OPT_DAX), >> + fsparam_enum("dax", OPT_DAX_ENUM, dax_param_enums), >> {} >> }; >> >> @@ -110,7 +119,10 @@ static int virtio_fs_parse_param(struct fs_context *fc, >> >> switch (opt) { >> case OPT_DAX: >> - ctx->dax = 1; >> + ctx->dax = FUSE_DAX_MOUNT_ALWAYS; >> + break; >> + case OPT_DAX_ENUM: >> + ctx->dax = result.uint_32; > > Do we want to check here if result.uint_32 has one of the allowed values. > FUSE_DAX_MOUNT_INODE, FUSE_DAX_MOUNT_ALWAYS or FUSE_DAX_MOUNT_NEVER. Or > VFS has already taken care of that? VFS will ensure that the returned result.uint_32 can only be among the values defined in 'struct constant_table', or fs_parse() will return -EINVAL.
diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c index c6f4e82e65f3..a478e824c2d0 100644 --- a/fs/fuse/dax.c +++ b/fs/fuse/dax.c @@ -70,6 +70,9 @@ struct fuse_inode_dax { }; struct fuse_conn_dax { + /** dax mode: FUSE_DAX_MOUNT_* (always, never or per-file) **/ + unsigned int mode; + /* DAX device */ struct dax_device *dev; @@ -1288,7 +1291,8 @@ static int fuse_dax_mem_range_init(struct fuse_conn_dax *fcd) return ret; } -int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev) +int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode, + struct dax_device *dax_dev) { struct fuse_conn_dax *fcd; int err; @@ -1301,6 +1305,7 @@ int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev) return -ENOMEM; spin_lock_init(&fcd->lock); + fcd->mode = mode; fcd->dev = dax_dev; err = fuse_dax_mem_range_init(fcd); if (err) { @@ -1339,10 +1344,16 @@ static const struct address_space_operations fuse_dax_file_aops = { static bool fuse_should_enable_dax(struct inode *inode) { struct fuse_conn *fc = get_fuse_conn(inode); + unsigned int mode; if (!fc->dax) return false; + mode = fc->dax->mode; + + if (mode == FUSE_DAX_MOUNT_NEVER) + return false; + return true; } diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 07829ce78695..f29018323845 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -487,6 +487,12 @@ struct fuse_dev { struct list_head entry; }; +enum { + FUSE_DAX_MOUNT_INODE, + FUSE_DAX_MOUNT_ALWAYS, + FUSE_DAX_MOUNT_NEVER, +}; + struct fuse_fs_context { int fd; unsigned int rootmode; @@ -503,7 +509,7 @@ struct fuse_fs_context { bool no_control:1; bool no_force_umount:1; bool legacy_opts_show:1; - bool dax:1; + unsigned int dax; unsigned int max_read; unsigned int blksize; const char *subtype; @@ -1242,7 +1248,8 @@ ssize_t fuse_dax_read_iter(struct kiocb *iocb, struct iov_iter *to); ssize_t fuse_dax_write_iter(struct kiocb *iocb, struct iov_iter *from); int fuse_dax_mmap(struct file *file, struct vm_area_struct *vma); int fuse_dax_break_layouts(struct inode *inode, u64 dmap_start, u64 dmap_end); -int fuse_dax_conn_alloc(struct fuse_conn *fc, struct dax_device *dax_dev); +int fuse_dax_conn_alloc(struct fuse_conn *fc, unsigned int mode, + struct dax_device *dax_dev); void fuse_dax_conn_free(struct fuse_conn *fc); bool fuse_dax_inode_alloc(struct super_block *sb, struct fuse_inode *fi); void fuse_dax_inode_init(struct inode *inode); diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c index b9beb39a4a18..f6b46395edb2 100644 --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -1434,7 +1434,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx) sb->s_subtype = ctx->subtype; ctx->subtype = NULL; if (IS_ENABLED(CONFIG_FUSE_DAX)) { - err = fuse_dax_conn_alloc(fc, ctx->dax_dev); + err = fuse_dax_conn_alloc(fc, ctx->dax, ctx->dax_dev); if (err) goto err; } diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c index 8f52cdaa8445..561f711d1945 100644 --- a/fs/fuse/virtio_fs.c +++ b/fs/fuse/virtio_fs.c @@ -88,12 +88,21 @@ struct virtio_fs_req_work { static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq, struct fuse_req *req, bool in_flight); +static const struct constant_table dax_param_enums[] = { + {"inode", FUSE_DAX_MOUNT_INODE }, + {"always", FUSE_DAX_MOUNT_ALWAYS }, + {"never", FUSE_DAX_MOUNT_NEVER }, + {} +}; + enum { OPT_DAX, + OPT_DAX_ENUM, }; static const struct fs_parameter_spec virtio_fs_parameters[] = { fsparam_flag("dax", OPT_DAX), + fsparam_enum("dax", OPT_DAX_ENUM, dax_param_enums), {} }; @@ -110,7 +119,10 @@ static int virtio_fs_parse_param(struct fs_context *fc, switch (opt) { case OPT_DAX: - ctx->dax = 1; + ctx->dax = FUSE_DAX_MOUNT_ALWAYS; + break; + case OPT_DAX_ENUM: + ctx->dax = result.uint_32; break; default: return -EINVAL; @@ -1326,7 +1338,7 @@ static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc) /* virtiofs allocates and installs its own fuse devices */ ctx->fudptr = NULL; - if (ctx->dax) { + if (ctx->dax != FUSE_DAX_MOUNT_NEVER) { if (!fs->dax_dev) { err = -EINVAL; pr_err("virtio-fs: dax can't be enabled as filesystem"
We add 'always', 'never', and 'inode' (default). '-o dax' continues to operate the same which is equivalent to 'always'. By the time this patch is applied, 'inode' mode is actually equal to 'always' mode, before the per-file DAX flag is introduced in the following patch. Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com> --- fs/fuse/dax.c | 13 ++++++++++++- fs/fuse/fuse_i.h | 11 +++++++++-- fs/fuse/inode.c | 2 +- fs/fuse/virtio_fs.c | 16 ++++++++++++++-- 4 files changed, 36 insertions(+), 6 deletions(-)