linux-kernel - RE: [PATCH] exfat: do not clear VolumeDirty in writeback

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <HK2PR04MB3891BFAB9DD271F5330D37A1812D9@HK2PR04MB3891.apcprd04.prod.outlook.com>
Date:   Tue, 8 Feb 2022 06:43:23 +0000
From:   "Yuezhang.Mo@...y.com" <Yuezhang.Mo@...y.com>
To:     "linkinjeon@...nel.org" <linkinjeon@...nel.org>,
        "sj1557.seo@...sung.com" <sj1557.seo@...sung.com>
CC:     "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] exfat: do not clear VolumeDirty in writeback

From the exFAT spec, VolumeDirty should be cleared after updating the directory entry.

1. Set the value of the VolumeDirty field to 1
2. Update the active FAT, if necessary
3. Update the active Allocation Bitmap
4. Create or update the directory entry, if necessary
5. Clear the value of the VolumeDirty field to 0, if its value prior to the first step was 0

But VolumeDirty will be cleared first in writeback if 'dirsync' or 'sync' is not enabled.
Refer the blktrace log of 'mkdir /mnt/test/dir1' as an example.

179,3    0        1     0.000000000    10  C  WS 2623488 + 1 [0]         <= Set VolumeDirty
179,3    3        1     5.052262001    26  C   W 2623488 + 1 [0]         <= Clear VolumeDirty
179,3    3        2     5.054685667    26  C   W 2627584 + 1 [0]         <= Bitmap
179,3    3        3     5.056795667    26  C   W 2628352 + 1 [0]         <= Body data of /test/
179,3    3        4     5.066790001    26  C   W 2628608 + 128 [0]       <= Body data of /test/dir1/
179,3    3        5     5.075998667    26  C   W 2628736 + 128 [0]       <= Body data of /test/dir1/
179,3    3        6     5.078409334     0  C  WS 2623488 + 1 [0]         <= Set VolumeDirty
179,3    3        7    20.239486002     0  C   W 2628096 + 1 [0]         <= Body data of /

After applying this patch, VolumeDirty will not be cleared until 'sync' or 'umount' is performed.

179,3    2        1     0.000000000     0  C  WS 2623488 + 1 [0]         <= Set VolumeDirty
179,3    0        1    30.221475670     9  C   W 2627584 + 1 [0]         <= Bitmap
179,3    0        2    30.223794337     9  C   W 2628352 + 1 [0]         <= Body data of /test/
179,3    0        3    30.233164003     9  C   W 2629376 + 128 [0]       <= Body data of /test/dir1/
179,3    0        4    30.242449670     9  C   W 2629504 + 128 [0]       <= Body data of /test/dir1/
179,3    0        5    60.445985007     9  C   W 2628096 + 1 [0]         <= Body data of /


-----Original Message-----
From: Mo, Yuezhang 
Sent: Tuesday, February 8, 2022 1:19 PM
To: linkinjeon@...nel.org; sj1557.seo@...sung.com
Cc: linux-fsdevel@...r.kernel.org; linux-kernel@...r.kernel.org
Subject: [PATCH] exfat: do not clear VolumeDirty in writeback

Before this commit, VolumeDirty will be cleared first in writeback if 'dirsync' or 'sync' is not enabled. If the power is suddenly cut off after cleaning VolumeDirty but other updates are not written, the exFAT filesystem will not be able to detect the power failure in the next mount.

And VolumeDirty will be set again when updating the parent directory. It means that BootSector will be written twice in each writeback, that will shorten the life of the device.

Reviewed-by: Andy.Wu <Andy.Wu@...y.com>
Reviewed-by: Aoyama, Wataru <wataru.aoyama@...y.com>
Signed-off-by: Yuezhang.Mo <Yuezhang.Mo@...y.com>
---
 fs/exfat/super.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/exfat/super.c b/fs/exfat/super.c index 8c9fb7dcec16..f4906c17475e 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -25,6 +25,8 @@
 static char exfat_default_iocharset[] = CONFIG_EXFAT_DEFAULT_IOCHARSET;  static struct kmem_cache *exfat_inode_cachep;
 
+static int __exfat_clear_volume_dirty(struct super_block *sb);
+
 static void exfat_free_iocharset(struct exfat_sb_info *sbi)  {
 	if (sbi->options.iocharset != exfat_default_iocharset) @@ -64,7 +66,7 @@ static int exfat_sync_fs(struct super_block *sb, int wait)
 	/* If there are some dirty buffers in the bdev inode */
 	mutex_lock(&sbi->s_lock);
 	sync_blockdev(sb->s_bdev);
-	if (exfat_clear_volume_dirty(sb))
+	if (__exfat_clear_volume_dirty(sb))
 		err = -EIO;
 	mutex_unlock(&sbi->s_lock);
 	return err;
@@ -139,13 +141,21 @@ int exfat_set_volume_dirty(struct super_block *sb)
 	return exfat_set_vol_flags(sb, sbi->vol_flags | VOLUME_DIRTY);  }
 
-int exfat_clear_volume_dirty(struct super_block *sb)
+static int __exfat_clear_volume_dirty(struct super_block *sb)
 {
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
 
 	return exfat_set_vol_flags(sb, sbi->vol_flags & ~VOLUME_DIRTY);  }
 
+int exfat_clear_volume_dirty(struct super_block *sb) {
+	if (sb->s_flags & (SB_SYNCHRONOUS | SB_DIRSYNC))
+		return __exfat_clear_volume_dirty(sb);
+
+	return 0;
+}
+
 static int exfat_show_options(struct seq_file *m, struct dentry *root)  {
 	struct super_block *sb = root->d_sb;
--
2.25.1