Bug 1848084

See also https://lore.kernel.org/linux-fsdevel/20200616202123.12656-1-msys.mizuma@gmail.com/ and followups.

XFS doesn't manipulate this flag on remount, and the behavior is not unique to xfs:

# mount -o loop,i_version fsfile test_iversion
# grep test_iversion /proc/mounts
/dev/loop4 /tmp/test_iversion ext4 rw,seclabel,relatime,i_version 0 0
# mount -o remount test_iversion
# grep test_iversion /proc/mounts
/dev/loop4 /tmp/test_iversion ext4 rw,seclabel,relatime 0 0
# uname -a
Linux <hostname> 5.7.0-rc4+ #7 SMP Wed Jun 10 14:01:34 EDT 2020 x86_64 x86_64 x86_64 GNU/Linux

Worth testing:  if "iversion" was exposed in /proc/mounts when SB_I_VERSION is set, I bet mount -o remount would DTRT.

(iversion is what mount(8) uses; I don't know if that pre- or post-dates ext4's "i_version")

I've sent an xfs-specific fix upstream for this:

https://lore.kernel.org/linux-xfs/e8bfbbec-4cb9-0267-08eb-03580e2aedc6@redhat.com/

but it will need slight changes for RHEL8 since we don't yet have the mount api rework.

The general vfs-level fix to export "iversion" in /proc/mounts was rejected, so every filesystem will need to manually fix this.

It seems that ext4 is also affected, because although it exports its custom mount option "i_version" in /proc/mounts, mount(2) doesn't parse it on remount (?).

Do other filesystems need to be investigated for RHEL8?

Thanks,
-Eric

Lukas said he's take a look at the ext4 side of this; we can send both patches (and any others required) under this one bug I suppose to save bug traffic.

FYI ext4 is interesting, although i_version is present in /proc/mounts and therefore does actually get passed back in on remount it still seems to get lost, at least in RHEL8:

# strace -emount mount -o loop,i_version fsfile mnt
mount("/dev/loop0", "/root/mnt", "ext4", MS_MGC_VAL, "i_version") = 0
+++ exited with 0 +++

# grep ext4 /proc/mounts
/dev/loop0 /root/mnt ext4 rw,seclabel,relatime,i_version 0 0


# strace -emount mount -o remount  mnt
mount("/dev/loop0", "/root/mnt", 0x55fdb39b17b0, MS_REMOUNT|MS_RELATIME, "seclabel,i_version") = 0
+++ exited with 0 +++

# grep ext4 /proc/mounts
/dev/loop0 /root/mnt ext4 rw,seclabel,relatime 0 0

(In reply to Eric Sandeen from comment #5)
> I've sent an xfs-specific fix upstream for this:
> 
> https://lore.kernel.org/linux-xfs/e8bfbbec-4cb9-0267-08eb-
> 03580e2aedc6/
> 
> but it will need slight changes for RHEL8 since we don't yet have the mount
> api rework.

I'm pretty depressed about the prospect of worrying about people exporting XFS with noiversion for the next however many years.  Are you sure we really need it?  What will the plan be to deprecate it?

iversion prevents races that can cause NFS clients to miss file updates.  In theory it could cause performance regressions, in practice we've seen no complaints about performance on recent kernels.

> The general vfs-level fix to export "iversion" in /proc/mounts was rejected,
> so every filesystem will need to manually fix this.
> 
> It seems that ext4 is also affected, because although it exports its custom
> mount option "i_version" in /proc/mounts, mount(2) doesn't parse it on
> remount (?).
> 
> Do other filesystems need to be investigated for RHEL8?

I've never actually tried to inventory all the exportable filesystems in RHEL8, I'll take a look....  But, yes, I expect XFS and ext4 are the ones we really care about.  (And ext4 still defaults to noiversion, unfortunately.)

Argh, sorry, I hadn't looked at your latest patch, looks like it does just keep SB_I_VERSION on all the time for v5.  Thank you, and sorry for the noise!

And, yes, I think the only other filesystem we'd care about for now is btrfs and it's not in RHEL8.

(In reply to J. Bruce Fields from comment #9)
> Argh, sorry, I hadn't looked at your latest patch, looks like it does just
> keep SB_I_VERSION on all the time for v5.  Thank you, and sorry for the
> noise!

Right, no need to be depressed!

> And, yes, I think the only other filesystem we'd care about for now is btrfs
> and it's not in RHEL8.

File a Fedora bug ;)

I went ahead and filed a Fedora bug to notify the folks who will be maintaining the btrfs code in Fedora.

Bug #1858535

The xfs commit is in upstream:

https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/commit/?h=for-next&id=ea52eff66dcd6359e07821e29a46b27951eecb42

and Lukas has sent the ext4 fix, and it has one review on the list:

https://lore.kernel.org/linux-ext4/20200727164622.B88874C044@d06av22.portsmouth.uk.ibm.com/

Bruce, is it critical that we get this into RHEL8.3 or will 8.4 suffice?

The bug is not a regression, and to hit it you need to remount an NFS-exported filesystem, which probably isn't that common, and then probably need frequent IO to one file from multiple clients.

There's an easy workaround, though I don't expect users to find it--they'll just see this as occasional hard-to-debug NFS unreliability.

It looks like a really minor patch.  I'd rather get it in.  But given that we've lived with it forever I guess I'm OK with waiting.

(In reply to Eric Sandeen from comment #12)
> The xfs commit is in upstream:
> 
> https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/commit/?h=for-
> next&id=ea52eff66dcd6359e07821e29a46b27951eecb42
> 
> and Lukas has sent the ext4 fix, and it has one review on the list:
> 
> https://lore.kernel.org/linux-ext4/20200727164622.B88874C044@d06av22.
> portsmouth.uk.ibm.com/
> 
> Bruce, is it critical that we get this into RHEL8.3 or will 8.4 suffice?

Who's submitting this for 8.4?  Should it be split into xfs and ext4 bugs, or can they both go in under this bug?

(In reply to J. Bruce Fields from comment #15)
> (In reply to Eric Sandeen from comment #12)
> > The xfs commit is in upstream:
> > 
> > https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/commit/?h=for-
> > next&id=ea52eff66dcd6359e07821e29a46b27951eecb42
> > 
> > and Lukas has sent the ext4 fix, and it has one review on the list:
> > 
> > https://lore.kernel.org/linux-ext4/20200727164622.B88874C044@d06av22.
> > portsmouth.uk.ibm.com/
> > 
> > Bruce, is it critical that we get this into RHEL8.3 or will 8.4 suffice?
> 
> Who's submitting this for 8.4?  Should it be split into xfs and ext4 bugs,
> or can they both go in under this bug?

I'll be sending this for ext4. If it's too much work to split into xfs and ext4 I can do both I don't mind. No preference here.

-Lukas

Lukas, if you'd like to send the trivial xfs fix along with the ext4 fix I think that would work well.  It hit 5.9 upstream so it's not in the update we did for RHEL8.4 xfs.

Thanks,
-Eric

Actually  -Pavel, if you can take this one it'd be great.  May want to queue it up after any ext4 updates Lukas has planned, but you guys can work out that timing.  For XFS, should base it on the updates bill & carlos have already sent, if there is any interaction or dependencies.

Testing may be the only trick, I'm not sure there is a reliable way for userspace to know for sure if inode versioning is on or off.  

xfs: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/xfs?id=4750a171c3290f9bbebca16c6372db723a4cfa3b
ext4: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/ext4?id=f25391ebb475d3ffb3aa61bb90e3594c841749ef

(In reply to Eric Sandeen from comment #18)
> Testing may be the only trick, I'm not sure there is a reliable way for
> userspace to know for sure if inode versioning is on or off.

If you can find sb->s_flags in kernel memory you could check whether SB_I_VERSION is set.

Or export the filesystem over NFS and run the SATT15 pynfs 4.0 test--it does several SETATTRs and checks the change attribute in between to verify that it changes.  Each setattr has to be committed to storage before it returns, so if your storage is too slow compared to clock resolution then this won't necessarily catch buggy i_version; maybe run the test on a ramdisk?

We will do this fix for both xfs and ext4 in this BZ.

Lukas Czerner will do the fix for ext4.
Pavel Reichl will do the fix for xfs.

Zorro Lang agreed to test both xfs and ext4.

(In reply to Pavel Reichl from comment #20)
> We will do this fix for both xfs and ext4 in this BZ.
> 
> Lukas Czerner will do the fix for ext4.
> Pavel Reichl will do the fix for xfs.
> 
> Zorro Lang agreed to test both xfs and ext4.

I'm good to test both fs together in this bug, if fs-devel would like to fix both of ext4 and xfs together.

I mentioned this to Pavel in our talk, but figured I'd put it here for posterity.

One manual way to confirm whether or not SB_I_VERSION is set would be to look at the s_flags for the superblock of the mount point in crash, I think:

crash> mount | grep -w "MOUNT\|xfs"
     MOUNT           SUPERBLK     TYPE   DEVNAME   DIRNAME
ffff96309525ca80 ffff963095f51000 xfs    /dev/mapper/cl-root /         
crash> super_block.s_flags -x ffff963095f51000
  s_flags = 0x70010000
crash> 


I also wonder if using systemtap might be someting that QE could build a less manual test around, i.e. add a printk to the show_mounts path to indicate whether or not i_version is still set.

(In reply to Eric Sandeen from comment #22)
> I mentioned this to Pavel in our talk, but figured I'd put it here for
> posterity.
> 
> One manual way to confirm whether or not SB_I_VERSION is set would be to
> look at the s_flags for the superblock of the mount point in crash, I think:
> 
> crash> mount | grep -w "MOUNT\|xfs"
>      MOUNT           SUPERBLK     TYPE   DEVNAME   DIRNAME
> ffff96309525ca80 ffff963095f51000 xfs    /dev/mapper/cl-root /         
> crash> super_block.s_flags -x ffff963095f51000
>   s_flags = 0x70010000
> crash> 
> 
> 
> I also wonder if using systemtap might be someting that QE could build a
> less manual test around, i.e. add a printk to the show_mounts path to
> indicate whether or not i_version is still set.

Thanks Eric, I'm planning to use some tricks to test this bug too, I don't have a better way to write a common test case for xfstests.

Thanks,
Zorro

Patch(es) available on kernel-4.18.0-255.el8.dt3

xfstests generic/230 (and some other cases) easily fails on kernel 255.el8.dt3 with "fsync Input/Output error"[1], it's nearly 100% reproducible for me on *ppc64le* machine. Bad news is I can't reproduce it with kernel 225.el8.dt2 even on same ppc64le machine [2], and can't reproduce it on ext4 either[3]. Also can't reproduce this bug on kernel 259.el8. I'm not sure if this's a regression from this bug fix, 255.el8.dt3 contains 2 xfs related bugs (bz1848084 and bz1696580).

FSTYP         -- xfs (non-debug)
PLATFORM      -- Linux/ppc64le ibm-p9z-20-lp23 4.18.0-255.el8.dt3.ppc64le+debug #1 SMP Thu Dec 3 03:21:24 EST 2020
MKFS_OPTIONS  -- -f -m crc=1,finobt=1,rmapbt=1,reflink=1 -i sparse=1 /dev/sda3
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda3 /mnt/xfstests/scratch

[1]
# ./check generic/230
FSTYP         -- xfs (non-debug)
PLATFORM      -- Linux/ppc64le ibm-p9z-20-lp23 4.18.0-255.el8.dt3.ppc64le+debug #1 SMP Thu Dec 3 03:21:24 EST 2020
MKFS_OPTIONS  -- -f -bsize=4096 /dev/sda3
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda3 /mnt/xfstests/scratch

generic/230 21s ... - output mismatch (see /var/lib/xfstests/results//generic/230.out.bad)
    --- tests/generic/230.out   2020-12-07 02:11:07.510848569 -0500
    +++ /var/lib/xfstests/results//generic/230.out.bad  2020-12-07 06:38:36.521919716 -0500
    @@ -8,6 +8,7 @@
     Rewrite 250 blocks plus 1 byte...
     Write 250 blocks...
     pwrite: Disk quota exceeded
    +fsync: Input/output error
     Write 1 block...
     pwrite: Disk quota exceeded
     Touch 3+4
    ...
    (Run 'diff -u /var/lib/xfstests/tests/generic/230.out /var/lib/xfstests/results//generic/230.out.bad'  to see the entire diff)
Ran: generic/230
Failures: generic/230
Failed 1 of 1 tests

[2]
FSTYP         -- xfs (non-debug)
PLATFORM      -- Linux/ppc64le ibm-p9z-20-lp23 4.18.0-255.el8.dt2.ppc64le+debug #1 SMP Mon Nov 30 04:31:02 EST 2020
MKFS_OPTIONS  -- -f -bsize=4096 /dev/sda3
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/sda3 /mnt/xfstests/scratch

generic/230 20s ...  21s
Ran: generic/230
Passed all 1 tests

[3]
FSTYP         -- ext4
PLATFORM      -- Linux/ppc64le ibm-p9z-20-lp23 4.18.0-255.el8.dt3.ppc64le+debug #1 SMP Thu Dec 3 03:21:24 EST 2020
MKFS_OPTIONS  -- /dev/sda3
MOUNT_OPTIONS -- -o acl,user_xattr -o context=system_u:object_r:root_t:s0 /dev/sda3 /mnt/xfstests/scratch

generic/230 21s ...  23s
Ran: generic/230
Passed all 1 tests

Bisect test has done, the regression issue on comment 29 not from this bug fix, refer to
https://bugzilla.redhat.com/show_bug.cgi?id=1696580#c10

Ext4 shows i_version in /proc/mounts, so it's easy to test on ext4. For xfs, I use a trick to make XFS show its i_version option.

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 6d8a0f805dae..87a5d8af4be3 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -168,7 +168,8 @@ xfs_fs_show_options(
                { XFS_MOUNT_DAX_NEVER,          ",dax=never" },
                { 0, NULL }
        };
-       struct xfs_mount        *mp = XFS_M(root->d_sb);
+       struct super_block      *sb = root->d_sb;
+       struct xfs_mount        *mp = XFS_M(sb);
        struct proc_xfs_info    *xfs_infop;
 
        for (xfs_infop = xfs_info_set; xfs_infop->flag; xfs_infop++) {
@@ -221,6 +222,10 @@ xfs_fs_show_options(
        if (!(mp->m_qflags & XFS_ALL_QUOTA_ACCT))
                seq_puts(m, ",noquota");
 
+       if (sb->s_flags & SB_I_VERSION) {
+               seq_puts(m, ",iversion");
+       }
+
        return 0;
 }


Test on V5 XFS:
#1 mkfs.xfs -f -m crc=1 /dev/sda3
#2 mount /dev/sda3 /mnt/scratch
#3 grep sda3 /proc/mounts 
/dev/sda3 /mnt/scratch xfs rw,seclabel,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota,iversion 0 0
#4 mount -oremount /mnt/scratch
#5 grep sda3 /proc/mounts 
/dev/sda3 /mnt/scratch xfs rw,seclabel,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota,iversion 0 0
#6 umount /dev/sda3
#7 mount /dev/sda3 /mnt/scratch -o noiversion
#8 grep sda3 /proc/mounts 
/dev/sda3 /mnt/scratch xfs rw,seclabel,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota,iversion 0 0
#9 umount /dev/sda3

From step1~5, we can see V5 xfs enable iversion by default, and it keep iversion after remount.
From step7~9, we can see V5 XFS *always* enable iversion, even if we specified -o noiversion, the iversion is still enabled.

Test on V4 XFS:
#1 mkfs.xfs -f -m crc=0 /dev/sda3
#2 mount /dev/sda3 /mnt/scratch
#3 grep sda3 /proc/mounts 
/dev/sda3 /mnt/scratch xfs rw,seclabel,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota 0 0
#4 mount -oremount /mnt/scratch
#5 grep sda3 /proc/mounts 
/dev/sda3 /mnt/scratch xfs rw,seclabel,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota 0 0
#6 umount /dev/sda3
#7 mount /dev/sda3 /mnt/scratch -o iversion
#8 grep sda3 /proc/mounts 
/dev/sda3 /mnt/scratch xfs rw,seclabel,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota,iversion 0 0
#9 mount -oremount /mnt/scratch
#10 grep sda3 /proc/mounts 
/dev/sda3 /mnt/scratch xfs rw,seclabel,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota,iversion 0 0

From step1~5, we can see V4 XFS disable iversion, and keep it disabled after remount.
From step7~10, we can see V4 XFS enable iversion if mount with -oiversion, and keep iversion after remount.

Test on ext4:
#1 mount /dev/sda3 /mnt/scratch
#2 grep sda3 /proc/mounts 
/dev/sda3 /mnt/scratch ext4 rw,seclabel,relatime 0 0
#3 mount /dev/sda3 /mnt/scratch -oiversion
#4 grep sda3 /proc/mounts 
/dev/sda3 /mnt/scratch ext4 rw,seclabel,relatime,i_version 0 0
#5 mount -oremount /mnt/scratch
#6 grep sda3 /proc/mounts 
/dev/sda3 /mnt/scratch ext4 rw,seclabel,relatime,i_version 0 0

From step1~6, we can see ext4 enable iversion if mount with -o iversion, and keep it after remount.

So I think the bug fix looks good to me, set Tested to this bug.

Patch(es) available on kernel-4.18.0-261.el8

Test passed on kernel 263.el8 by the method of comment 32.

[root@hpe-dl380pgen8-02-vm-2 ~]# mkfs.xfs -f /dev/loop0      
meta-data=/dev/loop0             isize=512    agcount=4, agsize=327680 blks
         =                       sectsz=512   attr=2, projid32bit=1                                                                                                                           
         =                       crc=1        finobt=1, sparse=1, rmapbt=0
         =                       reflink=1              
data     =                       bsize=4096   blocks=1310720, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0, ftype=1
log      =internal log           bsize=4096   blocks=2560, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0
Discarding blocks...Done.                                   
[root@hpe-dl380pgen8-02-vm-2 ~]# mkdir /mnt/scratch
[root@hpe-dl380pgen8-02-vm-2 ~]# mount /dev/loop0 /mnt/scratch
[root@hpe-dl380pgen8-02-vm-2 ~]# grep loop0 /proc/mounts
/dev/loop0 /mnt/scratch xfs rw,seclabel,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota,iversion 0 0
[root@hpe-dl380pgen8-02-vm-2 ~]# mount -oremount /mnt/scratch  
[root@hpe-dl380pgen8-02-vm-2 ~]# grep loop0 /proc/mounts
/dev/loop0 /mnt/scratch xfs rw,seclabel,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota,iversion 0 0
[root@hpe-dl380pgen8-02-vm-2 ~]# umount  /dev/loop0     
[root@hpe-dl380pgen8-02-vm-2 ~]# mount /dev/loop0 /mnt/scratch -onoiversion
[root@hpe-dl380pgen8-02-vm-2 ~]# grep loop0 /proc/mounts    
/dev/loop0 /mnt/scratch xfs rw,seclabel,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota,iversion 0 0
[root@hpe-dl380pgen8-02-vm-2 ~]# umount /mnt/scratch
[root@hpe-dl380pgen8-02-vm-2 ~]# mkfs.xfs -f -m crc=0 /dev/loop0              
meta-data=/dev/loop0             isize=256    agcount=4, agsize=327680 blks
         =                       sectsz=512   attr=2, projid32bit=1      
         =                       crc=0        finobt=0, sparse=0, rmapbt=0
         =                       reflink=0                     
data     =                       bsize=4096   blocks=1310720, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0, ftype=1
log      =internal log           bsize=4096   blocks=2560, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0
Discarding blocks...Done.
[root@hpe-dl380pgen8-02-vm-2 ~]# mount /dev/loop0 /mnt/scratch
[root@hpe-dl380pgen8-02-vm-2 ~]# grep loop0 /proc/mounts
/dev/loop0 /mnt/scratch xfs rw,seclabel,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota 0 0
[root@hpe-dl380pgen8-02-vm-2 ~]# umount /mnt/scratch
[root@hpe-dl380pgen8-02-vm-2 ~]# mount /dev/loop0 /mnt/scratch -oiversion
[root@hpe-dl380pgen8-02-vm-2 ~]# grep loop0 /proc/mounts
/dev/loop0 /mnt/scratch xfs rw,seclabel,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota,iversion 0 0
[root@hpe-dl380pgen8-02-vm-2 ~]# mount -oremount /mnt/scratch
[root@hpe-dl380pgen8-02-vm-2 ~]# grep loop0 /proc/mounts
/dev/loop0 /mnt/scratch xfs rw,seclabel,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota,iversion 0 0
[root@hpe-dl380pgen8-02-vm-2 ~]# umount /mnt/scratch
[root@hpe-dl380pgen8-02-vm-2 ~]# mkfs.ext4 -F /dev/loop0
mke2fs 1.45.6 (20-Mar-2020)
/dev/loop0 contains a xfs file system
Discarding device blocks: done
Creating filesystem with 1310720 4k blocks and 327680 inodes
Filesystem UUID: 6c1027ce-8d80-4147-9ea5-ccdc2c64c637
Superblock backups stored on blocks:
        32768, 98304, 163840, 229376, 294912, 819200, 884736

Allocating group tables: done
Writing inode tables: done
Creating journal (16384 blocks): done
Writing superblocks and filesystem accounting information: done

[root@hpe-dl380pgen8-02-vm-2 ~]# mount /dev/loop0 /mnt/scratch
[root@hpe-dl380pgen8-02-vm-2 ~]# grep loop0 /proc/mounts
/dev/loop0 /mnt/scratch ext4 rw,seclabel,relatime 0 0
[root@hpe-dl380pgen8-02-vm-2 ~]# umount /mnt/scratch
[root@hpe-dl380pgen8-02-vm-2 ~]# mount /dev/loop0 /mnt/scratch -oiversion
[root@hpe-dl380pgen8-02-vm-2 ~]# grep loop0 /proc/mounts
/dev/loop0 /mnt/scratch ext4 rw,seclabel,relatime,i_version 0 0
[root@hpe-dl380pgen8-02-vm-2 ~]# mount -oremount /mnt/scratch
[root@hpe-dl380pgen8-02-vm-2 ~]# grep loop0 /proc/mounts
/dev/loop0 /mnt/scratch ext4 rw,seclabel,relatime,i_version 0 0

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Important: kernel security, bug fix, and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2021:1578

The needinfo request[s] on this closed bug have been removed as they have been unresolved for 500 days