Josef Bacik
2013-Jul-01 20:12 UTC
[PATCH] Btrfs: only do the tree_mod_log_free_eb if this is our last ref
There is another bug in the tree mod log stuff in that we''re calling tree_mod_log_free_eb every single time a block is cow''ed. The problem with this is that if this block is shared by multiple snapshots we will call this multiple times per block, so if we go to rewind the mod log for this block we''ll BUG_ON() in __tree_mod_log_rewind because we try to rewind a free twice. We only want to call tree_mod_log_free_eb if we are actually freeing the block. With this patch I no longer hit the panic in __tree_mod_log_rewind. Thanks, Signed-off-by: Josef Bacik <jbacik@fusionio.com> --- fs/btrfs/ctree.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index 32e30ad..127e1fd 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -1093,7 +1093,8 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans, btrfs_set_node_ptr_generation(parent, parent_slot, trans->transid); btrfs_mark_buffer_dirty(parent); - tree_mod_log_free_eb(root->fs_info, buf); + if (last_ref) + tree_mod_log_free_eb(root->fs_info, buf); btrfs_free_tree_block(trans, root, buf, parent_start, last_ref); } -- 1.7.7.6 -- 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
Jan Schmidt
2013-Jul-02 07:36 UTC
Re: [PATCH] Btrfs: only do the tree_mod_log_free_eb if this is our last ref
(resent to list) On Mon, July 01, 2013 at 22:12 (+0200), Josef Bacik wrote:> There is another bug in the tree mod log stuff in that we''re calling > tree_mod_log_free_eb every single time a block is cow''ed. The problem with this > is that if this block is shared by multiple snapshots we will call this multiple > times per block, so if we go to rewind the mod log for this block we''ll BUG_ON() > in __tree_mod_log_rewind because we try to rewind a free twice. We only want to > call tree_mod_log_free_eb if we are actually freeing the block. With this patch > I no longer hit the panic in __tree_mod_log_rewind. Thanks, > > Signed-off-by: Josef Bacik <jbacik@fusionio.com>Strange that never really popped up largely so far, should be quite easy to hit. Anyway, Reviewed-by: Jan Schmidt <list.btrfs@jan-o-sch.net>> --- > fs/btrfs/ctree.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > index 32e30ad..127e1fd 100644 > --- a/fs/btrfs/ctree.c > +++ b/fs/btrfs/ctree.c > @@ -1093,7 +1093,8 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans, > btrfs_set_node_ptr_generation(parent, parent_slot, > trans->transid); > btrfs_mark_buffer_dirty(parent); > - tree_mod_log_free_eb(root->fs_info, buf); > + if (last_ref) > + tree_mod_log_free_eb(root->fs_info, buf); > btrfs_free_tree_block(trans, root, buf, parent_start, > last_ref); > } >-- 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
Liu Bo
2013-Jul-03 03:26 UTC
Re: [PATCH] Btrfs: only do the tree_mod_log_free_eb if this is our last ref
On Mon, Jul 01, 2013 at 04:12:27PM -0400, Josef Bacik wrote:> There is another bug in the tree mod log stuff in that we''re calling > tree_mod_log_free_eb every single time a block is cow''ed. The problem with this > is that if this block is shared by multiple snapshots we will call this multiple > times per block, so if we go to rewind the mod log for this block we''ll BUG_ON() > in __tree_mod_log_rewind because we try to rewind a free twice. We only want to > call tree_mod_log_free_eb if we are actually freeing the block. With this patch > I no longer hit the panic in __tree_mod_log_rewind. Thanks,This fixes my reproducer for __tree_mod_log_rewind() BUG_ON in line 1183. Tested-by: Liu Bo <bo.li.liu@oracle.com>> > Signed-off-by: Josef Bacik <jbacik@fusionio.com> > --- > fs/btrfs/ctree.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > index 32e30ad..127e1fd 100644 > --- a/fs/btrfs/ctree.c > +++ b/fs/btrfs/ctree.c > @@ -1093,7 +1093,8 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans, > btrfs_set_node_ptr_generation(parent, parent_slot, > trans->transid); > btrfs_mark_buffer_dirty(parent); > - tree_mod_log_free_eb(root->fs_info, buf); > + if (last_ref) > + tree_mod_log_free_eb(root->fs_info, buf); > btrfs_free_tree_block(trans, root, buf, parent_start, > last_ref); > } > -- > 1.7.7.6 > > -- > 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-- 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