All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Namjae Jeon" <namjae.jeon@samsung.com>
To: "'Tetsuhiro Kohada'" <kohada.t2@gmail.com>
Cc: <kohada.tetsuhiro@dc.mitsubishielectric.co.jp>,
	<mori.takahiro@ab.mitsubishielectric.co.jp>,
	<motai.hirotaka@aj.mitsubishielectric.co.jp>,
	"'Sungjong Seo'" <sj1557.seo@samsung.com>,
	<linux-fsdevel@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 3/3] exfat: set EXFAT_SB_DIRTY and VOL_DIRTY at the same timing
Date: Fri, 5 Jun 2020 16:32:53 +0900	[thread overview]
Message-ID: <000401d63b0b$8664f290$932ed7b0$@samsung.com> (raw)
In-Reply-To: <20200604084445.19205-3-kohada.t2@gmail.com>

> Set EXFAT_SB_DIRTY flag in exfat_put_super().
> 
> In some cases, can't clear VOL_DIRTY with 'sync'.
> 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.
Good catch.

Can you split this patch into two? (Don't set VOL_DIRTY on -ENOTEMPTY and Setting EXFAT_SB_DIRTY is
merged into exfat_set_vol_flag). I need to check the second one more.

Thanks!
> 
> Signed-off-by: Tetsuhiro Kohada <kohada.t2@gmail.com>
> ---
>  fs/exfat/balloc.c   |  4 ++--
>  fs/exfat/dir.c      | 18 ++++++++----------
>  fs/exfat/exfat_fs.h |  2 +-
>  fs/exfat/fatent.c   |  6 +-----
>  fs/exfat/misc.c     |  3 +--
>  fs/exfat/namei.c    | 12 ++++++------
>  fs/exfat/super.c    |  3 +++
>  7 files changed, 22 insertions(+), 26 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 3eb8386fb5f2..96c9a817d928 100644
> --- a/fs/exfat/dir.c
> +++ b/fs/exfat/dir.c
> @@ -468,7 +468,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); @@ -478,7 +478,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;
> @@ -514,7 +514,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;
> @@ -536,7 +536,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); @@ -545,7 +545,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++) { @@ -554,7 +554,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;
>  	}
> @@ -578,7 +578,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);
>  	}
> 
> @@ -606,10 +606,8 @@ int exfat_free_dentry_set(struct exfat_entry_set_cache *es, int sync)  {
>  	int i, err = 0;
> 
> -	if (es->modified) {
> -		set_bit(EXFAT_SB_DIRTY, &EXFAT_SB(es->sb)->s_state);
> +	if (es->modified)
>  		err = exfat_update_bhs(es->bh, es->num_bh, sync);
> -	}
> 
>  	for (i = 0; i < es->num_bh; i++)
>  		err ? bforget(es->bh[i]):brelse(es->bh[i]);
> diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h index f4fa0e833486..0e094d186612 100644
> --- a/fs/exfat/exfat_fs.h
> +++ b/fs/exfat/exfat_fs.h
> @@ -514,7 +514,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);
>  int exfat_update_bhs(struct buffer_head **bhs, int nr_bhs, int sync);  void exfat_chain_set(struct
> exfat_chain *ec, unsigned int dir,
>  		unsigned int size, unsigned char flags); diff --git a/fs/exfat/fatent.c
> b/fs/exfat/fatent.c index 5d11bc2f1b68..f8171183b4c1 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) { @@ -261,7 +260,6 @@ int exfat_zeroed_cluster(struct
> inode *dir, unsigned int clu)
>  			memset(bhs[n]->b_data, 0, sb->s_blocksize);
>  		}
> 
> -		set_bit(EXFAT_SB_DIRTY, &sbi->s_state);
>  		err = exfat_update_bhs(bhs, n, IS_DIRSYNC(dir));
>  		if (err)
>  			goto release_bhs;
> @@ -326,8 +324,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 dc34968e99d3..564718747fb2 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 5b0f35329d63..e36c9fc4a5d6 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);
> 
> @@ -1083,7 +1083,7 @@ static int exfat_rename_file(struct inode *inode, struct exfat_chain *p_dir,
>  			return -EIO;
> 
>  		memcpy(epnew, epold, DENTRY_SIZE);
> -		exfat_update_bh(sb, new_bh, sync);
> +		exfat_update_bh(new_bh, sync);
>  		brelse(old_bh);
>  		brelse(new_bh);
> 
> @@ -1100,7 +1100,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);
> @@ -1155,7 +1155,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);
> 
> @@ -1167,7 +1167,7 @@ static int exfat_move_file(struct inode *inode, struct exfat_chain *p_olddir,
>  		return -EIO;
> 
>  	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..199a1e78f9e5 100644
> --- a/fs/exfat/super.c
> +++ b/fs/exfat/super.c
> @@ -104,6 +104,9 @@ int exfat_set_vol_flags(struct super_block *sb, unsigned short new_flag)
>  	struct boot_sector *p_boot = (struct boot_sector *)sbi->boot_bh->b_data;
>  	bool sync;
> 
> +	if (new_flag == VOL_DIRTY)
> +		set_bit(EXFAT_SB_DIRTY, &sbi->s_state);
> +
>  	/* flags are not changed */
>  	if (sbi->vol_flag == new_flag)
>  		return 0;
> --
> 2.25.1



  reply	other threads:[~2020-06-05  7:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-04  8:44 [PATCH 1/3] exfat: add error check when updating dir-entries Tetsuhiro Kohada
2020-06-04  8:44 ` [PATCH 2/3] exfat: optimize exfat_zeroed_cluster() Tetsuhiro Kohada
2020-06-04  8:44 ` [PATCH 3/3] exfat: set EXFAT_SB_DIRTY and VOL_DIRTY at the same timing Tetsuhiro Kohada
2020-06-05  7:32   ` Namjae Jeon [this message]
2020-06-06  9:22     ` Tetsuhiro Kohada
2020-06-07  0:26       ` Namjae Jeon
2020-06-08  5:02         ` Kohada.Tetsuhiro
2020-06-09  2:02           ` Namjae Jeon
2020-06-07  0:33 ` [PATCH 1/3] exfat: add error check when updating dir-entries Namjae Jeon
2020-06-09  8:10   ` Tetsuhiro Kohada

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='000401d63b0b$8664f290$932ed7b0$@samsung.com' \
    --to=namjae.jeon@samsung.com \
    --cc=kohada.t2@gmail.com \
    --cc=kohada.tetsuhiro@dc.mitsubishielectric.co.jp \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mori.takahiro@ab.mitsubishielectric.co.jp \
    --cc=motai.hirotaka@aj.mitsubishielectric.co.jp \
    --cc=sj1557.seo@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.