Re: [PATCH v2 3/4] fuse: add per-file DAX flag

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 7/21/21 3:27 AM, Vivek Goyal wrote:
> On Tue, Jul 20, 2021 at 02:51:34PM +0800, JeffleXu wrote:
>>
>>
>> On 7/20/21 3:44 AM, Vivek Goyal wrote:
>>> On Fri, Jul 16, 2021 at 06:47:52PM +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.
>>>>
>>>> Signed-off-by: Jeffle Xu <jefflexu@xxxxxxxxxxxxxxxxx>
>>>> ---
>>>>  fs/fuse/dax.c             | 21 +++++++++++++++++----
>>>>  fs/fuse/file.c            |  4 ++--
>>>>  fs/fuse/fuse_i.h          |  5 +++--
>>>>  fs/fuse/inode.c           |  5 ++++-
>>>>  include/uapi/linux/fuse.h |  5 +++++
>>>>  5 files changed, 31 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
>>>> index a478e824c2d0..0e862119757a 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,31 @@ 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_dax_dontcache(struct inode *inode, bool newdax)
>>>> +{
>>>> +	struct fuse_conn *fc = get_fuse_conn(inode);
>>>> +
>>>> +	if (fc->dax && fc->dax->mode == FUSE_DAX_MOUNT_INODE &&
>>>> +	    IS_DAX(inode) != newdax)
>>>> +		d_mark_dontcache(inode);
>>>> +}
>>>> +
>>>
>>> This capability to mark an inode dontcache should probably be in a
>>> separate patch. These seem to logically two functionalities. One is
>>> enabling DAX on an inode. And second is making sure how soon you
>>> see the effect of that change and hence marking inode dontcache.
>>
>> OK, sounds reasonable.
>>
>>>
>>> Not sure how useful this is. In cache=none mode we should get rid of
>>> inode ASAP. In cache=auto mode we will get rid of after 1 second (or
>>> after a user specified timeout). So only place this seems to be
>>> useful is cache=always.
>>
>> Actually dontcache here is used to avoid dynamic switching between DAX
>> and non-DAX state while file is opened. The complexity of dynamic
>> switching is that, you have to clear the address_space, since page cache
>> and DAX entry can not coexist in the address space. Besides,
>> inode->a_ops also needs to be changed dynamically.
>>
>> With dontcache, dynamic switching is no longer needed and the DAX state
>> will be decided only when inode (in memory) is initialized. The downside
>> is that the new DAX state won't be updated until the file is closed and
>> reopened later.
>>
>> 'cache=none' only invalidates dentry, while the inode (in memory) is
>> still there (with address_space uncleared and a_ops unchanged).
> 
> Aha.., that's a good point.
>>
>> The dynamic switching may be done, though it's not such straightforward.
>> Currently, ext4/xfs are all implemented in this dontcache way, i.e., the
>> new DAX state won't be seen until the file is closed and reopened later.
> 
> Got it. Agreed that dontcache seems reasonable if file's DAX state
> has changed. Keep it in separate patch though with proper commit
> logs.
> 
> Also, please copy virtiofs list (virtio-fs@xxxxxxxxxx) when you post
> patches next time.
> 

Got it. By the way, what's the git repository of virtiofsd? AFAIK,
virtiofsd included in qemu (git@xxxxxxxxxx:qemu/qemu.git) doesn't
support DAX yet?

-- 
Thanks,
Jeffle



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux