Joseph Qi
2020-Mar-16 04:46 UTC
[Ocfs2-devel] [PATCH] ocfs2: the parent directory reference count should be increased only after the ocfs2_add_entry function is successfully called
On 2020/3/16 00:59, wangjian wrote:> Under some conditions, the directory cannot be deleted. > The specific scenarios are as follows: (for example, > /mnt/ocfs2 is the mount point) > > 1. Create the /mnt/ocfs2/p_dir directory. At this time, > the i_nlink corresponding to the inode of > the /mnt/ocfs2/p_dir directory is equal to 2. > > 2. During the process of creating the > /mnt/ocfs2/p_dir/s_dir directory, if the call to the inc_nlink > function in ocfs2_mknod succeeds, the functions such as > ocfs2_init_acl, ocfs2_init_security_set, and ocfs2_dentry_attach_lock fail. > At this time, the i_nlink corresponding to the inode of the > /mnt/ocfs2/p_dir directory is equal to 3, but /mnt/ocfs2/p_dir/s_dir > is not added to the /mnt/ocfs2/p_dir directory entry. > > 3. Delete the /mnt/ocfs2/p_dir directory (rm -rf /mnt/ocfs2/p_dir). > At this time, it is found that the i_nlink corresponding to > the inode corresponding to the /mnt/ocfs2/p_dir directory is equal to 3. > Therefore, the /mnt/ocfs2/p_dir directory cannot be deleted. > > Signed-off-by: Jian wang <wangjian161 at huawei.com> > --- > ?ocfs2/namei.c | 24 +++++++++++++----------- > ?1 file changed, 13 insertions(+), 11 deletions(-) > > diff --git a/ocfs2/namei.c b/ocfs2/namei.c > index 8ea51cf..19543b4 100644 > --- a/ocfs2/namei.c > +++ b/ocfs2/namei.c > @@ -388,17 +388,6 @@ static int ocfs2_mknod(struct inode *dir, > ???????????? mlog_errno(status); > ???????????? goto leave; > ???????? } > - > -??????? status = ocfs2_journal_access_di(handle, INODE_CACHE(dir), > -???????????????????????? parent_fe_bh, > -???????????????????????? OCFS2_JOURNAL_ACCESS_WRITE); > -??????? if (status < 0) { > -??????????? mlog_errno(status); > -??????????? goto leave; > -??????? }We can't simply move it down since we should get journal access before modification. Thanks, Joseph> -??????? ocfs2_add_links_count(dirfe, 1); > -??????? ocfs2_journal_dirty(handle, parent_fe_bh); > -??????? inc_nlink(dir); > ???? } > ? > ???? status = ocfs2_init_acl(handle, inode, dir, new_fe_bh, parent_fe_bh, > @@ -440,6 +429,19 @@ static int ocfs2_mknod(struct inode *dir, > ???????? goto leave; > ???? } > ? > +??? if (S_ISDIR(mode)) { > +??????? status = ocfs2_journal_access_di(handle, INODE_CACHE(dir), > +???????????????????????? parent_fe_bh, > +???????????????????????? OCFS2_JOURNAL_ACCESS_WRITE); > +??????? if (status < 0) { > +??????????? mlog_errno(status); > +??????????? goto leave; > +??????? } > +??????? ocfs2_add_links_count(dirfe, 1); > +??????? ocfs2_journal_dirty(handle, parent_fe_bh); > +??????? inc_nlink(dir); > +??? } > + > ???? insert_inode_hash(inode); > ???? d_instantiate(dentry, inode); > ???? status = 0;
wangjian
2020-Mar-16 07:27 UTC
[Ocfs2-devel] [PATCH] ocfs2: the parent directory reference count should be increased only after the ocfs2_add_entry function is successfully called
good suggestion. Can we just move the three statements ocfs2_add_links_count (dirfe, 1), ocfs2_journal_dirty (handle, parent_fe_bh), inc_nlink (dir) to the following? This seems to be no problem. Or reset the reference count back in case of failure? Thanks, Jian On 3/16/2020 12:46 PM, Joseph Qi wrote:> > On 2020/3/16 00:59, wangjian wrote: >> Under some conditions, the directory cannot be deleted. >> The specific scenarios are as follows: (for example, >> /mnt/ocfs2 is the mount point) >> >> 1. Create the /mnt/ocfs2/p_dir directory. At this time, >> the i_nlink corresponding to the inode of >> the /mnt/ocfs2/p_dir directory is equal to 2. >> >> 2. During the process of creating the >> /mnt/ocfs2/p_dir/s_dir directory, if the call to the inc_nlink >> function in ocfs2_mknod succeeds, the functions such as >> ocfs2_init_acl, ocfs2_init_security_set, and ocfs2_dentry_attach_lock fail. >> At this time, the i_nlink corresponding to the inode of the >> /mnt/ocfs2/p_dir directory is equal to 3, but /mnt/ocfs2/p_dir/s_dir >> is not added to the /mnt/ocfs2/p_dir directory entry. >> >> 3. Delete the /mnt/ocfs2/p_dir directory (rm -rf /mnt/ocfs2/p_dir). >> At this time, it is found that the i_nlink corresponding to >> the inode corresponding to the /mnt/ocfs2/p_dir directory is equal to 3. >> Therefore, the /mnt/ocfs2/p_dir directory cannot be deleted. >> >> Signed-off-by: Jian wang <wangjian161 at huawei.com> >> --- >> ?ocfs2/namei.c | 24 +++++++++++++----------- >> ?1 file changed, 13 insertions(+), 11 deletions(-) >> >> diff --git a/ocfs2/namei.c b/ocfs2/namei.c >> index 8ea51cf..19543b4 100644 >> --- a/ocfs2/namei.c >> +++ b/ocfs2/namei.c >> @@ -388,17 +388,6 @@ static int ocfs2_mknod(struct inode *dir, >> ???????????? mlog_errno(status); >> ???????????? goto leave; >> ???????? } >> - >> -??????? status = ocfs2_journal_access_di(handle, INODE_CACHE(dir), >> -???????????????????????? parent_fe_bh, >> -???????????????????????? OCFS2_JOURNAL_ACCESS_WRITE); >> -??????? if (status < 0) { >> -??????????? mlog_errno(status); >> -??????????? goto leave; >> -??????? } > We can't simply move it down since we should get journal access before > modification. > > Thanks, > Joseph > >> -??????? ocfs2_add_links_count(dirfe, 1); >> -??????? ocfs2_journal_dirty(handle, parent_fe_bh); >> -??????? inc_nlink(dir); >> ???? } >> >> ???? status = ocfs2_init_acl(handle, inode, dir, new_fe_bh, parent_fe_bh, >> @@ -440,6 +429,19 @@ static int ocfs2_mknod(struct inode *dir, >> ???????? goto leave; >> ???? } >> >> +??? if (S_ISDIR(mode)) { >> +??????? status = ocfs2_journal_access_di(handle, INODE_CACHE(dir), >> +???????????????????????? parent_fe_bh, >> +???????????????????????? OCFS2_JOURNAL_ACCESS_WRITE); >> +??????? if (status < 0) { >> +??????????? mlog_errno(status); >> +??????????? goto leave; >> +??????? } >> +??????? ocfs2_add_links_count(dirfe, 1); >> +??????? ocfs2_journal_dirty(handle, parent_fe_bh); >> +??????? inc_nlink(dir); >> +??? } >> + >> ???? insert_inode_hash(inode); >> ???? d_instantiate(dentry, inode); >> ???? status = 0; > . >