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@xxxxxxxxxxxxxxxxx>
>> ---
>> 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.
--
Thanks,
Jeffle
