RO mount of ext4 filesystem causes writes

eee00237fa5ec8f704f7323b54e48cc34e2d9168 is the first bad commit commit eee00237fa5ec8f704f7323b54e48cc34e2d9168

Author: Ye Bin <[email protected]>
Date:   Tue Mar 7 14:17:02 2023 +0800

ext4: commit super block if fs record error when journal record without error

Hello, folks.

I noticed a change in behavior of ext4 in recent kernels. I make use of several luks loopback images formatted as ext4 that I mount read-only most of the time. I use rsync to synchronize the backing images between machines. In the past, mouning the images as read-only would not touch the backing image contents at all, but recently this changed. Every mount, even ones that are RO from the start, will cause some small writes to the backing image and thus force rsync to scan the whole file.

I confirmed that the issue is still present on v6.4.rc6, so I performed a bisect and landed on the following commit:

Quoted 5 lines

eee00237fa5ec8f704f7323b54e48cc34e2d9168 is the first bad commit commit eee00237fa5ec8f704f7323b54e48cc34e2d9168

Author: Ye Bin <[email protected]>
Date:   Tue Mar 7 14:17:02 2023 +0800

Quoted 1 line

ext4: commit super block if fs record error when journal record without error

That certainly looks like a likely cause of my issue, but I'm not familiar enough with the ext4 code to diagnose any further. Please let me know if you need any additional information, or if you would like me to test anything.

Quoted 18 lines · Sean Greenslade

Hello, folks.

I noticed a change in behavior of ext4 in recent kernels. I make use of several luks loopback images formatted as ext4 that I mount read-only most of the time. I use rsync to synchronize the backing images between machines. In the past, mouning the images as read-only would not touch the backing image contents at all, but recently this changed. Every mount, even ones that are RO from the start, will cause some small writes to the backing image and thus force rsync to scan the whole file.

I confirmed that the issue is still present on v6.4.rc6, so I performed a bisect and landed on the following commit:

Quoted 5 lines

eee00237fa5ec8f704f7323b54e48cc34e2d9168 is the first bad commit commit eee00237fa5ec8f704f7323b54e48cc34e2d9168

Author: Ye Bin <[email protected]>
Date:   Tue Mar 7 14:17:02 2023 +0800

Quoted 7 lines

Quoted 1 line

ext4: commit super block if fs record error when journal record without error

That certainly looks like a likely cause of my issue, but I'm not familiar enough with the ext4 code to diagnose any further. Please let me know if you need any additional information, or if you would like me to test anything.

Can you show dmesg when regression happens?

Ye: It looks like this regression is caused by your commit. Would you like to take a look on it?

Anyway, thanks for the bug report. I'm adding it to regzbot:

#regzbot ^introduced: eee00237fa5ec8
#regzbot title: commit super block writes even in read-only filesystems

Quoted 18 lines · Sean Greenslade

Hello, folks.

I noticed a change in behavior of ext4 in recent kernels. I make use of several luks loopback images formatted as ext4 that I mount read-only most of the time. I use rsync to synchronize the backing images between machines. In the past, mouning the images as read-only would not touch the backing image contents at all, but recently this changed. Every mount, even ones that are RO from the start, will cause some small writes to the backing image and thus force rsync to scan the whole file.

I confirmed that the issue is still present on v6.4.rc6, so I performed a bisect and landed on the following commit:

Quoted 5 lines

eee00237fa5ec8f704f7323b54e48cc34e2d9168 is the first bad commit commit eee00237fa5ec8f704f7323b54e48cc34e2d9168

Author: Ye Bin <[email protected]>
Date:   Tue Mar 7 14:17:02 2023 +0800

Quoted 7 lines

Quoted 1 line

ext4: commit super block if fs record error when journal record without error

That certainly looks like a likely cause of my issue, but I'm not familiar enough with the ext4 code to diagnose any further. Please let me know if you need any additional information, or if you would like me to test anything.

Can you show dmesg when regression happens?

Ye: It looks like this regression is caused by your commit. Would you like to take a look on it?

Anyway, thanks for the bug report. I'm adding it to regzbot:

#regzbot ^introduced: eee00237fa5ec8
#regzbot title: commit super block writes even in read-only filesystems

No reply so far from the culprit author (Ye Bin) nor from Ted. Can you help in this case?

Quoted 3 lines · Bagas Sanjaya

No reply so far from the culprit author (Ye Bin) nor from Ted. Can you help in this case?

There's been no reply because I haven't been able to replicate it, and I didn't have the time do enough work to convince myself the report was bogus. At this point, I have spent time trying to reproduce it, and I've had no luck.

So, unless you can give me a simple set of reproduction instructions, I'm going to have to treat this report is invalid.

Regards,

- Ted

Note: this test was done using kvm-xfstests which can be found
https://github.com/tytso/xfstests-bld using the install-kconfig and the kbuild script that can also be found in this report. So if you want to play along from home, feel free. :-)

root@kvm-xfstests:~# mkfs.ext4 /dev/vdc
mke2fs 1.47.0 (5-Feb-2023)
Discarding device blocks: done                            
Creating filesystem with 1310720 4k blocks and 327680 inodes
Filesystem UUID: fe434060-6731-4b40-a94a-3a8517df0660
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@kvm-xfstests:~# md5sum /dev/vdc
fd38f9f8476ad63a744d179846ee7e18  /dev/vdc
root@kvm-xfstests:~# mount -o ro /dev/vdc /mnt
[  472.893614] EXT4-fs (vdc): orphan cleanup on readonly fs
[  472.894022] EXT4-fs (vdc): mounted filesystem fe434060-6731-4b40-a94a-3a8517df0660 ro with ordered data mode. Quota mode: none.
root@kvm-xfstests:~# umount /mnt
[  475.698053] EXT4-fs (vdc): unmounting filesystem fe434060-6731-4b40-a94a-3a8517df0660.
root@kvm-xfstests:~# md5sum /dev/vdc
fd38f9f8476ad63a744d179846ee7e18  /dev/vdc

Hmm.... OK, let's try it with LUKS, even though that *really* shouldn't make a difference. The cryptsetup lukeFormat and mkfs.ext4 steps are skipped here. Also, note that I had to manually edit the .config file to enable CONFIG_DM_CRYPT, since I dm_crypt is used by xfstests, so my install-kconfig script doesn't enable CONFIG_DM_CRYPT.

root@kvm-xfstests:~# uname -a
Linux kvm-xfstests 6.4.0-rc6-xfstests-lockdep #200 SMP PREEMPT_DYNAMIC Fri Jun 23 00:33:39 EDT 2023 x86_64 GNU/Linux

root@kvm-xfstests:~# md5sum /dev/vdc
28b75cc094e1e2a62ac25a730fc1dfee  /dev/vdc
root@kvm-xfstests:~# cryptsetup luksOpen /dev/vdc test
Enter passphrase for /dev/vdc: 
root@kvm-xfstests:~# mount -o ro /dev/mapper/test /mnt
[  812.073771] EXT4-fs (dm-0): orphan cleanup on readonly fs
[  812.074306] EXT4-fs (dm-0): mounted filesystem ac3f76f1-da0a-426e-85b2-08526afb2224 ro with ordered data mode. Quota mode: none.
root@kvm-xfstests:~# umount /mnt
[  814.383016] EXT4-fs (dm-0): unmounting filesystem ac3f76f1-da0a-426e-85b2-08526afb2224.
root@kvm-xfstests:~# cryptsetup luksClose /dev/mapper/test
[  830.001992] dm-0: detected capacity change from 10452992 to 0
root@kvm-xfstests:~# md5sum /dev/vdc
28b75cc094e1e2a62ac25a730fc1dfee  /dev/vdc

I perhaps should have been more explicit in my report. The issue is not that the image is any different after the mount; indeed, the md5sums are identical before and after on my machine as well. The issue is that something is issuing writes to the backing image, which bumps the mtime of the backing image. When handling the images with rsync, a difference in mtime causes the whole image to need to be read.

Quoted 6 lines · Sean Greenslade

I perhaps should have been more explicit in my report. The issue is not that the image is any different after the mount; indeed, the md5sums are identical before and after on my machine as well. The issue is that something is issuing writes to the backing image, which bumps the mtime of the backing image. When handling the images with rsync, a difference in mtime causes the whole image to need to be read.

Ah, yes, your initial report said "small writes", but it didn't specify whether the issue was that writes were modifying the image, or just simply touching the mtime field of the backing file. I assume these must be largish fs images, since it must have made the increased rsync time noticeable?

This appears to fix the problem for me, given the clarified reproduction information. Could you please try it on your end?

- Ted

From 6bb438fa0aac4c08acd626d408cb6d4b745df7fd Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <[email protected]>
Date: Fri, 23 Jun 2023 10:18:51 -0400
Subject: [PATCH] ext4: avoid updating the superblock on a r/o mount if not
needed

This was noticed by a user who noticied that the mtime of a file backing a loopback device was getting bumped when the loopback device is mounted read/only. Note: This doesn't show up when doing a loopback mount of a file directly, via "mount -o ro /tmp/foo.img /mnt", since the loop device is set read-only when mount automatically creates loop device. However, this is noticeable for a LUKS loop device like this:

% cryptsetup luksOpen /tmp/foo.img test
% mount -o ro /dev/loop0 /mnt ; umount /mnt

or, if LUKS is not in use, if the user manually creates the loop device like this:

% losetup /dev/loop0 /tmp/foo.img
% mount -o ro /dev/loop0 /mnt ; umount /mnt

The modified mtime causes rsync to do a rolling checksum scan of the file on the local and remote side, incrementally increasing the time to rsync the not-modified-but-touched image file.

Fixes eee00237fa5e ("ext4: commit super block if fs record error when journal record without error")

Cc [email protected]

Link https://lore.kernel.org/r/ZIauBR7YiV3rVAHL@glitch open here

Reported-by Sean Greenslade <[email protected]>

Signed-off-by Theodore Ts'o <[email protected]>

---
 fs/ext4/super.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

fs/ext4/super.c +2 -0

index b3819e70093e..c638b0db3b2b 100644
@@ -5997,19 +5997,27 @@ static int ext4_load_journal(struct super_block *sb,
 		err = jbd2_journal_wipe(journal, !really_read_only);
 	if (!err) {
 		char *save = kmalloc(EXT4_S_ERR_LEN, GFP_KERNEL);
+		__le16 orig_state;
+		bool changed = false;
 
 		if (save)

Signature · 1 line

2.31.0

It seems in the original code what we were trying to do was to preseve the error information area of superblock across journal load (which I am not sure why though?)

In the new code we see if the journal load changed that area and if yes we change that back to original log but we also marked changed = true. Why?

Ah, yes, your initial report said "small writes", but it didn't specify whether the issue was that writes were modifying the image, or just simply touching the mtime field of the backing file. I assume these must be largish fs images, since it must have made the increased rsync time noticeable?

This appears to fix the problem for me, given the clarified reproduction information. Could you please try it on your end?

- Ted

From 6bb438fa0aac4c08acd626d408cb6d4b745df7fd Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <[email protected]>
Date: Fri, 23 Jun 2023 10:18:51 -0400
Subject: [PATCH] ext4: avoid updating the superblock on a r/o mount if not
needed

This was noticed by a user who noticied that the mtime of a file backing a loopback device was getting bumped when the loopback device is mounted read/only. Note: This doesn't show up when doing a loopback mount of a file directly, via "mount -o ro /tmp/foo.img /mnt", since the loop device is set read-only when mount automatically creates loop device. However, this is noticeable for a LUKS loop device like this:

% cryptsetup luksOpen /tmp/foo.img test
% mount -o ro /dev/loop0 /mnt ; umount /mnt

or, if LUKS is not in use, if the user manually creates the loop device like this:

% losetup /dev/loop0 /tmp/foo.img
% mount -o ro /dev/loop0 /mnt ; umount /mnt

The modified mtime causes rsync to do a rolling checksum scan of the file on the local and remote side, incrementally increasing the time to rsync the not-modified-but-touched image file.

Fixes eee00237fa5e ("ext4: commit super block if fs record error when journal record without error")

Cc [email protected]

Link https://lore.kernel.org/r/ZIauBR7YiV3rVAHL@glitch open here

Reported-by Sean Greenslade <[email protected]>

Signed-off-by Theodore Ts'o <[email protected]>

---
 fs/ext4/super.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

fs/ext4/super.c +10 -2

index b3819e70093e..c638b0db3b2b 100644
@@ -5997,19 +5997,27 @@ static int ext4_load_journal(struct super_block *sb,
 		err = jbd2_journal_wipe(journal, !really_read_only);
 	if (!err) {
 		char *save = kmalloc(EXT4_S_ERR_LEN, GFP_KERNEL);
+		__le16 orig_state;
+		bool changed = false;
 
 		if (save)
 			memcpy(save, ((char *) es) +
 			       EXT4_S_ERR_START, EXT4_S_ERR_LEN);
 		err = jbd2_journal_load(journal);
-		if (save)
+		if (save && memcmp(((char *) es) + EXT4_S_ERR_START,
+				   save, EXT4_S_ERR_LEN)) {
 			memcpy(((char *) es) + EXT4_S_ERR_START,
 			       save, EXT4_S_ERR_LEN);
+			changed = true;
+		}
 		kfree(save);
+		orig_state = es->s_state;
 		es->s_state |= cpu_to_le16(EXT4_SB(sb)->s_mount_state &
 					   EXT4_ERROR_FS);
+		if (orig_state != es->s_state)
+			changed = true;
 		/* Write out restored error information to the superblock */
-		if (!bdev_read_only(sb->s_bdev)) {
+		if (changed && !really_read_only) {
 			int err2;
 			err2 = ext4_commit_super(sb);
 			err = err ? : err2;

Signature · 1 line

2.31.0