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

Groups > linux.kernel > #1498968 > unrolled thread

[PATCH 1/8] f2fs: clear nlink if fail to add_link

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/8] f2fs: clear nlink if fail to add_link Chao Yu <chao@kernel.org> - 2016-10-11 17:10 +0200
    [PATCH 8/8] f2fs: fix error handling in fsync_node_pages Chao Yu <chao@kernel.org> - 2016-10-11 17:10 +0200
    [PATCH 2/8] f2fs: fix to release discard entries during checkpoint Chao Yu <chao@kernel.org> - 2016-10-11 17:10 +0200
    [PATCH 4/8] f2fs: add missing f2fs_balance_fs in f2fs_zero_range Chao Yu <chao@kernel.org> - 2016-10-11 17:10 +0200
    Re: [PATCH 1/8] f2fs: clear nlink if fail to add_link Jaegeuk Kim <jaegeuk@kernel.org> - 2016-10-12 00:30 +0200
      Re: [PATCH 1/8] f2fs: clear nlink if fail to add_link Chao Yu <chao@kernel.org> - 2016-10-12 17:30 +0200
        Re: [PATCH 1/8] f2fs: clear nlink if fail to add_link Jaegeuk Kim <jaegeuk@kernel.org> - 2016-10-12 19:30 +0200

#1498968 — [PATCH 1/8] f2fs: clear nlink if fail to add_link

From: Chao Yu <yuchao0@huawei.com>

We don't need to keep incomplete created inode in cache, so if we fail to
add link into directory during new inode creation, it's better to set
nlink of inode to zero, then we can evict inode immediately. Otherwise
release of nid belong to inode will be delayed until inode cache is being
shrunk, it may cause a seemingly endless loop while allocating free nids
in time of testing generic/269 case of fstest suit.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/inode.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index d736989..34ae03c 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -384,6 +384,8 @@ retry:
 		f2fs_lock_op(sbi);
 		err = remove_inode_page(inode);
 		f2fs_unlock_op(sbi);
+		if (err == -ENOENT)
+			err = 0;
 	}
 
 	/* give more chances, if ENOMEM case */
@@ -424,6 +426,12 @@ void handle_failed_inode(struct inode *inode)
 	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 	struct node_info ni;
 
+	/*
+	 * clear nlink of inode in order to release resource of inode
+	 * immediately.
+	 */
+	clear_nlink(inode);
+
 	/* don't make bad inode, since it becomes a regular file. */
 	unlock_new_inode(inode);
 
-- 
2.10.1

[toc] | [next] | [standalone]

#1498969 — [PATCH 8/8] f2fs: fix error handling in fsync_node_pages

From: Chao Yu <yuchao0@huawei.com>

In fsync_node_pages, if f2fs was taged with CP_ERROR_FLAG, make sure bio
cache was flushed before return.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/node.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 3a393e3..14f137d 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1339,7 +1339,8 @@ retry:
 			if (unlikely(f2fs_cp_error(sbi))) {
 				f2fs_put_page(last_page, 0);
 				pagevec_release(&pvec);
-				return -EIO;
+				ret = -EIO;
+				goto out;
 			}
 
 			if (!IS_DNODE(page) || !is_cold_node(page))
@@ -1412,7 +1413,7 @@ continue_unlock:
 		unlock_page(last_page);
 		goto retry;
 	}
-
+out:
 	if (nwritten)
 		f2fs_submit_merged_bio_cond(sbi, NULL, NULL, ino, NODE, WRITE);
 	return ret ? -EIO: 0;
-- 
2.10.1

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

#1498970 — [PATCH 2/8] f2fs: fix to release discard entries during checkpoint

From: Chao Yu <yuchao0@huawei.com>

In f2fs_fill_super, if there is any IO error occurs during recovery,
cached discard entries will be leaked, in order to avoid this, make
write_checkpoint() handle memory release by itself, besides, move
clear_prefree_segments to write_checkpoint for readability.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/checkpoint.c | 9 ++++++---
 fs/f2fs/super.c      | 1 -
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index eacc697..654f5d7 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1184,7 +1184,6 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 	if (unlikely(f2fs_cp_error(sbi)))
 		return -EIO;
 
-	clear_prefree_segments(sbi, cpc);
 	clear_sbi_flag(sbi, SBI_IS_DIRTY);
 	clear_sbi_flag(sbi, SBI_NEED_CP);
 
@@ -1261,8 +1260,12 @@ int write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 
 	/* unlock all the fs_lock[] in do_checkpoint() */
 	err = do_checkpoint(sbi, cpc);
-
-	f2fs_wait_all_discard_bio(sbi);
+	if (err) {
+		release_discard_addrs(sbi);
+	} else {
+		clear_prefree_segments(sbi, cpc);
+		f2fs_wait_all_discard_bio(sbi);
+	}
 
 	unblock_operations(sbi);
 	stat_inc_cp_count(sbi->stat_info);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 6132b4c..bfa4414 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -738,7 +738,6 @@ static void f2fs_put_super(struct super_block *sb)
 	 * In addition, EIO will skip do checkpoint, we need this as well.
 	 */
 	release_ino_entry(sbi, true);
-	release_discard_addrs(sbi);
 
 	f2fs_leave_shrinker(sbi);
 	mutex_unlock(&sbi->umount_mutex);
-- 
2.10.1

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

#1498972 — [PATCH 4/8] f2fs: add missing f2fs_balance_fs in f2fs_zero_range

From: Chao Yu <yuchao0@huawei.com>

f2fs_balance_fs should be called in between node page updating, otherwise
node page count will exceeded far beyond watermark of triggering
foreground garbage collection, result in facing high risk of hitting LFS
allocation failure.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/file.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 8f48fdd..0081c79 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1260,6 +1260,9 @@ static int f2fs_zero_range(struct inode *inode, loff_t offset, loff_t len,
 			ret = f2fs_do_zero_range(&dn, index, end);
 			f2fs_put_dnode(&dn);
 			f2fs_unlock_op(sbi);
+
+			f2fs_balance_fs(sbi, dn.node_changed);
+
 			if (ret)
 				goto out;
 
-- 
2.10.1

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

#1499275

Hi Chao,

On Tue, Oct 11, 2016 at 10:56:59PM +0800, Chao Yu wrote:
> From: Chao Yu <yuchao0@huawei.com>
> 
> We don't need to keep incomplete created inode in cache, so if we fail to
> add link into directory during new inode creation, it's better to set
> nlink of inode to zero, then we can evict inode immediately. Otherwise
> release of nid belong to inode will be delayed until inode cache is being
> shrunk, it may cause a seemingly endless loop while allocating free nids
> in time of testing generic/269 case of fstest suit.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fs/f2fs/inode.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index d736989..34ae03c 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -384,6 +384,8 @@ retry:
>  		f2fs_lock_op(sbi);
>  		err = remove_inode_page(inode);
>  		f2fs_unlock_op(sbi);
> +		if (err == -ENOENT)
> +			err = 0;
>  	}
>  
>  	/* give more chances, if ENOMEM case */
> @@ -424,6 +426,12 @@ void handle_failed_inode(struct inode *inode)
>  	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>  	struct node_info ni;
>  
> +	/*
> +	 * clear nlink of inode in order to release resource of inode
> +	 * immediately.
> +	 */
> +	clear_nlink(inode);

We must call update_inode_page() here to avoid kernel panic.
Otherwise, this inode is kept in the gdirty list, resulting in kernel panic
when flushg dirty inodes, since it was already evicted.

I fixed this and started a round of tests.

Thanks,

> +
>  	/* don't make bad inode, since it becomes a regular file. */
>  	unlock_new_inode(inode);
>  
> -- 
> 2.10.1

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

#1499775

Hi Jaegeuk,

On 2016/10/12 6:19, Jaegeuk Kim wrote:
> Hi Chao,
> 
> On Tue, Oct 11, 2016 at 10:56:59PM +0800, Chao Yu wrote:
>> From: Chao Yu <yuchao0@huawei.com>
>>
>> We don't need to keep incomplete created inode in cache, so if we fail to
>> add link into directory during new inode creation, it's better to set
>> nlink of inode to zero, then we can evict inode immediately. Otherwise
>> release of nid belong to inode will be delayed until inode cache is being
>> shrunk, it may cause a seemingly endless loop while allocating free nids
>> in time of testing generic/269 case of fstest suit.
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>  fs/f2fs/inode.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
>> index d736989..34ae03c 100644
>> --- a/fs/f2fs/inode.c
>> +++ b/fs/f2fs/inode.c
>> @@ -384,6 +384,8 @@ retry:
>>  		f2fs_lock_op(sbi);
>>  		err = remove_inode_page(inode);
>>  		f2fs_unlock_op(sbi);
>> +		if (err == -ENOENT)
>> +			err = 0;
>>  	}
>>  
>>  	/* give more chances, if ENOMEM case */
>> @@ -424,6 +426,12 @@ void handle_failed_inode(struct inode *inode)
>>  	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>>  	struct node_info ni;
>>  
>> +	/*
>> +	 * clear nlink of inode in order to release resource of inode
>> +	 * immediately.
>> +	 */
>> +	clear_nlink(inode);
> 
> We must call update_inode_page() here to avoid kernel panic.
> Otherwise, this inode is kept in the gdirty list, resulting in kernel panic
> when flushg dirty inodes, since it was already evicted.

Thanks for fixing this, is this panic produced with fault injection?

Thanks,

> 
> I fixed this and started a round of tests.
> 
> Thanks,
> 
>> +
>>  	/* don't make bad inode, since it becomes a regular file. */
>>  	unlock_new_inode(inode);
>>  
>> -- 
>> 2.10.1

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

#1499844

On Wed, Oct 12, 2016 at 11:24:28PM +0800, Chao Yu wrote:
> Hi Jaegeuk,
> 
> On 2016/10/12 6:19, Jaegeuk Kim wrote:
> > Hi Chao,
> > 
> > On Tue, Oct 11, 2016 at 10:56:59PM +0800, Chao Yu wrote:
> >> From: Chao Yu <yuchao0@huawei.com>
> >>
> >> We don't need to keep incomplete created inode in cache, so if we fail to
> >> add link into directory during new inode creation, it's better to set
> >> nlink of inode to zero, then we can evict inode immediately. Otherwise
> >> release of nid belong to inode will be delayed until inode cache is being
> >> shrunk, it may cause a seemingly endless loop while allocating free nids
> >> in time of testing generic/269 case of fstest suit.
> >>
> >> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> >> ---
> >>  fs/f2fs/inode.c | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> >> index d736989..34ae03c 100644
> >> --- a/fs/f2fs/inode.c
> >> +++ b/fs/f2fs/inode.c
> >> @@ -384,6 +384,8 @@ retry:
> >>  		f2fs_lock_op(sbi);
> >>  		err = remove_inode_page(inode);
> >>  		f2fs_unlock_op(sbi);
> >> +		if (err == -ENOENT)
> >> +			err = 0;
> >>  	}
> >>  
> >>  	/* give more chances, if ENOMEM case */
> >> @@ -424,6 +426,12 @@ void handle_failed_inode(struct inode *inode)
> >>  	struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> >>  	struct node_info ni;
> >>  
> >> +	/*
> >> +	 * clear nlink of inode in order to release resource of inode
> >> +	 * immediately.
> >> +	 */
> >> +	clear_nlink(inode);
> > 
> > We must call update_inode_page() here to avoid kernel panic.
> > Otherwise, this inode is kept in the gdirty list, resulting in kernel panic
> > when flushg dirty inodes, since it was already evicted.
> 
> Thanks for fixing this, is this panic produced with fault injection?

No, it happened xfstests/027 under encrypted mode only.

Thanks,

> 
> Thanks,
> 
> > 
> > I fixed this and started a round of tests.
> > 
> > Thanks,
> > 
> >> +
> >>  	/* don't make bad inode, since it becomes a regular file. */
> >>  	unlock_new_inode(inode);
> >>  
> >> -- 
> >> 2.10.1

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web