Re: [PATCH v6 2/7] fuse: make DAX mount option a tri-state

From: JeffleXu <jefflexu@linux.alibaba.com>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: stefanha@redhat.com, miklos@szeredi.hub,
	linux-fsdevel@vger.kernel.org, virtio-fs@redhat.com,
	bo.liu@linux.alibaba.com, joseph.qi@linux.alibaba.com
Subject: Re: [PATCH v6 2/7] fuse: make DAX mount option a tri-state
Date: Fri, 29 Oct 2021 16:33:06 +0800	[thread overview]
Message-ID: <eb0c9711-66cb-bf79-0cf6-c6d6eec5ceea@linux.alibaba.com> (raw)
In-Reply-To: <YXAsV3xp3aeOjaeh@redhat.com>

On 10/20/21 10:48 PM, Vivek Goyal wrote:
> On Wed, Oct 20, 2021 at 10:52:38AM +0800, JeffleXu wrote:
>>
>>
>> On 10/18/21 10:10 PM, Vivek Goyal wrote:
>>> On Mon, Oct 11, 2021 at 11:00:47AM +0800, Jeffle Xu wrote:
>>>> We add 'always', 'never', and 'inode' (default). '-o dax' continues to
>>>> operate the same which is equivalent to 'always'. To be consistemt with
>>>> ext4/xfs's tri-state mount option, when neither '-o dax' nor '-o dax='
>>>> option is specified, the default behaviour is equal to 'inode'.
>>>
>>> Hi Jeffle,
>>>
>>> I am not sure when  -o "dax=inode"  is used as a default? If user
>>> specifies, "-o dax" then it is equal to "-o dax=always", otherwise
>>> user will explicitly specify "-o dax=always/never/inode". So when
>>> is dax=inode is used as default?
>>
>> That means when neither '-o dax' nor '-o dax=always/never/inode' is
>> specified, it is actually equal to '-o dax=inode', which is also how
>> per-file DAX on ext4/xfs works.
>>
>> This default behaviour for local filesystem, e.g. ext4/xfs, may be
>> straightforward, since the disk inode will be read into memory during
>> the inode instantiation, and checking for persistent inode attribute
>> shall be realatively cheap, except that the default behaviour has
>> changed from 'dax=never' to 'dax=inode'.
> 
> Interesting that ext4/xfs allowed for this behavior change.
> 
>>
>> Come back to virtiofs, when neither '-o dax' nor '-o
>> dax=always/never/inode' is specified, and it actually behaves as '-o
>> dax=inode', as long as '-o dax=server/attr' option is not specified for
>> virtiofsd, virtiofsd will always clear FUSE_ATTR_DAX and thus guest will
>> always disable DAX. IOWs, the guest virtiofs atually behaves as '-o
>> dax=never' when neither '-o dax' nor '-o dax=always/never/inode' is
>> specified, and '-o dax=server/attr' option is not specified for virtiofsd.
>>
>> But I'm okay if we need to change the default behaviour for virtiofs.
> 
> This is change of behavior from client's perspective. Even if client
> did not opt-in for DAX, DAX can be enabled based on server's setting.
> Not that there is anything wrong with it, but change of behavior part
> concerns me.
> 
> In case of virtiofs, lot of features we are controlling from server.
> Client typically just calls "mount" and there are not many options
> users can specify for mount.  
> 
> Given we already allowed to make client a choice about DAX behavior,
> I will feel more comfortable that we don't change it and let client
> request a specific DAX mode and if client does not specify anything,
> then DAX is not enabled.
> 

Hi Vivek,

How about the following design about the default behavior to move this
patchset forward, considering the discussion on another thread [1]?

- guest side: '-o dax=inode' acts as the default, keeping consistent
with xfs/ext4
- virtiofsd: the default behavior is like, neither file size based
policy ('dax=server') nor persistent inode flags based policy
('dax=attr') is used, though virtiofsd indeed advertises that
it supports per inode DAX feature (so that it won't fail FUSE_INIT
negotiation phase when guest advertises dax=inode by default)... In
fact, it acts like 'dax=never' in this case.

Then when guest opts-in and specifies '-o dax=inode' manually, then it
shall realize that proper configuration for virtiofsd is also needed (-o
dax=server|attr).

[1] https://www.spinics.net/lists/linux-xfs/msg56642.html

-- 
Thanks,
Jeffle