Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1485843 > unrolled thread

[PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags

Started byChao Yu <chao@kernel.org>
First post2016-09-18 17:40 +0200
Last post2016-09-20 05:00 +0200
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags Chao Yu <chao@kernel.org> - 2016-09-18 17:40 +0200
    Re: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of  ckpt_flags Jaegeuk Kim <jaegeuk@kernel.org> - 2016-09-19 23:50 +0200
      Re: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of  ckpt_flags Chao Yu <yuchao0@huawei.com> - 2016-09-20 03:50 +0200
        Re: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of  ckpt_flags Jaegeuk Kim <jaegeuk@kernel.org> - 2016-09-20 04:50 +0200
          Re: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of  ckpt_flags Chao Yu <yuchao0@huawei.com> - 2016-09-20 05:00 +0200

#1485843 — [PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags

FromChao Yu <chao@kernel.org>
Date2016-09-18 17:40 +0200
Subject[PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags
Message-ID<siMmd-1ol-15@gated-at.bofh.it>
From: Chao Yu <yuchao0@huawei.com>

This patch introduces spinlock to protect updating process of ckpt_flags
field in struct f2fs_checkpoint, it avoids incorrectly updating in race
condition.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/checkpoint.c | 24 ++++++++++++------------
 fs/f2fs/f2fs.h       | 29 +++++++++++++++++++++--------
 fs/f2fs/recovery.c   |  2 +-
 fs/f2fs/segment.c    |  4 ++--
 fs/f2fs/super.c      |  5 +++--
 5 files changed, 39 insertions(+), 25 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index df56a43..0338f8c 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -28,7 +28,7 @@ struct kmem_cache *inode_entry_slab;
 
 void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io)
 {
-	set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
+	set_ckpt_flags(sbi, CP_ERROR_FLAG);
 	sbi->sb->s_flags |= MS_RDONLY;
 	if (!end_io)
 		f2fs_flush_merged_bios(sbi);
@@ -571,7 +571,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
 	block_t start_blk, orphan_blocks, i, j;
 	int err;
 
-	if (!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG))
+	if (!is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG))
 		return 0;
 
 	start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi);
@@ -595,7 +595,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
 		f2fs_put_page(page, 1);
 	}
 	/* clear Orphan Flag */
-	clear_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG);
+	clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
 	return 0;
 }
 
@@ -1054,9 +1054,9 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 	/* 2 cp  + n data seg summary + orphan inode blocks */
 	data_sum_blocks = npages_for_summary_flush(sbi, false);
 	if (data_sum_blocks < NR_CURSEG_DATA_TYPE)
-		set_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG);
+		set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG);
 	else
-		clear_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG);
+		clear_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG);
 
 	orphan_blocks = GET_ORPHAN_BLOCKS(orphan_num);
 	ckpt->cp_pack_start_sum = cpu_to_le32(1 + cp_payload_blks +
@@ -1072,22 +1072,22 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 				orphan_blocks);
 
 	if (cpc->reason == CP_UMOUNT)
-		set_ckpt_flags(ckpt, CP_UMOUNT_FLAG);
+		set_ckpt_flags(sbi, CP_UMOUNT_FLAG);
 	else
-		clear_ckpt_flags(ckpt, CP_UMOUNT_FLAG);
+		clear_ckpt_flags(sbi, CP_UMOUNT_FLAG);
 
 	if (cpc->reason == CP_FASTBOOT)
-		set_ckpt_flags(ckpt, CP_FASTBOOT_FLAG);
+		set_ckpt_flags(sbi, CP_FASTBOOT_FLAG);
 	else
-		clear_ckpt_flags(ckpt, CP_FASTBOOT_FLAG);
+		clear_ckpt_flags(sbi, CP_FASTBOOT_FLAG);
 
 	if (orphan_num)
-		set_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG);
+		set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
 	else
-		clear_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG);
+		clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
 
 	if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
-		set_ckpt_flags(ckpt, CP_FSCK_FLAG);
+		set_ckpt_flags(sbi, CP_FSCK_FLAG);
 
 	/* update SIT/NAT bitmap */
 	get_sit_bitmap(sbi, __bitmap_ptr(sbi, SIT_BITMAP));
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index c30f744b..b615f3f 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -814,6 +814,7 @@ struct f2fs_sb_info {
 
 	/* for checkpoint */
 	struct f2fs_checkpoint *ckpt;		/* raw checkpoint pointer */
+	spinlock_t cp_lock;			/* for flag in ckpt */
 	struct inode *meta_inode;		/* cache meta blocks */
 	struct mutex cp_mutex;			/* checkpoint procedure lock */
 	struct rw_semaphore cp_rwsem;		/* blocking FS operations */
@@ -1083,24 +1084,36 @@ static inline unsigned long long cur_cp_version(struct f2fs_checkpoint *cp)
 	return le64_to_cpu(cp->checkpoint_ver);
 }
 
-static inline bool is_set_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f)
+static inline bool is_set_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f)
 {
+	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
 	unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags);
+
 	return ckpt_flags & f;
 }
 
-static inline void set_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f)
+static inline void set_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f)
 {
-	unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags);
+	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
+	unsigned int ckpt_flags;
+
+	spin_lock(&sbi->cp_lock);
+	ckpt_flags = le32_to_cpu(cp->ckpt_flags);
 	ckpt_flags |= f;
 	cp->ckpt_flags = cpu_to_le32(ckpt_flags);
+	spin_unlock(&sbi->cp_lock);
 }
 
-static inline void clear_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f)
+static inline void clear_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f)
 {
-	unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags);
+	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
+	unsigned int ckpt_flags;
+
+	spin_lock(&sbi->cp_lock);
+	ckpt_flags = le32_to_cpu(cp->ckpt_flags);
 	ckpt_flags &= (~f);
 	cp->ckpt_flags = cpu_to_le32(ckpt_flags);
+	spin_unlock(&sbi->cp_lock);
 }
 
 static inline bool f2fs_discard_en(struct f2fs_sb_info *sbi)
@@ -1148,8 +1161,8 @@ static inline bool __remain_node_summaries(int reason)
 
 static inline bool __exist_node_summaries(struct f2fs_sb_info *sbi)
 {
-	return (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG) ||
-			is_set_ckpt_flags(F2FS_CKPT(sbi), CP_FASTBOOT_FLAG));
+	return (is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG) ||
+			is_set_ckpt_flags(sbi, CP_FASTBOOT_FLAG));
 }
 
 /*
@@ -1851,7 +1864,7 @@ static inline int f2fs_readonly(struct super_block *sb)
 
 static inline bool f2fs_cp_error(struct f2fs_sb_info *sbi)
 {
-	return is_set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
+	return is_set_ckpt_flags(sbi, CP_ERROR_FLAG);
 }
 
 static inline bool is_dot_dotdot(const struct qstr *str)
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index ad748e5..37d99d2 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -649,7 +649,7 @@ out:
 			invalidate_mapping_pages(META_MAPPING(sbi),
 							blkaddr, blkaddr);
 
-		set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
+		set_ckpt_flags(sbi, CP_ERROR_FLAG);
 		mutex_unlock(&sbi->cp_mutex);
 	} else if (need_writecp) {
 		struct cp_control cpc = {
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 101b58f..dc68f30 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1825,7 +1825,7 @@ static int restore_curseg_summaries(struct f2fs_sb_info *sbi)
 	int type = CURSEG_HOT_DATA;
 	int err;
 
-	if (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_COMPACT_SUM_FLAG)) {
+	if (is_set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG)) {
 		int npages = npages_for_summary_flush(sbi, true);
 
 		if (npages >= 2)
@@ -1922,7 +1922,7 @@ static void write_normal_summaries(struct f2fs_sb_info *sbi,
 
 void write_data_summaries(struct f2fs_sb_info *sbi, block_t start_blk)
 {
-	if (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_COMPACT_SUM_FLAG))
+	if (is_set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG))
 		write_compacted_summaries(sbi, start_blk);
 	else
 		write_normal_summaries(sbi, start_blk, CURSEG_HOT_DATA);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 555217f..f809729 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -735,7 +735,7 @@ static void f2fs_put_super(struct super_block *sb)
 	 * clean checkpoint again.
 	 */
 	if (is_sbi_flag_set(sbi, SBI_IS_DIRTY) ||
-			!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG)) {
+			!is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) {
 		struct cp_control cpc = {
 			.reason = CP_UMOUNT,
 		};
@@ -1477,6 +1477,7 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
 	mutex_init(&sbi->umount_mutex);
 	mutex_init(&sbi->wio_mutex[NODE]);
 	mutex_init(&sbi->wio_mutex[DATA]);
+	spin_lock_init(&sbi->cp_lock);
 
 #ifdef CONFIG_F2FS_FS_ENCRYPTION
 	memcpy(sbi->key_prefix, F2FS_KEY_DESC_PREFIX,
@@ -1818,7 +1819,7 @@ try_onemore:
 		 * previous checkpoint was not done by clean system shutdown.
 		 */
 		if (bdev_read_only(sb->s_bdev) &&
-				!is_set_ckpt_flags(sbi->ckpt, CP_UMOUNT_FLAG)) {
+				!is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) {
 			err = -EROFS;
 			goto free_kobj;
 		}
-- 
2.7.2

[toc] | [next] | [standalone]

#1486897 — Re: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-09-19 23:50 +0200
SubjectRe: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags
Message-ID<sjeBP-2xm-1@gated-at.bofh.it>
In reply to#1485843
Hi Chao,

On Sun, Sep 18, 2016 at 11:30:06PM +0800, Chao Yu wrote:
> From: Chao Yu <yuchao0@huawei.com>
> 
> This patch introduces spinlock to protect updating process of ckpt_flags
> field in struct f2fs_checkpoint, it avoids incorrectly updating in race
> condition.

So, I'm seeing a race condition between f2fs_stop_checkpoint(),
write_checkpoint(), and f2fs_fill_super().

How about just adding f2fs_lock_op() in f2fs_stop_checkpoint()?

Thanks,

> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/checkpoint.c | 24 ++++++++++++------------
>  fs/f2fs/f2fs.h       | 29 +++++++++++++++++++++--------
>  fs/f2fs/recovery.c   |  2 +-
>  fs/f2fs/segment.c    |  4 ++--
>  fs/f2fs/super.c      |  5 +++--
>  5 files changed, 39 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index df56a43..0338f8c 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -28,7 +28,7 @@ struct kmem_cache *inode_entry_slab;
>  
>  void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io)
>  {
> -	set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
> +	set_ckpt_flags(sbi, CP_ERROR_FLAG);
>  	sbi->sb->s_flags |= MS_RDONLY;
>  	if (!end_io)
>  		f2fs_flush_merged_bios(sbi);
> @@ -571,7 +571,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
>  	block_t start_blk, orphan_blocks, i, j;
>  	int err;
>  
> -	if (!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG))
> +	if (!is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG))
>  		return 0;
>  
>  	start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi);
> @@ -595,7 +595,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
>  		f2fs_put_page(page, 1);
>  	}
>  	/* clear Orphan Flag */
> -	clear_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG);
> +	clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
>  	return 0;
>  }
>  
> @@ -1054,9 +1054,9 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  	/* 2 cp  + n data seg summary + orphan inode blocks */
>  	data_sum_blocks = npages_for_summary_flush(sbi, false);
>  	if (data_sum_blocks < NR_CURSEG_DATA_TYPE)
> -		set_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG);
> +		set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG);
>  	else
> -		clear_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG);
> +		clear_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG);
>  
>  	orphan_blocks = GET_ORPHAN_BLOCKS(orphan_num);
>  	ckpt->cp_pack_start_sum = cpu_to_le32(1 + cp_payload_blks +
> @@ -1072,22 +1072,22 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  				orphan_blocks);
>  
>  	if (cpc->reason == CP_UMOUNT)
> -		set_ckpt_flags(ckpt, CP_UMOUNT_FLAG);
> +		set_ckpt_flags(sbi, CP_UMOUNT_FLAG);
>  	else
> -		clear_ckpt_flags(ckpt, CP_UMOUNT_FLAG);
> +		clear_ckpt_flags(sbi, CP_UMOUNT_FLAG);
>  
>  	if (cpc->reason == CP_FASTBOOT)
> -		set_ckpt_flags(ckpt, CP_FASTBOOT_FLAG);
> +		set_ckpt_flags(sbi, CP_FASTBOOT_FLAG);
>  	else
> -		clear_ckpt_flags(ckpt, CP_FASTBOOT_FLAG);
> +		clear_ckpt_flags(sbi, CP_FASTBOOT_FLAG);
>  
>  	if (orphan_num)
> -		set_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG);
> +		set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
>  	else
> -		clear_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG);
> +		clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
>  
>  	if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
> -		set_ckpt_flags(ckpt, CP_FSCK_FLAG);
> +		set_ckpt_flags(sbi, CP_FSCK_FLAG);
>  
>  	/* update SIT/NAT bitmap */
>  	get_sit_bitmap(sbi, __bitmap_ptr(sbi, SIT_BITMAP));
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index c30f744b..b615f3f 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -814,6 +814,7 @@ struct f2fs_sb_info {
>  
>  	/* for checkpoint */
>  	struct f2fs_checkpoint *ckpt;		/* raw checkpoint pointer */
> +	spinlock_t cp_lock;			/* for flag in ckpt */
>  	struct inode *meta_inode;		/* cache meta blocks */
>  	struct mutex cp_mutex;			/* checkpoint procedure lock */
>  	struct rw_semaphore cp_rwsem;		/* blocking FS operations */
> @@ -1083,24 +1084,36 @@ static inline unsigned long long cur_cp_version(struct f2fs_checkpoint *cp)
>  	return le64_to_cpu(cp->checkpoint_ver);
>  }
>  
> -static inline bool is_set_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f)
> +static inline bool is_set_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f)
>  {
> +	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
>  	unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags);
> +
>  	return ckpt_flags & f;
>  }
>  
> -static inline void set_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f)
> +static inline void set_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f)
>  {
> -	unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags);
> +	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
> +	unsigned int ckpt_flags;
> +
> +	spin_lock(&sbi->cp_lock);
> +	ckpt_flags = le32_to_cpu(cp->ckpt_flags);
>  	ckpt_flags |= f;
>  	cp->ckpt_flags = cpu_to_le32(ckpt_flags);
> +	spin_unlock(&sbi->cp_lock);
>  }
>  
> -static inline void clear_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f)
> +static inline void clear_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f)
>  {
> -	unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags);
> +	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
> +	unsigned int ckpt_flags;
> +
> +	spin_lock(&sbi->cp_lock);
> +	ckpt_flags = le32_to_cpu(cp->ckpt_flags);
>  	ckpt_flags &= (~f);
>  	cp->ckpt_flags = cpu_to_le32(ckpt_flags);
> +	spin_unlock(&sbi->cp_lock);
>  }
>  
>  static inline bool f2fs_discard_en(struct f2fs_sb_info *sbi)
> @@ -1148,8 +1161,8 @@ static inline bool __remain_node_summaries(int reason)
>  
>  static inline bool __exist_node_summaries(struct f2fs_sb_info *sbi)
>  {
> -	return (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG) ||
> -			is_set_ckpt_flags(F2FS_CKPT(sbi), CP_FASTBOOT_FLAG));
> +	return (is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG) ||
> +			is_set_ckpt_flags(sbi, CP_FASTBOOT_FLAG));
>  }
>  
>  /*
> @@ -1851,7 +1864,7 @@ static inline int f2fs_readonly(struct super_block *sb)
>  
>  static inline bool f2fs_cp_error(struct f2fs_sb_info *sbi)
>  {
> -	return is_set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
> +	return is_set_ckpt_flags(sbi, CP_ERROR_FLAG);
>  }
>  
>  static inline bool is_dot_dotdot(const struct qstr *str)
> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> index ad748e5..37d99d2 100644
> --- a/fs/f2fs/recovery.c
> +++ b/fs/f2fs/recovery.c
> @@ -649,7 +649,7 @@ out:
>  			invalidate_mapping_pages(META_MAPPING(sbi),
>  							blkaddr, blkaddr);
>  
> -		set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
> +		set_ckpt_flags(sbi, CP_ERROR_FLAG);
>  		mutex_unlock(&sbi->cp_mutex);
>  	} else if (need_writecp) {
>  		struct cp_control cpc = {
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 101b58f..dc68f30 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1825,7 +1825,7 @@ static int restore_curseg_summaries(struct f2fs_sb_info *sbi)
>  	int type = CURSEG_HOT_DATA;
>  	int err;
>  
> -	if (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_COMPACT_SUM_FLAG)) {
> +	if (is_set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG)) {
>  		int npages = npages_for_summary_flush(sbi, true);
>  
>  		if (npages >= 2)
> @@ -1922,7 +1922,7 @@ static void write_normal_summaries(struct f2fs_sb_info *sbi,
>  
>  void write_data_summaries(struct f2fs_sb_info *sbi, block_t start_blk)
>  {
> -	if (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_COMPACT_SUM_FLAG))
> +	if (is_set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG))
>  		write_compacted_summaries(sbi, start_blk);
>  	else
>  		write_normal_summaries(sbi, start_blk, CURSEG_HOT_DATA);
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 555217f..f809729 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -735,7 +735,7 @@ static void f2fs_put_super(struct super_block *sb)
>  	 * clean checkpoint again.
>  	 */
>  	if (is_sbi_flag_set(sbi, SBI_IS_DIRTY) ||
> -			!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG)) {
> +			!is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) {
>  		struct cp_control cpc = {
>  			.reason = CP_UMOUNT,
>  		};
> @@ -1477,6 +1477,7 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
>  	mutex_init(&sbi->umount_mutex);
>  	mutex_init(&sbi->wio_mutex[NODE]);
>  	mutex_init(&sbi->wio_mutex[DATA]);
> +	spin_lock_init(&sbi->cp_lock);
>  
>  #ifdef CONFIG_F2FS_FS_ENCRYPTION
>  	memcpy(sbi->key_prefix, F2FS_KEY_DESC_PREFIX,
> @@ -1818,7 +1819,7 @@ try_onemore:
>  		 * previous checkpoint was not done by clean system shutdown.
>  		 */
>  		if (bdev_read_only(sb->s_bdev) &&
> -				!is_set_ckpt_flags(sbi->ckpt, CP_UMOUNT_FLAG)) {
> +				!is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) {
>  			err = -EROFS;
>  			goto free_kobj;
>  		}
> -- 
> 2.7.2

[toc] | [prev] | [next] | [standalone]

#1487004 — Re: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags

FromChao Yu <yuchao0@huawei.com>
Date2016-09-20 03:50 +0200
SubjectRe: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags
Message-ID<sjim5-4S8-5@gated-at.bofh.it>
In reply to#1486897
Hi Jaegeuk,

On 2016/9/20 5:49, Jaegeuk Kim wrote:
> Hi Chao,
> 
> On Sun, Sep 18, 2016 at 11:30:06PM +0800, Chao Yu wrote:
>> From: Chao Yu <yuchao0@huawei.com>
>>
>> This patch introduces spinlock to protect updating process of ckpt_flags
>> field in struct f2fs_checkpoint, it avoids incorrectly updating in race
>> condition.
> 
> So, I'm seeing a race condition between f2fs_stop_checkpoint(),
> write_checkpoint(), and f2fs_fill_super().
> 
> How about just adding f2fs_lock_op() in f2fs_stop_checkpoint()?

I'm afraid there will be a potential deadlock here:

Thread A				Thread B
- write_checkpoint
 - block_operations
  - f2fs_lock_all
 - do_checkpoint
 - wait_on_all_pages_writeback
					- f2fs_write_end_io
					 - f2fs_stop_checkpoint
					  - f2fs_lock_op
					 - end_page_writeback
					 - atomic_dec_and_test
					 - wake_up

Right?

Thanks,

> 
> Thanks,
> 
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>  fs/f2fs/checkpoint.c | 24 ++++++++++++------------
>>  fs/f2fs/f2fs.h       | 29 +++++++++++++++++++++--------
>>  fs/f2fs/recovery.c   |  2 +-
>>  fs/f2fs/segment.c    |  4 ++--
>>  fs/f2fs/super.c      |  5 +++--
>>  5 files changed, 39 insertions(+), 25 deletions(-)
>>
>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>> index df56a43..0338f8c 100644
>> --- a/fs/f2fs/checkpoint.c
>> +++ b/fs/f2fs/checkpoint.c
>> @@ -28,7 +28,7 @@ struct kmem_cache *inode_entry_slab;
>>  
>>  void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io)
>>  {
>> -	set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
>> +	set_ckpt_flags(sbi, CP_ERROR_FLAG);
>>  	sbi->sb->s_flags |= MS_RDONLY;
>>  	if (!end_io)
>>  		f2fs_flush_merged_bios(sbi);
>> @@ -571,7 +571,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
>>  	block_t start_blk, orphan_blocks, i, j;
>>  	int err;
>>  
>> -	if (!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG))
>> +	if (!is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG))
>>  		return 0;
>>  
>>  	start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi);
>> @@ -595,7 +595,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
>>  		f2fs_put_page(page, 1);
>>  	}
>>  	/* clear Orphan Flag */
>> -	clear_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG);
>> +	clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
>>  	return 0;
>>  }
>>  
>> @@ -1054,9 +1054,9 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>  	/* 2 cp  + n data seg summary + orphan inode blocks */
>>  	data_sum_blocks = npages_for_summary_flush(sbi, false);
>>  	if (data_sum_blocks < NR_CURSEG_DATA_TYPE)
>> -		set_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG);
>> +		set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG);
>>  	else
>> -		clear_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG);
>> +		clear_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG);
>>  
>>  	orphan_blocks = GET_ORPHAN_BLOCKS(orphan_num);
>>  	ckpt->cp_pack_start_sum = cpu_to_le32(1 + cp_payload_blks +
>> @@ -1072,22 +1072,22 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>  				orphan_blocks);
>>  
>>  	if (cpc->reason == CP_UMOUNT)
>> -		set_ckpt_flags(ckpt, CP_UMOUNT_FLAG);
>> +		set_ckpt_flags(sbi, CP_UMOUNT_FLAG);
>>  	else
>> -		clear_ckpt_flags(ckpt, CP_UMOUNT_FLAG);
>> +		clear_ckpt_flags(sbi, CP_UMOUNT_FLAG);
>>  
>>  	if (cpc->reason == CP_FASTBOOT)
>> -		set_ckpt_flags(ckpt, CP_FASTBOOT_FLAG);
>> +		set_ckpt_flags(sbi, CP_FASTBOOT_FLAG);
>>  	else
>> -		clear_ckpt_flags(ckpt, CP_FASTBOOT_FLAG);
>> +		clear_ckpt_flags(sbi, CP_FASTBOOT_FLAG);
>>  
>>  	if (orphan_num)
>> -		set_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG);
>> +		set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
>>  	else
>> -		clear_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG);
>> +		clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
>>  
>>  	if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
>> -		set_ckpt_flags(ckpt, CP_FSCK_FLAG);
>> +		set_ckpt_flags(sbi, CP_FSCK_FLAG);
>>  
>>  	/* update SIT/NAT bitmap */
>>  	get_sit_bitmap(sbi, __bitmap_ptr(sbi, SIT_BITMAP));
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index c30f744b..b615f3f 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -814,6 +814,7 @@ struct f2fs_sb_info {
>>  
>>  	/* for checkpoint */
>>  	struct f2fs_checkpoint *ckpt;		/* raw checkpoint pointer */
>> +	spinlock_t cp_lock;			/* for flag in ckpt */
>>  	struct inode *meta_inode;		/* cache meta blocks */
>>  	struct mutex cp_mutex;			/* checkpoint procedure lock */
>>  	struct rw_semaphore cp_rwsem;		/* blocking FS operations */
>> @@ -1083,24 +1084,36 @@ static inline unsigned long long cur_cp_version(struct f2fs_checkpoint *cp)
>>  	return le64_to_cpu(cp->checkpoint_ver);
>>  }
>>  
>> -static inline bool is_set_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f)
>> +static inline bool is_set_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f)
>>  {
>> +	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
>>  	unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags);
>> +
>>  	return ckpt_flags & f;
>>  }
>>  
>> -static inline void set_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f)
>> +static inline void set_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f)
>>  {
>> -	unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags);
>> +	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
>> +	unsigned int ckpt_flags;
>> +
>> +	spin_lock(&sbi->cp_lock);
>> +	ckpt_flags = le32_to_cpu(cp->ckpt_flags);
>>  	ckpt_flags |= f;
>>  	cp->ckpt_flags = cpu_to_le32(ckpt_flags);
>> +	spin_unlock(&sbi->cp_lock);
>>  }
>>  
>> -static inline void clear_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f)
>> +static inline void clear_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f)
>>  {
>> -	unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags);
>> +	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
>> +	unsigned int ckpt_flags;
>> +
>> +	spin_lock(&sbi->cp_lock);
>> +	ckpt_flags = le32_to_cpu(cp->ckpt_flags);
>>  	ckpt_flags &= (~f);
>>  	cp->ckpt_flags = cpu_to_le32(ckpt_flags);
>> +	spin_unlock(&sbi->cp_lock);
>>  }
>>  
>>  static inline bool f2fs_discard_en(struct f2fs_sb_info *sbi)
>> @@ -1148,8 +1161,8 @@ static inline bool __remain_node_summaries(int reason)
>>  
>>  static inline bool __exist_node_summaries(struct f2fs_sb_info *sbi)
>>  {
>> -	return (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG) ||
>> -			is_set_ckpt_flags(F2FS_CKPT(sbi), CP_FASTBOOT_FLAG));
>> +	return (is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG) ||
>> +			is_set_ckpt_flags(sbi, CP_FASTBOOT_FLAG));
>>  }
>>  
>>  /*
>> @@ -1851,7 +1864,7 @@ static inline int f2fs_readonly(struct super_block *sb)
>>  
>>  static inline bool f2fs_cp_error(struct f2fs_sb_info *sbi)
>>  {
>> -	return is_set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
>> +	return is_set_ckpt_flags(sbi, CP_ERROR_FLAG);
>>  }
>>  
>>  static inline bool is_dot_dotdot(const struct qstr *str)
>> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
>> index ad748e5..37d99d2 100644
>> --- a/fs/f2fs/recovery.c
>> +++ b/fs/f2fs/recovery.c
>> @@ -649,7 +649,7 @@ out:
>>  			invalidate_mapping_pages(META_MAPPING(sbi),
>>  							blkaddr, blkaddr);
>>  
>> -		set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
>> +		set_ckpt_flags(sbi, CP_ERROR_FLAG);
>>  		mutex_unlock(&sbi->cp_mutex);
>>  	} else if (need_writecp) {
>>  		struct cp_control cpc = {
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 101b58f..dc68f30 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -1825,7 +1825,7 @@ static int restore_curseg_summaries(struct f2fs_sb_info *sbi)
>>  	int type = CURSEG_HOT_DATA;
>>  	int err;
>>  
>> -	if (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_COMPACT_SUM_FLAG)) {
>> +	if (is_set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG)) {
>>  		int npages = npages_for_summary_flush(sbi, true);
>>  
>>  		if (npages >= 2)
>> @@ -1922,7 +1922,7 @@ static void write_normal_summaries(struct f2fs_sb_info *sbi,
>>  
>>  void write_data_summaries(struct f2fs_sb_info *sbi, block_t start_blk)
>>  {
>> -	if (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_COMPACT_SUM_FLAG))
>> +	if (is_set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG))
>>  		write_compacted_summaries(sbi, start_blk);
>>  	else
>>  		write_normal_summaries(sbi, start_blk, CURSEG_HOT_DATA);
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index 555217f..f809729 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -735,7 +735,7 @@ static void f2fs_put_super(struct super_block *sb)
>>  	 * clean checkpoint again.
>>  	 */
>>  	if (is_sbi_flag_set(sbi, SBI_IS_DIRTY) ||
>> -			!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG)) {
>> +			!is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) {
>>  		struct cp_control cpc = {
>>  			.reason = CP_UMOUNT,
>>  		};
>> @@ -1477,6 +1477,7 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
>>  	mutex_init(&sbi->umount_mutex);
>>  	mutex_init(&sbi->wio_mutex[NODE]);
>>  	mutex_init(&sbi->wio_mutex[DATA]);
>> +	spin_lock_init(&sbi->cp_lock);
>>  
>>  #ifdef CONFIG_F2FS_FS_ENCRYPTION
>>  	memcpy(sbi->key_prefix, F2FS_KEY_DESC_PREFIX,
>> @@ -1818,7 +1819,7 @@ try_onemore:
>>  		 * previous checkpoint was not done by clean system shutdown.
>>  		 */
>>  		if (bdev_read_only(sb->s_bdev) &&
>> -				!is_set_ckpt_flags(sbi->ckpt, CP_UMOUNT_FLAG)) {
>> +				!is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) {
>>  			err = -EROFS;
>>  			goto free_kobj;
>>  		}
>> -- 
>> 2.7.2
> 
> .
>

[toc] | [prev] | [next] | [standalone]

#1487028 — Re: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags

FromJaegeuk Kim <jaegeuk@kernel.org>
Date2016-09-20 04:50 +0200
SubjectRe: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags
Message-ID<sjjia-5z5-13@gated-at.bofh.it>
In reply to#1487004
On Tue, Sep 20, 2016 at 09:47:20AM +0800, Chao Yu wrote:
> Hi Jaegeuk,
> 
> On 2016/9/20 5:49, Jaegeuk Kim wrote:
> > Hi Chao,
> > 
> > On Sun, Sep 18, 2016 at 11:30:06PM +0800, Chao Yu wrote:
> >> From: Chao Yu <yuchao0@huawei.com>
> >>
> >> This patch introduces spinlock to protect updating process of ckpt_flags
> >> field in struct f2fs_checkpoint, it avoids incorrectly updating in race
> >> condition.
> > 
> > So, I'm seeing a race condition between f2fs_stop_checkpoint(),
> > write_checkpoint(), and f2fs_fill_super().
> > 
> > How about just adding f2fs_lock_op() in f2fs_stop_checkpoint()?
> 
> I'm afraid there will be a potential deadlock here:
> 
> Thread A				Thread B
> - write_checkpoint
>  - block_operations
>   - f2fs_lock_all
>  - do_checkpoint
>  - wait_on_all_pages_writeback
> 					- f2fs_write_end_io
> 					 - f2fs_stop_checkpoint
> 					  - f2fs_lock_op
> 					 - end_page_writeback
> 					 - atomic_dec_and_test
> 					 - wake_up
> 
> Right?

Okay, I see. Let me try to understand in more details.
Basically, there'd be no problem if there is no f2fs_stop_checkpoint(), since
every {set|clear}_ckpt_flags() are called under f2fs_lock_op() or in
fill_super(). And, you're probably concerned about any breakage of ckpt->flags
due to accidental f2fs_stop_checkpoint() trigger. So, we're able to lose
ERROR_FLAG because of data race.

Oh, I found one potential corruption case in f2fs_write_checkpoint().
Before writing the last checkpoint block, we used to check its IO error.
But, if set_ckpt_flags() and f2fs_stop_checkpoint() were called concurrently,
ckpt_flags was able to lose ERROR_FLAG, resulting in finalizing its checkpoint
pack. So, we can get valid checkpoint pack with EIO'ed metadata block.

BTW, we can do multiple set/clear flags in do_checkpoint() with single spin_lock
call, tho.

Thanks,

> > 
> >> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> >> ---
> >>  fs/f2fs/checkpoint.c | 24 ++++++++++++------------
> >>  fs/f2fs/f2fs.h       | 29 +++++++++++++++++++++--------
> >>  fs/f2fs/recovery.c   |  2 +-
> >>  fs/f2fs/segment.c    |  4 ++--
> >>  fs/f2fs/super.c      |  5 +++--
> >>  5 files changed, 39 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> >> index df56a43..0338f8c 100644
> >> --- a/fs/f2fs/checkpoint.c
> >> +++ b/fs/f2fs/checkpoint.c
> >> @@ -28,7 +28,7 @@ struct kmem_cache *inode_entry_slab;
> >>  
> >>  void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io)
> >>  {
> >> -	set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
> >> +	set_ckpt_flags(sbi, CP_ERROR_FLAG);
> >>  	sbi->sb->s_flags |= MS_RDONLY;
> >>  	if (!end_io)
> >>  		f2fs_flush_merged_bios(sbi);
> >> @@ -571,7 +571,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
> >>  	block_t start_blk, orphan_blocks, i, j;
> >>  	int err;
> >>  
> >> -	if (!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG))
> >> +	if (!is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG))
> >>  		return 0;
> >>  
> >>  	start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi);
> >> @@ -595,7 +595,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
> >>  		f2fs_put_page(page, 1);
> >>  	}
> >>  	/* clear Orphan Flag */
> >> -	clear_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG);
> >> +	clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
> >>  	return 0;
> >>  }
> >>  
> >> @@ -1054,9 +1054,9 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
> >>  	/* 2 cp  + n data seg summary + orphan inode blocks */
> >>  	data_sum_blocks = npages_for_summary_flush(sbi, false);
> >>  	if (data_sum_blocks < NR_CURSEG_DATA_TYPE)
> >> -		set_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG);
> >> +		set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG);
> >>  	else
> >> -		clear_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG);
> >> +		clear_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG);
> >>  
> >>  	orphan_blocks = GET_ORPHAN_BLOCKS(orphan_num);
> >>  	ckpt->cp_pack_start_sum = cpu_to_le32(1 + cp_payload_blks +
> >> @@ -1072,22 +1072,22 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
> >>  				orphan_blocks);
> >>  
> >>  	if (cpc->reason == CP_UMOUNT)
> >> -		set_ckpt_flags(ckpt, CP_UMOUNT_FLAG);
> >> +		set_ckpt_flags(sbi, CP_UMOUNT_FLAG);
> >>  	else
> >> -		clear_ckpt_flags(ckpt, CP_UMOUNT_FLAG);
> >> +		clear_ckpt_flags(sbi, CP_UMOUNT_FLAG);
> >>  
> >>  	if (cpc->reason == CP_FASTBOOT)
> >> -		set_ckpt_flags(ckpt, CP_FASTBOOT_FLAG);
> >> +		set_ckpt_flags(sbi, CP_FASTBOOT_FLAG);
> >>  	else
> >> -		clear_ckpt_flags(ckpt, CP_FASTBOOT_FLAG);
> >> +		clear_ckpt_flags(sbi, CP_FASTBOOT_FLAG);
> >>  
> >>  	if (orphan_num)
> >> -		set_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG);
> >> +		set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
> >>  	else
> >> -		clear_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG);
> >> +		clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
> >>  
> >>  	if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
> >> -		set_ckpt_flags(ckpt, CP_FSCK_FLAG);
> >> +		set_ckpt_flags(sbi, CP_FSCK_FLAG);
> >>  
> >>  	/* update SIT/NAT bitmap */
> >>  	get_sit_bitmap(sbi, __bitmap_ptr(sbi, SIT_BITMAP));
> >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >> index c30f744b..b615f3f 100644
> >> --- a/fs/f2fs/f2fs.h
> >> +++ b/fs/f2fs/f2fs.h
> >> @@ -814,6 +814,7 @@ struct f2fs_sb_info {
> >>  
> >>  	/* for checkpoint */
> >>  	struct f2fs_checkpoint *ckpt;		/* raw checkpoint pointer */
> >> +	spinlock_t cp_lock;			/* for flag in ckpt */
> >>  	struct inode *meta_inode;		/* cache meta blocks */
> >>  	struct mutex cp_mutex;			/* checkpoint procedure lock */
> >>  	struct rw_semaphore cp_rwsem;		/* blocking FS operations */
> >> @@ -1083,24 +1084,36 @@ static inline unsigned long long cur_cp_version(struct f2fs_checkpoint *cp)
> >>  	return le64_to_cpu(cp->checkpoint_ver);
> >>  }
> >>  
> >> -static inline bool is_set_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f)
> >> +static inline bool is_set_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f)
> >>  {
> >> +	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
> >>  	unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags);
> >> +
> >>  	return ckpt_flags & f;
> >>  }
> >>  
> >> -static inline void set_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f)
> >> +static inline void set_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f)
> >>  {
> >> -	unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags);
> >> +	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
> >> +	unsigned int ckpt_flags;
> >> +
> >> +	spin_lock(&sbi->cp_lock);
> >> +	ckpt_flags = le32_to_cpu(cp->ckpt_flags);
> >>  	ckpt_flags |= f;
> >>  	cp->ckpt_flags = cpu_to_le32(ckpt_flags);
> >> +	spin_unlock(&sbi->cp_lock);
> >>  }
> >>  
> >> -static inline void clear_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f)
> >> +static inline void clear_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f)
> >>  {
> >> -	unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags);
> >> +	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
> >> +	unsigned int ckpt_flags;
> >> +
> >> +	spin_lock(&sbi->cp_lock);
> >> +	ckpt_flags = le32_to_cpu(cp->ckpt_flags);
> >>  	ckpt_flags &= (~f);
> >>  	cp->ckpt_flags = cpu_to_le32(ckpt_flags);
> >> +	spin_unlock(&sbi->cp_lock);
> >>  }
> >>  
> >>  static inline bool f2fs_discard_en(struct f2fs_sb_info *sbi)
> >> @@ -1148,8 +1161,8 @@ static inline bool __remain_node_summaries(int reason)
> >>  
> >>  static inline bool __exist_node_summaries(struct f2fs_sb_info *sbi)
> >>  {
> >> -	return (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG) ||
> >> -			is_set_ckpt_flags(F2FS_CKPT(sbi), CP_FASTBOOT_FLAG));
> >> +	return (is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG) ||
> >> +			is_set_ckpt_flags(sbi, CP_FASTBOOT_FLAG));
> >>  }
> >>  
> >>  /*
> >> @@ -1851,7 +1864,7 @@ static inline int f2fs_readonly(struct super_block *sb)
> >>  
> >>  static inline bool f2fs_cp_error(struct f2fs_sb_info *sbi)
> >>  {
> >> -	return is_set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
> >> +	return is_set_ckpt_flags(sbi, CP_ERROR_FLAG);
> >>  }
> >>  
> >>  static inline bool is_dot_dotdot(const struct qstr *str)
> >> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> >> index ad748e5..37d99d2 100644
> >> --- a/fs/f2fs/recovery.c
> >> +++ b/fs/f2fs/recovery.c
> >> @@ -649,7 +649,7 @@ out:
> >>  			invalidate_mapping_pages(META_MAPPING(sbi),
> >>  							blkaddr, blkaddr);
> >>  
> >> -		set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
> >> +		set_ckpt_flags(sbi, CP_ERROR_FLAG);
> >>  		mutex_unlock(&sbi->cp_mutex);
> >>  	} else if (need_writecp) {
> >>  		struct cp_control cpc = {
> >> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> >> index 101b58f..dc68f30 100644
> >> --- a/fs/f2fs/segment.c
> >> +++ b/fs/f2fs/segment.c
> >> @@ -1825,7 +1825,7 @@ static int restore_curseg_summaries(struct f2fs_sb_info *sbi)
> >>  	int type = CURSEG_HOT_DATA;
> >>  	int err;
> >>  
> >> -	if (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_COMPACT_SUM_FLAG)) {
> >> +	if (is_set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG)) {
> >>  		int npages = npages_for_summary_flush(sbi, true);
> >>  
> >>  		if (npages >= 2)
> >> @@ -1922,7 +1922,7 @@ static void write_normal_summaries(struct f2fs_sb_info *sbi,
> >>  
> >>  void write_data_summaries(struct f2fs_sb_info *sbi, block_t start_blk)
> >>  {
> >> -	if (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_COMPACT_SUM_FLAG))
> >> +	if (is_set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG))
> >>  		write_compacted_summaries(sbi, start_blk);
> >>  	else
> >>  		write_normal_summaries(sbi, start_blk, CURSEG_HOT_DATA);
> >> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> >> index 555217f..f809729 100644
> >> --- a/fs/f2fs/super.c
> >> +++ b/fs/f2fs/super.c
> >> @@ -735,7 +735,7 @@ static void f2fs_put_super(struct super_block *sb)
> >>  	 * clean checkpoint again.
> >>  	 */
> >>  	if (is_sbi_flag_set(sbi, SBI_IS_DIRTY) ||
> >> -			!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG)) {
> >> +			!is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) {
> >>  		struct cp_control cpc = {
> >>  			.reason = CP_UMOUNT,
> >>  		};
> >> @@ -1477,6 +1477,7 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
> >>  	mutex_init(&sbi->umount_mutex);
> >>  	mutex_init(&sbi->wio_mutex[NODE]);
> >>  	mutex_init(&sbi->wio_mutex[DATA]);
> >> +	spin_lock_init(&sbi->cp_lock);
> >>  
> >>  #ifdef CONFIG_F2FS_FS_ENCRYPTION
> >>  	memcpy(sbi->key_prefix, F2FS_KEY_DESC_PREFIX,
> >> @@ -1818,7 +1819,7 @@ try_onemore:
> >>  		 * previous checkpoint was not done by clean system shutdown.
> >>  		 */
> >>  		if (bdev_read_only(sb->s_bdev) &&
> >> -				!is_set_ckpt_flags(sbi->ckpt, CP_UMOUNT_FLAG)) {
> >> +				!is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) {
> >>  			err = -EROFS;
> >>  			goto free_kobj;
> >>  		}
> >> -- 
> >> 2.7.2
> > 
> > .
> >

[toc] | [prev] | [next] | [standalone]

#1487032 — Re: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags

FromChao Yu <yuchao0@huawei.com>
Date2016-09-20 05:00 +0200
SubjectRe: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags
Message-ID<sjjrP-5Ci-3@gated-at.bofh.it>
In reply to#1487028
On 2016/9/20 10:41, Jaegeuk Kim wrote:
> On Tue, Sep 20, 2016 at 09:47:20AM +0800, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> On 2016/9/20 5:49, Jaegeuk Kim wrote:
>>> Hi Chao,
>>>
>>> On Sun, Sep 18, 2016 at 11:30:06PM +0800, Chao Yu wrote:
>>>> From: Chao Yu <yuchao0@huawei.com>
>>>>
>>>> This patch introduces spinlock to protect updating process of ckpt_flags
>>>> field in struct f2fs_checkpoint, it avoids incorrectly updating in race
>>>> condition.
>>>
>>> So, I'm seeing a race condition between f2fs_stop_checkpoint(),
>>> write_checkpoint(), and f2fs_fill_super().
>>>
>>> How about just adding f2fs_lock_op() in f2fs_stop_checkpoint()?
>>
>> I'm afraid there will be a potential deadlock here:
>>
>> Thread A				Thread B
>> - write_checkpoint
>>  - block_operations
>>   - f2fs_lock_all
>>  - do_checkpoint
>>  - wait_on_all_pages_writeback
>> 					- f2fs_write_end_io
>> 					 - f2fs_stop_checkpoint
>> 					  - f2fs_lock_op
>> 					 - end_page_writeback
>> 					 - atomic_dec_and_test
>> 					 - wake_up
>>
>> Right?
> 
> Okay, I see. Let me try to understand in more details.
> Basically, there'd be no problem if there is no f2fs_stop_checkpoint(), since
> every {set|clear}_ckpt_flags() are called under f2fs_lock_op() or in
> fill_super(). And, you're probably concerned about any breakage of ckpt->flags
> due to accidental f2fs_stop_checkpoint() trigger. So, we're able to lose
> ERROR_FLAG because of data race.
> 
> Oh, I found one potential corruption case in f2fs_write_checkpoint().
> Before writing the last checkpoint block, we used to check its IO error.
> But, if set_ckpt_flags() and f2fs_stop_checkpoint() were called concurrently,
> ckpt_flags was able to lose ERROR_FLAG, resulting in finalizing its checkpoint
> pack. So, we can get valid checkpoint pack with EIO'ed metadata block.

That's right.

> 
> BTW, we can do multiple set/clear flags in do_checkpoint() with single spin_lock
> call, tho.

Agree, let me refactor the code. :)

Thanks,

> 
> Thanks,
> 
>>>
>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>> ---
>>>>  fs/f2fs/checkpoint.c | 24 ++++++++++++------------
>>>>  fs/f2fs/f2fs.h       | 29 +++++++++++++++++++++--------
>>>>  fs/f2fs/recovery.c   |  2 +-
>>>>  fs/f2fs/segment.c    |  4 ++--
>>>>  fs/f2fs/super.c      |  5 +++--
>>>>  5 files changed, 39 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>>> index df56a43..0338f8c 100644
>>>> --- a/fs/f2fs/checkpoint.c
>>>> +++ b/fs/f2fs/checkpoint.c
>>>> @@ -28,7 +28,7 @@ struct kmem_cache *inode_entry_slab;
>>>>  
>>>>  void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io)
>>>>  {
>>>> -	set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
>>>> +	set_ckpt_flags(sbi, CP_ERROR_FLAG);
>>>>  	sbi->sb->s_flags |= MS_RDONLY;
>>>>  	if (!end_io)
>>>>  		f2fs_flush_merged_bios(sbi);
>>>> @@ -571,7 +571,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
>>>>  	block_t start_blk, orphan_blocks, i, j;
>>>>  	int err;
>>>>  
>>>> -	if (!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG))
>>>> +	if (!is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG))
>>>>  		return 0;
>>>>  
>>>>  	start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi);
>>>> @@ -595,7 +595,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
>>>>  		f2fs_put_page(page, 1);
>>>>  	}
>>>>  	/* clear Orphan Flag */
>>>> -	clear_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG);
>>>> +	clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
>>>>  	return 0;
>>>>  }
>>>>  
>>>> @@ -1054,9 +1054,9 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>>  	/* 2 cp  + n data seg summary + orphan inode blocks */
>>>>  	data_sum_blocks = npages_for_summary_flush(sbi, false);
>>>>  	if (data_sum_blocks < NR_CURSEG_DATA_TYPE)
>>>> -		set_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG);
>>>> +		set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG);
>>>>  	else
>>>> -		clear_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG);
>>>> +		clear_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG);
>>>>  
>>>>  	orphan_blocks = GET_ORPHAN_BLOCKS(orphan_num);
>>>>  	ckpt->cp_pack_start_sum = cpu_to_le32(1 + cp_payload_blks +
>>>> @@ -1072,22 +1072,22 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>>>>  				orphan_blocks);
>>>>  
>>>>  	if (cpc->reason == CP_UMOUNT)
>>>> -		set_ckpt_flags(ckpt, CP_UMOUNT_FLAG);
>>>> +		set_ckpt_flags(sbi, CP_UMOUNT_FLAG);
>>>>  	else
>>>> -		clear_ckpt_flags(ckpt, CP_UMOUNT_FLAG);
>>>> +		clear_ckpt_flags(sbi, CP_UMOUNT_FLAG);
>>>>  
>>>>  	if (cpc->reason == CP_FASTBOOT)
>>>> -		set_ckpt_flags(ckpt, CP_FASTBOOT_FLAG);
>>>> +		set_ckpt_flags(sbi, CP_FASTBOOT_FLAG);
>>>>  	else
>>>> -		clear_ckpt_flags(ckpt, CP_FASTBOOT_FLAG);
>>>> +		clear_ckpt_flags(sbi, CP_FASTBOOT_FLAG);
>>>>  
>>>>  	if (orphan_num)
>>>> -		set_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG);
>>>> +		set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
>>>>  	else
>>>> -		clear_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG);
>>>> +		clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG);
>>>>  
>>>>  	if (is_sbi_flag_set(sbi, SBI_NEED_FSCK))
>>>> -		set_ckpt_flags(ckpt, CP_FSCK_FLAG);
>>>> +		set_ckpt_flags(sbi, CP_FSCK_FLAG);
>>>>  
>>>>  	/* update SIT/NAT bitmap */
>>>>  	get_sit_bitmap(sbi, __bitmap_ptr(sbi, SIT_BITMAP));
>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>> index c30f744b..b615f3f 100644
>>>> --- a/fs/f2fs/f2fs.h
>>>> +++ b/fs/f2fs/f2fs.h
>>>> @@ -814,6 +814,7 @@ struct f2fs_sb_info {
>>>>  
>>>>  	/* for checkpoint */
>>>>  	struct f2fs_checkpoint *ckpt;		/* raw checkpoint pointer */
>>>> +	spinlock_t cp_lock;			/* for flag in ckpt */
>>>>  	struct inode *meta_inode;		/* cache meta blocks */
>>>>  	struct mutex cp_mutex;			/* checkpoint procedure lock */
>>>>  	struct rw_semaphore cp_rwsem;		/* blocking FS operations */
>>>> @@ -1083,24 +1084,36 @@ static inline unsigned long long cur_cp_version(struct f2fs_checkpoint *cp)
>>>>  	return le64_to_cpu(cp->checkpoint_ver);
>>>>  }
>>>>  
>>>> -static inline bool is_set_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f)
>>>> +static inline bool is_set_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f)
>>>>  {
>>>> +	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
>>>>  	unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags);
>>>> +
>>>>  	return ckpt_flags & f;
>>>>  }
>>>>  
>>>> -static inline void set_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f)
>>>> +static inline void set_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f)
>>>>  {
>>>> -	unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags);
>>>> +	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
>>>> +	unsigned int ckpt_flags;
>>>> +
>>>> +	spin_lock(&sbi->cp_lock);
>>>> +	ckpt_flags = le32_to_cpu(cp->ckpt_flags);
>>>>  	ckpt_flags |= f;
>>>>  	cp->ckpt_flags = cpu_to_le32(ckpt_flags);
>>>> +	spin_unlock(&sbi->cp_lock);
>>>>  }
>>>>  
>>>> -static inline void clear_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f)
>>>> +static inline void clear_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f)
>>>>  {
>>>> -	unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags);
>>>> +	struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
>>>> +	unsigned int ckpt_flags;
>>>> +
>>>> +	spin_lock(&sbi->cp_lock);
>>>> +	ckpt_flags = le32_to_cpu(cp->ckpt_flags);
>>>>  	ckpt_flags &= (~f);
>>>>  	cp->ckpt_flags = cpu_to_le32(ckpt_flags);
>>>> +	spin_unlock(&sbi->cp_lock);
>>>>  }
>>>>  
>>>>  static inline bool f2fs_discard_en(struct f2fs_sb_info *sbi)
>>>> @@ -1148,8 +1161,8 @@ static inline bool __remain_node_summaries(int reason)
>>>>  
>>>>  static inline bool __exist_node_summaries(struct f2fs_sb_info *sbi)
>>>>  {
>>>> -	return (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG) ||
>>>> -			is_set_ckpt_flags(F2FS_CKPT(sbi), CP_FASTBOOT_FLAG));
>>>> +	return (is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG) ||
>>>> +			is_set_ckpt_flags(sbi, CP_FASTBOOT_FLAG));
>>>>  }
>>>>  
>>>>  /*
>>>> @@ -1851,7 +1864,7 @@ static inline int f2fs_readonly(struct super_block *sb)
>>>>  
>>>>  static inline bool f2fs_cp_error(struct f2fs_sb_info *sbi)
>>>>  {
>>>> -	return is_set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
>>>> +	return is_set_ckpt_flags(sbi, CP_ERROR_FLAG);
>>>>  }
>>>>  
>>>>  static inline bool is_dot_dotdot(const struct qstr *str)
>>>> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
>>>> index ad748e5..37d99d2 100644
>>>> --- a/fs/f2fs/recovery.c
>>>> +++ b/fs/f2fs/recovery.c
>>>> @@ -649,7 +649,7 @@ out:
>>>>  			invalidate_mapping_pages(META_MAPPING(sbi),
>>>>  							blkaddr, blkaddr);
>>>>  
>>>> -		set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG);
>>>> +		set_ckpt_flags(sbi, CP_ERROR_FLAG);
>>>>  		mutex_unlock(&sbi->cp_mutex);
>>>>  	} else if (need_writecp) {
>>>>  		struct cp_control cpc = {
>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>> index 101b58f..dc68f30 100644
>>>> --- a/fs/f2fs/segment.c
>>>> +++ b/fs/f2fs/segment.c
>>>> @@ -1825,7 +1825,7 @@ static int restore_curseg_summaries(struct f2fs_sb_info *sbi)
>>>>  	int type = CURSEG_HOT_DATA;
>>>>  	int err;
>>>>  
>>>> -	if (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_COMPACT_SUM_FLAG)) {
>>>> +	if (is_set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG)) {
>>>>  		int npages = npages_for_summary_flush(sbi, true);
>>>>  
>>>>  		if (npages >= 2)
>>>> @@ -1922,7 +1922,7 @@ static void write_normal_summaries(struct f2fs_sb_info *sbi,
>>>>  
>>>>  void write_data_summaries(struct f2fs_sb_info *sbi, block_t start_blk)
>>>>  {
>>>> -	if (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_COMPACT_SUM_FLAG))
>>>> +	if (is_set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG))
>>>>  		write_compacted_summaries(sbi, start_blk);
>>>>  	else
>>>>  		write_normal_summaries(sbi, start_blk, CURSEG_HOT_DATA);
>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>> index 555217f..f809729 100644
>>>> --- a/fs/f2fs/super.c
>>>> +++ b/fs/f2fs/super.c
>>>> @@ -735,7 +735,7 @@ static void f2fs_put_super(struct super_block *sb)
>>>>  	 * clean checkpoint again.
>>>>  	 */
>>>>  	if (is_sbi_flag_set(sbi, SBI_IS_DIRTY) ||
>>>> -			!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG)) {
>>>> +			!is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) {
>>>>  		struct cp_control cpc = {
>>>>  			.reason = CP_UMOUNT,
>>>>  		};
>>>> @@ -1477,6 +1477,7 @@ static void init_sb_info(struct f2fs_sb_info *sbi)
>>>>  	mutex_init(&sbi->umount_mutex);
>>>>  	mutex_init(&sbi->wio_mutex[NODE]);
>>>>  	mutex_init(&sbi->wio_mutex[DATA]);
>>>> +	spin_lock_init(&sbi->cp_lock);
>>>>  
>>>>  #ifdef CONFIG_F2FS_FS_ENCRYPTION
>>>>  	memcpy(sbi->key_prefix, F2FS_KEY_DESC_PREFIX,
>>>> @@ -1818,7 +1819,7 @@ try_onemore:
>>>>  		 * previous checkpoint was not done by clean system shutdown.
>>>>  		 */
>>>>  		if (bdev_read_only(sb->s_bdev) &&
>>>> -				!is_set_ckpt_flags(sbi->ckpt, CP_UMOUNT_FLAG)) {
>>>> +				!is_set_ckpt_flags(sbi, CP_UMOUNT_FLAG)) {
>>>>  			err = -EROFS;
>>>>  			goto free_kobj;
>>>>  		}
>>>> -- 
>>>> 2.7.2
>>>
>>> .
>>>
> 
> .
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web