[RFC PATCH v1 3/6] exfat: use atomic bit ops for volume dirty flag

Chi Zhiling <[email protected]>
Newsgroups dev.linux.lists.exfat,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
From: Chi Zhiling <[email protected]>

After converting s_lock to a reader-writer lock, multiple operations can
execute concurrently under the read lock. As a result, multiple threads may
concurrently update sbi->vol_flags, causing a race in the existing
read-modify-write sequence.

Convert sbi->vol_flags to an unsigned long bitmap and use
test_and_set_bit() / test_and_clear_bit() to atomically update the volume
dirty bit. Only the thread performing the 0 -> 1 transition updates the
boot sector, avoiding concurrent unsynchronized modifications of vol_flags
and the boot-sector buffer.

Remove vol_flags_persistent, since the bitmap can directly retain the
MEDIA_FAILURE state while preserving the complete 16-bit on-disk volume
flags field when writing it back.

Signed-off-by: Chi Zhiling <[email protected]>
---
 fs/exfat/exfat_fs.h  |  3 +--
 fs/exfat/exfat_raw.h |  4 ++--
 fs/exfat/super.c     | 28 ++++++++++++----------------
 3 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
index a9131fe03302..f1505c013248 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -232,8 +232,7 @@ struct exfat_sb_info {
 	unsigned int num_FAT_sectors; /* num of FAT sectors */
 	unsigned int root_dir; /* root dir cluster */
 	unsigned int dentries_per_clu; /* num of dentries per cluster */
-	unsigned int vol_flags; /* volume flags */
-	unsigned int vol_flags_persistent; /* volume flags to retain */
+	unsigned long vol_flags; /* volume flags (bitmap) */
 	struct buffer_head *boot_bh; /* buffer_head of BOOT sector */
 
 	unsigned int map_clu; /* allocation bitmap start cluster */
diff --git a/fs/exfat/exfat_raw.h b/fs/exfat/exfat_raw.h
index ec70cd35bba0..222fee5c2adf 100644
--- a/fs/exfat/exfat_raw.h
+++ b/fs/exfat/exfat_raw.h
@@ -14,8 +14,8 @@
 
 #define EXFAT_MAX_FILE_LEN	255
 
-#define VOLUME_DIRTY		0x0002
-#define MEDIA_FAILURE		0x0004
+#define VOLUME_DIRTY_BIT	1
+#define MEDIA_FAILURE_BIT	2
 
 #define EXFAT_EOF_CLUSTER	0xFFFFFFFFu
 #define EXFAT_BAD_CLUSTER	0xFFFFFFF7u
diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index a9ea36ba2693..491273d8eeb6 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -69,27 +69,18 @@ static int exfat_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return 0;
 }
 
-static int exfat_set_vol_flags(struct super_block *sb, unsigned short new_flags)
+static int exfat_sync_vol_flags(struct super_block *sb)
 {
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
 	struct boot_sector *p_boot = (struct boot_sector *)sbi->boot_bh->b_data;
 
-	/* retain persistent-flags */
-	new_flags |= sbi->vol_flags_persistent;
-
-	/* flags are not changed */
-	if (sbi->vol_flags == new_flags)
-		return 0;
-
-	sbi->vol_flags = new_flags;
-
 	/* skip updating volume dirty flag,
 	 * if this volume has been mounted with read-only
 	 */
 	if (sb_rdonly(sb))
 		return 0;
 
-	p_boot->vol_flags = cpu_to_le16(new_flags);
+	p_boot->vol_flags = cpu_to_le16((unsigned short)READ_ONCE(sbi->vol_flags));
 
 	set_buffer_uptodate(sbi->boot_bh);
 	mark_buffer_dirty(sbi->boot_bh);
@@ -103,14 +94,20 @@ int exfat_set_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);
+	if (test_and_set_bit(VOLUME_DIRTY_BIT, &sbi->vol_flags))
+		return 0;
+
+	return exfat_sync_vol_flags(sb);
 }
 
 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);
+	if (!test_and_clear_bit(VOLUME_DIRTY_BIT, &sbi->vol_flags))
+		return 0;
+
+	return exfat_sync_vol_flags(sb);
 }
 
 static int exfat_show_options(struct seq_file *m, struct dentry *root)
@@ -509,7 +506,6 @@ static int exfat_read_boot_sector(struct super_block *sb)
 		(sbi->cluster_size_bits - DENTRY_SIZE_BITS);
 
 	sbi->vol_flags = le16_to_cpu(p_boot->vol_flags);
-	sbi->vol_flags_persistent = sbi->vol_flags & (VOLUME_DIRTY | MEDIA_FAILURE);
 	sbi->clu_srch_ptr = EXFAT_FIRST_CLUSTER;
 
 	/* check consistencies */
@@ -526,9 +522,9 @@ static int exfat_read_boot_sector(struct super_block *sb)
 		return -EINVAL;
 	}
 
-	if (sbi->vol_flags & VOLUME_DIRTY)
+	if (test_bit(VOLUME_DIRTY_BIT, &sbi->vol_flags))
 		exfat_warn(sb, "Volume was not properly unmounted. Some data may be corrupt. Please run fsck.");
-	if (sbi->vol_flags & MEDIA_FAILURE)
+	if (test_bit(MEDIA_FAILURE_BIT, &sbi->vol_flags))
 		exfat_warn(sb, "Medium has reported failures. Some data may be lost.");
 
 	/*
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.