On 08/01/2013 03:58 PM, Jaegeuk Kim wrote: > This patch fixes mishandling of the sbi->n_orphans variable. > > If users request lots of f2fs_unlink(), check_orphan_space() could be > contended. > In such the case, sbi->n_orphans can be read incorrectly so that f2fs_unlink() > would fall into the wrong state which results in the failure of > add_orphan_inode(). > > So, let's increment sbi->n_orphans virtually prior to the actual orphan inode > stuffs. After that, let's release sbi->n_orphans by calling > release_orphan_inode > or remove_orphan_inode.
Hi Kim, The key point is that we did not reduce sbi->n_orphans when we release/remove orphan inode, so just adding the reduction step can fix this issue. But why moving the increment of sbi->n_orphans before we add orphan inode? It seems that we can not get benefit from it, and it makes the procedure a bit complex, because we should reduce the sbi->n_orphans in some fail pathes before we really add orphan inode. Thanks, Gu > > Signed-off-by: Jaegeuk Kim <[email protected]> > --- > fs/f2fs/checkpoint.c | 13 ++++++++++--- > fs/f2fs/dir.c | 2 ++ > fs/f2fs/f2fs.h | 3 ++- > fs/f2fs/namei.c | 19 ++++++++++++++----- > 4 files changed, 28 insertions(+), 9 deletions(-) > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > index fe91773..c5a5c39 100644 > --- a/fs/f2fs/checkpoint.c > +++ b/fs/f2fs/checkpoint.c > @@ -182,7 +182,7 @@ const struct address_space_operations f2fs_meta_aops = { > .set_page_dirty = f2fs_set_meta_page_dirty, > }; > > -int check_orphan_space(struct f2fs_sb_info *sbi) > +int acquire_orphan_inode(struct f2fs_sb_info *sbi) > { > unsigned int max_orphans; > int err = 0; > @@ -197,10 +197,19 @@ int check_orphan_space(struct f2fs_sb_info *sbi) > mutex_lock(&sbi->orphan_inode_mutex); > if (sbi->n_orphans >= max_orphans) > err = -ENOSPC; > + else > + sbi->n_orphans++; > mutex_unlock(&sbi->orphan_inode_mutex); > return err; > } > > +void release_orphan_inode(struct f2fs_sb_info *sbi) > +{ > + mutex_lock(&sbi->orphan_inode_mutex); > + sbi->n_orphans--; > + mutex_unlock(&sbi->orphan_inode_mutex); > +} > + > void add_orphan_inode(struct f2fs_sb_info *sbi, nid_t ino) > { > struct list_head *head, *this; > @@ -229,8 +238,6 @@ retry: > list_add(&new->list, this->prev); > else > list_add_tail(&new->list, head); > - > - sbi->n_orphans++; > out: > mutex_unlock(&sbi->orphan_inode_mutex); > } > diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c > index d1bb260..384c6da 100644 > --- a/fs/f2fs/dir.c > +++ b/fs/f2fs/dir.c > @@ -572,6 +572,8 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, > struct page *page, > > if (inode->i_nlink == 0) > add_orphan_inode(sbi, inode->i_ino); > + else > + release_orphan_inode(sbi); > } > > if (bit_pos == NR_DENTRY_IN_BLOCK) { > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index a6858c7..78777cd 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -1044,7 +1044,8 @@ void destroy_segment_manager(struct f2fs_sb_info *); > struct page *grab_meta_page(struct f2fs_sb_info *, pgoff_t); > struct page *get_meta_page(struct f2fs_sb_info *, pgoff_t); > long sync_meta_pages(struct f2fs_sb_info *, enum page_type, long); > -int check_orphan_space(struct f2fs_sb_info *); > +int acquire_orphan_inode(struct f2fs_sb_info *); > +void release_orphan_inode(struct f2fs_sb_info *); > void add_orphan_inode(struct f2fs_sb_info *, nid_t); > void remove_orphan_inode(struct f2fs_sb_info *, nid_t); > int recover_orphan_inodes(struct f2fs_sb_info *); > diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c > index 3297278..4e47518 100644 > --- a/fs/f2fs/namei.c > +++ b/fs/f2fs/namei.c > @@ -239,7 +239,7 @@ static int f2fs_unlink(struct inode *dir, struct dentry > *dentry) > if (!de) > goto fail; > > - err = check_orphan_space(sbi); > + err = acquire_orphan_inode(sbi); > if (err) { > kunmap(page); > f2fs_put_page(page, 0); > @@ -393,7 +393,7 @@ static int f2fs_rename(struct inode *old_dir, struct > dentry *old_dentry, > struct inode *old_inode = old_dentry->d_inode; > struct inode *new_inode = new_dentry->d_inode; > struct page *old_dir_page; > - struct page *old_page; > + struct page *old_page, *new_page; > struct f2fs_dir_entry *old_dir_entry = NULL; > struct f2fs_dir_entry *old_entry; > struct f2fs_dir_entry *new_entry; > @@ -415,7 +415,6 @@ static int f2fs_rename(struct inode *old_dir, struct > dentry *old_dentry, > ilock = mutex_lock_op(sbi); > > if (new_inode) { > - struct page *new_page; > > err = -ENOTEMPTY; > if (old_dir_entry && !f2fs_empty_dir(new_inode)) > @@ -427,9 +426,13 @@ static int f2fs_rename(struct inode *old_dir, struct > dentry *old_dentry, > if (!new_entry) > goto out_dir; > > + err = acquire_orphan_inode(sbi); > + if (err) > + goto put_out_dir; > + > if (update_dent_inode(old_inode, &new_dentry->d_name)) { > - f2fs_put_page(new_page, 1); > - goto out_dir; > + release_orphan_inode(sbi); > + goto put_out_dir; > } > > f2fs_set_link(new_dir, new_entry, new_page, old_inode); > @@ -438,8 +441,12 @@ static int f2fs_rename(struct inode *old_dir, struct > dentry *old_dentry, > if (old_dir_entry) > drop_nlink(new_inode); > drop_nlink(new_inode); > + > if (!new_inode->i_nlink) > add_orphan_inode(sbi, new_inode->i_ino); > + else > + release_orphan_inode(sbi); > + > update_inode_page(new_inode); > } else { > err = f2fs_add_link(new_dentry, old_inode); > @@ -472,6 +479,8 @@ static int f2fs_rename(struct inode *old_dir, struct > dentry *old_dentry, > mutex_unlock_op(sbi, ilock); > return 0; > > +put_out_dir: > + f2fs_put_page(new_page, 1); > out_dir: > if (old_dir_entry) { > kunmap(old_dir_page); ------------------------------------------------------------------------------ Get your SQL database under version control now! Version control is standard for application code, but databases havent caught up. So what steps can you take to put your SQL databases under version control? Why should you start doing it? Read more to find out. http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
