* [PATCH 3/3] Make max_size consistent with nr
@ 2017-04-28 9:13 Christophe de Dinechin
2017-04-28 9:16 ` Roman Mamedov
0 siblings, 1 reply; 6+ messages in thread
From: Christophe de Dinechin @ 2017-04-28 9:13 UTC (permalink / raw)
To: Btrfs BTRFS
Since we memset tmpl, max_size==0. This does not seem consistent with nr = 1.
In check_extent_refs, we will call:
set_extent_dirty(root->fs_info->excluded_extents,
rec->start,
rec->start + rec->max_size - 1);
This ends up with BUG_ON(end < start) in insert_state.
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
---
cmds-check.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/cmds-check.c b/cmds-check.c
index 58e65d6..774e9b6 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -6193,6 +6193,7 @@ static int add_tree_backref(struct cache_tree *extent_cache, u64 bytenr,
tmpl.start = bytenr;
tmpl.nr = 1;
tmpl.metadata = 1;
+ tmpl.max_size = 1;
ret = add_extent_rec_nolookup(extent_cache, &tmpl);
if (ret)
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3/3] Make max_size consistent with nr
2017-04-28 9:13 [PATCH 3/3] Make max_size consistent with nr Christophe de Dinechin
@ 2017-04-28 9:16 ` Roman Mamedov
2017-04-28 9:50 ` [PATCH 2/3 v2] Prevent attempt to insert extent record with max_size==0 Christophe de Dinechin
2017-04-28 9:51 ` [PATCH 3/3 v2] Make max_size consistent with nr Christophe de Dinechin
0 siblings, 2 replies; 6+ messages in thread
From: Roman Mamedov @ 2017-04-28 9:16 UTC (permalink / raw)
To: Christophe de Dinechin; +Cc: Btrfs BTRFS
On Fri, 28 Apr 2017 11:13:36 +0200
Christophe de Dinechin <dinechin@redhat.com> wrote:
> Since we memset tmpl, max_size==0. This does not seem consistent with nr = 1.
> In check_extent_refs, we will call:
>
> set_extent_dirty(root->fs_info->excluded_extents,
> rec->start,
> rec->start + rec->max_size - 1);
>
> This ends up with BUG_ON(end < start) in insert_state.
>
> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
> ---
> cmds-check.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/cmds-check.c b/cmds-check.c
> index 58e65d6..774e9b6 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -6193,6 +6193,7 @@ static int add_tree_backref(struct cache_tree *extent_cache, u64 bytenr,
> tmpl.start = bytenr;
> tmpl.nr = 1;
> tmpl.metadata = 1;
> + tmpl.max_size = 1;
>
> ret = add_extent_rec_nolookup(extent_cache, &tmpl);
> if (ret)
The original code uses Tab characters for indent, but your addition uses
spaces. Also same problem in patch 2/3.
--
With respect,
Roman
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3 v2] Prevent attempt to insert extent record with max_size==0
2017-04-28 9:16 ` Roman Mamedov
@ 2017-04-28 9:50 ` Christophe de Dinechin
2017-05-02 16:46 ` David Sterba
2017-04-28 9:51 ` [PATCH 3/3 v2] Make max_size consistent with nr Christophe de Dinechin
1 sibling, 1 reply; 6+ messages in thread
From: Christophe de Dinechin @ 2017-04-28 9:50 UTC (permalink / raw)
To: Btrfs BTRFS; +Cc: Roman Mamedov
When this happens, we will trip a BUG_ON(end < start) in insert_state
because in check_extent_refs, we use this max_size expecting it's not zero:
set_extent_dirty(root->fs_info->excluded_extents,
rec->start,
rec->start + rec->max_size - 1);
See https://bugzilla.redhat.com/show_bug.cgi?id=1435567
for an example where this scenario occurs.
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
---
cmds-check.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/cmds-check.c b/cmds-check.c
index 2d3ebc1..c13f900 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -6029,6 +6029,7 @@ static int add_extent_rec_nolookup(struct cache_tree *extent_cache,
struct extent_record *rec;
int ret = 0;
+ BUG_ON(tmpl->max_size == 0);
rec = malloc(sizeof(*rec));
if (!rec)
return -ENOMEM;
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/3 v2] Prevent attempt to insert extent record with max_size==0
2017-04-28 9:50 ` [PATCH 2/3 v2] Prevent attempt to insert extent record with max_size==0 Christophe de Dinechin
@ 2017-05-02 16:46 ` David Sterba
0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2017-05-02 16:46 UTC (permalink / raw)
To: Christophe de Dinechin; +Cc: Btrfs BTRFS, Roman Mamedov
On Fri, Apr 28, 2017 at 11:50:23AM +0200, Christophe de Dinechin wrote:
> When this happens, we will trip a BUG_ON(end < start) in insert_state
> because in check_extent_refs, we use this max_size expecting it's not zero:
>
> set_extent_dirty(root->fs_info->excluded_extents,
> rec->start,
> rec->start + rec->max_size - 1);
>
> See https://bugzilla.redhat.com/show_bug.cgi?id=1435567
> for an example where this scenario occurs.
>
> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3 v2] Make max_size consistent with nr
2017-04-28 9:16 ` Roman Mamedov
2017-04-28 9:50 ` [PATCH 2/3 v2] Prevent attempt to insert extent record with max_size==0 Christophe de Dinechin
@ 2017-04-28 9:51 ` Christophe de Dinechin
2017-05-02 16:53 ` David Sterba
1 sibling, 1 reply; 6+ messages in thread
From: Christophe de Dinechin @ 2017-04-28 9:51 UTC (permalink / raw)
To: Btrfs BTRFS; +Cc: Roman Mamedov
Since we memset tmpl, max_size==0. This does not seem consistent with nr = 1.
In check_extent_refs, we will call:
set_extent_dirty(root->fs_info->excluded_extents,
rec->start,
rec->start + rec->max_size - 1);
This ends up with BUG_ON(end < start) in insert_state.
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
---
cmds-check.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/cmds-check.c b/cmds-check.c
index c13f900..d5e2966 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -6193,6 +6193,7 @@ static int add_tree_backref(struct cache_tree *extent_cache, u64 bytenr,
tmpl.start = bytenr;
tmpl.nr = 1;
tmpl.metadata = 1;
+ tmpl.max_size = 1;
ret = add_extent_rec_nolookup(extent_cache, &tmpl);
if (ret)
--
2.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3/3 v2] Make max_size consistent with nr
2017-04-28 9:51 ` [PATCH 3/3 v2] Make max_size consistent with nr Christophe de Dinechin
@ 2017-05-02 16:53 ` David Sterba
0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2017-05-02 16:53 UTC (permalink / raw)
To: Christophe de Dinechin; +Cc: Btrfs BTRFS, Roman Mamedov
On Fri, Apr 28, 2017 at 11:51:21AM +0200, Christophe de Dinechin wrote:
> Since we memset tmpl, max_size==0. This does not seem consistent with nr = 1.
> In check_extent_refs, we will call:
>
> set_extent_dirty(root->fs_info->excluded_extents,
> rec->start,
> rec->start + rec->max_size - 1);
>
> This ends up with BUG_ON(end < start) in insert_state.
>
> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
Applied. Thanks for debugging the issue!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-02 16:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-28 9:13 [PATCH 3/3] Make max_size consistent with nr Christophe de Dinechin
2017-04-28 9:16 ` Roman Mamedov
2017-04-28 9:50 ` [PATCH 2/3 v2] Prevent attempt to insert extent record with max_size==0 Christophe de Dinechin
2017-05-02 16:46 ` David Sterba
2017-04-28 9:51 ` [PATCH 3/3 v2] Make max_size consistent with nr Christophe de Dinechin
2017-05-02 16:53 ` David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox