[PATCH 2/2] exfat: remove exfat_update_parent_info()

[PATCH 2/2] exfat: remove exfat_update_parent_info()

[Yuezhang.Mo]

[-- Attachment #1: Type: text/plain, Size: 2198 bytes --]

exfat_update_parent_info() is a workaround for the wrong parent
directory information being used after renaming. Now that bug is
fixed, this is no longer needed, so remove it.

Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
Reviewed-by: Andy Wu <Andy.Wu@sony.com>
Reviewed-by: Aoyama Wataru <wataru.aoyama@sony.com>
Reviewed-by: Daniel Palmer <daniel.palmer@sony.com>
---
 fs/exfat/namei.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index e7adb6bfd9d5..76acc3721951 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -1168,28 +1168,6 @@ static int exfat_move_file(struct inode *inode, struct exfat_chain *p_olddir,
 	return 0;
 }
 
-static void exfat_update_parent_info(struct exfat_inode_info *ei,
-		struct inode *parent_inode)
-{
-	struct exfat_sb_info *sbi = EXFAT_SB(parent_inode->i_sb);
-	struct exfat_inode_info *parent_ei = EXFAT_I(parent_inode);
-	loff_t parent_isize = i_size_read(parent_inode);
-
-	/*
-	 * the problem that struct exfat_inode_info caches wrong parent info.
-	 *
-	 * because of flag-mismatch of ei->dir,
-	 * there is abnormal traversing cluster chain.
-	 */
-	if (unlikely(parent_ei->flags != ei->dir.flags ||
-		     parent_isize != EXFAT_CLU_TO_B(ei->dir.size, sbi) ||
-		     parent_ei->start_clu != ei->dir.dir)) {
-		exfat_chain_set(&ei->dir, parent_ei->start_clu,
-			EXFAT_B_TO_CLU_ROUND_UP(parent_isize, sbi),
-			parent_ei->flags);
-	}
-}
-
 /* rename or move a old file into a new file */
 static int __exfat_rename(struct inode *old_parent_inode,
 		struct exfat_inode_info *ei, struct inode *new_parent_inode,
@@ -1220,8 +1198,6 @@ static int __exfat_rename(struct inode *old_parent_inode,
 		return -ENOENT;
 	}
 
-	exfat_update_parent_info(ei, old_parent_inode);
-
 	exfat_chain_dup(&olddir, &ei->dir);
 	dentry = ei->entry;
 
@@ -1242,8 +1218,6 @@ static int __exfat_rename(struct inode *old_parent_inode,
 			goto out;
 		}
 
-		exfat_update_parent_info(new_ei, new_parent_inode);
-
 		p_dir = &(new_ei->dir);
 		new_entry = new_ei->entry;
 		ep = exfat_get_dentry(sb, p_dir, new_entry, &new_bh);
-- 
2.25.1

[-- Attachment #2: 0002-exfat-remove-exfat_update_parent_info.patch --]
[-- Type: application/octet-stream, Size: 2134 bytes --]

exfat_update_parent_info() is a workaround for the wrong parent
directory information being used after renaming. Now that bug is
fixed, this is no longer needed, so remove it.

Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
Reviewed-by: Andy Wu <Andy.Wu@sony.com>
Reviewed-by: Aoyama Wataru <wataru.aoyama@sony.com>
Reviewed-by: Daniel Palmer <daniel.palmer@sony.com>
---
 fs/exfat/namei.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index e7adb6bfd9d5..76acc3721951 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -1168,28 +1168,6 @@ static int exfat_move_file(struct inode *inode, struct exfat_chain *p_olddir,
 	return 0;
 }
 
-static void exfat_update_parent_info(struct exfat_inode_info *ei,
-		struct inode *parent_inode)
-{
-	struct exfat_sb_info *sbi = EXFAT_SB(parent_inode->i_sb);
-	struct exfat_inode_info *parent_ei = EXFAT_I(parent_inode);
-	loff_t parent_isize = i_size_read(parent_inode);
-
-	/*
-	 * the problem that struct exfat_inode_info caches wrong parent info.
-	 *
-	 * because of flag-mismatch of ei->dir,
-	 * there is abnormal traversing cluster chain.
-	 */
-	if (unlikely(parent_ei->flags != ei->dir.flags ||
-		     parent_isize != EXFAT_CLU_TO_B(ei->dir.size, sbi) ||
-		     parent_ei->start_clu != ei->dir.dir)) {
-		exfat_chain_set(&ei->dir, parent_ei->start_clu,
-			EXFAT_B_TO_CLU_ROUND_UP(parent_isize, sbi),
-			parent_ei->flags);
-	}
-}
-
 /* rename or move a old file into a new file */
 static int __exfat_rename(struct inode *old_parent_inode,
 		struct exfat_inode_info *ei, struct inode *new_parent_inode,
@@ -1220,8 +1198,6 @@ static int __exfat_rename(struct inode *old_parent_inode,
 		return -ENOENT;
 	}
 
-	exfat_update_parent_info(ei, old_parent_inode);
-
 	exfat_chain_dup(&olddir, &ei->dir);
 	dentry = ei->entry;
 
@@ -1242,8 +1218,6 @@ static int __exfat_rename(struct inode *old_parent_inode,
 			goto out;
 		}
 
-		exfat_update_parent_info(new_ei, new_parent_inode);
-
 		p_dir = &(new_ei->dir);
 		new_entry = new_ei->entry;
 		ep = exfat_get_dentry(sb, p_dir, new_entry, &new_bh);
-- 
2.25.1

RE: [PATCH 2/2] exfat: remove exfat_update_parent_info()

[Sungjong Seo]

> exfat_update_parent_info() is a workaround for the wrong parent directory
> information being used after renaming. Now that bug is fixed, this is no
> longer needed, so remove it.
> 
> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
> Reviewed-by: Andy Wu <Andy.Wu@sony.com>
> Reviewed-by: Aoyama Wataru <wataru.aoyama@sony.com>
> Reviewed-by: Daniel Palmer <daniel.palmer@sony.com>

As you said, exfat_update_parent_info() seems to be a workaround
that exists from the legacy code to resolve the inconsistency of
parent node information.

Thanks for your patch!
Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com>

> ---
>  fs/exfat/namei.c | 26 --------------------------
>  1 file changed, 26 deletions(-)
> 
> diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c index
> e7adb6bfd9d5..76acc3721951 100644
> --- a/fs/exfat/namei.c
> +++ b/fs/exfat/namei.c
> @@ -1168,28 +1168,6 @@ static int exfat_move_file(struct inode *inode,
> struct exfat_chain *p_olddir,
>  	return 0;
>  }
> 
> -static void exfat_update_parent_info(struct exfat_inode_info *ei,
> -		struct inode *parent_inode)
> -{
> -	struct exfat_sb_info *sbi = EXFAT_SB(parent_inode->i_sb);
> -	struct exfat_inode_info *parent_ei = EXFAT_I(parent_inode);
> -	loff_t parent_isize = i_size_read(parent_inode);
> -
> -	/*
> -	 * the problem that struct exfat_inode_info caches wrong parent
> info.
> -	 *
> -	 * because of flag-mismatch of ei->dir,
> -	 * there is abnormal traversing cluster chain.
> -	 */
> -	if (unlikely(parent_ei->flags != ei->dir.flags ||
> -		     parent_isize != EXFAT_CLU_TO_B(ei->dir.size, sbi) ||
> -		     parent_ei->start_clu != ei->dir.dir)) {
> -		exfat_chain_set(&ei->dir, parent_ei->start_clu,
> -			EXFAT_B_TO_CLU_ROUND_UP(parent_isize, sbi),
> -			parent_ei->flags);
> -	}
> -}
> -
>  /* rename or move a old file into a new file */  static int
> __exfat_rename(struct inode *old_parent_inode,
>  		struct exfat_inode_info *ei, struct inode *new_parent_inode,
> @@ -1220,8 +1198,6 @@ static int __exfat_rename(struct inode
> *old_parent_inode,
>  		return -ENOENT;
>  	}
> 
> -	exfat_update_parent_info(ei, old_parent_inode);
> -
>  	exfat_chain_dup(&olddir, &ei->dir);
>  	dentry = ei->entry;
> 
> @@ -1242,8 +1218,6 @@ static int __exfat_rename(struct inode
> *old_parent_inode,
>  			goto out;
>  		}
> 
> -		exfat_update_parent_info(new_ei, new_parent_inode);
> -
>  		p_dir = &(new_ei->dir);
>  		new_entry = new_ei->entry;
>  		ep = exfat_get_dentry(sb, p_dir, new_entry, &new_bh);
> --
> 2.25.1

Re: [PATCH 2/2] exfat: remove exfat_update_parent_info()

[Namjae Jeon]

2022-04-01 19:34 GMT+09:00, Sungjong Seo <sj1557.seo@samsung.com>:
>> exfat_update_parent_info() is a workaround for the wrong parent directory
>> information being used after renaming. Now that bug is fixed, this is no
>> longer needed, so remove it.
>>
>> Signed-off-by: Yuezhang Mo <Yuezhang.Mo@sony.com>
>> Reviewed-by: Andy Wu <Andy.Wu@sony.com>
>> Reviewed-by: Aoyama Wataru <wataru.aoyama@sony.com>
>> Reviewed-by: Daniel Palmer <daniel.palmer@sony.com>
>
> As you said, exfat_update_parent_info() seems to be a workaround
> that exists from the legacy code to resolve the inconsistency of
> parent node information.
>
> Thanks for your patch!
> Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com>
Hi Yuezhang,

I don't think there's any reason to split this patch from patch 1/2.
Any thought to combine them to the one ?

Thanks.
>
>> ---
>>  fs/exfat/namei.c | 26 --------------------------
>>  1 file changed, 26 deletions(-)
>>
>> diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c index
>> e7adb6bfd9d5..76acc3721951 100644
>> --- a/fs/exfat/namei.c
>> +++ b/fs/exfat/namei.c
>> @@ -1168,28 +1168,6 @@ static int exfat_move_file(struct inode *inode,
>> struct exfat_chain *p_olddir,
>>  	return 0;
>>  }
>>
>> -static void exfat_update_parent_info(struct exfat_inode_info *ei,
>> -		struct inode *parent_inode)
>> -{
>> -	struct exfat_sb_info *sbi = EXFAT_SB(parent_inode->i_sb);
>> -	struct exfat_inode_info *parent_ei = EXFAT_I(parent_inode);
>> -	loff_t parent_isize = i_size_read(parent_inode);
>> -
>> -	/*
>> -	 * the problem that struct exfat_inode_info caches wrong parent
>> info.
>> -	 *
>> -	 * because of flag-mismatch of ei->dir,
>> -	 * there is abnormal traversing cluster chain.
>> -	 */
>> -	if (unlikely(parent_ei->flags != ei->dir.flags ||
>> -		     parent_isize != EXFAT_CLU_TO_B(ei->dir.size, sbi) ||
>> -		     parent_ei->start_clu != ei->dir.dir)) {
>> -		exfat_chain_set(&ei->dir, parent_ei->start_clu,
>> -			EXFAT_B_TO_CLU_ROUND_UP(parent_isize, sbi),
>> -			parent_ei->flags);
>> -	}
>> -}
>> -
>>  /* rename or move a old file into a new file */  static int
>> __exfat_rename(struct inode *old_parent_inode,
>>  		struct exfat_inode_info *ei, struct inode *new_parent_inode,
>> @@ -1220,8 +1198,6 @@ static int __exfat_rename(struct inode
>> *old_parent_inode,
>>  		return -ENOENT;
>>  	}
>>
>> -	exfat_update_parent_info(ei, old_parent_inode);
>> -
>>  	exfat_chain_dup(&olddir, &ei->dir);
>>  	dentry = ei->entry;
>>
>> @@ -1242,8 +1218,6 @@ static int __exfat_rename(struct inode
>> *old_parent_inode,
>>  			goto out;
>>  		}
>>
>> -		exfat_update_parent_info(new_ei, new_parent_inode);
>> -
>>  		p_dir = &(new_ei->dir);
>>  		new_entry = new_ei->entry;
>>  		ep = exfat_get_dentry(sb, p_dir, new_entry, &new_bh);
>> --
>> 2.25.1
>
>

RE: [PATCH 2/2] exfat: remove exfat_update_parent_info()

[Yuezhang.Mo]

Dear Namjae,

> I don't think there's any reason to split this patch from patch 1/2.
> Any thought to combine them to the one ?

The purpose of splitting this patch from patch 1/2 is to highlight the fix point for easier review.
If you think it is not necessary, it is OK to squash them to one.


Best Regards,
Yuezhang Mo