[PATCH 1/3] f2fs: turn off atomic writes when deteting abnormal behaviors

[PATCH 1/3] f2fs: turn off atomic writes when deteting abnormal behaviors

[Jaegeuk Kim]

In order to prevent abusing atomic writes by abnormal users, we've added a
threshold, 20% over memory footprint, which disallows further atomic writes.
Previously, however, SQLite doesn't know the files became normal, so that
it could write stale data and commit on revoked normal database file.

Once f2fs detects such the abnormal behavior, this patch simply disables
all the atomic operations such as:
- write_begin() gives EINVAL to avoid stale data writes, and SQLite will call
  F2FS_IOC_ABORT_VOLATILE_WRITE to notify aborting the transaction,
- F2FS_IOC_START_ATOMIC_WRITE gives EINVAL for SQLite to fall back normal
  journal_mode,
- F2FS_IOC_COMMIT_ATOMIC_WRITE gives EINVAL, if the file was revoked, so that
  Framework retries to submit the transaction given propagated SQLite error.

Note that, this patch also turns off atomic operations, if foreground GC tries
to move atomic files too frequently.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c    |  7 ++++---
 fs/f2fs/f2fs.h    |  4 ++--
 fs/f2fs/file.c    |  6 +++++-
 fs/f2fs/gc.c      |  4 +---
 fs/f2fs/segment.c | 21 +++++++++++----------
 5 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 5e53d210e222..c9e75aa61c24 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2247,8 +2247,9 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
 	trace_f2fs_write_begin(inode, pos, len, flags);
 
 	if (f2fs_is_atomic_file(inode) &&
-			!f2fs_available_free_memory(sbi, INMEM_PAGES)) {
-		err = -ENOMEM;
+			(is_sbi_flag_set(sbi, SBI_DISABLE_ATOMIC_WRITE) ||
+			!f2fs_available_free_memory(sbi, INMEM_PAGES))) {
+		err = -EINVAL;
 		drop_atomic = true;
 		goto fail;
 	}
@@ -2331,7 +2332,7 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
 	f2fs_put_page(page, 1);
 	f2fs_write_failed(mapping, pos + len);
 	if (drop_atomic)
-		f2fs_drop_inmem_pages_all(sbi, false);
+		f2fs_disable_atomic_write(sbi);
 	return err;
 }
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 4d8b1de83143..1d5c8d543eda 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -621,7 +621,6 @@ enum {
 
 enum {
 	GC_FAILURE_PIN,
-	GC_FAILURE_ATOMIC,
 	MAX_GC_FAILURE
 };
 
@@ -1066,6 +1065,7 @@ enum {
 	SBI_POR_DOING,				/* recovery is doing or not */
 	SBI_NEED_SB_WRITE,			/* need to recover superblock */
 	SBI_NEED_CP,				/* need to checkpoint */
+	SBI_DISABLE_ATOMIC_WRITE,		/* turn off atomic write */
 };
 
 enum {
@@ -2833,7 +2833,7 @@ void f2fs_destroy_node_manager_caches(void);
  */
 bool f2fs_need_SSR(struct f2fs_sb_info *sbi);
 void f2fs_register_inmem_page(struct inode *inode, struct page *page);
-void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure);
+void f2fs_disable_atomic_write(struct f2fs_sb_info *sbi);
 void f2fs_drop_inmem_pages(struct inode *inode);
 void f2fs_drop_inmem_page(struct inode *inode, struct page *page);
 int f2fs_commit_inmem_pages(struct inode *inode);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 6880c6f78d58..b029a4ed3bb0 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1682,6 +1682,9 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
 	if (!S_ISREG(inode->i_mode))
 		return -EINVAL;
 
+	if (is_sbi_flag_set(F2FS_I_SB(inode), SBI_DISABLE_ATOMIC_WRITE))
+		return -EINVAL;
+
 	ret = mnt_want_write_file(filp);
 	if (ret)
 		return ret;
@@ -1750,7 +1753,6 @@ static int f2fs_ioc_commit_atomic_write(struct file *filp)
 		ret = f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true);
 		if (!ret) {
 			clear_inode_flag(inode, FI_ATOMIC_FILE);
-			F2FS_I(inode)->i_gc_failures[GC_FAILURE_ATOMIC] = 0;
 			stat_dec_atomic_write(inode);
 		}
 	} else {
@@ -1853,6 +1855,8 @@ static int f2fs_ioc_abort_volatile_write(struct file *filp)
 		ret = f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true);
 	}
 
+	clear_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
+
 	inode_unlock(inode);
 
 	mnt_drop_write_file(filp);
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 9093be6e7a7d..6d762f3cdfc7 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -629,7 +629,6 @@ static void move_data_block(struct inode *inode, block_t bidx,
 		goto out;
 
 	if (f2fs_is_atomic_file(inode)) {
-		F2FS_I(inode)->i_gc_failures[GC_FAILURE_ATOMIC]++;
 		F2FS_I_SB(inode)->skipped_atomic_files[gc_type]++;
 		goto out;
 	}
@@ -745,7 +744,6 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
 		goto out;
 
 	if (f2fs_is_atomic_file(inode)) {
-		F2FS_I(inode)->i_gc_failures[GC_FAILURE_ATOMIC]++;
 		F2FS_I_SB(inode)->skipped_atomic_files[gc_type]++;
 		goto out;
 	}
@@ -1100,7 +1098,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
 		if (has_not_enough_free_secs(sbi, sec_freed, 0)) {
 			if (skipped_round > MAX_SKIP_ATOMIC_COUNT &&
 				skipped_round * 2 >= round)
-				f2fs_drop_inmem_pages_all(sbi, true);
+				f2fs_disable_atomic_write(sbi);
 			segno = NULL_SEGNO;
 			goto gc_more;
 		}
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 9efce174c51a..05877a2f1894 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -274,11 +274,14 @@ static int __revoke_inmem_pages(struct inode *inode,
 	return err;
 }
 
-void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure)
+void f2fs_disable_atomic_write(struct f2fs_sb_info *sbi)
 {
 	struct list_head *head = &sbi->inode_list[ATOMIC_FILE];
 	struct inode *inode;
 	struct f2fs_inode_info *fi;
+
+	/* just turn it off */
+	set_sbi_flag(sbi, SBI_DISABLE_ATOMIC_WRITE);
 next:
 	spin_lock(&sbi->inode_lock[ATOMIC_FILE]);
 	if (list_empty(head)) {
@@ -290,17 +293,16 @@ void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure)
 	spin_unlock(&sbi->inode_lock[ATOMIC_FILE]);
 
 	if (inode) {
-		if (gc_failure) {
-			if (fi->i_gc_failures[GC_FAILURE_ATOMIC])
-				goto drop;
-			goto skip;
+		inode_lock(inode);
+		/* need to check whether it was already revoked */
+		if (f2fs_is_atomic_file(inode)) {
+			f2fs_drop_inmem_pages(inode);
+			set_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
 		}
-drop:
-		set_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
-		f2fs_drop_inmem_pages(inode);
+		inode_unlock(inode);
 		iput(inode);
 	}
-skip:
+
 	congestion_wait(BLK_RW_ASYNC, HZ/50);
 	cond_resched();
 	goto next;
@@ -320,7 +322,6 @@ void f2fs_drop_inmem_pages(struct inode *inode)
 	mutex_unlock(&fi->inmem_lock);
 
 	clear_inode_flag(inode, FI_ATOMIC_FILE);
-	fi->i_gc_failures[GC_FAILURE_ATOMIC] = 0;
 	stat_dec_atomic_write(inode);
 }
 
-- 
2.17.0.441.gb46fe60e1d-goog

[PATCH 2/3] f2fs: keep meta pages in cp_error state

[Jaegeuk Kim]

It turns out losing meta pages in shutdown period makes f2fs very unstable
so that I could see many unexpected error conditions.

Let's keep meta pages for fault injection and sudden power-off tests.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/checkpoint.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 9f1c96caebda..37ec0f16448e 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -242,11 +242,8 @@ static int __f2fs_write_meta_page(struct page *page,
 
 	trace_f2fs_writepage(page, META);
 
-	if (unlikely(f2fs_cp_error(sbi))) {
-		dec_page_count(sbi, F2FS_DIRTY_META);
-		unlock_page(page);
-		return 0;
-	}
+	if (unlikely(f2fs_cp_error(sbi)))
+		goto redirty_out;
 	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
 		goto redirty_out;
 	if (wbc->for_reclaim && page->index < GET_SUM_BLOCK(sbi, 0))
@@ -1129,6 +1126,9 @@ static void wait_on_all_pages_writeback(struct f2fs_sb_info *sbi)
 		if (!get_pages(sbi, F2FS_WB_CP_DATA))
 			break;
 
+		if (unlikely(f2fs_cp_error(sbi)))
+			break;
+
 		io_schedule_timeout(5*HZ);
 	}
 	finish_wait(&sbi->cp_wait, &wait);
@@ -1202,8 +1202,12 @@ static void commit_checkpoint(struct f2fs_sb_info *sbi,
 
 	/* writeout cp pack 2 page */
 	err = __f2fs_write_meta_page(page, &wbc, FS_CP_META_IO);
-	f2fs_bug_on(sbi, err);
+	if (unlikely(err && f2fs_cp_error(sbi))) {
+		f2fs_put_page(page, 1);
+		return;
+	}
 
+	f2fs_bug_on(sbi, err);
 	f2fs_put_page(page, 0);
 
 	/* submit checkpoint (with barrier if NOBARRIER is not set) */
@@ -1229,7 +1233,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 	while (get_pages(sbi, F2FS_DIRTY_META)) {
 		f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_CP_META_IO);
 		if (unlikely(f2fs_cp_error(sbi)))
-			return -EIO;
+			break;
 	}
 
 	/*
@@ -1309,7 +1313,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 			f2fs_sync_meta_pages(sbi, META, LONG_MAX,
 							FS_CP_META_IO);
 			if (unlikely(f2fs_cp_error(sbi)))
-				return -EIO;
+				break;
 		}
 	}
 
@@ -1350,9 +1354,6 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 	/* wait for previous submitted meta pages writeback */
 	wait_on_all_pages_writeback(sbi);
 
-	if (unlikely(f2fs_cp_error(sbi)))
-		return -EIO;
-
 	/* flush all device cache */
 	err = f2fs_flush_device_cache(sbi);
 	if (err)
@@ -1364,9 +1365,6 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 
 	f2fs_release_ino_entry(sbi, false);
 
-	if (unlikely(f2fs_cp_error(sbi)))
-		return -EIO;
-
 	clear_sbi_flag(sbi, SBI_IS_DIRTY);
 	clear_sbi_flag(sbi, SBI_NEED_CP);
 	__set_cp_next_pack(sbi);
@@ -1381,7 +1379,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 
 	f2fs_bug_on(sbi, get_pages(sbi, F2FS_DIRTY_DENTS));
 
-	return 0;
+	return unlikely(f2fs_cp_error(sbi));
 }
 
 /*
-- 
2.17.0.441.gb46fe60e1d-goog

[PATCH 3/3] f2fs: indicate shutdown f2fs to allow unmount successfully

[Jaegeuk Kim]

Once we shutdown f2fs, we have to flush stale pages in order to unmount
the system. In order to make stable, we need to stop fault injection as well.

Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/checkpoint.c | 1 +
 fs/f2fs/f2fs.h       | 7 +++++++
 fs/f2fs/file.c       | 4 ++++
 fs/f2fs/inode.c      | 3 +++
 fs/f2fs/node.c       | 3 ++-
 fs/f2fs/super.c      | 5 +----
 6 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 37ec0f16448e..3837fd6505f4 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -28,6 +28,7 @@ struct kmem_cache *f2fs_inode_entry_slab;
 
 void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io)
 {
+	f2fs_build_fault_attr(sbi, 0);
 	set_ckpt_flags(sbi, CP_ERROR_FLAG);
 	if (!end_io)
 		f2fs_flush_merged_writes(sbi);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 1d5c8d543eda..a3a62600f0bc 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1066,6 +1066,7 @@ enum {
 	SBI_NEED_SB_WRITE,			/* need to recover superblock */
 	SBI_NEED_CP,				/* need to checkpoint */
 	SBI_DISABLE_ATOMIC_WRITE,		/* turn off atomic write */
+	SBI_IS_SHUTDOWN,			/* shutdown by ioctl */
 };
 
 enum {
@@ -3373,4 +3374,10 @@ static inline bool f2fs_force_buffered_io(struct inode *inode, int rw)
 			F2FS_I_SB(inode)->s_ndevs);
 }
 
+#ifdef CONFIG_F2FS_FAULT_INJECTION
+extern void f2fs_build_fault_attr(struct f2fs_sb_info *sbi, unsigned int rate);
+#else
+#define f2fs_build_fault_attr(sbi, rate)		do { } while (0)
+#endif
+
 #endif
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index b029a4ed3bb0..e84c5288c60b 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1893,6 +1893,7 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
 		}
 		if (sb) {
 			f2fs_stop_checkpoint(sbi, false);
+			set_sbi_flag(sbi, SBI_IS_SHUTDOWN);
 			thaw_bdev(sb->s_bdev, sb);
 		}
 		break;
@@ -1902,13 +1903,16 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg)
 		if (ret)
 			goto out;
 		f2fs_stop_checkpoint(sbi, false);
+		set_sbi_flag(sbi, SBI_IS_SHUTDOWN);
 		break;
 	case F2FS_GOING_DOWN_NOSYNC:
 		f2fs_stop_checkpoint(sbi, false);
+		set_sbi_flag(sbi, SBI_IS_SHUTDOWN);
 		break;
 	case F2FS_GOING_DOWN_METAFLUSH:
 		f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO);
 		f2fs_stop_checkpoint(sbi, false);
+		set_sbi_flag(sbi, SBI_IS_SHUTDOWN);
 		break;
 	default:
 		ret = -EINVAL;
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index f121c864f4c0..f91dd017a65c 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -159,6 +159,9 @@ bool f2fs_inode_chksum_verify(struct f2fs_sb_info *sbi, struct page *page)
 	struct f2fs_inode *ri;
 	__u32 provided, calculated;
 
+	if (unlikely(is_sbi_flag_set(sbi, SBI_IS_SHUTDOWN)))
+		return true;
+
 	if (!f2fs_enable_inode_chksum(sbi, page) ||
 			PageDirty(page) || PageWriteback(page))
 		return true;
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index b0267d3823b4..1061dd18b09c 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1146,7 +1146,8 @@ static int read_node_page(struct page *page, int op_flags)
 
 	f2fs_get_node_info(sbi, page->index, &ni);
 
-	if (unlikely(ni.blk_addr == NULL_ADDR)) {
+	if (unlikely(ni.blk_addr == NULL_ADDR) ||
+			is_sbi_flag_set(sbi, SBI_IS_SHUTDOWN)) {
 		ClearPageUptodate(page);
 		return -ENOENT;
 	}
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 1dc6809fac38..1cb5d1e4fcfd 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -57,8 +57,7 @@ char *fault_name[FAULT_MAX] = {
 	[FAULT_CHECKPOINT]	= "checkpoint error",
 };
 
-static void f2fs_build_fault_attr(struct f2fs_sb_info *sbi,
-						unsigned int rate)
+void f2fs_build_fault_attr(struct f2fs_sb_info *sbi, unsigned int rate)
 {
 	struct f2fs_fault_info *ffi = &F2FS_OPTION(sbi).fault_info;
 
@@ -1379,9 +1378,7 @@ static void default_options(struct f2fs_sb_info *sbi)
 	set_opt(sbi, POSIX_ACL);
 #endif
 
-#ifdef CONFIG_F2FS_FAULT_INJECTION
 	f2fs_build_fault_attr(sbi, 0);
-#endif
 }
 
 #ifdef CONFIG_QUOTA
-- 
2.17.0.441.gb46fe60e1d-goog

Re: [PATCH 1/3] f2fs: turn off atomic writes when deteting abnormal behaviors

[Chao Yu]

On 2018/7/15 9:11, Jaegeuk Kim wrote:
> In order to prevent abusing atomic writes by abnormal users, we've added a
> threshold, 20% over memory footprint, which disallows further atomic writes.
> Previously, however, SQLite doesn't know the files became normal, so that
> it could write stale data and commit on revoked normal database file.
> 
> Once f2fs detects such the abnormal behavior, this patch simply disables
> all the atomic operations such as:
> - write_begin() gives EINVAL to avoid stale data writes, and SQLite will call
>   F2FS_IOC_ABORT_VOLATILE_WRITE to notify aborting the transaction,
> - F2FS_IOC_START_ATOMIC_WRITE gives EINVAL for SQLite to fall back normal
>   journal_mode,
> - F2FS_IOC_COMMIT_ATOMIC_WRITE gives EINVAL, if the file was revoked, so that
>   Framework retries to submit the transaction given propagated SQLite error.
> 
> Note that, this patch also turns off atomic operations, if foreground GC tries
> to move atomic files too frequently.

Well, how about just keeping original implementation: shutdown atomic write for
those files which are really affect fggc? Since intention of the original
behavior is targeting to abnormal atomic write usage, e.g. open atomic_write
file for very long time, then fggc will be blocked each time when moving its
block. So shutdown it, fggc will recover.

Thanks,

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/data.c    |  7 ++++---
>  fs/f2fs/f2fs.h    |  4 ++--
>  fs/f2fs/file.c    |  6 +++++-
>  fs/f2fs/gc.c      |  4 +---
>  fs/f2fs/segment.c | 21 +++++++++++----------
>  5 files changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 5e53d210e222..c9e75aa61c24 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2247,8 +2247,9 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
>  	trace_f2fs_write_begin(inode, pos, len, flags);
>  
>  	if (f2fs_is_atomic_file(inode) &&
> -			!f2fs_available_free_memory(sbi, INMEM_PAGES)) {
> -		err = -ENOMEM;
> +			(is_sbi_flag_set(sbi, SBI_DISABLE_ATOMIC_WRITE) ||
> +			!f2fs_available_free_memory(sbi, INMEM_PAGES))) {
> +		err = -EINVAL;
>  		drop_atomic = true;
>  		goto fail;
>  	}
> @@ -2331,7 +2332,7 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
>  	f2fs_put_page(page, 1);
>  	f2fs_write_failed(mapping, pos + len);
>  	if (drop_atomic)
> -		f2fs_drop_inmem_pages_all(sbi, false);
> +		f2fs_disable_atomic_write(sbi);
>  	return err;
>  }
>  
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 4d8b1de83143..1d5c8d543eda 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -621,7 +621,6 @@ enum {
>  
>  enum {
>  	GC_FAILURE_PIN,
> -	GC_FAILURE_ATOMIC,
>  	MAX_GC_FAILURE
>  };
>  
> @@ -1066,6 +1065,7 @@ enum {
>  	SBI_POR_DOING,				/* recovery is doing or not */
>  	SBI_NEED_SB_WRITE,			/* need to recover superblock */
>  	SBI_NEED_CP,				/* need to checkpoint */
> +	SBI_DISABLE_ATOMIC_WRITE,		/* turn off atomic write */
>  };
>  
>  enum {
> @@ -2833,7 +2833,7 @@ void f2fs_destroy_node_manager_caches(void);
>   */
>  bool f2fs_need_SSR(struct f2fs_sb_info *sbi);
>  void f2fs_register_inmem_page(struct inode *inode, struct page *page);
> -void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure);
> +void f2fs_disable_atomic_write(struct f2fs_sb_info *sbi);
>  void f2fs_drop_inmem_pages(struct inode *inode);
>  void f2fs_drop_inmem_page(struct inode *inode, struct page *page);
>  int f2fs_commit_inmem_pages(struct inode *inode);
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 6880c6f78d58..b029a4ed3bb0 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1682,6 +1682,9 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
>  	if (!S_ISREG(inode->i_mode))
>  		return -EINVAL;
>  
> +	if (is_sbi_flag_set(F2FS_I_SB(inode), SBI_DISABLE_ATOMIC_WRITE))
> +		return -EINVAL;
> +
>  	ret = mnt_want_write_file(filp);
>  	if (ret)
>  		return ret;
> @@ -1750,7 +1753,6 @@ static int f2fs_ioc_commit_atomic_write(struct file *filp)
>  		ret = f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true);
>  		if (!ret) {
>  			clear_inode_flag(inode, FI_ATOMIC_FILE);
> -			F2FS_I(inode)->i_gc_failures[GC_FAILURE_ATOMIC] = 0;
>  			stat_dec_atomic_write(inode);
>  		}
>  	} else {
> @@ -1853,6 +1855,8 @@ static int f2fs_ioc_abort_volatile_write(struct file *filp)
>  		ret = f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true);
>  	}
>  
> +	clear_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
> +
>  	inode_unlock(inode);
>  
>  	mnt_drop_write_file(filp);
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 9093be6e7a7d..6d762f3cdfc7 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -629,7 +629,6 @@ static void move_data_block(struct inode *inode, block_t bidx,
>  		goto out;
>  
>  	if (f2fs_is_atomic_file(inode)) {
> -		F2FS_I(inode)->i_gc_failures[GC_FAILURE_ATOMIC]++;
>  		F2FS_I_SB(inode)->skipped_atomic_files[gc_type]++;
>  		goto out;
>  	}
> @@ -745,7 +744,6 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
>  		goto out;
>  
>  	if (f2fs_is_atomic_file(inode)) {
> -		F2FS_I(inode)->i_gc_failures[GC_FAILURE_ATOMIC]++;
>  		F2FS_I_SB(inode)->skipped_atomic_files[gc_type]++;
>  		goto out;
>  	}
> @@ -1100,7 +1098,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>  		if (has_not_enough_free_secs(sbi, sec_freed, 0)) {
>  			if (skipped_round > MAX_SKIP_ATOMIC_COUNT &&
>  				skipped_round * 2 >= round)
> -				f2fs_drop_inmem_pages_all(sbi, true);
> +				f2fs_disable_atomic_write(sbi);
>  			segno = NULL_SEGNO;
>  			goto gc_more;
>  		}
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 9efce174c51a..05877a2f1894 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -274,11 +274,14 @@ static int __revoke_inmem_pages(struct inode *inode,
>  	return err;
>  }
>  
> -void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure)
> +void f2fs_disable_atomic_write(struct f2fs_sb_info *sbi)
>  {
>  	struct list_head *head = &sbi->inode_list[ATOMIC_FILE];
>  	struct inode *inode;
>  	struct f2fs_inode_info *fi;
> +
> +	/* just turn it off */
> +	set_sbi_flag(sbi, SBI_DISABLE_ATOMIC_WRITE);
>  next:
>  	spin_lock(&sbi->inode_lock[ATOMIC_FILE]);
>  	if (list_empty(head)) {
> @@ -290,17 +293,16 @@ void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure)
>  	spin_unlock(&sbi->inode_lock[ATOMIC_FILE]);
>  
>  	if (inode) {
> -		if (gc_failure) {
> -			if (fi->i_gc_failures[GC_FAILURE_ATOMIC])
> -				goto drop;
> -			goto skip;
> +		inode_lock(inode);
> +		/* need to check whether it was already revoked */
> +		if (f2fs_is_atomic_file(inode)) {
> +			f2fs_drop_inmem_pages(inode);
> +			set_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
>  		}
> -drop:
> -		set_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
> -		f2fs_drop_inmem_pages(inode);
> +		inode_unlock(inode);
>  		iput(inode);
>  	}
> -skip:
> +
>  	congestion_wait(BLK_RW_ASYNC, HZ/50);
>  	cond_resched();
>  	goto next;
> @@ -320,7 +322,6 @@ void f2fs_drop_inmem_pages(struct inode *inode)
>  	mutex_unlock(&fi->inmem_lock);
>  
>  	clear_inode_flag(inode, FI_ATOMIC_FILE);
> -	fi->i_gc_failures[GC_FAILURE_ATOMIC] = 0;
>  	stat_dec_atomic_write(inode);
>  }
>  
> 

Re: [PATCH 1/3] f2fs: turn off atomic writes when deteting abnormal behaviors

[Chao Yu]

On 2018/7/15 9:11, Jaegeuk Kim wrote:
> In order to prevent abusing atomic writes by abnormal users, we've added a
> threshold, 20% over memory footprint, which disallows further atomic writes.
> Previously, however, SQLite doesn't know the files became normal, so that
> it could write stale data and commit on revoked normal database file.
> 
> Once f2fs detects such the abnormal behavior, this patch simply disables
> all the atomic operations such as:
> - write_begin() gives EINVAL to avoid stale data writes, and SQLite will call
>   F2FS_IOC_ABORT_VOLATILE_WRITE to notify aborting the transaction,
> - F2FS_IOC_START_ATOMIC_WRITE gives EINVAL for SQLite to fall back normal
>   journal_mode,
> - F2FS_IOC_COMMIT_ATOMIC_WRITE gives EINVAL, if the file was revoked, so that
>   Framework retries to submit the transaction given propagated SQLite error.
> 
> Note that, this patch also turns off atomic operations, if foreground GC tries
> to move atomic files too frequently.

Well, how about just keeping original implementation: shutdown atomic write for
those files which are really affect fggc? Since intention of the original
behavior is targeting to abnormal atomic write usage, e.g. open atomic_write
file for very long time, then fggc will be blocked each time when moving its
block. So shutdown it, fggc will recover.

Thanks,

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/data.c    |  7 ++++---
>  fs/f2fs/f2fs.h    |  4 ++--
>  fs/f2fs/file.c    |  6 +++++-
>  fs/f2fs/gc.c      |  4 +---
>  fs/f2fs/segment.c | 21 +++++++++++----------
>  5 files changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 5e53d210e222..c9e75aa61c24 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2247,8 +2247,9 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
>  	trace_f2fs_write_begin(inode, pos, len, flags);
>  
>  	if (f2fs_is_atomic_file(inode) &&
> -			!f2fs_available_free_memory(sbi, INMEM_PAGES)) {
> -		err = -ENOMEM;
> +			(is_sbi_flag_set(sbi, SBI_DISABLE_ATOMIC_WRITE) ||
> +			!f2fs_available_free_memory(sbi, INMEM_PAGES))) {
> +		err = -EINVAL;
>  		drop_atomic = true;
>  		goto fail;
>  	}
> @@ -2331,7 +2332,7 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
>  	f2fs_put_page(page, 1);
>  	f2fs_write_failed(mapping, pos + len);
>  	if (drop_atomic)
> -		f2fs_drop_inmem_pages_all(sbi, false);
> +		f2fs_disable_atomic_write(sbi);
>  	return err;
>  }
>  
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 4d8b1de83143..1d5c8d543eda 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -621,7 +621,6 @@ enum {
>  
>  enum {
>  	GC_FAILURE_PIN,
> -	GC_FAILURE_ATOMIC,
>  	MAX_GC_FAILURE
>  };
>  
> @@ -1066,6 +1065,7 @@ enum {
>  	SBI_POR_DOING,				/* recovery is doing or not */
>  	SBI_NEED_SB_WRITE,			/* need to recover superblock */
>  	SBI_NEED_CP,				/* need to checkpoint */
> +	SBI_DISABLE_ATOMIC_WRITE,		/* turn off atomic write */
>  };
>  
>  enum {
> @@ -2833,7 +2833,7 @@ void f2fs_destroy_node_manager_caches(void);
>   */
>  bool f2fs_need_SSR(struct f2fs_sb_info *sbi);
>  void f2fs_register_inmem_page(struct inode *inode, struct page *page);
> -void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure);
> +void f2fs_disable_atomic_write(struct f2fs_sb_info *sbi);
>  void f2fs_drop_inmem_pages(struct inode *inode);
>  void f2fs_drop_inmem_page(struct inode *inode, struct page *page);
>  int f2fs_commit_inmem_pages(struct inode *inode);
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 6880c6f78d58..b029a4ed3bb0 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1682,6 +1682,9 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
>  	if (!S_ISREG(inode->i_mode))
>  		return -EINVAL;
>  
> +	if (is_sbi_flag_set(F2FS_I_SB(inode), SBI_DISABLE_ATOMIC_WRITE))
> +		return -EINVAL;
> +
>  	ret = mnt_want_write_file(filp);
>  	if (ret)
>  		return ret;
> @@ -1750,7 +1753,6 @@ static int f2fs_ioc_commit_atomic_write(struct file *filp)
>  		ret = f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true);
>  		if (!ret) {
>  			clear_inode_flag(inode, FI_ATOMIC_FILE);
> -			F2FS_I(inode)->i_gc_failures[GC_FAILURE_ATOMIC] = 0;
>  			stat_dec_atomic_write(inode);
>  		}
>  	} else {
> @@ -1853,6 +1855,8 @@ static int f2fs_ioc_abort_volatile_write(struct file *filp)
>  		ret = f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true);
>  	}
>  
> +	clear_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
> +
>  	inode_unlock(inode);
>  
>  	mnt_drop_write_file(filp);
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 9093be6e7a7d..6d762f3cdfc7 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -629,7 +629,6 @@ static void move_data_block(struct inode *inode, block_t bidx,
>  		goto out;
>  
>  	if (f2fs_is_atomic_file(inode)) {
> -		F2FS_I(inode)->i_gc_failures[GC_FAILURE_ATOMIC]++;
>  		F2FS_I_SB(inode)->skipped_atomic_files[gc_type]++;
>  		goto out;
>  	}
> @@ -745,7 +744,6 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
>  		goto out;
>  
>  	if (f2fs_is_atomic_file(inode)) {
> -		F2FS_I(inode)->i_gc_failures[GC_FAILURE_ATOMIC]++;
>  		F2FS_I_SB(inode)->skipped_atomic_files[gc_type]++;
>  		goto out;
>  	}
> @@ -1100,7 +1098,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>  		if (has_not_enough_free_secs(sbi, sec_freed, 0)) {
>  			if (skipped_round > MAX_SKIP_ATOMIC_COUNT &&
>  				skipped_round * 2 >= round)
> -				f2fs_drop_inmem_pages_all(sbi, true);
> +				f2fs_disable_atomic_write(sbi);
>  			segno = NULL_SEGNO;
>  			goto gc_more;
>  		}
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 9efce174c51a..05877a2f1894 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -274,11 +274,14 @@ static int __revoke_inmem_pages(struct inode *inode,
>  	return err;
>  }
>  
> -void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure)
> +void f2fs_disable_atomic_write(struct f2fs_sb_info *sbi)
>  {
>  	struct list_head *head = &sbi->inode_list[ATOMIC_FILE];
>  	struct inode *inode;
>  	struct f2fs_inode_info *fi;
> +
> +	/* just turn it off */
> +	set_sbi_flag(sbi, SBI_DISABLE_ATOMIC_WRITE);
>  next:
>  	spin_lock(&sbi->inode_lock[ATOMIC_FILE]);
>  	if (list_empty(head)) {
> @@ -290,17 +293,16 @@ void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure)
>  	spin_unlock(&sbi->inode_lock[ATOMIC_FILE]);
>  
>  	if (inode) {
> -		if (gc_failure) {
> -			if (fi->i_gc_failures[GC_FAILURE_ATOMIC])
> -				goto drop;
> -			goto skip;
> +		inode_lock(inode);
> +		/* need to check whether it was already revoked */
> +		if (f2fs_is_atomic_file(inode)) {
> +			f2fs_drop_inmem_pages(inode);
> +			set_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
>  		}
> -drop:
> -		set_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
> -		f2fs_drop_inmem_pages(inode);
> +		inode_unlock(inode);
>  		iput(inode);
>  	}
> -skip:
> +
>  	congestion_wait(BLK_RW_ASYNC, HZ/50);
>  	cond_resched();
>  	goto next;
> @@ -320,7 +322,6 @@ void f2fs_drop_inmem_pages(struct inode *inode)
>  	mutex_unlock(&fi->inmem_lock);
>  
>  	clear_inode_flag(inode, FI_ATOMIC_FILE);
> -	fi->i_gc_failures[GC_FAILURE_ATOMIC] = 0;
>  	stat_dec_atomic_write(inode);
>  }
>  
> 

Re: [PATCH 2/3] f2fs: keep meta pages in cp_error state

[Chao Yu]

On 2018/7/15 9:11, Jaegeuk Kim wrote:
> It turns out losing meta pages in shutdown period makes f2fs very unstable
> so that I could see many unexpected error conditions.
> 
> Let's keep meta pages for fault injection and sudden power-off tests.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/checkpoint.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 9f1c96caebda..37ec0f16448e 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -242,11 +242,8 @@ static int __f2fs_write_meta_page(struct page *page,
>  
>  	trace_f2fs_writepage(page, META);
>  
> -	if (unlikely(f2fs_cp_error(sbi))) {
> -		dec_page_count(sbi, F2FS_DIRTY_META);
> -		unlock_page(page);
> -		return 0;
> -	}
> +	if (unlikely(f2fs_cp_error(sbi)))
> +		goto redirty_out;
>  	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
>  		goto redirty_out;
>  	if (wbc->for_reclaim && page->index < GET_SUM_BLOCK(sbi, 0))
> @@ -1129,6 +1126,9 @@ static void wait_on_all_pages_writeback(struct f2fs_sb_info *sbi)
>  		if (!get_pages(sbi, F2FS_WB_CP_DATA))
>  			break;
>  
> +		if (unlikely(f2fs_cp_error(sbi)))
> +			break;
> +
>  		io_schedule_timeout(5*HZ);
>  	}
>  	finish_wait(&sbi->cp_wait, &wait);
> @@ -1202,8 +1202,12 @@ static void commit_checkpoint(struct f2fs_sb_info *sbi,
>  
>  	/* writeout cp pack 2 page */
>  	err = __f2fs_write_meta_page(page, &wbc, FS_CP_META_IO);
> -	f2fs_bug_on(sbi, err);
> +	if (unlikely(err && f2fs_cp_error(sbi))) {
> +		f2fs_put_page(page, 1);
> +		return;
> +	}
>  
> +	f2fs_bug_on(sbi, err);
>  	f2fs_put_page(page, 0);
>  
>  	/* submit checkpoint (with barrier if NOBARRIER is not set) */
> @@ -1229,7 +1233,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  	while (get_pages(sbi, F2FS_DIRTY_META)) {
>  		f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_CP_META_IO);
>  		if (unlikely(f2fs_cp_error(sbi)))
> -			return -EIO;
> +			break;
>  	}
>  
>  	/*
> @@ -1309,7 +1313,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  			f2fs_sync_meta_pages(sbi, META, LONG_MAX,
>  							FS_CP_META_IO);
>  			if (unlikely(f2fs_cp_error(sbi)))
> -				return -EIO;
> +				break;
>  		}
>  	}
>  
> @@ -1350,9 +1354,6 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  	/* wait for previous submitted meta pages writeback */
>  	wait_on_all_pages_writeback(sbi);
>  
> -	if (unlikely(f2fs_cp_error(sbi)))
> -		return -EIO;
> -
>  	/* flush all device cache */
>  	err = f2fs_flush_device_cache(sbi);
>  	if (err)
> @@ -1364,9 +1365,6 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  
>  	f2fs_release_ino_entry(sbi, false);
>  
> -	if (unlikely(f2fs_cp_error(sbi)))
> -		return -EIO;
> -
>  	clear_sbi_flag(sbi, SBI_IS_DIRTY);
>  	clear_sbi_flag(sbi, SBI_NEED_CP);
>  	__set_cp_next_pack(sbi);
> @@ -1381,7 +1379,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  
>  	f2fs_bug_on(sbi, get_pages(sbi, F2FS_DIRTY_DENTS));
>  
> -	return 0;
> +	return unlikely(f2fs_cp_error(sbi));

return unlikely(f2fs_cp_error(sbi)) ? -EIO : 0;

>  }
>  
>  /*
> 

Re: [PATCH 2/3] f2fs: keep meta pages in cp_error state

[Chao Yu]

On 2018/7/15 9:11, Jaegeuk Kim wrote:
> It turns out losing meta pages in shutdown period makes f2fs very unstable
> so that I could see many unexpected error conditions.
> 
> Let's keep meta pages for fault injection and sudden power-off tests.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/checkpoint.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 9f1c96caebda..37ec0f16448e 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -242,11 +242,8 @@ static int __f2fs_write_meta_page(struct page *page,
>  
>  	trace_f2fs_writepage(page, META);
>  
> -	if (unlikely(f2fs_cp_error(sbi))) {
> -		dec_page_count(sbi, F2FS_DIRTY_META);
> -		unlock_page(page);
> -		return 0;
> -	}
> +	if (unlikely(f2fs_cp_error(sbi)))
> +		goto redirty_out;
>  	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
>  		goto redirty_out;
>  	if (wbc->for_reclaim && page->index < GET_SUM_BLOCK(sbi, 0))
> @@ -1129,6 +1126,9 @@ static void wait_on_all_pages_writeback(struct f2fs_sb_info *sbi)
>  		if (!get_pages(sbi, F2FS_WB_CP_DATA))
>  			break;
>  
> +		if (unlikely(f2fs_cp_error(sbi)))
> +			break;
> +
>  		io_schedule_timeout(5*HZ);
>  	}
>  	finish_wait(&sbi->cp_wait, &wait);
> @@ -1202,8 +1202,12 @@ static void commit_checkpoint(struct f2fs_sb_info *sbi,
>  
>  	/* writeout cp pack 2 page */
>  	err = __f2fs_write_meta_page(page, &wbc, FS_CP_META_IO);
> -	f2fs_bug_on(sbi, err);
> +	if (unlikely(err && f2fs_cp_error(sbi))) {
> +		f2fs_put_page(page, 1);
> +		return;
> +	}
>  
> +	f2fs_bug_on(sbi, err);
>  	f2fs_put_page(page, 0);
>  
>  	/* submit checkpoint (with barrier if NOBARRIER is not set) */
> @@ -1229,7 +1233,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  	while (get_pages(sbi, F2FS_DIRTY_META)) {
>  		f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_CP_META_IO);
>  		if (unlikely(f2fs_cp_error(sbi)))
> -			return -EIO;
> +			break;
>  	}
>  
>  	/*
> @@ -1309,7 +1313,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  			f2fs_sync_meta_pages(sbi, META, LONG_MAX,
>  							FS_CP_META_IO);
>  			if (unlikely(f2fs_cp_error(sbi)))
> -				return -EIO;
> +				break;
>  		}
>  	}
>  
> @@ -1350,9 +1354,6 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  	/* wait for previous submitted meta pages writeback */
>  	wait_on_all_pages_writeback(sbi);
>  
> -	if (unlikely(f2fs_cp_error(sbi)))
> -		return -EIO;
> -
>  	/* flush all device cache */
>  	err = f2fs_flush_device_cache(sbi);
>  	if (err)
> @@ -1364,9 +1365,6 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  
>  	f2fs_release_ino_entry(sbi, false);
>  
> -	if (unlikely(f2fs_cp_error(sbi)))
> -		return -EIO;
> -
>  	clear_sbi_flag(sbi, SBI_IS_DIRTY);
>  	clear_sbi_flag(sbi, SBI_NEED_CP);
>  	__set_cp_next_pack(sbi);
> @@ -1381,7 +1379,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  
>  	f2fs_bug_on(sbi, get_pages(sbi, F2FS_DIRTY_DENTS));
>  
> -	return 0;
> +	return unlikely(f2fs_cp_error(sbi));

return unlikely(f2fs_cp_error(sbi)) ? -EIO : 0;

>  }
>  
>  /*
> 

Re: [PATCH 2/3] f2fs: keep meta pages in cp_error state

[Jaegeuk Kim]

On 07/16, Chao Yu wrote:
> On 2018/7/15 9:11, Jaegeuk Kim wrote:
> > It turns out losing meta pages in shutdown period makes f2fs very unstable
> > so that I could see many unexpected error conditions.
> > 
> > Let's keep meta pages for fault injection and sudden power-off tests.
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/checkpoint.c | 28 +++++++++++++---------------
> >  1 file changed, 13 insertions(+), 15 deletions(-)
> > 
> > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > index 9f1c96caebda..37ec0f16448e 100644
> > --- a/fs/f2fs/checkpoint.c
> > +++ b/fs/f2fs/checkpoint.c
> > @@ -242,11 +242,8 @@ static int __f2fs_write_meta_page(struct page *page,
> >  
> >  	trace_f2fs_writepage(page, META);
> >  
> > -	if (unlikely(f2fs_cp_error(sbi))) {
> > -		dec_page_count(sbi, F2FS_DIRTY_META);
> > -		unlock_page(page);
> > -		return 0;
> > -	}
> > +	if (unlikely(f2fs_cp_error(sbi)))
> > +		goto redirty_out;
> >  	if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
> >  		goto redirty_out;
> >  	if (wbc->for_reclaim && page->index < GET_SUM_BLOCK(sbi, 0))
> > @@ -1129,6 +1126,9 @@ static void wait_on_all_pages_writeback(struct f2fs_sb_info *sbi)
> >  		if (!get_pages(sbi, F2FS_WB_CP_DATA))
> >  			break;
> >  
> > +		if (unlikely(f2fs_cp_error(sbi)))
> > +			break;
> > +
> >  		io_schedule_timeout(5*HZ);
> >  	}
> >  	finish_wait(&sbi->cp_wait, &wait);
> > @@ -1202,8 +1202,12 @@ static void commit_checkpoint(struct f2fs_sb_info *sbi,
> >  
> >  	/* writeout cp pack 2 page */
> >  	err = __f2fs_write_meta_page(page, &wbc, FS_CP_META_IO);
> > -	f2fs_bug_on(sbi, err);
> > +	if (unlikely(err && f2fs_cp_error(sbi))) {
> > +		f2fs_put_page(page, 1);
> > +		return;
> > +	}
> >  
> > +	f2fs_bug_on(sbi, err);
> >  	f2fs_put_page(page, 0);
> >  
> >  	/* submit checkpoint (with barrier if NOBARRIER is not set) */
> > @@ -1229,7 +1233,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
> >  	while (get_pages(sbi, F2FS_DIRTY_META)) {
> >  		f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_CP_META_IO);
> >  		if (unlikely(f2fs_cp_error(sbi)))
> > -			return -EIO;
> > +			break;
> >  	}
> >  
> >  	/*
> > @@ -1309,7 +1313,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
> >  			f2fs_sync_meta_pages(sbi, META, LONG_MAX,
> >  							FS_CP_META_IO);
> >  			if (unlikely(f2fs_cp_error(sbi)))
> > -				return -EIO;
> > +				break;
> >  		}
> >  	}
> >  
> > @@ -1350,9 +1354,6 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
> >  	/* wait for previous submitted meta pages writeback */
> >  	wait_on_all_pages_writeback(sbi);
> >  
> > -	if (unlikely(f2fs_cp_error(sbi)))
> > -		return -EIO;
> > -
> >  	/* flush all device cache */
> >  	err = f2fs_flush_device_cache(sbi);
> >  	if (err)
> > @@ -1364,9 +1365,6 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
> >  
> >  	f2fs_release_ino_entry(sbi, false);
> >  
> > -	if (unlikely(f2fs_cp_error(sbi)))
> > -		return -EIO;
> > -
> >  	clear_sbi_flag(sbi, SBI_IS_DIRTY);
> >  	clear_sbi_flag(sbi, SBI_NEED_CP);
> >  	__set_cp_next_pack(sbi);
> > @@ -1381,7 +1379,7 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
> >  
> >  	f2fs_bug_on(sbi, get_pages(sbi, F2FS_DIRTY_DENTS));
> >  
> > -	return 0;
> > +	return unlikely(f2fs_cp_error(sbi));
> 
> return unlikely(f2fs_cp_error(sbi)) ? -EIO : 0;
> 

Done.

> >  }
> >  
> >  /*
> > 

Re: [PATCH 1/3] f2fs: turn off atomic writes when deteting abnormal behaviors

[Jaegeuk Kim]

On 07/16, Chao Yu wrote:
> On 2018/7/15 9:11, Jaegeuk Kim wrote:
> > In order to prevent abusing atomic writes by abnormal users, we've added a
> > threshold, 20% over memory footprint, which disallows further atomic writes.
> > Previously, however, SQLite doesn't know the files became normal, so that
> > it could write stale data and commit on revoked normal database file.
> > 
> > Once f2fs detects such the abnormal behavior, this patch simply disables
> > all the atomic operations such as:
> > - write_begin() gives EINVAL to avoid stale data writes, and SQLite will call
> >   F2FS_IOC_ABORT_VOLATILE_WRITE to notify aborting the transaction,
> > - F2FS_IOC_START_ATOMIC_WRITE gives EINVAL for SQLite to fall back normal
> >   journal_mode,
> > - F2FS_IOC_COMMIT_ATOMIC_WRITE gives EINVAL, if the file was revoked, so that
> >   Framework retries to submit the transaction given propagated SQLite error.
> > 
> > Note that, this patch also turns off atomic operations, if foreground GC tries
> > to move atomic files too frequently.
> 
> Well, how about just keeping original implementation: shutdown atomic write for
> those files which are really affect fggc? Since intention of the original
> behavior is targeting to abnormal atomic write usage, e.g. open atomic_write
> file for very long time, then fggc will be blocked each time when moving its
> block. So shutdown it, fggc will recover.

The point here is stopping sqlite to keep going wrong data writes even after
we already revoked blocks.

Thanks,

> 
> Thanks,
> 
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/data.c    |  7 ++++---
> >  fs/f2fs/f2fs.h    |  4 ++--
> >  fs/f2fs/file.c    |  6 +++++-
> >  fs/f2fs/gc.c      |  4 +---
> >  fs/f2fs/segment.c | 21 +++++++++++----------
> >  5 files changed, 23 insertions(+), 19 deletions(-)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 5e53d210e222..c9e75aa61c24 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -2247,8 +2247,9 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
> >  	trace_f2fs_write_begin(inode, pos, len, flags);
> >  
> >  	if (f2fs_is_atomic_file(inode) &&
> > -			!f2fs_available_free_memory(sbi, INMEM_PAGES)) {
> > -		err = -ENOMEM;
> > +			(is_sbi_flag_set(sbi, SBI_DISABLE_ATOMIC_WRITE) ||
> > +			!f2fs_available_free_memory(sbi, INMEM_PAGES))) {
> > +		err = -EINVAL;
> >  		drop_atomic = true;
> >  		goto fail;
> >  	}
> > @@ -2331,7 +2332,7 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
> >  	f2fs_put_page(page, 1);
> >  	f2fs_write_failed(mapping, pos + len);
> >  	if (drop_atomic)
> > -		f2fs_drop_inmem_pages_all(sbi, false);
> > +		f2fs_disable_atomic_write(sbi);
> >  	return err;
> >  }
> >  
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 4d8b1de83143..1d5c8d543eda 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -621,7 +621,6 @@ enum {
> >  
> >  enum {
> >  	GC_FAILURE_PIN,
> > -	GC_FAILURE_ATOMIC,
> >  	MAX_GC_FAILURE
> >  };
> >  
> > @@ -1066,6 +1065,7 @@ enum {
> >  	SBI_POR_DOING,				/* recovery is doing or not */
> >  	SBI_NEED_SB_WRITE,			/* need to recover superblock */
> >  	SBI_NEED_CP,				/* need to checkpoint */
> > +	SBI_DISABLE_ATOMIC_WRITE,		/* turn off atomic write */
> >  };
> >  
> >  enum {
> > @@ -2833,7 +2833,7 @@ void f2fs_destroy_node_manager_caches(void);
> >   */
> >  bool f2fs_need_SSR(struct f2fs_sb_info *sbi);
> >  void f2fs_register_inmem_page(struct inode *inode, struct page *page);
> > -void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure);
> > +void f2fs_disable_atomic_write(struct f2fs_sb_info *sbi);
> >  void f2fs_drop_inmem_pages(struct inode *inode);
> >  void f2fs_drop_inmem_page(struct inode *inode, struct page *page);
> >  int f2fs_commit_inmem_pages(struct inode *inode);
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index 6880c6f78d58..b029a4ed3bb0 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -1682,6 +1682,9 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
> >  	if (!S_ISREG(inode->i_mode))
> >  		return -EINVAL;
> >  
> > +	if (is_sbi_flag_set(F2FS_I_SB(inode), SBI_DISABLE_ATOMIC_WRITE))
> > +		return -EINVAL;
> > +
> >  	ret = mnt_want_write_file(filp);
> >  	if (ret)
> >  		return ret;
> > @@ -1750,7 +1753,6 @@ static int f2fs_ioc_commit_atomic_write(struct file *filp)
> >  		ret = f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true);
> >  		if (!ret) {
> >  			clear_inode_flag(inode, FI_ATOMIC_FILE);
> > -			F2FS_I(inode)->i_gc_failures[GC_FAILURE_ATOMIC] = 0;
> >  			stat_dec_atomic_write(inode);
> >  		}
> >  	} else {
> > @@ -1853,6 +1855,8 @@ static int f2fs_ioc_abort_volatile_write(struct file *filp)
> >  		ret = f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true);
> >  	}
> >  
> > +	clear_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
> > +
> >  	inode_unlock(inode);
> >  
> >  	mnt_drop_write_file(filp);
> > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> > index 9093be6e7a7d..6d762f3cdfc7 100644
> > --- a/fs/f2fs/gc.c
> > +++ b/fs/f2fs/gc.c
> > @@ -629,7 +629,6 @@ static void move_data_block(struct inode *inode, block_t bidx,
> >  		goto out;
> >  
> >  	if (f2fs_is_atomic_file(inode)) {
> > -		F2FS_I(inode)->i_gc_failures[GC_FAILURE_ATOMIC]++;
> >  		F2FS_I_SB(inode)->skipped_atomic_files[gc_type]++;
> >  		goto out;
> >  	}
> > @@ -745,7 +744,6 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
> >  		goto out;
> >  
> >  	if (f2fs_is_atomic_file(inode)) {
> > -		F2FS_I(inode)->i_gc_failures[GC_FAILURE_ATOMIC]++;
> >  		F2FS_I_SB(inode)->skipped_atomic_files[gc_type]++;
> >  		goto out;
> >  	}
> > @@ -1100,7 +1098,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
> >  		if (has_not_enough_free_secs(sbi, sec_freed, 0)) {
> >  			if (skipped_round > MAX_SKIP_ATOMIC_COUNT &&
> >  				skipped_round * 2 >= round)
> > -				f2fs_drop_inmem_pages_all(sbi, true);
> > +				f2fs_disable_atomic_write(sbi);
> >  			segno = NULL_SEGNO;
> >  			goto gc_more;
> >  		}
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 9efce174c51a..05877a2f1894 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -274,11 +274,14 @@ static int __revoke_inmem_pages(struct inode *inode,
> >  	return err;
> >  }
> >  
> > -void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure)
> > +void f2fs_disable_atomic_write(struct f2fs_sb_info *sbi)
> >  {
> >  	struct list_head *head = &sbi->inode_list[ATOMIC_FILE];
> >  	struct inode *inode;
> >  	struct f2fs_inode_info *fi;
> > +
> > +	/* just turn it off */
> > +	set_sbi_flag(sbi, SBI_DISABLE_ATOMIC_WRITE);
> >  next:
> >  	spin_lock(&sbi->inode_lock[ATOMIC_FILE]);
> >  	if (list_empty(head)) {
> > @@ -290,17 +293,16 @@ void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure)
> >  	spin_unlock(&sbi->inode_lock[ATOMIC_FILE]);
> >  
> >  	if (inode) {
> > -		if (gc_failure) {
> > -			if (fi->i_gc_failures[GC_FAILURE_ATOMIC])
> > -				goto drop;
> > -			goto skip;
> > +		inode_lock(inode);
> > +		/* need to check whether it was already revoked */
> > +		if (f2fs_is_atomic_file(inode)) {
> > +			f2fs_drop_inmem_pages(inode);
> > +			set_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
> >  		}
> > -drop:
> > -		set_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
> > -		f2fs_drop_inmem_pages(inode);
> > +		inode_unlock(inode);
> >  		iput(inode);
> >  	}
> > -skip:
> > +
> >  	congestion_wait(BLK_RW_ASYNC, HZ/50);
> >  	cond_resched();
> >  	goto next;
> > @@ -320,7 +322,6 @@ void f2fs_drop_inmem_pages(struct inode *inode)
> >  	mutex_unlock(&fi->inmem_lock);
> >  
> >  	clear_inode_flag(inode, FI_ATOMIC_FILE);
> > -	fi->i_gc_failures[GC_FAILURE_ATOMIC] = 0;
> >  	stat_dec_atomic_write(inode);
> >  }
> >  
> > 

Re: [f2fs-dev] [PATCH 1/3] f2fs: turn off atomic writes when deteting abnormal behaviors

[Chao Yu]

On 2018/7/23 21:03, Jaegeuk Kim wrote:
> On 07/16, Chao Yu wrote:
>> On 2018/7/15 9:11, Jaegeuk Kim wrote:
>>> In order to prevent abusing atomic writes by abnormal users, we've added a
>>> threshold, 20% over memory footprint, which disallows further atomic writes.
>>> Previously, however, SQLite doesn't know the files became normal, so that
>>> it could write stale data and commit on revoked normal database file.
>>>
>>> Once f2fs detects such the abnormal behavior, this patch simply disables
>>> all the atomic operations such as:
>>> - write_begin() gives EINVAL to avoid stale data writes, and SQLite will call
>>>   F2FS_IOC_ABORT_VOLATILE_WRITE to notify aborting the transaction,
>>> - F2FS_IOC_START_ATOMIC_WRITE gives EINVAL for SQLite to fall back normal
>>>   journal_mode,
>>> - F2FS_IOC_COMMIT_ATOMIC_WRITE gives EINVAL, if the file was revoked, so that
>>>   Framework retries to submit the transaction given propagated SQLite error.
>>>
>>> Note that, this patch also turns off atomic operations, if foreground GC tries
>>> to move atomic files too frequently.
>>
>> Well, how about just keeping original implementation: shutdown atomic write for
>> those files which are really affect fggc? Since intention of the original
>> behavior is targeting to abnormal atomic write usage, e.g. open atomic_write
>> file for very long time, then fggc will be blocked each time when moving its
>> block. So shutdown it, fggc will recover.
> 
> The point here is stopping sqlite to keep going wrong data writes even after
> we already revoked blocks.

Yes, that's correct, what I mean is that if we can do that with smaller
granularity like just revoke blocks for file which is blocking fggc, it will
affect system/sqlite flow much less than forcing closing all atomic_write.

Thanks,

> 
> Thanks,
> 
>>
>> Thanks,
>>
>>>
>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>> ---
>>>  fs/f2fs/data.c    |  7 ++++---
>>>  fs/f2fs/f2fs.h    |  4 ++--
>>>  fs/f2fs/file.c    |  6 +++++-
>>>  fs/f2fs/gc.c      |  4 +---
>>>  fs/f2fs/segment.c | 21 +++++++++++----------
>>>  5 files changed, 23 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index 5e53d210e222..c9e75aa61c24 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -2247,8 +2247,9 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
>>>  	trace_f2fs_write_begin(inode, pos, len, flags);
>>>  
>>>  	if (f2fs_is_atomic_file(inode) &&
>>> -			!f2fs_available_free_memory(sbi, INMEM_PAGES)) {
>>> -		err = -ENOMEM;
>>> +			(is_sbi_flag_set(sbi, SBI_DISABLE_ATOMIC_WRITE) ||
>>> +			!f2fs_available_free_memory(sbi, INMEM_PAGES))) {
>>> +		err = -EINVAL;
>>>  		drop_atomic = true;
>>>  		goto fail;
>>>  	}
>>> @@ -2331,7 +2332,7 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
>>>  	f2fs_put_page(page, 1);
>>>  	f2fs_write_failed(mapping, pos + len);
>>>  	if (drop_atomic)
>>> -		f2fs_drop_inmem_pages_all(sbi, false);
>>> +		f2fs_disable_atomic_write(sbi);
>>>  	return err;
>>>  }
>>>  
>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>> index 4d8b1de83143..1d5c8d543eda 100644
>>> --- a/fs/f2fs/f2fs.h
>>> +++ b/fs/f2fs/f2fs.h
>>> @@ -621,7 +621,6 @@ enum {
>>>  
>>>  enum {
>>>  	GC_FAILURE_PIN,
>>> -	GC_FAILURE_ATOMIC,
>>>  	MAX_GC_FAILURE
>>>  };
>>>  
>>> @@ -1066,6 +1065,7 @@ enum {
>>>  	SBI_POR_DOING,				/* recovery is doing or not */
>>>  	SBI_NEED_SB_WRITE,			/* need to recover superblock */
>>>  	SBI_NEED_CP,				/* need to checkpoint */
>>> +	SBI_DISABLE_ATOMIC_WRITE,		/* turn off atomic write */
>>>  };
>>>  
>>>  enum {
>>> @@ -2833,7 +2833,7 @@ void f2fs_destroy_node_manager_caches(void);
>>>   */
>>>  bool f2fs_need_SSR(struct f2fs_sb_info *sbi);
>>>  void f2fs_register_inmem_page(struct inode *inode, struct page *page);
>>> -void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure);
>>> +void f2fs_disable_atomic_write(struct f2fs_sb_info *sbi);
>>>  void f2fs_drop_inmem_pages(struct inode *inode);
>>>  void f2fs_drop_inmem_page(struct inode *inode, struct page *page);
>>>  int f2fs_commit_inmem_pages(struct inode *inode);
>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>> index 6880c6f78d58..b029a4ed3bb0 100644
>>> --- a/fs/f2fs/file.c
>>> +++ b/fs/f2fs/file.c
>>> @@ -1682,6 +1682,9 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
>>>  	if (!S_ISREG(inode->i_mode))
>>>  		return -EINVAL;
>>>  
>>> +	if (is_sbi_flag_set(F2FS_I_SB(inode), SBI_DISABLE_ATOMIC_WRITE))
>>> +		return -EINVAL;
>>> +
>>>  	ret = mnt_want_write_file(filp);
>>>  	if (ret)
>>>  		return ret;
>>> @@ -1750,7 +1753,6 @@ static int f2fs_ioc_commit_atomic_write(struct file *filp)
>>>  		ret = f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true);
>>>  		if (!ret) {
>>>  			clear_inode_flag(inode, FI_ATOMIC_FILE);
>>> -			F2FS_I(inode)->i_gc_failures[GC_FAILURE_ATOMIC] = 0;
>>>  			stat_dec_atomic_write(inode);
>>>  		}
>>>  	} else {
>>> @@ -1853,6 +1855,8 @@ static int f2fs_ioc_abort_volatile_write(struct file *filp)
>>>  		ret = f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true);
>>>  	}
>>>  
>>> +	clear_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
>>> +
>>>  	inode_unlock(inode);
>>>  
>>>  	mnt_drop_write_file(filp);
>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>> index 9093be6e7a7d..6d762f3cdfc7 100644
>>> --- a/fs/f2fs/gc.c
>>> +++ b/fs/f2fs/gc.c
>>> @@ -629,7 +629,6 @@ static void move_data_block(struct inode *inode, block_t bidx,
>>>  		goto out;
>>>  
>>>  	if (f2fs_is_atomic_file(inode)) {
>>> -		F2FS_I(inode)->i_gc_failures[GC_FAILURE_ATOMIC]++;
>>>  		F2FS_I_SB(inode)->skipped_atomic_files[gc_type]++;
>>>  		goto out;
>>>  	}
>>> @@ -745,7 +744,6 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
>>>  		goto out;
>>>  
>>>  	if (f2fs_is_atomic_file(inode)) {
>>> -		F2FS_I(inode)->i_gc_failures[GC_FAILURE_ATOMIC]++;
>>>  		F2FS_I_SB(inode)->skipped_atomic_files[gc_type]++;
>>>  		goto out;
>>>  	}
>>> @@ -1100,7 +1098,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>>>  		if (has_not_enough_free_secs(sbi, sec_freed, 0)) {
>>>  			if (skipped_round > MAX_SKIP_ATOMIC_COUNT &&
>>>  				skipped_round * 2 >= round)
>>> -				f2fs_drop_inmem_pages_all(sbi, true);
>>> +				f2fs_disable_atomic_write(sbi);
>>>  			segno = NULL_SEGNO;
>>>  			goto gc_more;
>>>  		}
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 9efce174c51a..05877a2f1894 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -274,11 +274,14 @@ static int __revoke_inmem_pages(struct inode *inode,
>>>  	return err;
>>>  }
>>>  
>>> -void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure)
>>> +void f2fs_disable_atomic_write(struct f2fs_sb_info *sbi)
>>>  {
>>>  	struct list_head *head = &sbi->inode_list[ATOMIC_FILE];
>>>  	struct inode *inode;
>>>  	struct f2fs_inode_info *fi;
>>> +
>>> +	/* just turn it off */
>>> +	set_sbi_flag(sbi, SBI_DISABLE_ATOMIC_WRITE);
>>>  next:
>>>  	spin_lock(&sbi->inode_lock[ATOMIC_FILE]);
>>>  	if (list_empty(head)) {
>>> @@ -290,17 +293,16 @@ void f2fs_drop_inmem_pages_all(struct f2fs_sb_info *sbi, bool gc_failure)
>>>  	spin_unlock(&sbi->inode_lock[ATOMIC_FILE]);
>>>  
>>>  	if (inode) {
>>> -		if (gc_failure) {
>>> -			if (fi->i_gc_failures[GC_FAILURE_ATOMIC])
>>> -				goto drop;
>>> -			goto skip;
>>> +		inode_lock(inode);
>>> +		/* need to check whether it was already revoked */
>>> +		if (f2fs_is_atomic_file(inode)) {
>>> +			f2fs_drop_inmem_pages(inode);
>>> +			set_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
>>>  		}
>>> -drop:
>>> -		set_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
>>> -		f2fs_drop_inmem_pages(inode);
>>> +		inode_unlock(inode);
>>>  		iput(inode);
>>>  	}
>>> -skip:
>>> +
>>>  	congestion_wait(BLK_RW_ASYNC, HZ/50);
>>>  	cond_resched();
>>>  	goto next;
>>> @@ -320,7 +322,6 @@ void f2fs_drop_inmem_pages(struct inode *inode)
>>>  	mutex_unlock(&fi->inmem_lock);
>>>  
>>>  	clear_inode_flag(inode, FI_ATOMIC_FILE);
>>> -	fi->i_gc_failures[GC_FAILURE_ATOMIC] = 0;
>>>  	stat_dec_atomic_write(inode);
>>>  }
>>>  
>>>
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 

Re: [f2fs-dev] [PATCH 1/3] f2fs: turn off atomic writes when deteting abnormal behaviors

[Jaegeuk Kim]

On 07/23, Chao Yu wrote:
> On 2018/7/23 21:03, Jaegeuk Kim wrote:
> > On 07/16, Chao Yu wrote:
> >> On 2018/7/15 9:11, Jaegeuk Kim wrote:
> >>> In order to prevent abusing atomic writes by abnormal users, we've added a
> >>> threshold, 20% over memory footprint, which disallows further atomic writes.
> >>> Previously, however, SQLite doesn't know the files became normal, so that
> >>> it could write stale data and commit on revoked normal database file.
> >>>
> >>> Once f2fs detects such the abnormal behavior, this patch simply disables
> >>> all the atomic operations such as:
> >>> - write_begin() gives EINVAL to avoid stale data writes, and SQLite will call
> >>>   F2FS_IOC_ABORT_VOLATILE_WRITE to notify aborting the transaction,
> >>> - F2FS_IOC_START_ATOMIC_WRITE gives EINVAL for SQLite to fall back normal
> >>>   journal_mode,
> >>> - F2FS_IOC_COMMIT_ATOMIC_WRITE gives EINVAL, if the file was revoked, so that
> >>>   Framework retries to submit the transaction given propagated SQLite error.
> >>>
> >>> Note that, this patch also turns off atomic operations, if foreground GC tries
> >>> to move atomic files too frequently.
> >>
> >> Well, how about just keeping original implementation: shutdown atomic write for
> >> those files which are really affect fggc? Since intention of the original
> >> behavior is targeting to abnormal atomic write usage, e.g. open atomic_write
> >> file for very long time, then fggc will be blocked each time when moving its
> >> block. So shutdown it, fggc will recover.
> > 
> > The point here is stopping sqlite to keep going wrong data writes even after
> > we already revoked blocks.
> 
> Yes, that's correct, what I mean is that if we can do that with smaller
> granularity like just revoke blocks for file which is blocking fggc, it will
> affect system/sqlite flow much less than forcing closing all atomic_write.

How about this?

From 64d2becb82a496c2e2c04abeed42efa3b401ee20 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Fri, 27 Jul 2018 18:15:11 +0900
Subject: [PATCH] f2fs: don't allow any writes on aborted atomic writes

In order to prevent abusing atomic writes by abnormal users, we've added a
threshold, 20% over memory footprint, which disallows further atomic writes.
Previously, however, SQLite doesn't know the files became normal, so that
it could write stale data and commit on revoked normal database file.

Once f2fs detects such the abnormal behavior, this patch tries to avoid further
writes in write_begin().

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c | 3 ++-
 fs/f2fs/file.c | 7 ++++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 22dd00c6e241..02ec2603725f 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2295,7 +2295,8 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
 	trace_f2fs_write_begin(inode, pos, len, flags);
 
 	if (f2fs_is_atomic_file(inode) &&
-			!f2fs_available_free_memory(sbi, INMEM_PAGES)) {
+			(is_inode_flag_set(inode, FI_ATOMIC_REVOKE_REQUEST) ||
+			!f2fs_available_free_memory(sbi, INMEM_PAGES))) {
 		err = -ENOMEM;
 		drop_atomic = true;
 		goto fail;
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index ff2cb8fb6934..c2c47f3248c4 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1708,8 +1708,11 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
 
 	down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
 
-	if (f2fs_is_atomic_file(inode))
+	if (f2fs_is_atomic_file(inode)) {
+		if (is_inode_flag_set(inode, FI_ATOMIC_REVOKE_REQUEST))
+			ret = -EINVAL;
 		goto out;
+	}
 
 	ret = f2fs_convert_inline_inode(inode);
 	if (ret)
@@ -1871,6 +1874,8 @@ static int f2fs_ioc_abort_volatile_write(struct file *filp)
 		ret = f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true);
 	}
 
+	clear_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
+
 	inode_unlock(inode);
 
 	mnt_drop_write_file(filp);
-- 
2.17.0.441.gb46fe60e1d-goog

Re: [f2fs-dev] [PATCH 1/3] f2fs: turn off atomic writes when deteting abnormal behaviors

[Jaegeuk Kim]

On 07/23, Chao Yu wrote:
> On 2018/7/23 21:03, Jaegeuk Kim wrote:
> > On 07/16, Chao Yu wrote:
> >> On 2018/7/15 9:11, Jaegeuk Kim wrote:
> >>> In order to prevent abusing atomic writes by abnormal users, we've added a
> >>> threshold, 20% over memory footprint, which disallows further atomic writes.
> >>> Previously, however, SQLite doesn't know the files became normal, so that
> >>> it could write stale data and commit on revoked normal database file.
> >>>
> >>> Once f2fs detects such the abnormal behavior, this patch simply disables
> >>> all the atomic operations such as:
> >>> - write_begin() gives EINVAL to avoid stale data writes, and SQLite will call
> >>>   F2FS_IOC_ABORT_VOLATILE_WRITE to notify aborting the transaction,
> >>> - F2FS_IOC_START_ATOMIC_WRITE gives EINVAL for SQLite to fall back normal
> >>>   journal_mode,
> >>> - F2FS_IOC_COMMIT_ATOMIC_WRITE gives EINVAL, if the file was revoked, so that
> >>>   Framework retries to submit the transaction given propagated SQLite error.
> >>>
> >>> Note that, this patch also turns off atomic operations, if foreground GC tries
> >>> to move atomic files too frequently.
> >>
> >> Well, how about just keeping original implementation: shutdown atomic write for
> >> those files which are really affect fggc? Since intention of the original
> >> behavior is targeting to abnormal atomic write usage, e.g. open atomic_write
> >> file for very long time, then fggc will be blocked each time when moving its
> >> block. So shutdown it, fggc will recover.
> > 
> > The point here is stopping sqlite to keep going wrong data writes even after
> > we already revoked blocks.
> 
> Yes, that's correct, what I mean is that if we can do that with smaller
> granularity like just revoke blocks for file which is blocking fggc, it will
> affect system/sqlite flow much less than forcing closing all atomic_write.

How about this?

>From 64d2becb82a496c2e2c04abeed42efa3b401ee20 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Fri, 27 Jul 2018 18:15:11 +0900
Subject: [PATCH] f2fs: don't allow any writes on aborted atomic writes

In order to prevent abusing atomic writes by abnormal users, we've added a
threshold, 20% over memory footprint, which disallows further atomic writes.
Previously, however, SQLite doesn't know the files became normal, so that
it could write stale data and commit on revoked normal database file.

Once f2fs detects such the abnormal behavior, this patch tries to avoid further
writes in write_begin().

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c | 3 ++-
 fs/f2fs/file.c | 7 ++++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 22dd00c6e241..02ec2603725f 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2295,7 +2295,8 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
 	trace_f2fs_write_begin(inode, pos, len, flags);
 
 	if (f2fs_is_atomic_file(inode) &&
-			!f2fs_available_free_memory(sbi, INMEM_PAGES)) {
+			(is_inode_flag_set(inode, FI_ATOMIC_REVOKE_REQUEST) ||
+			!f2fs_available_free_memory(sbi, INMEM_PAGES))) {
 		err = -ENOMEM;
 		drop_atomic = true;
 		goto fail;
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index ff2cb8fb6934..c2c47f3248c4 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1708,8 +1708,11 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
 
 	down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
 
-	if (f2fs_is_atomic_file(inode))
+	if (f2fs_is_atomic_file(inode)) {
+		if (is_inode_flag_set(inode, FI_ATOMIC_REVOKE_REQUEST))
+			ret = -EINVAL;
 		goto out;
+	}
 
 	ret = f2fs_convert_inline_inode(inode);
 	if (ret)
@@ -1871,6 +1874,8 @@ static int f2fs_ioc_abort_volatile_write(struct file *filp)
 		ret = f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true);
 	}
 
+	clear_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
+
 	inode_unlock(inode);
 
 	mnt_drop_write_file(filp);
-- 
2.17.0.441.gb46fe60e1d-goog

Re: [f2fs-dev] [PATCH 1/3] f2fs: turn off atomic writes when deteting abnormal behaviors

[Chao Yu]

On 2018/7/27 17:17, Jaegeuk Kim wrote:
> On 07/23, Chao Yu wrote:
>> On 2018/7/23 21:03, Jaegeuk Kim wrote:
>>> On 07/16, Chao Yu wrote:
>>>> On 2018/7/15 9:11, Jaegeuk Kim wrote:
>>>>> In order to prevent abusing atomic writes by abnormal users, we've added a
>>>>> threshold, 20% over memory footprint, which disallows further atomic writes.
>>>>> Previously, however, SQLite doesn't know the files became normal, so that
>>>>> it could write stale data and commit on revoked normal database file.
>>>>>
>>>>> Once f2fs detects such the abnormal behavior, this patch simply disables
>>>>> all the atomic operations such as:
>>>>> - write_begin() gives EINVAL to avoid stale data writes, and SQLite will call
>>>>>   F2FS_IOC_ABORT_VOLATILE_WRITE to notify aborting the transaction,
>>>>> - F2FS_IOC_START_ATOMIC_WRITE gives EINVAL for SQLite to fall back normal
>>>>>   journal_mode,
>>>>> - F2FS_IOC_COMMIT_ATOMIC_WRITE gives EINVAL, if the file was revoked, so that
>>>>>   Framework retries to submit the transaction given propagated SQLite error.
>>>>>
>>>>> Note that, this patch also turns off atomic operations, if foreground GC tries
>>>>> to move atomic files too frequently.
>>>>
>>>> Well, how about just keeping original implementation: shutdown atomic write for
>>>> those files which are really affect fggc? Since intention of the original
>>>> behavior is targeting to abnormal atomic write usage, e.g. open atomic_write
>>>> file for very long time, then fggc will be blocked each time when moving its
>>>> block. So shutdown it, fggc will recover.
>>>
>>> The point here is stopping sqlite to keep going wrong data writes even after
>>> we already revoked blocks.
>>
>> Yes, that's correct, what I mean is that if we can do that with smaller
>> granularity like just revoke blocks for file which is blocking fggc, it will
>> affect system/sqlite flow much less than forcing closing all atomic_write.
> 
> How about this?

Yes, it looks good to me. :)

> 
> From 64d2becb82a496c2e2c04abeed42efa3b401ee20 Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim <jaegeuk@kernel.org>
> Date: Fri, 27 Jul 2018 18:15:11 +0900
> Subject: [PATCH] f2fs: don't allow any writes on aborted atomic writes
> 
> In order to prevent abusing atomic writes by abnormal users, we've added a
> threshold, 20% over memory footprint, which disallows further atomic writes.
> Previously, however, SQLite doesn't know the files became normal, so that
> it could write stale data and commit on revoked normal database file.
> 
> Once f2fs detects such the abnormal behavior, this patch tries to avoid further
> writes in write_begin().
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

> ---
>  fs/f2fs/data.c | 3 ++-
>  fs/f2fs/file.c | 7 ++++++-
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 22dd00c6e241..02ec2603725f 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2295,7 +2295,8 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
>  	trace_f2fs_write_begin(inode, pos, len, flags);
>  
>  	if (f2fs_is_atomic_file(inode) &&
> -			!f2fs_available_free_memory(sbi, INMEM_PAGES)) {
> +			(is_inode_flag_set(inode, FI_ATOMIC_REVOKE_REQUEST) ||
> +			!f2fs_available_free_memory(sbi, INMEM_PAGES))) {
>  		err = -ENOMEM;
>  		drop_atomic = true;
>  		goto fail;
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index ff2cb8fb6934..c2c47f3248c4 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1708,8 +1708,11 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
>  
>  	down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
>  
> -	if (f2fs_is_atomic_file(inode))
> +	if (f2fs_is_atomic_file(inode)) {
> +		if (is_inode_flag_set(inode, FI_ATOMIC_REVOKE_REQUEST))
> +			ret = -EINVAL;
>  		goto out;
> +	}
>  
>  	ret = f2fs_convert_inline_inode(inode);
>  	if (ret)
> @@ -1871,6 +1874,8 @@ static int f2fs_ioc_abort_volatile_write(struct file *filp)
>  		ret = f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true);
>  	}
>  
> +	clear_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
> +
>  	inode_unlock(inode);
>  
>  	mnt_drop_write_file(filp);
> 

Re: [f2fs-dev] [PATCH 1/3] f2fs: turn off atomic writes when deteting abnormal behaviors

[Jaegeuk Kim]

On 07/27, Chao Yu wrote:
> On 2018/7/27 17:17, Jaegeuk Kim wrote:
> > On 07/23, Chao Yu wrote:
> >> On 2018/7/23 21:03, Jaegeuk Kim wrote:
> >>> On 07/16, Chao Yu wrote:
> >>>> On 2018/7/15 9:11, Jaegeuk Kim wrote:
> >>>>> In order to prevent abusing atomic writes by abnormal users, we've added a
> >>>>> threshold, 20% over memory footprint, which disallows further atomic writes.
> >>>>> Previously, however, SQLite doesn't know the files became normal, so that
> >>>>> it could write stale data and commit on revoked normal database file.
> >>>>>
> >>>>> Once f2fs detects such the abnormal behavior, this patch simply disables
> >>>>> all the atomic operations such as:
> >>>>> - write_begin() gives EINVAL to avoid stale data writes, and SQLite will call
> >>>>>   F2FS_IOC_ABORT_VOLATILE_WRITE to notify aborting the transaction,
> >>>>> - F2FS_IOC_START_ATOMIC_WRITE gives EINVAL for SQLite to fall back normal
> >>>>>   journal_mode,
> >>>>> - F2FS_IOC_COMMIT_ATOMIC_WRITE gives EINVAL, if the file was revoked, so that
> >>>>>   Framework retries to submit the transaction given propagated SQLite error.
> >>>>>
> >>>>> Note that, this patch also turns off atomic operations, if foreground GC tries
> >>>>> to move atomic files too frequently.
> >>>>
> >>>> Well, how about just keeping original implementation: shutdown atomic write for
> >>>> those files which are really affect fggc? Since intention of the original
> >>>> behavior is targeting to abnormal atomic write usage, e.g. open atomic_write
> >>>> file for very long time, then fggc will be blocked each time when moving its
> >>>> block. So shutdown it, fggc will recover.
> >>>
> >>> The point here is stopping sqlite to keep going wrong data writes even after
> >>> we already revoked blocks.
> >>
> >> Yes, that's correct, what I mean is that if we can do that with smaller
> >> granularity like just revoke blocks for file which is blocking fggc, it will
> >> affect system/sqlite flow much less than forcing closing all atomic_write.
> > 
> > How about this?
> 
> Yes, it looks good to me. :)
> 
> > 
> > From 64d2becb82a496c2e2c04abeed42efa3b401ee20 Mon Sep 17 00:00:00 2001
> > From: Jaegeuk Kim <jaegeuk@kernel.org>
> > Date: Fri, 27 Jul 2018 18:15:11 +0900
> > Subject: [PATCH] f2fs: don't allow any writes on aborted atomic writes
> > 
> > In order to prevent abusing atomic writes by abnormal users, we've added a
> > threshold, 20% over memory footprint, which disallows further atomic writes.
> > Previously, however, SQLite doesn't know the files became normal, so that
> > it could write stale data and commit on revoked normal database file.
> > 
> > Once f2fs detects such the abnormal behavior, this patch tries to avoid further
> > writes in write_begin().
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> 
> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> 
> Thanks,
> 
> > ---
> >  fs/f2fs/data.c | 3 ++-
> >  fs/f2fs/file.c | 7 ++++++-
> >  2 files changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 22dd00c6e241..02ec2603725f 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -2295,7 +2295,8 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
> >  	trace_f2fs_write_begin(inode, pos, len, flags);
> >  
> >  	if (f2fs_is_atomic_file(inode) &&
> > -			!f2fs_available_free_memory(sbi, INMEM_PAGES)) {
> > +			(is_inode_flag_set(inode, FI_ATOMIC_REVOKE_REQUEST) ||
> > +			!f2fs_available_free_memory(sbi, INMEM_PAGES))) {

       if ((f2fs_is_atomic_file(inode) &&
                       !f2fs_available_free_memory(sbi, INMEM_PAGES)) ||
                       is_inode_flag_set(inode, FI_ATOMIC_REVOKE_REQUEST)) {

-- Fixed wrong condition.

> >  		err = -ENOMEM;
> >  		drop_atomic = true;
> >  		goto fail;
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index ff2cb8fb6934..c2c47f3248c4 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -1708,8 +1708,11 @@ static int f2fs_ioc_start_atomic_write(struct file *filp)
> >  
> >  	down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> >  
> > -	if (f2fs_is_atomic_file(inode))
> > +	if (f2fs_is_atomic_file(inode)) {
> > +		if (is_inode_flag_set(inode, FI_ATOMIC_REVOKE_REQUEST))
> > +			ret = -EINVAL;
> >  		goto out;
> > +	}
> >  
> >  	ret = f2fs_convert_inline_inode(inode);
> >  	if (ret)
> > @@ -1871,6 +1874,8 @@ static int f2fs_ioc_abort_volatile_write(struct file *filp)
> >  		ret = f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true);
> >  	}
> >  
> > +	clear_inode_flag(inode, FI_ATOMIC_REVOKE_REQUEST);
> > +
> >  	inode_unlock(inode);
> >  
> >  	mnt_drop_write_file(filp);
> >