On 10/17/18 5:30 PM, Darrick J. Wong wrote: > On Wed, Oct 17, 2018 at 05:20:56PM -0500, Eric Sandeen wrote: >> Today we reject this flag on an already-reflinked inode, but we really >> need to reject it for any inode on a reflink-capable filesystem until >> reflink+dax is supported. >> >> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx> >> --- >> >> (um, do we need to catch this when reading from disk as well? If we >> found a dax-flagged inode on a reflinked fs, then what would we do with it?) > > Ignore the DAX flag. See xfs_inode_supports_dax. Oh, ok. Hohum, by that measure shouldn't we allow mount -o dax on reflink filesystems, and just ignore/reject dax behavior on actually reflinked inodes? >> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c >> index 0ef5ece5634c..63d579c652f2 100644 >> --- a/fs/xfs/xfs_ioctl.c >> +++ b/fs/xfs/xfs_ioctl.c >> @@ -1031,8 +1031,9 @@ xfs_ioctl_setattr_xflags( >> if ((fa->fsx_xflags & FS_XFLAG_REALTIME) && xfs_is_reflink_inode(ip)) >> ip->i_d.di_flags2 &= ~XFS_DIFLAG2_REFLINK; >> >> - /* Don't allow us to set DAX mode for a reflinked file for now. */ >> - if ((fa->fsx_xflags & FS_XFLAG_DAX) && xfs_is_reflink_inode(ip)) >> + /* Don't allow us to set DAX mode on a reflink filesystem for now. */ >> + if ((fa->fsx_xflags & FS_XFLAG_DAX) && >> + xfs_sb_version_hasreflink(&ip->i_mount->m_sb)) >> return -EINVAL; > > Not sure we need this, since DAX always loses to REFLINK, whether it's > at inode loading time or if someone's trying to set it in the ioctl. loses to a reflinked /inode/ but not to a reflink-capable fs, like mount does. > Ofc it's hard to say what the behavior should be since the dax iflag > semantics are poorly defined (it's been more or less advisory this whole > time)... > > ...but I gather you're sending this patch because you don't like this > wishy washy "ok you change this flag but it doesn't tell you if that had > any effect and there's no way to find out either" behavior? :) Just aiming for some semblance of consistency. Today we have: mount -o dax + xfs_sb_version_hasreflink => ignore mount option, disable dax for all inodes regardless of reflinked status chattr +x + xfs_sb_version_hasreflink => inode is dax until it gets reflinked, then it's ignored right? (and what happens if we have a daxified inode that gets reflinked later?) -Eric
