[RFC PATCH v2 1/2] f2fs: do not set LOST_PINO for newly created dir

[RFC PATCH v2 1/2] f2fs: do not set LOST_PINO for newly created dir

[Sheng Yong]

Since directories will be written back with checkpoint and fsync a
directory will always write CP, there is no need to set LOST_PINO
after creating a directory.

Signed-off-by: Sheng Yong <shengyong1@huawei.com>
---
 fs/f2fs/dir.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 94756f55a97e..37f9c7f55605 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -415,7 +415,8 @@ struct page *init_inode_metadata(struct inode *inode, struct inode *dir,
 	 * We lost i_pino from now on.
 	 */
 	if (is_inode_flag_set(inode, FI_INC_LINK)) {
-		file_lost_pino(inode);
+		if (!S_ISDIR(inode->i_mode))
+			file_lost_pino(inode);
 		/*
 		 * If link the tmpfile to alias through linkat path,
 		 * we should remove this inode from orphan list.
-- 
2.11.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[RFC PATCH v2 2/2] f2fs: do not set LOST_PINO for renamed dir

[Sheng Yong]

After renaming a directory, fsck could detect unmatched pino. The scenario
can be reproduced as the following:

	$ mkdir /bar/subbar /foo
	$ rename /bar/subbar /foo

Then fsck will report:
[ASSERT] (__chk_dots_dentries:1182)  --> Bad inode number[0x3] for '..', parent parent ino is [0x4]

Rename sets LOST_PINO for old_inode. However, the flag cannot be cleared,
since dir is written back with CP. So, let's get rid of LOST_PINO for a
renamed dir and fix the pino directly at the end of rename.

Signed-off-by: Sheng Yong <shengyong1@huawei.com>
---
 fs/f2fs/namei.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index c31b40e5f9cf..b75dc2f4ad57 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -772,7 +772,10 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
 	}
 
 	down_write(&F2FS_I(old_inode)->i_sem);
-	file_lost_pino(old_inode);
+	if (!old_dir_entry || whiteout)
+		file_lost_pino(old_inode);
+	else
+		F2FS_I(old_inode)->i_pino = new_dir->i_ino;
 	up_write(&F2FS_I(old_inode)->i_sem);
 
 	old_inode->i_ctime = current_time(old_inode);
-- 
2.11.0


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH v2 1/2] f2fs: do not set LOST_PINO for newly created dir

[Chao Yu]

On 2017/6/26 10:41, Sheng Yong wrote:
> Since directories will be written back with checkpoint and fsync a
> directory will always write CP, there is no need to set LOST_PINO
> after creating a directory.

Needs to cover rename/rename2?

Thanks,

> 
> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
> ---
>  fs/f2fs/dir.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index 94756f55a97e..37f9c7f55605 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -415,7 +415,8 @@ struct page *init_inode_metadata(struct inode *inode, struct inode *dir,
>  	 * We lost i_pino from now on.
>  	 */
>  	if (is_inode_flag_set(inode, FI_INC_LINK)) {
> -		file_lost_pino(inode);
> +		if (!S_ISDIR(inode->i_mode))
> +			file_lost_pino(inode);
>  		/*
>  		 * If link the tmpfile to alias through linkat path,
>  		 * we should remove this inode from orphan list.
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH v2 2/2] f2fs: do not set LOST_PINO for renamed dir

[Jaegeuk Kim]

Hi,

On 06/26, Sheng Yong wrote:
> After renaming a directory, fsck could detect unmatched pino. The scenario
> can be reproduced as the following:
> 
> 	$ mkdir /bar/subbar /foo
> 	$ rename /bar/subbar /foo
> 
> Then fsck will report:
> [ASSERT] (__chk_dots_dentries:1182)  --> Bad inode number[0x3] for '..', parent parent ino is [0x4]

Does fsck report this? If LOST_PINO is found, it must skip parent ino?

> 
> Rename sets LOST_PINO for old_inode. However, the flag cannot be cleared,
> since dir is written back with CP. So, let's get rid of LOST_PINO for a
> renamed dir and fix the pino directly at the end of rename.
> 
> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
> ---
>  fs/f2fs/namei.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
> index c31b40e5f9cf..b75dc2f4ad57 100644
> --- a/fs/f2fs/namei.c
> +++ b/fs/f2fs/namei.c
> @@ -772,7 +772,10 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
>  	}
>  
>  	down_write(&F2FS_I(old_inode)->i_sem);
> -	file_lost_pino(old_inode);
> +	if (!old_dir_entry || whiteout)
> +		file_lost_pino(old_inode);
> +	else
> +		F2FS_I(old_inode)->i_pino = new_dir->i_ino;
>  	up_write(&F2FS_I(old_inode)->i_sem);
>  
>  	old_inode->i_ctime = current_time(old_inode);
> -- 
> 2.11.0
> 
> 
> ------------------------------------------------------------------------------
> 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

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [RFC PATCH v2 2/2] f2fs: do not set LOST_PINO for renamed dir

[Sheng Yong]

Hi, Jaegeuk

On 2017/7/1 23:10, Jaegeuk Kim wrote:
> Hi,
> 
> On 06/26, Sheng Yong wrote:
>> After renaming a directory, fsck could detect unmatched pino. The scenario
>> can be reproduced as the following:
>>
>> 	$ mkdir /bar/subbar /foo
>> 	$ rename /bar/subbar /foo
>>
>> Then fsck will report:
>> [ASSERT] (__chk_dots_dentries:1182)  --> Bad inode number[0x3] for '..', parent parent ino is [0x4]
> 
> Does fsck report this? If LOST_PINO is found, it must skip parent ino?
Yes, fsck should skip checking pino if LOST_PINO is set. Meanwhile, I think
we don't have to set LOST_PINO when rename a dir, since dir is not involved
in recover and its pino will never get fixed.

thanks,
Sheng
> 
>>
>> Rename sets LOST_PINO for old_inode. However, the flag cannot be cleared,
>> since dir is written back with CP. So, let's get rid of LOST_PINO for a
>> renamed dir and fix the pino directly at the end of rename.
>>
>> Signed-off-by: Sheng Yong <shengyong1@huawei.com>
>> ---
>>   fs/f2fs/namei.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
>> index c31b40e5f9cf..b75dc2f4ad57 100644
>> --- a/fs/f2fs/namei.c
>> +++ b/fs/f2fs/namei.c
>> @@ -772,7 +772,10 @@ static int f2fs_rename(struct inode *old_dir, struct dentry *old_dentry,
>>   	}
>>   
>>   	down_write(&F2FS_I(old_inode)->i_sem);
>> -	file_lost_pino(old_inode);
>> +	if (!old_dir_entry || whiteout)
>> +		file_lost_pino(old_inode);
>> +	else
>> +		F2FS_I(old_inode)->i_pino = new_dir->i_ino;
>>   	up_write(&F2FS_I(old_inode)->i_sem);
>>   
>>   	old_inode->i_ctime = current_time(old_inode);
>> -- 
>> 2.11.0
>>
>>
>> ------------------------------------------------------------------------------
>> 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
> 
> .
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot