diff mbox series

[v3] exfat: do not clear VolumeDirty in writeback

Message ID HK2PR04MB3891EE32B58A61D3ED9944CE81139@HK2PR04MB3891.apcprd04.prod.outlook.com (mailing list archive)
State New, archived
Headers show
Series [v3] exfat: do not clear VolumeDirty in writeback | expand

Commit Message

Yuezhang.Mo@sony.com March 18, 2022, 7:15 a.m. UTC 
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 but not cleared when updating
the parent directory. It means that BootSector will be written at
least once in each write-back, which will shorten the life of the
device.

Reviewed-by: Andy Wu <Andy.Wu@sony.com>
Reviewed-by: Aoyama Wataru <wataru.aoyama@sony.com>
Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
---

Changes for v2:
  - Clear VolumeDirty until sync or umount is run

Changes for v3:
  - Add REQ_FUA and REQ_PREFLUSH to guarantee strict write ordering

 fs/exfat/file.c  |  2 --
 fs/exfat/namei.c |  5 -----
 fs/exfat/super.c | 10 ++--------
 3 files changed, 2 insertions(+), 15 deletions(-)

Comments

Kohada.Tetsuhiro@dc.MitsubishiElectric.co.jp March 22, 2022, 5:51 a.m. UTC | #1 
> 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 but not cleared when updating the 
> parent directory. It means that BootSector will be written at least once in each write-back, which will shorten the life of the device.
> 
> Reviewed-by: Andy Wu <Andy.Wu@sony.com>
> Reviewed-by: Aoyama Wataru <wataru.aoyama@sony.com>
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> ---
> 
> Changes for v2:
>   - Clear VolumeDirty until sync or umount is run
> 
> Changes for v3:
>   - Add REQ_FUA and REQ_PREFLUSH to guarantee strict write ordering
> 
>  fs/exfat/file.c  |  2 --
>  fs/exfat/namei.c |  5 -----
>  fs/exfat/super.c | 10 ++--------
>  3 files changed, 2 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/exfat/file.c b/fs/exfat/file.c index 
> d890fd34bb2d..2f5130059236 100644
> --- a/fs/exfat/file.c
> +++ b/fs/exfat/file.c
> @@ -218,8 +218,6 @@ int __exfat_truncate(struct inode *inode, loff_t new_size)
>  	if (exfat_free_cluster(inode, &clu))
>  		return -EIO;
> 
> -	exfat_clear_volume_dirty(sb);
> -
>  	return 0;
>  }
> 
> diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c index 
> af4eb39cc0c3..39c9bdd6b6aa 100644
> --- a/fs/exfat/namei.c
> +++ b/fs/exfat/namei.c
> @@ -554,7 +554,6 @@ static int exfat_create(struct user_namespace *mnt_userns, struct inode *dir,
>  	exfat_set_volume_dirty(sb);
>  	err = exfat_add_entry(dir, dentry->d_name.name, &cdir, TYPE_FILE,
>  		&info);
> -	exfat_clear_volume_dirty(sb);
>  	if (err)
>  		goto unlock;
> 
> @@ -812,7 +811,6 @@ static int exfat_unlink(struct inode *dir, struct 
> dentry *dentry)
> 
>  	/* This doesn't modify ei */
>  	ei->dir.dir = DIR_DELETED;
> -	exfat_clear_volume_dirty(sb);
> 
>  	inode_inc_iversion(dir);
>  	dir->i_mtime = dir->i_atime = current_time(dir); @@ -846,7 +844,6 @@ 
> static int exfat_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
>  	exfat_set_volume_dirty(sb);
>  	err = exfat_add_entry(dir, dentry->d_name.name, &cdir, TYPE_DIR,
>  		&info);
> -	exfat_clear_volume_dirty(sb);
>  	if (err)
>  		goto unlock;
> 
> @@ -976,7 +973,6 @@ static int exfat_rmdir(struct inode *dir, struct dentry *dentry)
>  		goto unlock;
>  	}
>  	ei->dir.dir = DIR_DELETED;
> -	exfat_clear_volume_dirty(sb);
> 
>  	inode_inc_iversion(dir);
>  	dir->i_mtime = dir->i_atime = current_time(dir); @@ -1311,7 +1307,6 
> @@ static int __exfat_rename(struct inode *old_parent_inode,
>  		 */
>  		new_ei->dir.dir = DIR_DELETED;
>  	}
> -	exfat_clear_volume_dirty(sb);
>  out:
>  	return ret;
>  }
> diff --git a/fs/exfat/super.c b/fs/exfat/super.c index 
> 8c9fb7dcec16..c1f7f7b7c4ab 100644
> --- a/fs/exfat/super.c
> +++ b/fs/exfat/super.c
> @@ -100,7 +100,6 @@ static int exfat_set_vol_flags(struct super_block *sb, unsigned short new_flags)  {
>  	struct exfat_sb_info *sbi = EXFAT_SB(sb);
>  	struct boot_sector *p_boot = (struct boot_sector *)sbi->boot_bh->b_data;
> -	bool sync;
> 
>  	/* retain persistent-flags */
>  	new_flags |= sbi->vol_flags_persistent; @@ -119,16 +118,11 @@ static 
> int exfat_set_vol_flags(struct super_block *sb, unsigned short 
> new_flags)
> 
>  	p_boot->vol_flags = cpu_to_le16(new_flags);
> 
> -	if ((new_flags & VOLUME_DIRTY) && !buffer_dirty(sbi->boot_bh))
> -		sync = true;
> -	else
> -		sync = false;
> -
>  	set_buffer_uptodate(sbi->boot_bh);
>  	mark_buffer_dirty(sbi->boot_bh);
> 
> -	if (sync)
> -		sync_dirty_buffer(sbi->boot_bh);
> +	__sync_dirty_buffer(sbi->boot_bh, REQ_SYNC | REQ_FUA | 
> +REQ_PREFLUSH);
> +
>  	return 0;
>  }

Looks good.

BR.
T.Kohada
diff mbox series

Patch

diff --git a/fs/exfat/file.c b/fs/exfat/file.c
index d890fd34bb2d..2f5130059236 100644
--- a/fs/exfat/file.c
+++ b/fs/exfat/file.c
@@ -218,8 +218,6 @@  int __exfat_truncate(struct inode *inode, loff_t new_size)
 	if (exfat_free_cluster(inode, &clu))
 		return -EIO;
 
-	exfat_clear_volume_dirty(sb);
-
 	return 0;
 }
 
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index af4eb39cc0c3..39c9bdd6b6aa 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -554,7 +554,6 @@  static int exfat_create(struct user_namespace *mnt_userns, struct inode *dir,
 	exfat_set_volume_dirty(sb);
 	err = exfat_add_entry(dir, dentry->d_name.name, &cdir, TYPE_FILE,
 		&info);
-	exfat_clear_volume_dirty(sb);
 	if (err)
 		goto unlock;
 
@@ -812,7 +811,6 @@  static int exfat_unlink(struct inode *dir, struct dentry *dentry)
 
 	/* This doesn't modify ei */
 	ei->dir.dir = DIR_DELETED;
-	exfat_clear_volume_dirty(sb);
 
 	inode_inc_iversion(dir);
 	dir->i_mtime = dir->i_atime = current_time(dir);
@@ -846,7 +844,6 @@  static int exfat_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
 	exfat_set_volume_dirty(sb);
 	err = exfat_add_entry(dir, dentry->d_name.name, &cdir, TYPE_DIR,
 		&info);
-	exfat_clear_volume_dirty(sb);
 	if (err)
 		goto unlock;
 
@@ -976,7 +973,6 @@  static int exfat_rmdir(struct inode *dir, struct dentry *dentry)
 		goto unlock;
 	}
 	ei->dir.dir = DIR_DELETED;
-	exfat_clear_volume_dirty(sb);
 
 	inode_inc_iversion(dir);
 	dir->i_mtime = dir->i_atime = current_time(dir);
@@ -1311,7 +1307,6 @@  static int __exfat_rename(struct inode *old_parent_inode,
 		 */
 		new_ei->dir.dir = DIR_DELETED;
 	}
-	exfat_clear_volume_dirty(sb);
 out:
 	return ret;
 }
diff --git a/fs/exfat/super.c b/fs/exfat/super.c
index 8c9fb7dcec16..c1f7f7b7c4ab 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -100,7 +100,6 @@  static int exfat_set_vol_flags(struct super_block *sb, unsigned short new_flags)
 {
 	struct exfat_sb_info *sbi = EXFAT_SB(sb);
 	struct boot_sector *p_boot = (struct boot_sector *)sbi->boot_bh->b_data;
-	bool sync;
 
 	/* retain persistent-flags */
 	new_flags |= sbi->vol_flags_persistent;
@@ -119,16 +118,11 @@  static int exfat_set_vol_flags(struct super_block *sb, unsigned short new_flags)
 
 	p_boot->vol_flags = cpu_to_le16(new_flags);
 
-	if ((new_flags & VOLUME_DIRTY) && !buffer_dirty(sbi->boot_bh))
-		sync = true;
-	else
-		sync = false;
-
 	set_buffer_uptodate(sbi->boot_bh);
 	mark_buffer_dirty(sbi->boot_bh);
 
-	if (sync)
-		sync_dirty_buffer(sbi->boot_bh);
+	__sync_dirty_buffer(sbi->boot_bh, REQ_SYNC | REQ_FUA | REQ_PREFLUSH);
+
 	return 0;
 }