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 <[email protected]>
Signed-off-by: Tetsuhiro Kohada <[email protected]>
---
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, §or);
@@ -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, §or);
@@ -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;
+
/* 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
> 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 <[email protected]>
> Signed-off-by: Tetsuhiro Kohada <[email protected]>
> ---
> 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, §or); @@ -
> 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, §or); @@ -
> 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.
> +
> /* 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
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 <[email protected]>
>> Signed-off-by: Tetsuhiro Kohada <[email protected]>
>> ---
>> 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, §or); @@ -
>> 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, §or); @@ -
>> 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
>
>