| Message ID | d4d168d0a592fa8f828174b3f93fa463b922d492.1605215645.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleanup error handling in relocation | expand |
On 2020/11/13 上午5:18, Josef Bacik wrote: > The following patches are going to address error handling in relocation, > in order to test those patches I need to be able to inject errors in > btrfs_search_slot and btrfs_cow_block, as we call both of these pretty > often in different cases during relocation. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/ctree.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > index d2d5854d51a7..a51e761bf00f 100644 > --- a/fs/btrfs/ctree.c > +++ b/fs/btrfs/ctree.c > @@ -1493,6 +1493,7 @@ noinline int btrfs_cow_block(struct btrfs_trans_handle *trans, > > return ret; > } > +ALLOW_ERROR_INJECTION(btrfs_cow_block, ERRNO); > > /* > * helper function for defrag to decide if two blocks pointed to by a > @@ -2870,6 +2871,7 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root, > btrfs_release_path(p); > return ret; > } > +ALLOW_ERROR_INJECTION(btrfs_search_slot, ERRNO); This concerns me a little. For error case, wouldn't we also free the path? But if we just override the error, the path is not freed by anyone, neither caller nor btrfs_search_slot() would free the path. Or did I miss something? Thanks, Qu > > /* > * Like btrfs_search_slot, this looks for a key in the given tree. It uses the >
On 11/12/20 7:02 PM, Qu Wenruo wrote: > > > On 2020/11/13 上午5:18, Josef Bacik wrote: >> The following patches are going to address error handling in relocation, >> in order to test those patches I need to be able to inject errors in >> btrfs_search_slot and btrfs_cow_block, as we call both of these pretty >> often in different cases during relocation. >> >> Signed-off-by: Josef Bacik <josef@toxicpanda.com> >> --- >> fs/btrfs/ctree.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c >> index d2d5854d51a7..a51e761bf00f 100644 >> --- a/fs/btrfs/ctree.c >> +++ b/fs/btrfs/ctree.c >> @@ -1493,6 +1493,7 @@ noinline int btrfs_cow_block(struct btrfs_trans_handle *trans, >> >> return ret; >> } >> +ALLOW_ERROR_INJECTION(btrfs_cow_block, ERRNO); >> >> /* >> * helper function for defrag to decide if two blocks pointed to by a >> @@ -2870,6 +2871,7 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root, >> btrfs_release_path(p); >> return ret; >> } >> +ALLOW_ERROR_INJECTION(btrfs_search_slot, ERRNO); > > This concerns me a little. > > For error case, wouldn't we also free the path? > But if we just override the error, the path is not freed by anyone, > neither caller nor btrfs_search_slot() would free the path. > > Or did I miss something? > You're missing that the caller is responsible for free'ing the path, failing btrfs_search_slot isn't going to leak anything unless there's a bug with the caller. Thanks, Josef
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index d2d5854d51a7..a51e761bf00f 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -1493,6 +1493,7 @@ noinline int btrfs_cow_block(struct btrfs_trans_handle *trans, return ret; } +ALLOW_ERROR_INJECTION(btrfs_cow_block, ERRNO); /* * helper function for defrag to decide if two blocks pointed to by a @@ -2870,6 +2871,7 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root, btrfs_release_path(p); return ret; } +ALLOW_ERROR_INJECTION(btrfs_search_slot, ERRNO); /* * Like btrfs_search_slot, this looks for a key in the given tree. It uses the
The following patches are going to address error handling in relocation, in order to test those patches I need to be able to inject errors in btrfs_search_slot and btrfs_cow_block, as we call both of these pretty often in different cases during relocation. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/ctree.c | 2 ++ 1 file changed, 2 insertions(+)