| Message ID | HK2PR04MB38911DEEC1C24C06E4C272D5811A9@HK2PR04MB3891.apcprd04.prod.outlook.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fix referencing wrong parent dir info after renaming | expand |
> 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
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 > >
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
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);