[PATCH 1/1] hfsplus: skip unnecessary volume header sync

[Sougata Santra <sougata@tuxera.com>]

hfsplus_sync_fs always updates volume header information to disk with every
sync. This causes problem for systems trying to monitor disk activity to
switch them to low power state. Also hfsplus_sync_fs is explicitly called
from mount/unmount, which is unnecessary. During mount/unmount we only want
to set/clear volume dirty sate.

Signed-off-by: Sougata Santra <sougata@tuxera.com>
---
 fs/hfsplus/super.c | 101 +++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 79 insertions(+), 22 deletions(-)

diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 4cf2024..5cacb06 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -170,12 +170,61 @@ static void hfsplus_evict_inode(struct inode *inode)
 	}
 }
 
+/*
+ * Helper to sync volume header state to disk. Caller must acquire
+ * volume header mutex (vh_mutex).
+ */
+static int hfsplus_sync_volume_header_locked(struct super_block *sb)
+{
+	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
+	int write_backup = 0;
+	int error = 0;
+
+	if (test_and_clear_bit(HFSPLUS_SB_WRITEBACKUP, &sbi->flags)) {
+		memcpy(sbi->s_backup_vhdr, sbi->s_vhdr, sizeof(*sbi->s_vhdr));
+		write_backup = 1;
+	}
+
+	error = hfsplus_submit_bio(sb,
+			sbi->part_start + HFSPLUS_VOLHEAD_SECTOR,
+			sbi->s_vhdr_buf, NULL, WRITE_SYNC);
+
+	if (error || !write_backup)
+		goto out;
+
+	error = hfsplus_submit_bio(sb,
+			sbi->part_start + sbi->sect_count - 2,
+			sbi->s_backup_vhdr_buf, NULL, WRITE_SYNC);
+out:
+	return error;
+}
+
+/* Sync dirty/clean volume header state to disk. */
+static int hfsplus_sync_volume_header(struct super_block *sb)
+{
+	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
+	int error = 0;
+
+	hfs_dbg(SUPER, "hfsplus_sync_volume_header\n");
+
+	mutex_lock(&sbi->vh_mutex);
+	error = hfsplus_sync_volume_header_locked(sb);
+	mutex_unlock(&sbi->vh_mutex);
+
+	if (!error && !test_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags))
+		blkdev_issue_flush(sb->s_bdev, GFP_KERNEL, NULL);
+
+	return error;
+}
+
 static int hfsplus_sync_fs(struct super_block *sb, int wait)
 {
 	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
 	struct hfsplus_vh *vhdr = sbi->s_vhdr;
-	int write_backup = 0;
+	int write_header = 0;
 	int error, error2;
+	u32 free_blocks, next_cnid;
+	u32 folder_count, file_count;
 
 	if (!wait)
 		return 0;
@@ -196,7 +245,8 @@ static int hfsplus_sync_fs(struct super_block *sb, int wait)
 		error = error2;
 	if (sbi->attr_tree) {
 		error2 =
-		    filemap_write_and_wait(sbi->attr_tree->inode->i_mapping);
+			filemap_write_and_wait(
+					sbi->attr_tree->inode->i_mapping);
 		if (!error)
 			error = error2;
 	}
@@ -206,34 +256,41 @@ static int hfsplus_sync_fs(struct super_block *sb, int wait)
 
 	mutex_lock(&sbi->vh_mutex);
 	mutex_lock(&sbi->alloc_mutex);
-	vhdr->free_blocks = cpu_to_be32(sbi->free_blocks);
-	vhdr->next_cnid = cpu_to_be32(sbi->next_cnid);
-	vhdr->folder_count = cpu_to_be32(sbi->folder_count);
-	vhdr->file_count = cpu_to_be32(sbi->file_count);
 
-	if (test_and_clear_bit(HFSPLUS_SB_WRITEBACKUP, &sbi->flags)) {
-		memcpy(sbi->s_backup_vhdr, sbi->s_vhdr, sizeof(*sbi->s_vhdr));
-		write_backup = 1;
+	free_blocks = cpu_to_be32(sbi->free_blocks);
+	next_cnid = cpu_to_be32(sbi->next_cnid);
+	folder_count = cpu_to_be32(sbi->folder_count);
+	file_count = cpu_to_be32(sbi->file_count);
+
+	/* Check if some attribute of volume header has changed. */
+	if (vhdr->free_blocks != free_blocks ||
+			vhdr->next_cnid != next_cnid ||
+			vhdr->folder_count != folder_count ||
+			vhdr->file_count != file_count) {
+		vhdr->free_blocks = free_blocks;
+		vhdr->next_cnid = next_cnid;
+		vhdr->folder_count = folder_count;
+		vhdr->file_count = file_count;
+		write_header = 1;
 	}
+	/*
+	 * Write volume header only when something has changed. Also there
+	 * is no need to write backup volume header if nothing has changed
+	 * in the the volume header itself.
+	 */
+	if (!write_header)
+		goto out;
 
-	error2 = hfsplus_submit_bio(sb,
-				   sbi->part_start + HFSPLUS_VOLHEAD_SECTOR,
-				   sbi->s_vhdr_buf, NULL, WRITE_SYNC);
+	error2 = hfsplus_sync_volume_header_locked(sb);
 	if (!error)
 		error = error2;
-	if (!write_backup)
-		goto out;
 
-	error2 = hfsplus_submit_bio(sb,
-				  sbi->part_start + sbi->sect_count - 2,
-				  sbi->s_backup_vhdr_buf, NULL, WRITE_SYNC);
-	if (!error)
-		error2 = error;
 out:
 	mutex_unlock(&sbi->alloc_mutex);
 	mutex_unlock(&sbi->vh_mutex);
 
-	if (!test_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags))
+	if (write_header && !error &&
+			!test_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags))
 		blkdev_issue_flush(sb->s_bdev, GFP_KERNEL, NULL);
 
 	return error;
@@ -287,7 +344,7 @@ static void hfsplus_put_super(struct super_block *sb)
 		vhdr->attributes |= cpu_to_be32(HFSPLUS_VOL_UNMNT);
 		vhdr->attributes &= cpu_to_be32(~HFSPLUS_VOL_INCNSTNT);
 
-		hfsplus_sync_fs(sb, 1);
+		hfsplus_sync_volume_header(sb);
 	}
 
 	hfs_btree_close(sbi->attr_tree);
@@ -539,7 +596,7 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent)
 		be32_add_cpu(&vhdr->write_count, 1);
 		vhdr->attributes &= cpu_to_be32(~HFSPLUS_VOL_UNMNT);
 		vhdr->attributes |= cpu_to_be32(HFSPLUS_VOL_INCNSTNT);
-		hfsplus_sync_fs(sb, 1);
+		hfsplus_sync_volume_header(sb);
 
 		if (!sbi->hidden_dir) {
 			mutex_lock(&sbi->vh_mutex);