* [PATCH v2] exfat: remove EXFAT_SB_DIRTY flag @ 2020-06-16 0:24 ` Tetsuhiro Kohada 2020-06-16 2:03 ` Sungjong Seo 0 siblings, 1 reply; 3+ messages in thread From: Tetsuhiro Kohada @ 2020-06-16 0:24 UTC (permalink / raw) To: kohada.t2 Cc: kohada.tetsuhiro, mori.takahiro, motai.hirotaka, Sungjong Seo, Namjae Jeon, linux-fsdevel, linux-kernel 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@samsung.com> Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* RE: [PATCH v2] exfat: remove EXFAT_SB_DIRTY flag 2020-06-16 0:24 ` [PATCH v2] exfat: remove EXFAT_SB_DIRTY flag Tetsuhiro Kohada @ 2020-06-16 2:03 ` Sungjong Seo 2020-06-16 2:23 ` Tetsuhiro Kohada 0 siblings, 1 reply; 3+ messages in thread From: Sungjong Seo @ 2020-06-16 2:03 UTC (permalink / raw) To: 'Tetsuhiro Kohada' Cc: kohada.tetsuhiro, mori.takahiro, motai.hirotaka, 'Namjae Jeon', linux-fsdevel, linux-kernel > 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@samsung.com> > Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com> > --- > 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] exfat: remove EXFAT_SB_DIRTY flag 2020-06-16 2:03 ` Sungjong Seo @ 2020-06-16 2:23 ` Tetsuhiro Kohada 0 siblings, 0 replies; 3+ messages in thread From: Tetsuhiro Kohada @ 2020-06-16 2:23 UTC (permalink / raw) To: Sungjong Seo Cc: kohada.tetsuhiro, mori.takahiro, motai.hirotaka, 'Namjae Jeon', linux-fsdevel, linux-kernel 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@samsung.com> >> Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com> >> --- >> 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 > > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-06-16 2:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20200616002500epcas1p19135f6f329fbf8c90df1b3ba8a1f67f7@epcas1p1.samsung.com>
2020-06-16 0:24 ` [PATCH v2] exfat: remove EXFAT_SB_DIRTY flag Tetsuhiro Kohada
2020-06-16 2:03 ` Sungjong Seo
2020-06-16 2:23 ` Tetsuhiro Kohada
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).