Re: [PATCH v2] exfat: remove EXFAT_SB_DIRTY flag

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

On 2020/06/16 11:03, Sungjong Seo wrote:

remove EXFAT_SB_DIRTY flag and related codes.

This flag is set/reset in exfat_put_super()/exfat_sync_fs() to avoid
sync_blockdev().
However ...
- exfat_put_super():
Before calling this, the VFS has already called sync_filesystem(), so sync
is never performed here.
- exfat_sync_fs():
After calling this, the VFS calls sync_blockdev(), so, it is meaningless
to check EXFAT_SB_DIRTY or to bypass sync_blockdev() here.
Not only that, but in some cases can't clear VOL_DIRTY.
ex:
VOL_DIRTY is set when rmdir starts, but when non-empty-dir is detected,
return error without setting EXFAT_SB_DIRTY.
If performe 'sync' in this state, VOL_DIRTY will not be cleared.

Remove the EXFAT_SB_DIRTY check to ensure synchronization.
And, remove the code related to the flag.

Suggested-by: Sungjong Seo <sj1557.seo@xxxxxxxxxxx>
Signed-off-by: Tetsuhiro Kohada <kohada.t2@xxxxxxxxx>
---
Changes in v2:
  - exfat_sync_fs() avoids synchronous processing when wait=0

  fs/exfat/balloc.c   |  4 ++--
  fs/exfat/dir.c      | 16 ++++++++--------
  fs/exfat/exfat_fs.h |  5 +----
  fs/exfat/fatent.c   |  7 ++-----
  fs/exfat/misc.c     |  3 +--
  fs/exfat/namei.c    | 12 ++++++------
  fs/exfat/super.c    | 14 ++++++--------
  7 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/fs/exfat/balloc.c b/fs/exfat/balloc.c index
4055eb00ea9b..a987919686c0 100644
--- a/fs/exfat/balloc.c
+++ b/fs/exfat/balloc.c
@@ -158,7 +158,7 @@ int exfat_set_bitmap(struct inode *inode, unsigned int
clu)
  	b = BITMAP_OFFSET_BIT_IN_SECTOR(sb, ent_idx);

  	set_bit_le(b, sbi->vol_amap[i]->b_data);
-	exfat_update_bh(sb, sbi->vol_amap[i], IS_DIRSYNC(inode));
+	exfat_update_bh(sbi->vol_amap[i], IS_DIRSYNC(inode));
  	return 0;
  }

@@ -180,7 +180,7 @@ void exfat_clear_bitmap(struct inode *inode, unsigned
int clu)
  	b = BITMAP_OFFSET_BIT_IN_SECTOR(sb, ent_idx);

  	clear_bit_le(b, sbi->vol_amap[i]->b_data);
-	exfat_update_bh(sb, sbi->vol_amap[i], IS_DIRSYNC(inode));
+	exfat_update_bh(sbi->vol_amap[i], IS_DIRSYNC(inode));

  	if (opts->discard) {
  		int ret_discard;
diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index
8e775bd5d523..02acbb6ddf02 100644
--- a/fs/exfat/dir.c
+++ b/fs/exfat/dir.c
@@ -470,7 +470,7 @@ int exfat_init_dir_entry(struct inode *inode, struct
exfat_chain *p_dir,
  			&ep->dentry.file.access_date,
  			NULL);

-	exfat_update_bh(sb, bh, IS_DIRSYNC(inode));
+	exfat_update_bh(bh, IS_DIRSYNC(inode));
  	brelse(bh);

  	ep = exfat_get_dentry(sb, p_dir, entry + 1, &bh, &sector); @@ -
480,7 +480,7 @@ int exfat_init_dir_entry(struct inode *inode, struct
exfat_chain *p_dir,
  	exfat_init_stream_entry(ep,
  		(type == TYPE_FILE) ? ALLOC_FAT_CHAIN : ALLOC_NO_FAT_CHAIN,
  		start_clu, size);
-	exfat_update_bh(sb, bh, IS_DIRSYNC(inode));
+	exfat_update_bh(bh, IS_DIRSYNC(inode));
  	brelse(bh);

  	return 0;
@@ -516,7 +516,7 @@ int exfat_update_dir_chksum(struct inode *inode,
struct exfat_chain *p_dir,
  	}

  	fep->dentry.file.checksum = cpu_to_le16(chksum);
-	exfat_update_bh(sb, fbh, IS_DIRSYNC(inode));
+	exfat_update_bh(fbh, IS_DIRSYNC(inode));
  release_fbh:
  	brelse(fbh);
  	return ret;
@@ -538,7 +538,7 @@ int exfat_init_ext_entry(struct inode *inode, struct
exfat_chain *p_dir,
  		return -EIO;

  	ep->dentry.file.num_ext = (unsigned char)(num_entries - 1);
-	exfat_update_bh(sb, bh, sync);
+	exfat_update_bh(bh, sync);
  	brelse(bh);

  	ep = exfat_get_dentry(sb, p_dir, entry + 1, &bh, &sector); @@ -
547,7 +547,7 @@ int exfat_init_ext_entry(struct inode *inode, struct
exfat_chain *p_dir,

  	ep->dentry.stream.name_len = p_uniname->name_len;
  	ep->dentry.stream.name_hash = cpu_to_le16(p_uniname->name_hash);
-	exfat_update_bh(sb, bh, sync);
+	exfat_update_bh(bh, sync);
  	brelse(bh);

  	for (i = EXFAT_FIRST_CLUSTER; i < num_entries; i++) { @@ -556,7
+556,7 @@ int exfat_init_ext_entry(struct inode *inode, struct exfat_chain
*p_dir,
  			return -EIO;

  		exfat_init_name_entry(ep, uniname);
-		exfat_update_bh(sb, bh, sync);
+		exfat_update_bh(bh, sync);
  		brelse(bh);
  		uniname += EXFAT_FILE_NAME_LEN;
  	}
@@ -580,7 +580,7 @@ int exfat_remove_entries(struct inode *inode, struct
exfat_chain *p_dir,
  			return -EIO;

  		exfat_set_entry_type(ep, TYPE_DELETED);
-		exfat_update_bh(sb, bh, IS_DIRSYNC(inode));
+		exfat_update_bh(bh, IS_DIRSYNC(inode));
  		brelse(bh);
  	}

@@ -610,7 +610,7 @@ void exfat_free_dentry_set(struct
exfat_entry_set_cache *es, int sync)

  	for (i = 0; i < es->num_bh; i++) {
  		if (es->modified)
-			exfat_update_bh(es->sb, es->bh[i], sync);
+			exfat_update_bh(es->bh[i], sync);
  		brelse(es->bh[i]);
  	}
  	kfree(es);
diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h index
595f3117f492..84664024e51e 100644
--- a/fs/exfat/exfat_fs.h
+++ b/fs/exfat/exfat_fs.h
@@ -13,8 +13,6 @@
  #define EXFAT_SUPER_MAGIC       0x2011BAB0UL
  #define EXFAT_ROOT_INO		1

-#define EXFAT_SB_DIRTY		0
-
  #define EXFAT_CLUSTERS_UNTRACKED (~0u)

  /*
@@ -238,7 +236,6 @@ struct exfat_sb_info {
  	unsigned int clu_srch_ptr; /* cluster search pointer */
  	unsigned int used_clusters; /* number of used clusters */

-	unsigned long s_state;
  	struct mutex s_lock; /* superblock lock */
  	struct exfat_mount_options options;
  	struct nls_table *nls_io; /* Charset used for input and display */
@@ -514,7 +511,7 @@ void exfat_set_entry_time(struct exfat_sb_info *sbi,
struct timespec64 *ts,
  		u8 *tz, __le16 *time, __le16 *date, u8 *time_cs);
  u16 exfat_calc_chksum16(void *data, int len, u16 chksum, int type);
  u32 exfat_calc_chksum32(void *data, int len, u32 chksum, int type); -void
exfat_update_bh(struct super_block *sb, struct buffer_head *bh, int sync);
+void exfat_update_bh(struct buffer_head *bh, int sync);
  void exfat_chain_set(struct exfat_chain *ec, unsigned int dir,
  		unsigned int size, unsigned char flags);  void
exfat_chain_dup(struct exfat_chain *dup, struct exfat_chain *ec); diff --
git a/fs/exfat/fatent.c b/fs/exfat/fatent.c index
4e5c5c9c0f2d..82ee8246c080 100644
--- a/fs/exfat/fatent.c
+++ b/fs/exfat/fatent.c
@@ -75,7 +75,7 @@ int exfat_ent_set(struct super_block *sb, unsigned int
loc,

  	fat_entry = (__le32 *)&(bh->b_data[off]);
  	*fat_entry = cpu_to_le32(content);
-	exfat_update_bh(sb, bh, sb->s_flags & SB_SYNCHRONOUS);
+	exfat_update_bh(bh, sb->s_flags & SB_SYNCHRONOUS);
  	exfat_mirror_bh(sb, sec, bh);
  	brelse(bh);
  	return 0;
@@ -174,7 +174,6 @@ int exfat_free_cluster(struct inode *inode, struct
exfat_chain *p_chain)
  		return -EIO;
  	}

-	set_bit(EXFAT_SB_DIRTY, &sbi->s_state);
  	clu = p_chain->dir;

  	if (p_chain->flags == ALLOC_NO_FAT_CHAIN) { @@ -274,7 +273,7 @@ int
exfat_zeroed_cluster(struct inode *dir, unsigned int clu)
  			goto release_bhs;
  		}
  		memset(bhs[n]->b_data, 0, sb->s_blocksize);
-		exfat_update_bh(sb, bhs[n], 0);
+		exfat_update_bh(bhs[n], 0);

  		n++;
  		blknr++;
@@ -358,8 +357,6 @@ int exfat_alloc_cluster(struct inode *inode, unsigned
int num_alloc,
  		}
  	}

-	set_bit(EXFAT_SB_DIRTY, &sbi->s_state);
-
  	p_chain->dir = EXFAT_EOF_CLUSTER;

  	while ((new_clu = exfat_find_free_bitmap(sb, hint_clu)) != diff --
git a/fs/exfat/misc.c b/fs/exfat/misc.c index 17d41f3d3709..8a3dde59052b
100644
--- a/fs/exfat/misc.c
+++ b/fs/exfat/misc.c
@@ -163,9 +163,8 @@ u32 exfat_calc_chksum32(void *data, int len, u32
chksum, int type)
  	return chksum;
  }

-void exfat_update_bh(struct super_block *sb, struct buffer_head *bh, int
sync)
+void exfat_update_bh(struct buffer_head *bh, int sync)
  {
-	set_bit(EXFAT_SB_DIRTY, &EXFAT_SB(sb)->s_state);
  	set_buffer_uptodate(bh);
  	mark_buffer_dirty(bh);

diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c index
edd8023865a0..5eef2217fcf2 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -387,7 +387,7 @@ static int exfat_find_empty_entry(struct inode *inode,
  			ep->dentry.stream.valid_size = cpu_to_le64(size);
  			ep->dentry.stream.size =

ep->dentry.stream.valid_size;

  			ep->dentry.stream.flags = p_dir->flags;
-			exfat_update_bh(sb, bh, IS_DIRSYNC(inode));
+			exfat_update_bh(bh, IS_DIRSYNC(inode));
  			brelse(bh);
  			if (exfat_update_dir_chksum(inode, &(ei->dir),
  			    ei->entry))
@@ -1071,7 +1071,7 @@ static int exfat_rename_file(struct inode *inode,
struct exfat_chain *p_dir,
  			epnew->dentry.file.attr |=

cpu_to_le16(ATTR_ARCHIVE);

  			ei->attr |= ATTR_ARCHIVE;
  		}
-		exfat_update_bh(sb, new_bh, sync);
+		exfat_update_bh(new_bh, sync);
  		brelse(old_bh);
  		brelse(new_bh);

@@ -1087,7 +1087,7 @@ static int exfat_rename_file(struct inode *inode,
struct exfat_chain *p_dir,
  		}

  		memcpy(epnew, epold, DENTRY_SIZE);
-		exfat_update_bh(sb, new_bh, sync);
+		exfat_update_bh(new_bh, sync);
  		brelse(old_bh);
  		brelse(new_bh);

@@ -1104,7 +1104,7 @@ static int exfat_rename_file(struct inode *inode,
struct exfat_chain *p_dir,
  			epold->dentry.file.attr |=

cpu_to_le16(ATTR_ARCHIVE);

  			ei->attr |= ATTR_ARCHIVE;
  		}
-		exfat_update_bh(sb, old_bh, sync);
+		exfat_update_bh(old_bh, sync);
  		brelse(old_bh);
  		ret = exfat_init_ext_entry(inode, p_dir, oldentry,
  			num_new_entries, p_uniname);
@@ -1159,7 +1159,7 @@ static int exfat_move_file(struct inode *inode,
struct exfat_chain *p_olddir,
  		epnew->dentry.file.attr |= cpu_to_le16(ATTR_ARCHIVE);
  		ei->attr |= ATTR_ARCHIVE;
  	}
-	exfat_update_bh(sb, new_bh, IS_DIRSYNC(inode));
+	exfat_update_bh(new_bh, IS_DIRSYNC(inode));
  	brelse(mov_bh);
  	brelse(new_bh);

@@ -1175,7 +1175,7 @@ static int exfat_move_file(struct inode *inode,
struct exfat_chain *p_olddir,
  	}

  	memcpy(epnew, epmov, DENTRY_SIZE);
-	exfat_update_bh(sb, new_bh, IS_DIRSYNC(inode));
+	exfat_update_bh(new_bh, IS_DIRSYNC(inode));
  	brelse(mov_bh);
  	brelse(new_bh);

diff --git a/fs/exfat/super.c b/fs/exfat/super.c index
e650e65536f8..49804d369b51 100644
--- a/fs/exfat/super.c
+++ b/fs/exfat/super.c
@@ -45,9 +45,6 @@ static void exfat_put_super(struct super_block *sb)
  	struct exfat_sb_info *sbi = EXFAT_SB(sb);

  	mutex_lock(&sbi->s_lock);
-	if (test_and_clear_bit(EXFAT_SB_DIRTY, &sbi->s_state))
-		sync_blockdev(sb->s_bdev);
-	exfat_set_vol_flags(sb, VOL_CLEAN);
  	exfat_free_bitmap(sbi);
  	brelse(sbi->boot_bh);
  	mutex_unlock(&sbi->s_lock);
@@ -60,13 +57,14 @@ static int exfat_sync_fs(struct super_block *sb, int
wait)
  	struct exfat_sb_info *sbi = EXFAT_SB(sb);
  	int err = 0;

+	if (!wait)
+		return;


Need to return 0.


I'm sorry.
Reconsider the check before posting.


+
  	/* If there are some dirty buffers in the bdev inode */
  	mutex_lock(&sbi->s_lock);
-	if (test_and_clear_bit(EXFAT_SB_DIRTY, &sbi->s_state)) {
-		sync_blockdev(sb->s_bdev);
-		if (exfat_set_vol_flags(sb, VOL_CLEAN))
-			err = -EIO;
-	}
+	sync_blockdev(sb->s_bdev);
+	if (exfat_set_vol_flags(sb, VOL_CLEAN))
+		err = -EIO;
  	mutex_unlock(&sbi->s_lock);
  	return err;
  }
--
2.25.1
[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux