* [PATCH 1/2] Btrfs: add missing error handling after doing leaf/node binary search
@ 2019-02-18 16:57 fdmanana
2019-02-18 17:07 ` [PATCH v2 " fdmanana
0 siblings, 1 reply; 5+ messages in thread
From: fdmanana @ 2019-02-18 16:57 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
The function map_private_extent_buffer() can return an -EINVAL error, and
it is called by generic_bin_search() which will return back the error. The
btrfs_bin_search() function in turn calls generic_bin_search() and the
key_search() function calls btrfs_bin_search(), so both can return the
-EINVAL error coming from the map_private_extent_buffer() function. Some
callers of these functions were ignoring that these functions can return
an error, so fix them to deal with error return values.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/ctree.c | 6 ++++++
fs/btrfs/relocation.c | 6 ++++++
fs/btrfs/tree-log.c | 2 ++
3 files changed, 14 insertions(+)
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 5a6c39b44c84..5b9f602fb9e2 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -3005,6 +3005,8 @@ int btrfs_search_old_slot(struct btrfs_root *root, const struct btrfs_key *key,
*/
prev_cmp = -1;
ret = key_search(b, key, level, &prev_cmp, &slot);
+ if (ret < 0)
+ goto done;
if (level != 0) {
int dec = 0;
@@ -5156,6 +5158,10 @@ int btrfs_search_forward(struct btrfs_root *root, struct btrfs_key *min_key,
nritems = btrfs_header_nritems(cur);
level = btrfs_header_level(cur);
sret = btrfs_bin_search(cur, min_key, level, &slot);
+ if (sret < 0) {
+ ret = sret;
+ goto out;
+ }
/* at the lowest level, we're done, setup the path and exit */
if (level == path->lowest_level) {
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 272b287f8cf0..434a5b159b65 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -1802,6 +1802,8 @@ int replace_path(struct btrfs_trans_handle *trans, struct reloc_control *rc,
BUG_ON(level < lowest_level);
ret = btrfs_bin_search(parent, &key, level, &slot);
+ if (ret < 0)
+ break;
if (ret && slot > 0)
slot--;
@@ -2685,6 +2687,8 @@ static int do_relocation(struct btrfs_trans_handle *trans,
if (!lowest) {
ret = btrfs_bin_search(upper->eb, key,
upper->level, &slot);
+ if (ret < 0)
+ break;
BUG_ON(ret);
bytenr = btrfs_node_blockptr(upper->eb, slot);
if (node->eb->start == bytenr)
@@ -2720,6 +2724,8 @@ static int do_relocation(struct btrfs_trans_handle *trans,
} else {
ret = btrfs_bin_search(upper->eb, key, upper->level,
&slot);
+ if (ret < 0)
+ break;
BUG_ON(ret);
}
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 8c9e87f5ec58..81a357a32837 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -3772,6 +3772,8 @@ static int drop_objectid_items(struct btrfs_trans_handle *trans,
found_key.type = 0;
ret = btrfs_bin_search(path->nodes[0], &found_key, 0,
&start_slot);
+ if (ret < 0)
+ break;
ret = btrfs_del_items(trans, log, path, start_slot,
path->slots[0] - start_slot + 1);
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] Btrfs: add missing error handling after doing leaf/node binary search
2019-02-18 16:57 [PATCH 1/2] Btrfs: add missing error handling after doing leaf/node binary search fdmanana
@ 2019-02-18 17:07 ` fdmanana
2019-02-18 17:11 ` Nikolay Borisov
2019-02-19 19:46 ` David Sterba
0 siblings, 2 replies; 5+ messages in thread
From: fdmanana @ 2019-02-18 17:07 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
The function map_private_extent_buffer() can return an -EINVAL error, and
it is called by generic_bin_search() which will return back the error. The
btrfs_bin_search() function in turn calls generic_bin_search() and the
key_search() function calls btrfs_bin_search(), so both can return the
-EINVAL error coming from the map_private_extent_buffer() function. Some
callers of these functions were ignoring that these functions can return
an error, so fix them to deal with error return values.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
V2: Fixed error handling in relocation, missed assignment of ret to err.
fs/btrfs/ctree.c | 6 ++++++
fs/btrfs/relocation.c | 10 ++++++++++
fs/btrfs/tree-log.c | 2 ++
3 files changed, 18 insertions(+)
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 5a6c39b44c84..5b9f602fb9e2 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -3005,6 +3005,8 @@ int btrfs_search_old_slot(struct btrfs_root *root, const struct btrfs_key *key,
*/
prev_cmp = -1;
ret = key_search(b, key, level, &prev_cmp, &slot);
+ if (ret < 0)
+ goto done;
if (level != 0) {
int dec = 0;
@@ -5156,6 +5158,10 @@ int btrfs_search_forward(struct btrfs_root *root, struct btrfs_key *min_key,
nritems = btrfs_header_nritems(cur);
level = btrfs_header_level(cur);
sret = btrfs_bin_search(cur, min_key, level, &slot);
+ if (sret < 0) {
+ ret = sret;
+ goto out;
+ }
/* at the lowest level, we're done, setup the path and exit */
if (level == path->lowest_level) {
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 272b287f8cf0..628ba528516d 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -1802,6 +1802,8 @@ int replace_path(struct btrfs_trans_handle *trans, struct reloc_control *rc,
BUG_ON(level < lowest_level);
ret = btrfs_bin_search(parent, &key, level, &slot);
+ if (ret < 0)
+ break;
if (ret && slot > 0)
slot--;
@@ -2685,6 +2687,10 @@ static int do_relocation(struct btrfs_trans_handle *trans,
if (!lowest) {
ret = btrfs_bin_search(upper->eb, key,
upper->level, &slot);
+ if (ret < 0) {
+ err = ret;
+ goto next;
+ }
BUG_ON(ret);
bytenr = btrfs_node_blockptr(upper->eb, slot);
if (node->eb->start == bytenr)
@@ -2720,6 +2726,10 @@ static int do_relocation(struct btrfs_trans_handle *trans,
} else {
ret = btrfs_bin_search(upper->eb, key, upper->level,
&slot);
+ if (ret < 0) {
+ err = ret;
+ goto next;
+ }
BUG_ON(ret);
}
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 8c9e87f5ec58..81a357a32837 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -3772,6 +3772,8 @@ static int drop_objectid_items(struct btrfs_trans_handle *trans,
found_key.type = 0;
ret = btrfs_bin_search(path->nodes[0], &found_key, 0,
&start_slot);
+ if (ret < 0)
+ break;
ret = btrfs_del_items(trans, log, path, start_slot,
path->slots[0] - start_slot + 1);
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] Btrfs: add missing error handling after doing leaf/node binary search
2019-02-18 17:07 ` [PATCH v2 " fdmanana
@ 2019-02-18 17:11 ` Nikolay Borisov
2019-02-18 17:15 ` Filipe Manana
2019-02-19 19:46 ` David Sterba
1 sibling, 1 reply; 5+ messages in thread
From: Nikolay Borisov @ 2019-02-18 17:11 UTC (permalink / raw)
To: fdmanana, linux-btrfs
On 18.02.19 г. 19:07 ч., fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> The function map_private_extent_buffer() can return an -EINVAL error, and
> it is called by generic_bin_search() which will return back the error. The
> btrfs_bin_search() function in turn calls generic_bin_search() and the
> key_search() function calls btrfs_bin_search(), so both can return the
> -EINVAL error coming from the map_private_extent_buffer() function. Some
> callers of these functions were ignoring that these functions can return
> an error, so fix them to deal with error return values.
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
>
> V2: Fixed error handling in relocation, missed assignment of ret to err.
>
> fs/btrfs/ctree.c | 6 ++++++
> fs/btrfs/relocation.c | 10 ++++++++++
> fs/btrfs/tree-log.c | 2 ++
> 3 files changed, 18 insertions(+)
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 5a6c39b44c84..5b9f602fb9e2 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -3005,6 +3005,8 @@ int btrfs_search_old_slot(struct btrfs_root *root, const struct btrfs_key *key,
> */
> prev_cmp = -1;
> ret = key_search(b, key, level, &prev_cmp, &slot);
> + if (ret < 0)
> + goto done;
>
> if (level != 0) {
> int dec = 0;
> @@ -5156,6 +5158,10 @@ int btrfs_search_forward(struct btrfs_root *root, struct btrfs_key *min_key,
> nritems = btrfs_header_nritems(cur);
> level = btrfs_header_level(cur);
> sret = btrfs_bin_search(cur, min_key, level, &slot);
> + if (sret < 0) {
> + ret = sret;
> + goto out;
> + }>
> /* at the lowest level, we're done, setup the path and exit */
> if (level == path->lowest_level) {
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 272b287f8cf0..628ba528516d 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -1802,6 +1802,8 @@ int replace_path(struct btrfs_trans_handle *trans, struct reloc_control *rc,
> BUG_ON(level < lowest_level);
>
> ret = btrfs_bin_search(parent, &key, level, &slot);
> + if (ret < 0)
> + break;
> if (ret && slot > 0)
> slot--;
>
> @@ -2685,6 +2687,10 @@ static int do_relocation(struct btrfs_trans_handle *trans,
> if (!lowest) {
> ret = btrfs_bin_search(upper->eb, key,
> upper->level, &slot);
> + if (ret < 0) {
> + err = ret;
> + goto next;
> + }
> BUG_ON(ret);
> bytenr = btrfs_node_blockptr(upper->eb, slot);
> if (node->eb->start == bytenr)
> @@ -2720,6 +2726,10 @@ static int do_relocation(struct btrfs_trans_handle *trans,
> } else {
> ret = btrfs_bin_search(upper->eb, key, upper->level,
> &slot);
> + if (ret < 0) {
> + err = ret;
> + goto next;
> + }
> BUG_ON(ret);
Ideally those BUG_ON should disappear and be replaced with, perhahps
-EUCLEAN or somesuch?
> }
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 8c9e87f5ec58..81a357a32837 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -3772,6 +3772,8 @@ static int drop_objectid_items(struct btrfs_trans_handle *trans,
> found_key.type = 0;
> ret = btrfs_bin_search(path->nodes[0], &found_key, 0,
> &start_slot);
> + if (ret < 0)
> + break;
>
> ret = btrfs_del_items(trans, log, path, start_slot,
> path->slots[0] - start_slot + 1);
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] Btrfs: add missing error handling after doing leaf/node binary search
2019-02-18 17:11 ` Nikolay Borisov
@ 2019-02-18 17:15 ` Filipe Manana
0 siblings, 0 replies; 5+ messages in thread
From: Filipe Manana @ 2019-02-18 17:15 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-btrfs
On Mon, Feb 18, 2019 at 5:11 PM Nikolay Borisov <nborisov@suse.com> wrote:
>
>
>
> On 18.02.19 г. 19:07 ч., fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> >
> > The function map_private_extent_buffer() can return an -EINVAL error, and
> > it is called by generic_bin_search() which will return back the error. The
> > btrfs_bin_search() function in turn calls generic_bin_search() and the
> > key_search() function calls btrfs_bin_search(), so both can return the
> > -EINVAL error coming from the map_private_extent_buffer() function. Some
> > callers of these functions were ignoring that these functions can return
> > an error, so fix them to deal with error return values.
> >
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
>
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
>
> > ---
> >
> > V2: Fixed error handling in relocation, missed assignment of ret to err.
> >
> > fs/btrfs/ctree.c | 6 ++++++
> > fs/btrfs/relocation.c | 10 ++++++++++
> > fs/btrfs/tree-log.c | 2 ++
> > 3 files changed, 18 insertions(+)
> >
> > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> > index 5a6c39b44c84..5b9f602fb9e2 100644
> > --- a/fs/btrfs/ctree.c
> > +++ b/fs/btrfs/ctree.c
> > @@ -3005,6 +3005,8 @@ int btrfs_search_old_slot(struct btrfs_root *root, const struct btrfs_key *key,
> > */
> > prev_cmp = -1;
> > ret = key_search(b, key, level, &prev_cmp, &slot);
> > + if (ret < 0)
> > + goto done;
> >
> > if (level != 0) {
> > int dec = 0;
> > @@ -5156,6 +5158,10 @@ int btrfs_search_forward(struct btrfs_root *root, struct btrfs_key *min_key,
> > nritems = btrfs_header_nritems(cur);
> > level = btrfs_header_level(cur);
> > sret = btrfs_bin_search(cur, min_key, level, &slot);
> > + if (sret < 0) {
> > + ret = sret;
> > + goto out;
> > + }>
> > /* at the lowest level, we're done, setup the path and exit */
> > if (level == path->lowest_level) {
> > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> > index 272b287f8cf0..628ba528516d 100644
> > --- a/fs/btrfs/relocation.c
> > +++ b/fs/btrfs/relocation.c
> > @@ -1802,6 +1802,8 @@ int replace_path(struct btrfs_trans_handle *trans, struct reloc_control *rc,
> > BUG_ON(level < lowest_level);
> >
> > ret = btrfs_bin_search(parent, &key, level, &slot);
> > + if (ret < 0)
> > + break;
> > if (ret && slot > 0)
> > slot--;
> >
> > @@ -2685,6 +2687,10 @@ static int do_relocation(struct btrfs_trans_handle *trans,
> > if (!lowest) {
> > ret = btrfs_bin_search(upper->eb, key,
> > upper->level, &slot);
> > + if (ret < 0) {
> > + err = ret;
> > + goto next;
> > + }
> > BUG_ON(ret);
> > bytenr = btrfs_node_blockptr(upper->eb, slot);
> > if (node->eb->start == bytenr)
> > @@ -2720,6 +2726,10 @@ static int do_relocation(struct btrfs_trans_handle *trans,
> > } else {
> > ret = btrfs_bin_search(upper->eb, key, upper->level,
> > &slot);
> > + if (ret < 0) {
> > + err = ret;
> > + goto next;
> > + }
> > BUG_ON(ret);
>
> Ideally those BUG_ON should disappear and be replaced with, perhahps
> -EUCLEAN or somesuch?
Different problem. Those assert the impossibility of the search
returning 1 (key not found), which would happen due to a relocation
algorithm flaw.
I'm dealing with missing error handling only, therefore won't do such
thing in this patch.
>
> > }
> >
> > diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> > index 8c9e87f5ec58..81a357a32837 100644
> > --- a/fs/btrfs/tree-log.c
> > +++ b/fs/btrfs/tree-log.c
> > @@ -3772,6 +3772,8 @@ static int drop_objectid_items(struct btrfs_trans_handle *trans,
> > found_key.type = 0;
> > ret = btrfs_bin_search(path->nodes[0], &found_key, 0,
> > &start_slot);
> > + if (ret < 0)
> > + break;
> >
> > ret = btrfs_del_items(trans, log, path, start_slot,
> > path->slots[0] - start_slot + 1);
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] Btrfs: add missing error handling after doing leaf/node binary search
2019-02-18 17:07 ` [PATCH v2 " fdmanana
2019-02-18 17:11 ` Nikolay Borisov
@ 2019-02-19 19:46 ` David Sterba
1 sibling, 0 replies; 5+ messages in thread
From: David Sterba @ 2019-02-19 19:46 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs
On Mon, Feb 18, 2019 at 05:07:30PM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> The function map_private_extent_buffer() can return an -EINVAL error, and
> it is called by generic_bin_search() which will return back the error. The
> btrfs_bin_search() function in turn calls generic_bin_search() and the
> key_search() function calls btrfs_bin_search(), so both can return the
> -EINVAL error coming from the map_private_extent_buffer() function. Some
> callers of these functions were ignoring that these functions can return
> an error, so fix them to deal with error return values.
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
The binary search functions are called from functions that return errors
and those are handled.
Grepping for map_private_extent_buffer pointed to the set/get function
template DEFINE_BTRFS_SETGET_BITS, handling errors there seems to be
more tricky.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-19 19:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-18 16:57 [PATCH 1/2] Btrfs: add missing error handling after doing leaf/node binary search fdmanana
2019-02-18 17:07 ` [PATCH v2 " fdmanana
2019-02-18 17:11 ` Nikolay Borisov
2019-02-18 17:15 ` Filipe Manana
2019-02-19 19:46 ` David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).