| Message ID | 1351000527-24952-3-git-send-email-list.btrfs@jan-o-sch.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/23/2012 09:55 PM, Jan Schmidt wrote: > Avoid the implicit free by tree_mod_log_set_root_pointer, which is wrong in > two places. Where needed, we call tree_mod_log_free_eb explicitly now. > > Signed-off-by: Jan Schmidt <list.btrfs@jan-o-sch.net> > --- > fs/btrfs/ctree.c | 10 ++-------- > 1 files changed, 2 insertions(+), 8 deletions(-) > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > index 44a7e25..e6b75cc 100644 > --- a/fs/btrfs/ctree.c > +++ b/fs/btrfs/ctree.c > @@ -647,8 +647,6 @@ tree_mod_log_insert_root(struct btrfs_fs_info *fs_info, > if (tree_mod_dont_log(fs_info, NULL)) > return 0; > > - __tree_mod_log_free_eb(fs_info, old_root); > - > ret = tree_mod_alloc(fs_info, flags, &tm); > if (ret < 0) > goto out; > @@ -926,12 +924,7 @@ static noinline int update_ref_for_cow(struct btrfs_trans_handle *trans, > ret = btrfs_dec_ref(trans, root, buf, 1, 1); > BUG_ON(ret); /* -ENOMEM */ > } > - /* > - * don't log freeing in case we're freeing the root node, this > - * is done by tree_mod_log_set_root_pointer later > - */ > - if (buf != root->node && btrfs_header_level(buf) != 0) > - tree_mod_log_free_eb(root->fs_info, buf); > + tree_mod_log_free_eb(root->fs_info, buf); Since we've owned a check for if eb's level is 0 in tree_mod_dont_log(), we can also get rid of these level checks in other places, can't we? > clean_tree_block(trans, root, buf); > *last_ref = 1; > } > @@ -1728,6 +1721,7 @@ static noinline int balance_level(struct btrfs_trans_handle *trans, > goto enospc; > } > > + tree_mod_log_free_eb(root->fs_info, root->node); > tree_mod_log_set_root_pointer(root, child); > rcu_assign_pointer(root->node, child); > > Reviewed-by: Liu Bo <bo.li.liu@oracle.com> Tested-by: Liu Bo <bo.li.liu@oracle.com> thanks, liubo -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, October 23, 2012 at 17:28 (+0200), Liu Bo wrote: > On 10/23/2012 09:55 PM, Jan Schmidt wrote: >> Avoid the implicit free by tree_mod_log_set_root_pointer, which is wrong in >> two places. Where needed, we call tree_mod_log_free_eb explicitly now. >> >> Signed-off-by: Jan Schmidt <list.btrfs@jan-o-sch.net> >> --- >> fs/btrfs/ctree.c | 10 ++-------- >> 1 files changed, 2 insertions(+), 8 deletions(-) >> >> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c >> index 44a7e25..e6b75cc 100644 >> --- a/fs/btrfs/ctree.c >> +++ b/fs/btrfs/ctree.c >> @@ -647,8 +647,6 @@ tree_mod_log_insert_root(struct btrfs_fs_info *fs_info, >> if (tree_mod_dont_log(fs_info, NULL)) >> return 0; >> >> - __tree_mod_log_free_eb(fs_info, old_root); >> - >> ret = tree_mod_alloc(fs_info, flags, &tm); >> if (ret < 0) >> goto out; >> @@ -926,12 +924,7 @@ static noinline int update_ref_for_cow(struct btrfs_trans_handle *trans, >> ret = btrfs_dec_ref(trans, root, buf, 1, 1); >> BUG_ON(ret); /* -ENOMEM */ >> } >> - /* >> - * don't log freeing in case we're freeing the root node, this >> - * is done by tree_mod_log_set_root_pointer later >> - */ >> - if (buf != root->node && btrfs_header_level(buf) != 0) >> - tree_mod_log_free_eb(root->fs_info, buf); >> + tree_mod_log_free_eb(root->fs_info, buf); > > Since we've owned a check for if eb's level is 0 in tree_mod_dont_log(), > we can also get rid of these level checks in other places, can't we? We can, yes. There may be hot paths where its worth an extra check to save the overhead. That would require looking closer at the individual sites. Here it definitely wasn't worth any extra check, so I dropped it en passant. Thanks for reviewing and testing! -Jan >> clean_tree_block(trans, root, buf); >> *last_ref = 1; >> } >> @@ -1728,6 +1721,7 @@ static noinline int balance_level(struct btrfs_trans_handle *trans, >> goto enospc; >> } >> >> + tree_mod_log_free_eb(root->fs_info, root->node); >> tree_mod_log_set_root_pointer(root, child); >> rcu_assign_pointer(root->node, child); >> >> > > Reviewed-by: Liu Bo <bo.li.liu@oracle.com> > Tested-by: Liu Bo <bo.li.liu@oracle.com> > > thanks, > liubo -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index 44a7e25..e6b75cc 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -647,8 +647,6 @@ tree_mod_log_insert_root(struct btrfs_fs_info *fs_info, if (tree_mod_dont_log(fs_info, NULL)) return 0; - __tree_mod_log_free_eb(fs_info, old_root); - ret = tree_mod_alloc(fs_info, flags, &tm); if (ret < 0) goto out; @@ -926,12 +924,7 @@ static noinline int update_ref_for_cow(struct btrfs_trans_handle *trans, ret = btrfs_dec_ref(trans, root, buf, 1, 1); BUG_ON(ret); /* -ENOMEM */ } - /* - * don't log freeing in case we're freeing the root node, this - * is done by tree_mod_log_set_root_pointer later - */ - if (buf != root->node && btrfs_header_level(buf) != 0) - tree_mod_log_free_eb(root->fs_info, buf); + tree_mod_log_free_eb(root->fs_info, buf); clean_tree_block(trans, root, buf); *last_ref = 1; } @@ -1728,6 +1721,7 @@ static noinline int balance_level(struct btrfs_trans_handle *trans, goto enospc; } + tree_mod_log_free_eb(root->fs_info, root->node); tree_mod_log_set_root_pointer(root, child); rcu_assign_pointer(root->node, child);
Avoid the implicit free by tree_mod_log_set_root_pointer, which is wrong in two places. Where needed, we call tree_mod_log_free_eb explicitly now. Signed-off-by: Jan Schmidt <list.btrfs@jan-o-sch.net> --- fs/btrfs/ctree.c | 10 ++-------- 1 files changed, 2 insertions(+), 8 deletions(-)