On 8/27/21 9:25 AM, Bill O'Donnell wrote:
On Fri, Aug 27, 2021 at 09:18:32AM -0500, Eric Sandeen wrote:
On 8/27/21 9:03 AM, Bill O'Donnell wrote:
On Thu, Aug 26, 2021 at 06:43:44PM -0500, Eric Sandeen wrote:
On 8/26/21 5:08 PM, Bill O'Donnell wrote:
On Thu, Aug 26, 2021 at 01:16:22PM -0500, Eric Sandeen wrote:
On 8/26/21 1:09 PM, Darrick J. Wong wrote:
On Thu, Aug 26, 2021 at 12:30:12PM -0500, Bill O'Donnell wrote:
@@ -1584,7 +1586,7 @@ xfs_fs_fill_super(
if (xfs_has_crc(mp))
sb->s_flags |= SB_I_VERSION;
- if (xfs_has_dax_always(mp)) {
+ if (xfs_has_dax_always(mp) || xfs_has_dax_inode(mp)) {
Er... can't this be done without burning another feature bit by:
if (xfs_has_dax_always(mp) || (!xfs_has_dax_always(mp) &&
!xfs_has_dax_never(mp))) {
...
xfs_warn(mp, "DAX IS EXPERIMENTAL");
}
changing this conditional in this way will also fail dax=inode mounts on
reflink-capable (i.e. default) filesystems, no?
Correct. My original patch tests fine, and still handles the reflink and dax
incompatibility. The new suggested logic is problematic.
-Bill
I think that both your proposed patch and Darrick's suggestion have this problem.
"mount -o dax=inode" makes your new xfs_has_dax_inode(mp) true, and in that
conditional, if the filesystem has reflink enabled, mount fails:
# mkfs.xfs -f /dev/pmem0p1
meta-data=/dev/pmem0p1 isize=512 agcount=4, agsize=4194304 blks
= sectsz=4096 attr=2, projid32bit=1
= crc=1 finobt=1, sparse=1, rmapbt=0
= reflink=1 bigtime=0 inobtcount=0
data = bsize=4096 blocks=16777216, imaxpct=25
= sunit=0 swidth=0 blks
naming =version 2 bsize=4096 ascii-ci=0, ftype=1
log =internal log bsize=4096 blocks=8192, version=2
= sectsz=4096 sunit=1 blks, lazy-count=1
realtime =none extsz=4096 blocks=0, rtextents=0
# mount -o dax=inode /dev/pmem0p1 /mnt/test
mount: wrong fs type, bad option, bad superblock on /dev/pmem0p1,
missing codepage or helper program, or other error
In some cases useful info is found in syslog - try
dmesg | tail or so.
# dmesg | tail -n 2
[ 192.691733] XFS (pmem0p1): DAX enabled. Warning: EXPERIMENTAL, use at your own risk
[ 192.700300] XFS (pmem0p1): DAX and reflink cannot be used together!
So, the "DAX enabled" is a misnomer in this case. However the incompatibility of DAX and reflink is
reflected in the next message, and indeed the mount fails. Is it now a matter of fixing
the message output so as not to indicate "DAX enabled..."?
The mount should not fail, and it does not fail prior to your change.
In the past, we did not allow any mixing of a reflink-capable
filesystem with dax in any way. Now, with per-inode dax, dax-enabled inodes and
reflink-enabled inodes can exist on the same filesystem, you just cannot have an
inode which is both dax-enabled and reflinked at the same time.
Ah. I missed that nuance. I had thought the incompatibility was
absolute. :/
The manpage for mkfs.xfs may need updating for the inode mode
(unless mine is old):
----------------snip------------------
"Note: the filesystem DAX mount option ( -o dax ) is incom‐
patible with reflink-enabled XFS filesystems. To use
filesystem DAX with XFS, specify the -m reflink=0 option to
mkfs.xfs to disable the reflink feature."
-------------------------------------
Hm, looks like the xfs(5) manpage got updated, but it seems mkfs.xfs(8) did not.
dax=value
Set CPU direct access (DAX) behavior for the current filesystem.
This mount option accepts the following values:
"dax=inode" DAX will be enabled only on regular files with
FS_XFLAG_DAX applied.
"dax=never" DAX will not be enabled for any files. FS_XFLAG_DAX
will be ignored.
"dax=always" DAX will be enabled for all regular files, regardless
of the FS_XFLAG_DAX state.
If no option is used when mounting a filesystem stored on a DAX
capable device, dax=inode will be used as default.
For details regarding DAX behavior in kernel, please refer to ker‐
nel's documentation
I'll send a patch to fix up the mkfs manpage, thanks.
Thanks,
-Eric
