On Fri, Jul 16, 2021 at 08:40:29AM +0800, Liu Bo wrote:> On Thu, Jul 15, 2021 at 05:30:31PM +0800, Jeffle Xu wrote:
> > Add one flag for fuse_attr.flags indicating if DAX shall be enabled
for
> > this file.
> >
> > When the per-file DAX flag changes for an *opened* file, the state of
> > the file won't be updated until this file is closed and reopened
later.
> >
> > Currently it is not implemented yet to change per-file DAX flag inside
> > guest kernel, e.g., by chattr(1).
>
> Thanks for the patch, it looks good to me.
>
> I think it's a good starting point, what I'd like to discuss here
is
> whether we're going to let chattr to toggle the dax flag.
I have the same question. Why not take chattr approach as taken
by ext4/xfs as well.
Vivek
>
> My usecase is like, on the fuse server side, if a file is marked as
> DAX, then it won't change any more. So this 'fuse_attr.flags'
works
> for me at least.
>
> thanks,
> liubo
>
> >
> > Signed-off-by: Jeffle Xu <jefflexu at linux.alibaba.com>
> > ---
> > fs/fuse/dax.c | 28 ++++++++++++++++++++++++----
> > fs/fuse/file.c | 4 ++--
> > fs/fuse/fuse_i.h | 5 +++--
> > fs/fuse/inode.c | 4 +++-
> > include/uapi/linux/fuse.h | 5 +++++
> > 5 files changed, 37 insertions(+), 9 deletions(-)
> >
> > diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
> > index 4873d764cb66..ed5a430364bb 100644
> > --- a/fs/fuse/dax.c
> > +++ b/fs/fuse/dax.c
> > @@ -1341,7 +1341,7 @@ static const struct address_space_operations
fuse_dax_file_aops = {
> > .invalidatepage = noop_invalidatepage,
> > };
> >
> > -static bool fuse_should_enable_dax(struct inode *inode)
> > +static bool fuse_should_enable_dax(struct inode *inode, unsigned int
flags)
> > {
> > struct fuse_conn *fc = get_fuse_conn(inode);
> > unsigned int mode;
> > @@ -1354,18 +1354,38 @@ static bool fuse_should_enable_dax(struct
inode *inode)
> > if (mode == FUSE_DAX_MOUNT_NEVER)
> > return false;
> >
> > - return true;
> > + if (mode == FUSE_DAX_MOUNT_ALWAYS)
> > + return true;
> > +
> > + WARN_ON(mode != FUSE_DAX_MOUNT_INODE);
> > + return flags & FUSE_ATTR_DAX;
> > }
> >
> > -void fuse_dax_inode_init(struct inode *inode)
> > +void fuse_dax_inode_init(struct inode *inode, unsigned int flags)
> > {
> > - if (!fuse_should_enable_dax(inode))
> > + if (!fuse_should_enable_dax(inode, flags))
> > return;
> >
> > inode->i_flags |= S_DAX;
> > inode->i_data.a_ops = &fuse_dax_file_aops;
> > }
> >
> > +void fuse_update_dax(struct inode *inode, unsigned int flags)
> > +{
> > + bool oldstate, newstate;
> > + struct fuse_conn *fc = get_fuse_conn(inode);
> > +
> > + if (!IS_ENABLED(CONFIG_FUSE_DAX) || !fc->dax ||
> > + fc->dax->mode != FUSE_DAX_MOUNT_INODE)
> > + return;
> > +
> > + oldstate = IS_DAX(inode);
> > + newstate = flags & FUSE_ATTR_DAX;
> > +
> > + if (oldstate != newstate)
> > + d_mark_dontcache(inode);
> > +}
> > +
> > bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int
map_alignment)
> > {
> > if (fc->dax && (map_alignment > FUSE_DAX_SHIFT)) {
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index 97f860cfc195..cf42af492146 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -3142,7 +3142,7 @@ static const struct address_space_operations
fuse_file_aops = {
> > .write_end = fuse_write_end,
> > };
> >
> > -void fuse_init_file_inode(struct inode *inode)
> > +void fuse_init_file_inode(struct inode *inode, struct fuse_attr
*attr)
> > {
> > struct fuse_inode *fi = get_fuse_inode(inode);
> >
> > @@ -3156,5 +3156,5 @@ void fuse_init_file_inode(struct inode *inode)
> > fi->writepages = RB_ROOT;
> >
> > if (IS_ENABLED(CONFIG_FUSE_DAX))
> > - fuse_dax_inode_init(inode);
> > + fuse_dax_inode_init(inode, attr->flags);
> > }
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index f29018323845..b0ecfffd0c7d 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -1000,7 +1000,7 @@ int fuse_notify_poll_wakeup(struct fuse_conn
*fc,
> > /**
> > * Initialize file operations on a regular file
> > */
> > -void fuse_init_file_inode(struct inode *inode);
> > +void fuse_init_file_inode(struct inode *inode, struct fuse_attr
*attr);
> >
> > /**
> > * Initialize inode operations on regular files and special files
> > @@ -1252,8 +1252,9 @@ 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);
> > +void fuse_dax_inode_init(struct inode *inode, unsigned int flags);
> > void fuse_dax_inode_cleanup(struct inode *inode);
> > +void fuse_update_dax(struct inode *inode, unsigned int flags);
> > bool fuse_dax_check_alignment(struct fuse_conn *fc, unsigned int
map_alignment);
> > void fuse_dax_cancel_work(struct fuse_conn *fc);
> >
> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > index f6b46395edb2..47ebb1a394d2 100644
> > --- a/fs/fuse/inode.c
> > +++ b/fs/fuse/inode.c
> > @@ -269,6 +269,8 @@ void fuse_change_attributes(struct inode *inode,
struct fuse_attr *attr,
> > if (inval)
> > invalidate_inode_pages2(inode->i_mapping);
> > }
> > +
> > + fuse_update_dax(inode, attr->flags);
> > }
> >
> > static void fuse_init_inode(struct inode *inode, struct fuse_attr
*attr)
> > @@ -281,7 +283,7 @@ static void fuse_init_inode(struct inode *inode,
struct fuse_attr *attr)
> > inode->i_ctime.tv_nsec = attr->ctimensec;
> > if (S_ISREG(inode->i_mode)) {
> > fuse_init_common(inode);
> > - fuse_init_file_inode(inode);
> > + fuse_init_file_inode(inode, attr);
> > } else if (S_ISDIR(inode->i_mode))
> > fuse_init_dir(inode);
> > else if (S_ISLNK(inode->i_mode))
> > diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> > index 36ed092227fa..9ee088ddbe2a 100644
> > --- a/include/uapi/linux/fuse.h
> > +++ b/include/uapi/linux/fuse.h
> > @@ -184,6 +184,9 @@
> > *
> > * 7.34
> > * - add FUSE_SYNCFS
> > + *
> > + * 7.35
> > + * - add FUSE_ATTR_DAX
> > */
> >
> > #ifndef _LINUX_FUSE_H
> > @@ -449,8 +452,10 @@ struct fuse_file_lock {
> > * fuse_attr flags
> > *
> > * FUSE_ATTR_SUBMOUNT: Object is a submount root
> > + * FUSE_ATTR_DAX: Enable DAX for this file in per-file DAX mode
> > */
> > #define FUSE_ATTR_SUBMOUNT (1 << 0)
> > +#define FUSE_ATTR_DAX (1 << 1)
> >
> > /**
> > * Open flags
> > --
> > 2.27.0
>