On 10/29/21 9:03 PM, Vivek Goyal wrote: > On Fri, Oct 29, 2021 at 04:33:06PM +0800, JeffleXu wrote: >> >> >> 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 > > This sounds good. > >> - 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. > > Not sure why virtiofsd needs to advertise that it supports per inode > DAX even if no per inode dax policy is in affect. Guest will know that > server is not supporting per inode DAX. But it will not return an > error to user space (because dax=inode seems to be advisory). > > IOW, this is very similar to the case of using dax=inode on a block > device which does not support DAX. No errors and no warnings. OK. I will adopt this behavior. That is, if virtiofsd is not specified with 'dax=server|attr' option, it won't advertise support for per inode DAX in FUSE_INIT either. And then client will fallback to 'dax=never' even if it is mounted with 'dax=inode'. > >> >> 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). > > I gave some comments w.r.t dax=server naming in your posting. Not sure if > you got a chance to look at it. > > Thanks > Vivek > >> >> [1] https://www.spinics.net/lists/linux-xfs/msg56642.html >> >> -- >> Thanks, >> Jeffle >> -- Thanks, Jeffle