| Message ID | 1394829413-1620-1-git-send-email-jbacik@fb.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Mar 14, 2014 at 04:36:53PM -0400, Josef Bacik wrote: > I'm not sure why we weren't aborting here in the first place, it is obviously a > bad time from the fact that we print the leaf and yell loudly about it. Fix > this up, otherwise we panic because our path could be pointing into oblivion. > Thanks, > > Signed-off-by: Josef Bacik <jbacik@fb.com> > --- > fs/btrfs/extent-tree.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 696f0b6..0015b02 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -5744,6 +5744,8 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, Adding context: 5748 } else if (WARN_ON(ret == -ENOENT)) { 5749 btrfs_print_leaf(extent_root, path->nodes[0]); 5750 btrfs_err(info, > "unable to find ref byte nr %llu parent %llu root %llu owner %llu offset %llu", > bytenr, parent, root_objectid, owner_objectid, > owner_offset); > + btrfs_abort_transaction(trans, extent_root, ret); Abort prints stacktrace on it's own and with the WARN_ON above it would be noisy and without any extra benefit, so I suggest to remove it. > + goto out; > } else { -- 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
Hi Josef, how about aborting the transaction also in place that you print: "umm, got %d back from search, was looking for %llu" You abort in case ret<0, otherwise the code just proceeds with extent_slot = path->slots[0]; which can't be right in that case. Thanks, Alex. On Mon, Mar 17, 2014 at 3:55 PM, David Sterba <dsterba@suse.cz> wrote: > On Fri, Mar 14, 2014 at 04:36:53PM -0400, Josef Bacik wrote: >> I'm not sure why we weren't aborting here in the first place, it is obviously a >> bad time from the fact that we print the leaf and yell loudly about it. Fix >> this up, otherwise we panic because our path could be pointing into oblivion. >> Thanks, >> >> Signed-off-by: Josef Bacik <jbacik@fb.com> >> --- >> fs/btrfs/extent-tree.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c >> index 696f0b6..0015b02 100644 >> --- a/fs/btrfs/extent-tree.c >> +++ b/fs/btrfs/extent-tree.c >> @@ -5744,6 +5744,8 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, > > Adding context: > > 5748 } else if (WARN_ON(ret == -ENOENT)) { > 5749 btrfs_print_leaf(extent_root, path->nodes[0]); > 5750 btrfs_err(info, > >> "unable to find ref byte nr %llu parent %llu root %llu owner %llu offset %llu", >> bytenr, parent, root_objectid, owner_objectid, >> owner_offset); >> + btrfs_abort_transaction(trans, extent_root, ret); > > Abort prints stacktrace on it's own and with the WARN_ON above it would > be noisy and without any extra benefit, so I suggest to remove it. > >> + goto out; >> } else { > -- > 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
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 696f0b6..0015b02 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -5744,6 +5744,8 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, "unable to find ref byte nr %llu parent %llu root %llu owner %llu offset %llu", bytenr, parent, root_objectid, owner_objectid, owner_offset); + btrfs_abort_transaction(trans, extent_root, ret); + goto out; } else { btrfs_abort_transaction(trans, extent_root, ret); goto out;
I'm not sure why we weren't aborting here in the first place, it is obviously a bad time from the fact that we print the leaf and yell loudly about it. Fix this up, otherwise we panic because our path could be pointing into oblivion. Thanks, Signed-off-by: Josef Bacik <jbacik@fb.com> --- fs/btrfs/extent-tree.c | 2 ++ 1 file changed, 2 insertions(+)