* [PATCH] Btrfs: fix crash when starting transaction
@ 2014-06-23 11:58 Filipe David Borba Manana
2014-06-24 2:34 ` Satoru Takeuchi
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Filipe David Borba Manana @ 2014-06-23 11:58 UTC (permalink / raw)
To: linux-btrfs; +Cc: Filipe David Borba Manana
Often when starting a transaction we commit the currently running transaction,
which can end up writing block group caches when the current process has its
journal_info set to NULL (and not to a transaction). This makes our assertion
at btrfs_check_data_free_space() (current_journal != NULL) fail, resulting
in a crash/hang. Therefore fix it by setting journal_info.
Two different traces of this issue follow below.
1)
[51502.241936] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
[51502.242213] ------------[ cut here ]------------
[51502.242493] kernel BUG at fs/btrfs/ctree.h:3964!
[51502.242669] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
(...)
[51502.244010] Call Trace:
[51502.244010] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
[51502.244010] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
[51502.244010] [<ffffffffa0357a6a>] commit_cowonly_roots+0x164/0x226 [btrfs]
[51502.244010] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
[51502.244010] [<ffffffff8168ec7b>] ? _raw_spin_unlock+0x2b/0x40
[51502.244010] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
[51502.244010] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
[51502.244010] [<ffffffffa02d73e1>] __unlink_start_trans+0x31/0xe0 [btrfs]
[51502.244010] [<ffffffffa02dea67>] btrfs_unlink+0x37/0xc0 [btrfs]
[51502.244010] [<ffffffff811bb054>] ? do_unlinkat+0x114/0x2a0
[51502.244010] [<ffffffff811baebc>] vfs_unlink+0xcc/0x150
[51502.244010] [<ffffffff811bb1a0>] do_unlinkat+0x260/0x2a0
[51502.244010] [<ffffffff811a9ef4>] ? filp_close+0x64/0x90
[51502.244010] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
[51502.244010] [<ffffffff81349cab>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[51502.244010] [<ffffffff811be9eb>] SyS_unlinkat+0x1b/0x40
[51502.244010] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
[51502.244010] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 71 13 36 a0 48 89 fe 31 c0 48 c7 c7 b8 43 36 a0 48 89 e5 e8 5d b0 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
[51502.244010] RIP [<ffffffffa03575da>] assfail.constprop.88+0x1e/0x20 [btrfs]
2)
[25405.097230] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
[25405.097488] ------------[ cut here ]------------
[25405.097767] kernel BUG at fs/btrfs/ctree.h:3964!
[25405.097940] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
(...)
[25405.100008] Call Trace:
[25405.100008] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
[25405.100008] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
[25405.100008] [<ffffffffa035755a>] commit_cowonly_roots+0x164/0x226 [btrfs]
[25405.100008] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
[25405.100008] [<ffffffff8109c170>] ? bit_waitqueue+0xc0/0xc0
[25405.100008] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
[25405.100008] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
[25405.100008] [<ffffffffa02e3407>] btrfs_create+0x47/0x210 [btrfs]
[25405.100008] [<ffffffffa02d74cc>] ? btrfs_permission+0x3c/0x80 [btrfs]
[25405.100008] [<ffffffff811bc63b>] vfs_create+0x9b/0x130
[25405.100008] [<ffffffff811bcf19>] do_last+0x849/0xe20
[25405.100008] [<ffffffff811b9409>] ? link_path_walk+0x79/0x820
[25405.100008] [<ffffffff811bd5b5>] path_openat+0xc5/0x690
[25405.100008] [<ffffffff810ab07d>] ? trace_hardirqs_on+0xd/0x10
[25405.100008] [<ffffffff811cdcd2>] ? __alloc_fd+0x32/0x1d0
[25405.100008] [<ffffffff811be2a3>] do_filp_open+0x43/0xa0
[25405.100008] [<ffffffff811cddf1>] ? __alloc_fd+0x151/0x1d0
[25405.100008] [<ffffffff811abcfc>] do_sys_open+0x13c/0x230
[25405.100008] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
[25405.100008] [<ffffffff811abe12>] SyS_open+0x22/0x30
[25405.100008] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
[25405.100008] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 51 13 36 a0 48 89 fe 31 c0 48 c7 c7 d0 43 36 a0 48 89 e5 e8 6d b5 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
[25405.100008] RIP [<ffffffffa03570ca>] assfail.constprop.88+0x1e/0x20 [btrfs]
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
fs/btrfs/transaction.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index ac984a3..fe4abe9 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -491,7 +491,11 @@ again:
smp_mb();
if (cur_trans->state >= TRANS_STATE_BLOCKED &&
may_wait_transaction(root, type)) {
+ void *journal_info = current->journal_info;
+ if (!journal_info)
+ current->journal_info = h;
btrfs_commit_transaction(h, root);
+ current->journal_info = journal_info;
goto again;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Btrfs: fix crash when starting transaction
2014-06-23 11:58 [PATCH] Btrfs: fix crash when starting transaction Filipe David Borba Manana
@ 2014-06-24 2:34 ` Satoru Takeuchi
2014-06-24 8:29 ` Filipe David Manana
2014-06-24 10:22 ` Miao Xie
2014-06-24 16:46 ` [PATCH v2] " Filipe David Borba Manana
2 siblings, 1 reply; 12+ messages in thread
From: Satoru Takeuchi @ 2014-06-24 2:34 UTC (permalink / raw)
To: Filipe David Borba Manana, linux-btrfs
Hi Filipe,
(2014/06/23 20:58), Filipe David Borba Manana wrote:
> Often when starting a transaction we commit the currently running transaction,
> which can end up writing block group caches when the current process has its
> journal_info set to NULL (and not to a transaction). This makes our assertion
> at btrfs_check_data_free_space() (current_journal != NULL) fail, resulting
> in a crash/hang. Therefore fix it by setting journal_info.
Do you have any reproducer?
Thanks,
Satoru
>
> Two different traces of this issue follow below.
>
> 1)
>
> [51502.241936] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
> [51502.242213] ------------[ cut here ]------------
> [51502.242493] kernel BUG at fs/btrfs/ctree.h:3964!
> [51502.242669] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
> (...)
> [51502.244010] Call Trace:
> [51502.244010] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
> [51502.244010] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
> [51502.244010] [<ffffffffa0357a6a>] commit_cowonly_roots+0x164/0x226 [btrfs]
> [51502.244010] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
> [51502.244010] [<ffffffff8168ec7b>] ? _raw_spin_unlock+0x2b/0x40
> [51502.244010] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
> [51502.244010] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
> [51502.244010] [<ffffffffa02d73e1>] __unlink_start_trans+0x31/0xe0 [btrfs]
> [51502.244010] [<ffffffffa02dea67>] btrfs_unlink+0x37/0xc0 [btrfs]
> [51502.244010] [<ffffffff811bb054>] ? do_unlinkat+0x114/0x2a0
> [51502.244010] [<ffffffff811baebc>] vfs_unlink+0xcc/0x150
> [51502.244010] [<ffffffff811bb1a0>] do_unlinkat+0x260/0x2a0
> [51502.244010] [<ffffffff811a9ef4>] ? filp_close+0x64/0x90
> [51502.244010] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
> [51502.244010] [<ffffffff81349cab>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [51502.244010] [<ffffffff811be9eb>] SyS_unlinkat+0x1b/0x40
> [51502.244010] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
> [51502.244010] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 71 13 36 a0 48 89 fe 31 c0 48 c7 c7 b8 43 36 a0 48 89 e5 e8 5d b0 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
> [51502.244010] RIP [<ffffffffa03575da>] assfail.constprop.88+0x1e/0x20 [btrfs]
>
> 2)
>
> [25405.097230] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
> [25405.097488] ------------[ cut here ]------------
> [25405.097767] kernel BUG at fs/btrfs/ctree.h:3964!
> [25405.097940] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
> (...)
> [25405.100008] Call Trace:
> [25405.100008] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
> [25405.100008] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
> [25405.100008] [<ffffffffa035755a>] commit_cowonly_roots+0x164/0x226 [btrfs]
> [25405.100008] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
> [25405.100008] [<ffffffff8109c170>] ? bit_waitqueue+0xc0/0xc0
> [25405.100008] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
> [25405.100008] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
> [25405.100008] [<ffffffffa02e3407>] btrfs_create+0x47/0x210 [btrfs]
> [25405.100008] [<ffffffffa02d74cc>] ? btrfs_permission+0x3c/0x80 [btrfs]
> [25405.100008] [<ffffffff811bc63b>] vfs_create+0x9b/0x130
> [25405.100008] [<ffffffff811bcf19>] do_last+0x849/0xe20
> [25405.100008] [<ffffffff811b9409>] ? link_path_walk+0x79/0x820
> [25405.100008] [<ffffffff811bd5b5>] path_openat+0xc5/0x690
> [25405.100008] [<ffffffff810ab07d>] ? trace_hardirqs_on+0xd/0x10
> [25405.100008] [<ffffffff811cdcd2>] ? __alloc_fd+0x32/0x1d0
> [25405.100008] [<ffffffff811be2a3>] do_filp_open+0x43/0xa0
> [25405.100008] [<ffffffff811cddf1>] ? __alloc_fd+0x151/0x1d0
> [25405.100008] [<ffffffff811abcfc>] do_sys_open+0x13c/0x230
> [25405.100008] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
> [25405.100008] [<ffffffff811abe12>] SyS_open+0x22/0x30
> [25405.100008] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
> [25405.100008] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 51 13 36 a0 48 89 fe 31 c0 48 c7 c7 d0 43 36 a0 48 89 e5 e8 6d b5 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
> [25405.100008] RIP [<ffffffffa03570ca>] assfail.constprop.88+0x1e/0x20 [btrfs]
>
> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> ---
> fs/btrfs/transaction.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index ac984a3..fe4abe9 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -491,7 +491,11 @@ again:
> smp_mb();
> if (cur_trans->state >= TRANS_STATE_BLOCKED &&
> may_wait_transaction(root, type)) {
> + void *journal_info = current->journal_info;
> + if (!journal_info)
> + current->journal_info = h;
> btrfs_commit_transaction(h, root);
> + current->journal_info = journal_info;
> goto again;
> }
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Btrfs: fix crash when starting transaction
2014-06-24 2:34 ` Satoru Takeuchi
@ 2014-06-24 8:29 ` Filipe David Manana
2014-06-24 9:35 ` Satoru Takeuchi
0 siblings, 1 reply; 12+ messages in thread
From: Filipe David Manana @ 2014-06-24 8:29 UTC (permalink / raw)
To: Satoru Takeuchi; +Cc: linux-btrfs@vger.kernel.org
On Tue, Jun 24, 2014 at 3:34 AM, Satoru Takeuchi
<takeuchi_satoru@jp.fujitsu.com> wrote:
> Hi Filipe,
>
> (2014/06/23 20:58), Filipe David Borba Manana wrote:
>> Often when starting a transaction we commit the currently running transaction,
>> which can end up writing block group caches when the current process has its
>> journal_info set to NULL (and not to a transaction). This makes our assertion
>> at btrfs_check_data_free_space() (current_journal != NULL) fail, resulting
>> in a crash/hang. Therefore fix it by setting journal_info.
>
> Do you have any reproducer?
Just run xfstests in a loop. This depends on specific timings, and at
least in my test machine it happens rarely. In 4 continuous days of
running all xfstests in a loop, I got this 3 times in different tests:
generic/083, generic/251 and generic/320.
Thanks.
>
> Thanks,
> Satoru
>
>>
>> Two different traces of this issue follow below.
>>
>> 1)
>>
>> [51502.241936] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
>> [51502.242213] ------------[ cut here ]------------
>> [51502.242493] kernel BUG at fs/btrfs/ctree.h:3964!
>> [51502.242669] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
>> (...)
>> [51502.244010] Call Trace:
>> [51502.244010] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
>> [51502.244010] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
>> [51502.244010] [<ffffffffa0357a6a>] commit_cowonly_roots+0x164/0x226 [btrfs]
>> [51502.244010] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
>> [51502.244010] [<ffffffff8168ec7b>] ? _raw_spin_unlock+0x2b/0x40
>> [51502.244010] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
>> [51502.244010] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
>> [51502.244010] [<ffffffffa02d73e1>] __unlink_start_trans+0x31/0xe0 [btrfs]
>> [51502.244010] [<ffffffffa02dea67>] btrfs_unlink+0x37/0xc0 [btrfs]
>> [51502.244010] [<ffffffff811bb054>] ? do_unlinkat+0x114/0x2a0
>> [51502.244010] [<ffffffff811baebc>] vfs_unlink+0xcc/0x150
>> [51502.244010] [<ffffffff811bb1a0>] do_unlinkat+0x260/0x2a0
>> [51502.244010] [<ffffffff811a9ef4>] ? filp_close+0x64/0x90
>> [51502.244010] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
>> [51502.244010] [<ffffffff81349cab>] ? trace_hardirqs_on_thunk+0x3a/0x3f
>> [51502.244010] [<ffffffff811be9eb>] SyS_unlinkat+0x1b/0x40
>> [51502.244010] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
>> [51502.244010] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 71 13 36 a0 48 89 fe 31 c0 48 c7 c7 b8 43 36 a0 48 89 e5 e8 5d b0 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
>> [51502.244010] RIP [<ffffffffa03575da>] assfail.constprop.88+0x1e/0x20 [btrfs]
>>
>> 2)
>>
>> [25405.097230] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
>> [25405.097488] ------------[ cut here ]------------
>> [25405.097767] kernel BUG at fs/btrfs/ctree.h:3964!
>> [25405.097940] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
>> (...)
>> [25405.100008] Call Trace:
>> [25405.100008] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
>> [25405.100008] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
>> [25405.100008] [<ffffffffa035755a>] commit_cowonly_roots+0x164/0x226 [btrfs]
>> [25405.100008] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
>> [25405.100008] [<ffffffff8109c170>] ? bit_waitqueue+0xc0/0xc0
>> [25405.100008] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
>> [25405.100008] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
>> [25405.100008] [<ffffffffa02e3407>] btrfs_create+0x47/0x210 [btrfs]
>> [25405.100008] [<ffffffffa02d74cc>] ? btrfs_permission+0x3c/0x80 [btrfs]
>> [25405.100008] [<ffffffff811bc63b>] vfs_create+0x9b/0x130
>> [25405.100008] [<ffffffff811bcf19>] do_last+0x849/0xe20
>> [25405.100008] [<ffffffff811b9409>] ? link_path_walk+0x79/0x820
>> [25405.100008] [<ffffffff811bd5b5>] path_openat+0xc5/0x690
>> [25405.100008] [<ffffffff810ab07d>] ? trace_hardirqs_on+0xd/0x10
>> [25405.100008] [<ffffffff811cdcd2>] ? __alloc_fd+0x32/0x1d0
>> [25405.100008] [<ffffffff811be2a3>] do_filp_open+0x43/0xa0
>> [25405.100008] [<ffffffff811cddf1>] ? __alloc_fd+0x151/0x1d0
>> [25405.100008] [<ffffffff811abcfc>] do_sys_open+0x13c/0x230
>> [25405.100008] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
>> [25405.100008] [<ffffffff811abe12>] SyS_open+0x22/0x30
>> [25405.100008] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
>> [25405.100008] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 51 13 36 a0 48 89 fe 31 c0 48 c7 c7 d0 43 36 a0 48 89 e5 e8 6d b5 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
>> [25405.100008] RIP [<ffffffffa03570ca>] assfail.constprop.88+0x1e/0x20 [btrfs]
>>
>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
>> ---
>> fs/btrfs/transaction.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>> index ac984a3..fe4abe9 100644
>> --- a/fs/btrfs/transaction.c
>> +++ b/fs/btrfs/transaction.c
>> @@ -491,7 +491,11 @@ again:
>> smp_mb();
>> if (cur_trans->state >= TRANS_STATE_BLOCKED &&
>> may_wait_transaction(root, type)) {
>> + void *journal_info = current->journal_info;
>> + if (!journal_info)
>> + current->journal_info = h;
>> btrfs_commit_transaction(h, root);
>> + current->journal_info = journal_info;
>> goto again;
>> }
>>
>>
>
--
Filipe David Manana,
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Btrfs: fix crash when starting transaction
2014-06-24 8:29 ` Filipe David Manana
@ 2014-06-24 9:35 ` Satoru Takeuchi
0 siblings, 0 replies; 12+ messages in thread
From: Satoru Takeuchi @ 2014-06-24 9:35 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs@vger.kernel.org
Hi Filipe,
(2014/06/24 17:29), Filipe David Manana wrote:
> On Tue, Jun 24, 2014 at 3:34 AM, Satoru Takeuchi
> <takeuchi_satoru@jp.fujitsu.com> wrote:
>> Hi Filipe,
>>
>> (2014/06/23 20:58), Filipe David Borba Manana wrote:
>>> Often when starting a transaction we commit the currently running transaction,
>>> which can end up writing block group caches when the current process has its
>>> journal_info set to NULL (and not to a transaction). This makes our assertion
>>> at btrfs_check_data_free_space() (current_journal != NULL) fail, resulting
>>> in a crash/hang. Therefore fix it by setting journal_info.
>>
>> Do you have any reproducer?
>
> Just run xfstests in a loop. This depends on specific timings, and at
> least in my test machine it happens rarely. In 4 continuous days of
> running all xfstests in a loop, I got this 3 times in different tests:
> generic/083, generic/251 and generic/320.
Oh... it seems to hard to reproduce for me. I'll take a look
at this patch anyway.
Thanks,
Satoru
>
> Thanks.
>
>>
>> Thanks,
>> Satoru
>>
>>>
>>> Two different traces of this issue follow below.
>>>
>>> 1)
>>>
>>> [51502.241936] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
>>> [51502.242213] ------------[ cut here ]------------
>>> [51502.242493] kernel BUG at fs/btrfs/ctree.h:3964!
>>> [51502.242669] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
>>> (...)
>>> [51502.244010] Call Trace:
>>> [51502.244010] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
>>> [51502.244010] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
>>> [51502.244010] [<ffffffffa0357a6a>] commit_cowonly_roots+0x164/0x226 [btrfs]
>>> [51502.244010] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
>>> [51502.244010] [<ffffffff8168ec7b>] ? _raw_spin_unlock+0x2b/0x40
>>> [51502.244010] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
>>> [51502.244010] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
>>> [51502.244010] [<ffffffffa02d73e1>] __unlink_start_trans+0x31/0xe0 [btrfs]
>>> [51502.244010] [<ffffffffa02dea67>] btrfs_unlink+0x37/0xc0 [btrfs]
>>> [51502.244010] [<ffffffff811bb054>] ? do_unlinkat+0x114/0x2a0
>>> [51502.244010] [<ffffffff811baebc>] vfs_unlink+0xcc/0x150
>>> [51502.244010] [<ffffffff811bb1a0>] do_unlinkat+0x260/0x2a0
>>> [51502.244010] [<ffffffff811a9ef4>] ? filp_close+0x64/0x90
>>> [51502.244010] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
>>> [51502.244010] [<ffffffff81349cab>] ? trace_hardirqs_on_thunk+0x3a/0x3f
>>> [51502.244010] [<ffffffff811be9eb>] SyS_unlinkat+0x1b/0x40
>>> [51502.244010] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
>>> [51502.244010] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 71 13 36 a0 48 89 fe 31 c0 48 c7 c7 b8 43 36 a0 48 89 e5 e8 5d b0 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
>>> [51502.244010] RIP [<ffffffffa03575da>] assfail.constprop.88+0x1e/0x20 [btrfs]
>>>
>>> 2)
>>>
>>> [25405.097230] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
>>> [25405.097488] ------------[ cut here ]------------
>>> [25405.097767] kernel BUG at fs/btrfs/ctree.h:3964!
>>> [25405.097940] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
>>> (...)
>>> [25405.100008] Call Trace:
>>> [25405.100008] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
>>> [25405.100008] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
>>> [25405.100008] [<ffffffffa035755a>] commit_cowonly_roots+0x164/0x226 [btrfs]
>>> [25405.100008] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
>>> [25405.100008] [<ffffffff8109c170>] ? bit_waitqueue+0xc0/0xc0
>>> [25405.100008] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
>>> [25405.100008] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
>>> [25405.100008] [<ffffffffa02e3407>] btrfs_create+0x47/0x210 [btrfs]
>>> [25405.100008] [<ffffffffa02d74cc>] ? btrfs_permission+0x3c/0x80 [btrfs]
>>> [25405.100008] [<ffffffff811bc63b>] vfs_create+0x9b/0x130
>>> [25405.100008] [<ffffffff811bcf19>] do_last+0x849/0xe20
>>> [25405.100008] [<ffffffff811b9409>] ? link_path_walk+0x79/0x820
>>> [25405.100008] [<ffffffff811bd5b5>] path_openat+0xc5/0x690
>>> [25405.100008] [<ffffffff810ab07d>] ? trace_hardirqs_on+0xd/0x10
>>> [25405.100008] [<ffffffff811cdcd2>] ? __alloc_fd+0x32/0x1d0
>>> [25405.100008] [<ffffffff811be2a3>] do_filp_open+0x43/0xa0
>>> [25405.100008] [<ffffffff811cddf1>] ? __alloc_fd+0x151/0x1d0
>>> [25405.100008] [<ffffffff811abcfc>] do_sys_open+0x13c/0x230
>>> [25405.100008] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
>>> [25405.100008] [<ffffffff811abe12>] SyS_open+0x22/0x30
>>> [25405.100008] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
>>> [25405.100008] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 51 13 36 a0 48 89 fe 31 c0 48 c7 c7 d0 43 36 a0 48 89 e5 e8 6d b5 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
>>> [25405.100008] RIP [<ffffffffa03570ca>] assfail.constprop.88+0x1e/0x20 [btrfs]
>>>
>>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
>>> ---
>>> fs/btrfs/transaction.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>>> index ac984a3..fe4abe9 100644
>>> --- a/fs/btrfs/transaction.c
>>> +++ b/fs/btrfs/transaction.c
>>> @@ -491,7 +491,11 @@ again:
>>> smp_mb();
>>> if (cur_trans->state >= TRANS_STATE_BLOCKED &&
>>> may_wait_transaction(root, type)) {
>>> + void *journal_info = current->journal_info;
>>> + if (!journal_info)
>>> + current->journal_info = h;
>>> btrfs_commit_transaction(h, root);
>>> + current->journal_info = journal_info;
>>> goto again;
>>> }
>>>
>>>
>>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Btrfs: fix crash when starting transaction
2014-06-23 11:58 [PATCH] Btrfs: fix crash when starting transaction Filipe David Borba Manana
2014-06-24 2:34 ` Satoru Takeuchi
@ 2014-06-24 10:22 ` Miao Xie
2014-06-24 10:29 ` Filipe David Manana
2014-06-24 16:46 ` [PATCH v2] " Filipe David Borba Manana
2 siblings, 1 reply; 12+ messages in thread
From: Miao Xie @ 2014-06-24 10:22 UTC (permalink / raw)
To: Filipe David Borba Manana, linux-btrfs; +Cc: Josef Bacik
CC Josef
On Mon, 23 Jun 2014 12:58:59 +0100, Filipe David Borba Manana wrote:
> Often when starting a transaction we commit the currently running transaction,
> which can end up writing block group caches when the current process has its
> journal_info set to NULL (and not to a transaction). This makes our assertion
> at btrfs_check_data_free_space() (current_journal != NULL) fail, resulting
> in a crash/hang. Therefore fix it by setting journal_info.
>
> Two different traces of this issue follow below.
>
> 1)
>
> [51502.241936] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
> [51502.242213] ------------[ cut here ]------------
> [51502.242493] kernel BUG at fs/btrfs/ctree.h:3964!
> [51502.242669] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
> (...)
> [51502.244010] Call Trace:
> [51502.244010] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
> [51502.244010] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
> [51502.244010] [<ffffffffa0357a6a>] commit_cowonly_roots+0x164/0x226 [btrfs]
> [51502.244010] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
> [51502.244010] [<ffffffff8168ec7b>] ? _raw_spin_unlock+0x2b/0x40
> [51502.244010] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
> [51502.244010] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
> [51502.244010] [<ffffffffa02d73e1>] __unlink_start_trans+0x31/0xe0 [btrfs]
> [51502.244010] [<ffffffffa02dea67>] btrfs_unlink+0x37/0xc0 [btrfs]
> [51502.244010] [<ffffffff811bb054>] ? do_unlinkat+0x114/0x2a0
> [51502.244010] [<ffffffff811baebc>] vfs_unlink+0xcc/0x150
> [51502.244010] [<ffffffff811bb1a0>] do_unlinkat+0x260/0x2a0
> [51502.244010] [<ffffffff811a9ef4>] ? filp_close+0x64/0x90
> [51502.244010] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
> [51502.244010] [<ffffffff81349cab>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [51502.244010] [<ffffffff811be9eb>] SyS_unlinkat+0x1b/0x40
> [51502.244010] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
> [51502.244010] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 71 13 36 a0 48 89 fe 31 c0 48 c7 c7 b8 43 36 a0 48 89 e5 e8 5d b0 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
> [51502.244010] RIP [<ffffffffa03575da>] assfail.constprop.88+0x1e/0x20 [btrfs]
>
> 2)
>
> [25405.097230] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
> [25405.097488] ------------[ cut here ]------------
> [25405.097767] kernel BUG at fs/btrfs/ctree.h:3964!
> [25405.097940] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
> (...)
> [25405.100008] Call Trace:
> [25405.100008] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
> [25405.100008] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
> [25405.100008] [<ffffffffa035755a>] commit_cowonly_roots+0x164/0x226 [btrfs]
> [25405.100008] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
> [25405.100008] [<ffffffff8109c170>] ? bit_waitqueue+0xc0/0xc0
> [25405.100008] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
> [25405.100008] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
> [25405.100008] [<ffffffffa02e3407>] btrfs_create+0x47/0x210 [btrfs]
> [25405.100008] [<ffffffffa02d74cc>] ? btrfs_permission+0x3c/0x80 [btrfs]
> [25405.100008] [<ffffffff811bc63b>] vfs_create+0x9b/0x130
> [25405.100008] [<ffffffff811bcf19>] do_last+0x849/0xe20
> [25405.100008] [<ffffffff811b9409>] ? link_path_walk+0x79/0x820
> [25405.100008] [<ffffffff811bd5b5>] path_openat+0xc5/0x690
> [25405.100008] [<ffffffff810ab07d>] ? trace_hardirqs_on+0xd/0x10
> [25405.100008] [<ffffffff811cdcd2>] ? __alloc_fd+0x32/0x1d0
> [25405.100008] [<ffffffff811be2a3>] do_filp_open+0x43/0xa0
> [25405.100008] [<ffffffff811cddf1>] ? __alloc_fd+0x151/0x1d0
> [25405.100008] [<ffffffff811abcfc>] do_sys_open+0x13c/0x230
> [25405.100008] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
> [25405.100008] [<ffffffff811abe12>] SyS_open+0x22/0x30
> [25405.100008] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
> [25405.100008] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 51 13 36 a0 48 89 fe 31 c0 48 c7 c7 d0 43 36 a0 48 89 e5 e8 6d b5 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
> [25405.100008] RIP [<ffffffffa03570ca>] assfail.constprop.88+0x1e/0x20 [btrfs]
>
> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> ---
> fs/btrfs/transaction.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index ac984a3..fe4abe9 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -491,7 +491,11 @@ again:
> smp_mb();
> if (cur_trans->state >= TRANS_STATE_BLOCKED &&
> may_wait_transaction(root, type)) {
> + void *journal_info = current->journal_info;
> + if (!journal_info)
> + current->journal_info = h;
> btrfs_commit_transaction(h, root);
> + current->journal_info = journal_info;
> goto again;
It seems it is impossible that the task who set BTRFS_SEND_TRANS_STUB would start a transaction,
that is if current->journal_info is not NULL, we are sure there is a transaction handle in it,
so we just join that handle and then go out. In other words, current->journal_info here must be NULL,
so the if sentence is unnecessary,
smp_mb();
if (cur_trans->state >= TRANS_STATE_BLOCKED &&
may_wait_transaction(root, type)) {
+ current->journal_info = h;
btrfs_commit_transaction(h, root);
goto again;
is OK.
Maybe there is something wrong in what I said, if so, please correct me.
Thanks
Miao
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Btrfs: fix crash when starting transaction
2014-06-24 10:22 ` Miao Xie
@ 2014-06-24 10:29 ` Filipe David Manana
2014-06-24 15:25 ` Josef Bacik
0 siblings, 1 reply; 12+ messages in thread
From: Filipe David Manana @ 2014-06-24 10:29 UTC (permalink / raw)
To: Miao Xie; +Cc: linux-btrfs@vger.kernel.org, Josef Bacik
On Tue, Jun 24, 2014 at 11:22 AM, Miao Xie <miaox@cn.fujitsu.com> wrote:
> CC Josef
>
> On Mon, 23 Jun 2014 12:58:59 +0100, Filipe David Borba Manana wrote:
>> Often when starting a transaction we commit the currently running transaction,
>> which can end up writing block group caches when the current process has its
>> journal_info set to NULL (and not to a transaction). This makes our assertion
>> at btrfs_check_data_free_space() (current_journal != NULL) fail, resulting
>> in a crash/hang. Therefore fix it by setting journal_info.
>>
>> Two different traces of this issue follow below.
>>
>> 1)
>>
>> [51502.241936] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
>> [51502.242213] ------------[ cut here ]------------
>> [51502.242493] kernel BUG at fs/btrfs/ctree.h:3964!
>> [51502.242669] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
>> (...)
>> [51502.244010] Call Trace:
>> [51502.244010] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
>> [51502.244010] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
>> [51502.244010] [<ffffffffa0357a6a>] commit_cowonly_roots+0x164/0x226 [btrfs]
>> [51502.244010] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
>> [51502.244010] [<ffffffff8168ec7b>] ? _raw_spin_unlock+0x2b/0x40
>> [51502.244010] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
>> [51502.244010] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
>> [51502.244010] [<ffffffffa02d73e1>] __unlink_start_trans+0x31/0xe0 [btrfs]
>> [51502.244010] [<ffffffffa02dea67>] btrfs_unlink+0x37/0xc0 [btrfs]
>> [51502.244010] [<ffffffff811bb054>] ? do_unlinkat+0x114/0x2a0
>> [51502.244010] [<ffffffff811baebc>] vfs_unlink+0xcc/0x150
>> [51502.244010] [<ffffffff811bb1a0>] do_unlinkat+0x260/0x2a0
>> [51502.244010] [<ffffffff811a9ef4>] ? filp_close+0x64/0x90
>> [51502.244010] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
>> [51502.244010] [<ffffffff81349cab>] ? trace_hardirqs_on_thunk+0x3a/0x3f
>> [51502.244010] [<ffffffff811be9eb>] SyS_unlinkat+0x1b/0x40
>> [51502.244010] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
>> [51502.244010] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 71 13 36 a0 48 89 fe 31 c0 48 c7 c7 b8 43 36 a0 48 89 e5 e8 5d b0 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
>> [51502.244010] RIP [<ffffffffa03575da>] assfail.constprop.88+0x1e/0x20 [btrfs]
>>
>> 2)
>>
>> [25405.097230] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
>> [25405.097488] ------------[ cut here ]------------
>> [25405.097767] kernel BUG at fs/btrfs/ctree.h:3964!
>> [25405.097940] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
>> (...)
>> [25405.100008] Call Trace:
>> [25405.100008] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
>> [25405.100008] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
>> [25405.100008] [<ffffffffa035755a>] commit_cowonly_roots+0x164/0x226 [btrfs]
>> [25405.100008] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
>> [25405.100008] [<ffffffff8109c170>] ? bit_waitqueue+0xc0/0xc0
>> [25405.100008] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
>> [25405.100008] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
>> [25405.100008] [<ffffffffa02e3407>] btrfs_create+0x47/0x210 [btrfs]
>> [25405.100008] [<ffffffffa02d74cc>] ? btrfs_permission+0x3c/0x80 [btrfs]
>> [25405.100008] [<ffffffff811bc63b>] vfs_create+0x9b/0x130
>> [25405.100008] [<ffffffff811bcf19>] do_last+0x849/0xe20
>> [25405.100008] [<ffffffff811b9409>] ? link_path_walk+0x79/0x820
>> [25405.100008] [<ffffffff811bd5b5>] path_openat+0xc5/0x690
>> [25405.100008] [<ffffffff810ab07d>] ? trace_hardirqs_on+0xd/0x10
>> [25405.100008] [<ffffffff811cdcd2>] ? __alloc_fd+0x32/0x1d0
>> [25405.100008] [<ffffffff811be2a3>] do_filp_open+0x43/0xa0
>> [25405.100008] [<ffffffff811cddf1>] ? __alloc_fd+0x151/0x1d0
>> [25405.100008] [<ffffffff811abcfc>] do_sys_open+0x13c/0x230
>> [25405.100008] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
>> [25405.100008] [<ffffffff811abe12>] SyS_open+0x22/0x30
>> [25405.100008] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
>> [25405.100008] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 51 13 36 a0 48 89 fe 31 c0 48 c7 c7 d0 43 36 a0 48 89 e5 e8 6d b5 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
>> [25405.100008] RIP [<ffffffffa03570ca>] assfail.constprop.88+0x1e/0x20 [btrfs]
>>
>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
>> ---
>> fs/btrfs/transaction.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>> index ac984a3..fe4abe9 100644
>> --- a/fs/btrfs/transaction.c
>> +++ b/fs/btrfs/transaction.c
>> @@ -491,7 +491,11 @@ again:
>> smp_mb();
>> if (cur_trans->state >= TRANS_STATE_BLOCKED &&
>> may_wait_transaction(root, type)) {
>> + void *journal_info = current->journal_info;
>> + if (!journal_info)
>> + current->journal_info = h;
>> btrfs_commit_transaction(h, root);
>> + current->journal_info = journal_info;
>> goto again;
>
> It seems it is impossible that the task who set BTRFS_SEND_TRANS_STUB would start a transaction,
> that is if current->journal_info is not NULL, we are sure there is a transaction handle in it,
> so we just join that handle and then go out. In other words, current->journal_info here must be NULL,
> so the if sentence is unnecessary,
I think that is true too (current->journal_info == NULL here).
The if test is done mostly because it's done some lines below too.
Thanks
> smp_mb();
> if (cur_trans->state >= TRANS_STATE_BLOCKED &&
> may_wait_transaction(root, type)) {
> + current->journal_info = h;
> btrfs_commit_transaction(h, root);
> goto again;
>
> is OK.
>
> Maybe there is something wrong in what I said, if so, please correct me.
>
> Thanks
> Miao
--
Filipe David Manana,
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Btrfs: fix crash when starting transaction
2014-06-24 10:29 ` Filipe David Manana
@ 2014-06-24 15:25 ` Josef Bacik
0 siblings, 0 replies; 12+ messages in thread
From: Josef Bacik @ 2014-06-24 15:25 UTC (permalink / raw)
To: fdmanana, Miao Xie; +Cc: linux-btrfs@vger.kernel.org
On 06/24/2014 03:29 AM, Filipe David Manana wrote:
> On Tue, Jun 24, 2014 at 11:22 AM, Miao Xie <miaox@cn.fujitsu.com> wrote:
>> CC Josef
>>
>> On Mon, 23 Jun 2014 12:58:59 +0100, Filipe David Borba Manana wrote:
>>> Often when starting a transaction we commit the currently running transaction,
>>> which can end up writing block group caches when the current process has its
>>> journal_info set to NULL (and not to a transaction). This makes our assertion
>>> at btrfs_check_data_free_space() (current_journal != NULL) fail, resulting
>>> in a crash/hang. Therefore fix it by setting journal_info.
>>>
>>> Two different traces of this issue follow below.
>>>
>>> 1)
>>>
>>> [51502.241936] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
>>> [51502.242213] ------------[ cut here ]------------
>>> [51502.242493] kernel BUG at fs/btrfs/ctree.h:3964!
>>> [51502.242669] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
>>> (...)
>>> [51502.244010] Call Trace:
>>> [51502.244010] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
>>> [51502.244010] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
>>> [51502.244010] [<ffffffffa0357a6a>] commit_cowonly_roots+0x164/0x226 [btrfs]
>>> [51502.244010] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
>>> [51502.244010] [<ffffffff8168ec7b>] ? _raw_spin_unlock+0x2b/0x40
>>> [51502.244010] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
>>> [51502.244010] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
>>> [51502.244010] [<ffffffffa02d73e1>] __unlink_start_trans+0x31/0xe0 [btrfs]
>>> [51502.244010] [<ffffffffa02dea67>] btrfs_unlink+0x37/0xc0 [btrfs]
>>> [51502.244010] [<ffffffff811bb054>] ? do_unlinkat+0x114/0x2a0
>>> [51502.244010] [<ffffffff811baebc>] vfs_unlink+0xcc/0x150
>>> [51502.244010] [<ffffffff811bb1a0>] do_unlinkat+0x260/0x2a0
>>> [51502.244010] [<ffffffff811a9ef4>] ? filp_close+0x64/0x90
>>> [51502.244010] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
>>> [51502.244010] [<ffffffff81349cab>] ? trace_hardirqs_on_thunk+0x3a/0x3f
>>> [51502.244010] [<ffffffff811be9eb>] SyS_unlinkat+0x1b/0x40
>>> [51502.244010] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
>>> [51502.244010] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 71 13 36 a0 48 89 fe 31 c0 48 c7 c7 b8 43 36 a0 48 89 e5 e8 5d b0 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
>>> [51502.244010] RIP [<ffffffffa03575da>] assfail.constprop.88+0x1e/0x20 [btrfs]
>>>
>>> 2)
>>>
>>> [25405.097230] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
>>> [25405.097488] ------------[ cut here ]------------
>>> [25405.097767] kernel BUG at fs/btrfs/ctree.h:3964!
>>> [25405.097940] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
>>> (...)
>>> [25405.100008] Call Trace:
>>> [25405.100008] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
>>> [25405.100008] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
>>> [25405.100008] [<ffffffffa035755a>] commit_cowonly_roots+0x164/0x226 [btrfs]
>>> [25405.100008] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
>>> [25405.100008] [<ffffffff8109c170>] ? bit_waitqueue+0xc0/0xc0
>>> [25405.100008] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
>>> [25405.100008] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
>>> [25405.100008] [<ffffffffa02e3407>] btrfs_create+0x47/0x210 [btrfs]
>>> [25405.100008] [<ffffffffa02d74cc>] ? btrfs_permission+0x3c/0x80 [btrfs]
>>> [25405.100008] [<ffffffff811bc63b>] vfs_create+0x9b/0x130
>>> [25405.100008] [<ffffffff811bcf19>] do_last+0x849/0xe20
>>> [25405.100008] [<ffffffff811b9409>] ? link_path_walk+0x79/0x820
>>> [25405.100008] [<ffffffff811bd5b5>] path_openat+0xc5/0x690
>>> [25405.100008] [<ffffffff810ab07d>] ? trace_hardirqs_on+0xd/0x10
>>> [25405.100008] [<ffffffff811cdcd2>] ? __alloc_fd+0x32/0x1d0
>>> [25405.100008] [<ffffffff811be2a3>] do_filp_open+0x43/0xa0
>>> [25405.100008] [<ffffffff811cddf1>] ? __alloc_fd+0x151/0x1d0
>>> [25405.100008] [<ffffffff811abcfc>] do_sys_open+0x13c/0x230
>>> [25405.100008] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
>>> [25405.100008] [<ffffffff811abe12>] SyS_open+0x22/0x30
>>> [25405.100008] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
>>> [25405.100008] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 51 13 36 a0 48 89 fe 31 c0 48 c7 c7 d0 43 36 a0 48 89 e5 e8 6d b5 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
>>> [25405.100008] RIP [<ffffffffa03570ca>] assfail.constprop.88+0x1e/0x20 [btrfs]
>>>
>>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
>>> ---
>>> fs/btrfs/transaction.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>>> index ac984a3..fe4abe9 100644
>>> --- a/fs/btrfs/transaction.c
>>> +++ b/fs/btrfs/transaction.c
>>> @@ -491,7 +491,11 @@ again:
>>> smp_mb();
>>> if (cur_trans->state >= TRANS_STATE_BLOCKED &&
>>> may_wait_transaction(root, type)) {
>>> + void *journal_info = current->journal_info;
>>> + if (!journal_info)
>>> + current->journal_info = h;
>>> btrfs_commit_transaction(h, root);
>>> + current->journal_info = journal_info;
>>> goto again;
>>
>> It seems it is impossible that the task who set BTRFS_SEND_TRANS_STUB would start a transaction,
>> that is if current->journal_info is not NULL, we are sure there is a transaction handle in it,
>> so we just join that handle and then go out. In other words, current->journal_info here must be NULL,
>> so the if sentence is unnecessary,
>
> I think that is true too (current->journal_info == NULL here).
> The if test is done mostly because it's done some lines below too.
>
Yeah journal_info should always == NULL here. In fact I'd actually like to put
an ASSERT() to make sure we don't have TRANS_SUB set coming into
start_transaction, but you can leave that for another day if you like. Good
find btw,
Josef
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] Btrfs: fix crash when starting transaction
2014-06-23 11:58 [PATCH] Btrfs: fix crash when starting transaction Filipe David Borba Manana
2014-06-24 2:34 ` Satoru Takeuchi
2014-06-24 10:22 ` Miao Xie
@ 2014-06-24 16:46 ` Filipe David Borba Manana
2014-07-03 8:30 ` Miao Xie
2 siblings, 1 reply; 12+ messages in thread
From: Filipe David Borba Manana @ 2014-06-24 16:46 UTC (permalink / raw)
To: linux-btrfs; +Cc: Filipe David Borba Manana
Often when starting a transaction we commit the currently running transaction,
which can end up writing block group caches when the current process has its
journal_info set to NULL (and not to a transaction). This makes our assertion
at btrfs_check_data_free_space() (current_journal != NULL) fail, resulting
in a crash/hang. Therefore fix it by setting journal_info.
Two different traces of this issue follow below.
1)
[51502.241936] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
[51502.242213] ------------[ cut here ]------------
[51502.242493] kernel BUG at fs/btrfs/ctree.h:3964!
[51502.242669] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
(...)
[51502.244010] Call Trace:
[51502.244010] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
[51502.244010] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
[51502.244010] [<ffffffffa0357a6a>] commit_cowonly_roots+0x164/0x226 [btrfs]
[51502.244010] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
[51502.244010] [<ffffffff8168ec7b>] ? _raw_spin_unlock+0x2b/0x40
[51502.244010] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
[51502.244010] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
[51502.244010] [<ffffffffa02d73e1>] __unlink_start_trans+0x31/0xe0 [btrfs]
[51502.244010] [<ffffffffa02dea67>] btrfs_unlink+0x37/0xc0 [btrfs]
[51502.244010] [<ffffffff811bb054>] ? do_unlinkat+0x114/0x2a0
[51502.244010] [<ffffffff811baebc>] vfs_unlink+0xcc/0x150
[51502.244010] [<ffffffff811bb1a0>] do_unlinkat+0x260/0x2a0
[51502.244010] [<ffffffff811a9ef4>] ? filp_close+0x64/0x90
[51502.244010] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
[51502.244010] [<ffffffff81349cab>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[51502.244010] [<ffffffff811be9eb>] SyS_unlinkat+0x1b/0x40
[51502.244010] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
[51502.244010] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 71 13 36 a0 48 89 fe 31 c0 48 c7 c7 b8 43 36 a0 48 89 e5 e8 5d b0 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
[51502.244010] RIP [<ffffffffa03575da>] assfail.constprop.88+0x1e/0x20 [btrfs]
2)
[25405.097230] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
[25405.097488] ------------[ cut here ]------------
[25405.097767] kernel BUG at fs/btrfs/ctree.h:3964!
[25405.097940] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
(...)
[25405.100008] Call Trace:
[25405.100008] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
[25405.100008] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
[25405.100008] [<ffffffffa035755a>] commit_cowonly_roots+0x164/0x226 [btrfs]
[25405.100008] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
[25405.100008] [<ffffffff8109c170>] ? bit_waitqueue+0xc0/0xc0
[25405.100008] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
[25405.100008] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
[25405.100008] [<ffffffffa02e3407>] btrfs_create+0x47/0x210 [btrfs]
[25405.100008] [<ffffffffa02d74cc>] ? btrfs_permission+0x3c/0x80 [btrfs]
[25405.100008] [<ffffffff811bc63b>] vfs_create+0x9b/0x130
[25405.100008] [<ffffffff811bcf19>] do_last+0x849/0xe20
[25405.100008] [<ffffffff811b9409>] ? link_path_walk+0x79/0x820
[25405.100008] [<ffffffff811bd5b5>] path_openat+0xc5/0x690
[25405.100008] [<ffffffff810ab07d>] ? trace_hardirqs_on+0xd/0x10
[25405.100008] [<ffffffff811cdcd2>] ? __alloc_fd+0x32/0x1d0
[25405.100008] [<ffffffff811be2a3>] do_filp_open+0x43/0xa0
[25405.100008] [<ffffffff811cddf1>] ? __alloc_fd+0x151/0x1d0
[25405.100008] [<ffffffff811abcfc>] do_sys_open+0x13c/0x230
[25405.100008] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
[25405.100008] [<ffffffff811abe12>] SyS_open+0x22/0x30
[25405.100008] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
[25405.100008] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 51 13 36 a0 48 89 fe 31 c0 48 c7 c7 d0 43 36 a0 48 89 e5 e8 6d b5 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
[25405.100008] RIP [<ffffffffa03570ca>] assfail.constprop.88+0x1e/0x20 [btrfs]
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
V2: Removed test for current->journal_info == NULL. At this point it's
always expected to be NULL.
fs/btrfs/transaction.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index ac984a3..614eac3 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -491,6 +491,7 @@ again:
smp_mb();
if (cur_trans->state >= TRANS_STATE_BLOCKED &&
may_wait_transaction(root, type)) {
+ current->journal_info = h;
btrfs_commit_transaction(h, root);
goto again;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] Btrfs: fix crash when starting transaction
2014-06-24 16:46 ` [PATCH v2] " Filipe David Borba Manana
@ 2014-07-03 8:30 ` Miao Xie
2014-07-03 10:32 ` Satoru Takeuchi
0 siblings, 1 reply; 12+ messages in thread
From: Miao Xie @ 2014-07-03 8:30 UTC (permalink / raw)
To: Filipe David Borba Manana, linux-btrfs
On Tue, 24 Jun 2014 17:46:58 +0100, Filipe David Borba Manana wrote:
> Often when starting a transaction we commit the currently running transaction,
> which can end up writing block group caches when the current process has its
> journal_info set to NULL (and not to a transaction). This makes our assertion
> at btrfs_check_data_free_space() (current_journal != NULL) fail, resulting
> in a crash/hang. Therefore fix it by setting journal_info.
>
> Two different traces of this issue follow below.
>
> 1)
>
> [51502.241936] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
> [51502.242213] ------------[ cut here ]------------
> [51502.242493] kernel BUG at fs/btrfs/ctree.h:3964!
> [51502.242669] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
> (...)
> [51502.244010] Call Trace:
> [51502.244010] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
> [51502.244010] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
> [51502.244010] [<ffffffffa0357a6a>] commit_cowonly_roots+0x164/0x226 [btrfs]
> [51502.244010] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
> [51502.244010] [<ffffffff8168ec7b>] ? _raw_spin_unlock+0x2b/0x40
> [51502.244010] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
> [51502.244010] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
> [51502.244010] [<ffffffffa02d73e1>] __unlink_start_trans+0x31/0xe0 [btrfs]
> [51502.244010] [<ffffffffa02dea67>] btrfs_unlink+0x37/0xc0 [btrfs]
> [51502.244010] [<ffffffff811bb054>] ? do_unlinkat+0x114/0x2a0
> [51502.244010] [<ffffffff811baebc>] vfs_unlink+0xcc/0x150
> [51502.244010] [<ffffffff811bb1a0>] do_unlinkat+0x260/0x2a0
> [51502.244010] [<ffffffff811a9ef4>] ? filp_close+0x64/0x90
> [51502.244010] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
> [51502.244010] [<ffffffff81349cab>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [51502.244010] [<ffffffff811be9eb>] SyS_unlinkat+0x1b/0x40
> [51502.244010] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
> [51502.244010] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 71 13 36 a0 48 89 fe 31 c0 48 c7 c7 b8 43 36 a0 48 89 e5 e8 5d b0 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
> [51502.244010] RIP [<ffffffffa03575da>] assfail.constprop.88+0x1e/0x20 [btrfs]
>
> 2)
>
> [25405.097230] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
> [25405.097488] ------------[ cut here ]------------
> [25405.097767] kernel BUG at fs/btrfs/ctree.h:3964!
> [25405.097940] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
> (...)
> [25405.100008] Call Trace:
> [25405.100008] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
> [25405.100008] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
> [25405.100008] [<ffffffffa035755a>] commit_cowonly_roots+0x164/0x226 [btrfs]
> [25405.100008] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
> [25405.100008] [<ffffffff8109c170>] ? bit_waitqueue+0xc0/0xc0
> [25405.100008] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
> [25405.100008] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
> [25405.100008] [<ffffffffa02e3407>] btrfs_create+0x47/0x210 [btrfs]
> [25405.100008] [<ffffffffa02d74cc>] ? btrfs_permission+0x3c/0x80 [btrfs]
> [25405.100008] [<ffffffff811bc63b>] vfs_create+0x9b/0x130
> [25405.100008] [<ffffffff811bcf19>] do_last+0x849/0xe20
> [25405.100008] [<ffffffff811b9409>] ? link_path_walk+0x79/0x820
> [25405.100008] [<ffffffff811bd5b5>] path_openat+0xc5/0x690
> [25405.100008] [<ffffffff810ab07d>] ? trace_hardirqs_on+0xd/0x10
> [25405.100008] [<ffffffff811cdcd2>] ? __alloc_fd+0x32/0x1d0
> [25405.100008] [<ffffffff811be2a3>] do_filp_open+0x43/0xa0
> [25405.100008] [<ffffffff811cddf1>] ? __alloc_fd+0x151/0x1d0
> [25405.100008] [<ffffffff811abcfc>] do_sys_open+0x13c/0x230
> [25405.100008] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
> [25405.100008] [<ffffffff811abe12>] SyS_open+0x22/0x30
> [25405.100008] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
> [25405.100008] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 51 13 36 a0 48 89 fe 31 c0 48 c7 c7 d0 43 36 a0 48 89 e5 e8 6d b5 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
> [25405.100008] RIP [<ffffffffa03570ca>] assfail.constprop.88+0x1e/0x20 [btrfs]
>
> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> ---
>
> V2: Removed test for current->journal_info == NULL. At this point it's
> always expected to be NULL.
Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
>
> fs/btrfs/transaction.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index ac984a3..614eac3 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -491,6 +491,7 @@ again:
> smp_mb();
> if (cur_trans->state >= TRANS_STATE_BLOCKED &&
> may_wait_transaction(root, type)) {
> + current->journal_info = h;
> btrfs_commit_transaction(h, root);
> goto again;
> }
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] Btrfs: fix crash when starting transaction
2014-07-03 8:30 ` Miao Xie
@ 2014-07-03 10:32 ` Satoru Takeuchi
2014-07-03 11:07 ` Miao Xie
0 siblings, 1 reply; 12+ messages in thread
From: Satoru Takeuchi @ 2014-07-03 10:32 UTC (permalink / raw)
To: miaox, Filipe David Borba Manana, linux-btrfs
(2014/07/03 17:30), Miao Xie wrote:
> On Tue, 24 Jun 2014 17:46:58 +0100, Filipe David Borba Manana wrote:
>> Often when starting a transaction we commit the currently running transaction,
>> which can end up writing block group caches when the current process has its
>> journal_info set to NULL (and not to a transaction). This makes our assertion
>> at btrfs_check_data_free_space() (current_journal != NULL) fail, resulting
>> in a crash/hang. Therefore fix it by setting journal_info.
>>
>> Two different traces of this issue follow below.
>>
>> 1)
>>
>> [51502.241936] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
>> [51502.242213] ------------[ cut here ]------------
>> [51502.242493] kernel BUG at fs/btrfs/ctree.h:3964!
>> [51502.242669] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
>> (...)
>> [51502.244010] Call Trace:
>> [51502.244010] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
>> [51502.244010] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
>> [51502.244010] [<ffffffffa0357a6a>] commit_cowonly_roots+0x164/0x226 [btrfs]
>> [51502.244010] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
>> [51502.244010] [<ffffffff8168ec7b>] ? _raw_spin_unlock+0x2b/0x40
>> [51502.244010] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
>> [51502.244010] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
>> [51502.244010] [<ffffffffa02d73e1>] __unlink_start_trans+0x31/0xe0 [btrfs]
>> [51502.244010] [<ffffffffa02dea67>] btrfs_unlink+0x37/0xc0 [btrfs]
>> [51502.244010] [<ffffffff811bb054>] ? do_unlinkat+0x114/0x2a0
>> [51502.244010] [<ffffffff811baebc>] vfs_unlink+0xcc/0x150
>> [51502.244010] [<ffffffff811bb1a0>] do_unlinkat+0x260/0x2a0
>> [51502.244010] [<ffffffff811a9ef4>] ? filp_close+0x64/0x90
>> [51502.244010] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
>> [51502.244010] [<ffffffff81349cab>] ? trace_hardirqs_on_thunk+0x3a/0x3f
>> [51502.244010] [<ffffffff811be9eb>] SyS_unlinkat+0x1b/0x40
>> [51502.244010] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
>> [51502.244010] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 71 13 36 a0 48 89 fe 31 c0 48 c7 c7 b8 43 36 a0 48 89 e5 e8 5d b0 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
>> [51502.244010] RIP [<ffffffffa03575da>] assfail.constprop.88+0x1e/0x20 [btrfs]
>>
>> 2)
>>
>> [25405.097230] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
>> [25405.097488] ------------[ cut here ]------------
>> [25405.097767] kernel BUG at fs/btrfs/ctree.h:3964!
>> [25405.097940] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
>> (...)
>> [25405.100008] Call Trace:
>> [25405.100008] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
>> [25405.100008] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
>> [25405.100008] [<ffffffffa035755a>] commit_cowonly_roots+0x164/0x226 [btrfs]
>> [25405.100008] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
>> [25405.100008] [<ffffffff8109c170>] ? bit_waitqueue+0xc0/0xc0
>> [25405.100008] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
>> [25405.100008] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
>> [25405.100008] [<ffffffffa02e3407>] btrfs_create+0x47/0x210 [btrfs]
>> [25405.100008] [<ffffffffa02d74cc>] ? btrfs_permission+0x3c/0x80 [btrfs]
>> [25405.100008] [<ffffffff811bc63b>] vfs_create+0x9b/0x130
>> [25405.100008] [<ffffffff811bcf19>] do_last+0x849/0xe20
>> [25405.100008] [<ffffffff811b9409>] ? link_path_walk+0x79/0x820
>> [25405.100008] [<ffffffff811bd5b5>] path_openat+0xc5/0x690
>> [25405.100008] [<ffffffff810ab07d>] ? trace_hardirqs_on+0xd/0x10
>> [25405.100008] [<ffffffff811cdcd2>] ? __alloc_fd+0x32/0x1d0
>> [25405.100008] [<ffffffff811be2a3>] do_filp_open+0x43/0xa0
>> [25405.100008] [<ffffffff811cddf1>] ? __alloc_fd+0x151/0x1d0
>> [25405.100008] [<ffffffff811abcfc>] do_sys_open+0x13c/0x230
>> [25405.100008] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
>> [25405.100008] [<ffffffff811abe12>] SyS_open+0x22/0x30
>> [25405.100008] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
>> [25405.100008] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 51 13 36 a0 48 89 fe 31 c0 48 c7 c7 d0 43 36 a0 48 89 e5 e8 6d b5 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
>> [25405.100008] RIP [<ffffffffa03570ca>] assfail.constprop.88+0x1e/0x20 [btrfs]
>>
>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
>> ---
>>
>> V2: Removed test for current->journal_info == NULL. At this point it's
>> always expected to be NULL.
>
> Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
Let me clarify my understanding since I'm not good at the transaction code.
* What is the route cause?
When start_transaction() is called with current->journal_transaction == NULL,
we hit BUG() in the following path.
start_transaction
-> btrfs_commit_transaction
-> commit_cowonly_roots
-> update_cowonly_root
-> btrfs_write_dirty_block_groups
-> cache_save_setup
-> btrfs_check_data_free_space
ASSERT(current->journal_info) # it fails and hits BUG()!
* Why NULL check is not necessary?
fs/btrfs/transaction.c:
===============================================================================
start_transaction(...) {
if (current->journal_info &&
current->journal_info != (void *)BTRFS_SEND_TRANS_STUB) {
...
goto got_it;
===============================================================================
To begin with, if current->journal_info == BTRFS_SEND_TRANS_STUB,
start_transaction() should not be called. Filipe's another patch
([PATCH] Btrfs: assert send doesn't attempt to start) is to change
the second condition to ASSERT().
If current->journal_transaction is a "real" transaction, the
above-mentioned code path is skipped because of "goto got_it".
Is my understanding correct?
BTW, Filipe, although I've tried to reproduce this problem for two days
by running xfstests loop, it didn't happen yet with my environment ;-(
So all I can do seems to be code review.
Thanks,
Satoru
>>
>> fs/btrfs/transaction.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>> index ac984a3..614eac3 100644
>> --- a/fs/btrfs/transaction.c
>> +++ b/fs/btrfs/transaction.c
>> @@ -491,6 +491,7 @@ again:
>> smp_mb();
>> if (cur_trans->state >= TRANS_STATE_BLOCKED &&
>> may_wait_transaction(root, type)) {
>> + current->journal_info = h;
>> btrfs_commit_transaction(h, root);
>> goto again;
>> }
>>
>
> --
> 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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] Btrfs: fix crash when starting transaction
2014-07-03 10:32 ` Satoru Takeuchi
@ 2014-07-03 11:07 ` Miao Xie
2014-07-03 23:13 ` Satoru Takeuchi
0 siblings, 1 reply; 12+ messages in thread
From: Miao Xie @ 2014-07-03 11:07 UTC (permalink / raw)
To: Satoru Takeuchi, Filipe David Borba Manana, linux-btrfs
On Thu, 3 Jul 2014 19:32:18 +0900, Satoru Takeuchi wrote:
> (2014/07/03 17:30), Miao Xie wrote:
>> On Tue, 24 Jun 2014 17:46:58 +0100, Filipe David Borba Manana wrote:
>>> Often when starting a transaction we commit the currently running transaction,
>>> which can end up writing block group caches when the current process has its
>>> journal_info set to NULL (and not to a transaction). This makes our assertion
>>> at btrfs_check_data_free_space() (current_journal != NULL) fail, resulting
>>> in a crash/hang. Therefore fix it by setting journal_info.
>>>
>>> Two different traces of this issue follow below.
>>>
>>> 1)
>>>
>>> [51502.241936] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
>>> [51502.242213] ------------[ cut here ]------------
>>> [51502.242493] kernel BUG at fs/btrfs/ctree.h:3964!
>>> [51502.242669] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
>>> (...)
>>> [51502.244010] Call Trace:
>>> [51502.244010] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
>>> [51502.244010] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
>>> [51502.244010] [<ffffffffa0357a6a>] commit_cowonly_roots+0x164/0x226 [btrfs]
>>> [51502.244010] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
>>> [51502.244010] [<ffffffff8168ec7b>] ? _raw_spin_unlock+0x2b/0x40
>>> [51502.244010] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
>>> [51502.244010] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
>>> [51502.244010] [<ffffffffa02d73e1>] __unlink_start_trans+0x31/0xe0 [btrfs]
>>> [51502.244010] [<ffffffffa02dea67>] btrfs_unlink+0x37/0xc0 [btrfs]
>>> [51502.244010] [<ffffffff811bb054>] ? do_unlinkat+0x114/0x2a0
>>> [51502.244010] [<ffffffff811baebc>] vfs_unlink+0xcc/0x150
>>> [51502.244010] [<ffffffff811bb1a0>] do_unlinkat+0x260/0x2a0
>>> [51502.244010] [<ffffffff811a9ef4>] ? filp_close+0x64/0x90
>>> [51502.244010] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
>>> [51502.244010] [<ffffffff81349cab>] ? trace_hardirqs_on_thunk+0x3a/0x3f
>>> [51502.244010] [<ffffffff811be9eb>] SyS_unlinkat+0x1b/0x40
>>> [51502.244010] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
>>> [51502.244010] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 71 13 36 a0 48 89 fe 31 c0 48 c7 c7 b8 43 36 a0 48 89 e5 e8 5d b0 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
>>> [51502.244010] RIP [<ffffffffa03575da>] assfail.constprop.88+0x1e/0x20 [btrfs]
>>>
>>> 2)
>>>
>>> [25405.097230] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
>>> [25405.097488] ------------[ cut here ]------------
>>> [25405.097767] kernel BUG at fs/btrfs/ctree.h:3964!
>>> [25405.097940] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
>>> (...)
>>> [25405.100008] Call Trace:
>>> [25405.100008] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
>>> [25405.100008] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
>>> [25405.100008] [<ffffffffa035755a>] commit_cowonly_roots+0x164/0x226 [btrfs]
>>> [25405.100008] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
>>> [25405.100008] [<ffffffff8109c170>] ? bit_waitqueue+0xc0/0xc0
>>> [25405.100008] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
>>> [25405.100008] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
>>> [25405.100008] [<ffffffffa02e3407>] btrfs_create+0x47/0x210 [btrfs]
>>> [25405.100008] [<ffffffffa02d74cc>] ? btrfs_permission+0x3c/0x80 [btrfs]
>>> [25405.100008] [<ffffffff811bc63b>] vfs_create+0x9b/0x130
>>> [25405.100008] [<ffffffff811bcf19>] do_last+0x849/0xe20
>>> [25405.100008] [<ffffffff811b9409>] ? link_path_walk+0x79/0x820
>>> [25405.100008] [<ffffffff811bd5b5>] path_openat+0xc5/0x690
>>> [25405.100008] [<ffffffff810ab07d>] ? trace_hardirqs_on+0xd/0x10
>>> [25405.100008] [<ffffffff811cdcd2>] ? __alloc_fd+0x32/0x1d0
>>> [25405.100008] [<ffffffff811be2a3>] do_filp_open+0x43/0xa0
>>> [25405.100008] [<ffffffff811cddf1>] ? __alloc_fd+0x151/0x1d0
>>> [25405.100008] [<ffffffff811abcfc>] do_sys_open+0x13c/0x230
>>> [25405.100008] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
>>> [25405.100008] [<ffffffff811abe12>] SyS_open+0x22/0x30
>>> [25405.100008] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
>>> [25405.100008] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 51 13 36 a0 48 89 fe 31 c0 48 c7 c7 d0 43 36 a0 48 89 e5 e8 6d b5 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
>>> [25405.100008] RIP [<ffffffffa03570ca>] assfail.constprop.88+0x1e/0x20 [btrfs]
>>>
>>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
>>> ---
>>>
>>> V2: Removed test for current->journal_info == NULL. At this point it's
>>> always expected to be NULL.
>>
>> Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
>
> Let me clarify my understanding since I'm not good at the transaction code.
>
> * What is the route cause?
>
> When start_transaction() is called with current->journal_transaction == NULL,
> we hit BUG() in the following path.
>
> start_transaction
> -> btrfs_commit_transaction
> -> commit_cowonly_roots
> -> update_cowonly_root
> -> btrfs_write_dirty_block_groups
> -> cache_save_setup
> -> btrfs_check_data_free_space
> ASSERT(current->journal_info) # it fails and hits BUG()!
Yes, you are right.
>
> * Why NULL check is not necessary?
>
> fs/btrfs/transaction.c:
> ===============================================================================
> start_transaction(...) {
> if (current->journal_info &&
> current->journal_info != (void *)BTRFS_SEND_TRANS_STUB) {
> ...
> goto got_it;
> ===============================================================================
> To begin with, if current->journal_info == BTRFS_SEND_TRANS_STUB,
> start_transaction() should not be called. Filipe's another patch
> ([PATCH] Btrfs: assert send doesn't attempt to start) is to change
> the second condition to ASSERT().
>
> If current->journal_transaction is a "real" transaction, the
> above-mentioned code path is skipped because of "goto got_it".
>
> Is my understanding correct?
Yes, you are right.
And only JOIN/JOIN_NOLOCK can join the current transaction handle, the others is
forbidden.
Thanks
Miao
>
> BTW, Filipe, although I've tried to reproduce this problem for two days
> by running xfstests loop, it didn't happen yet with my environment ;-(
> So all I can do seems to be code review.
>
> Thanks,
> Satoru
>
>>>
>>> fs/btrfs/transaction.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>>> index ac984a3..614eac3 100644
>>> --- a/fs/btrfs/transaction.c
>>> +++ b/fs/btrfs/transaction.c
>>> @@ -491,6 +491,7 @@ again:
>>> smp_mb();
>>> if (cur_trans->state >= TRANS_STATE_BLOCKED &&
>>> may_wait_transaction(root, type)) {
>>> + current->journal_info = h;
>>> btrfs_commit_transaction(h, root);
>>> goto again;
>>> }
>>>
>>
>> --
>> 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
>>
>
> .
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] Btrfs: fix crash when starting transaction
2014-07-03 11:07 ` Miao Xie
@ 2014-07-03 23:13 ` Satoru Takeuchi
0 siblings, 0 replies; 12+ messages in thread
From: Satoru Takeuchi @ 2014-07-03 23:13 UTC (permalink / raw)
To: miaox, Filipe David Borba Manana, linux-btrfs
(2014/07/03 20:07), Miao Xie wrote:
> On Thu, 3 Jul 2014 19:32:18 +0900, Satoru Takeuchi wrote:
>> (2014/07/03 17:30), Miao Xie wrote:
>>> On Tue, 24 Jun 2014 17:46:58 +0100, Filipe David Borba Manana wrote:
>>>> Often when starting a transaction we commit the currently running transaction,
>>>> which can end up writing block group caches when the current process has its
>>>> journal_info set to NULL (and not to a transaction). This makes our assertion
>>>> at btrfs_check_data_free_space() (current_journal != NULL) fail, resulting
>>>> in a crash/hang. Therefore fix it by setting journal_info.
>>>>
>>>> Two different traces of this issue follow below.
>>>>
>>>> 1)
>>>>
>>>> [51502.241936] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
>>>> [51502.242213] ------------[ cut here ]------------
>>>> [51502.242493] kernel BUG at fs/btrfs/ctree.h:3964!
>>>> [51502.242669] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
>>>> (...)
>>>> [51502.244010] Call Trace:
>>>> [51502.244010] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
>>>> [51502.244010] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
>>>> [51502.244010] [<ffffffffa0357a6a>] commit_cowonly_roots+0x164/0x226 [btrfs]
>>>> [51502.244010] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
>>>> [51502.244010] [<ffffffff8168ec7b>] ? _raw_spin_unlock+0x2b/0x40
>>>> [51502.244010] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
>>>> [51502.244010] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
>>>> [51502.244010] [<ffffffffa02d73e1>] __unlink_start_trans+0x31/0xe0 [btrfs]
>>>> [51502.244010] [<ffffffffa02dea67>] btrfs_unlink+0x37/0xc0 [btrfs]
>>>> [51502.244010] [<ffffffff811bb054>] ? do_unlinkat+0x114/0x2a0
>>>> [51502.244010] [<ffffffff811baebc>] vfs_unlink+0xcc/0x150
>>>> [51502.244010] [<ffffffff811bb1a0>] do_unlinkat+0x260/0x2a0
>>>> [51502.244010] [<ffffffff811a9ef4>] ? filp_close+0x64/0x90
>>>> [51502.244010] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
>>>> [51502.244010] [<ffffffff81349cab>] ? trace_hardirqs_on_thunk+0x3a/0x3f
>>>> [51502.244010] [<ffffffff811be9eb>] SyS_unlinkat+0x1b/0x40
>>>> [51502.244010] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
>>>> [51502.244010] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 71 13 36 a0 48 89 fe 31 c0 48 c7 c7 b8 43 36 a0 48 89 e5 e8 5d b0 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
>>>> [51502.244010] RIP [<ffffffffa03575da>] assfail.constprop.88+0x1e/0x20 [btrfs]
>>>>
>>>> 2)
>>>>
>>>> [25405.097230] BTRFS: assertion failed: current->journal_info, file: fs/btrfs/extent-tree.c, line: 3670
>>>> [25405.097488] ------------[ cut here ]------------
>>>> [25405.097767] kernel BUG at fs/btrfs/ctree.h:3964!
>>>> [25405.097940] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC
>>>> (...)
>>>> [25405.100008] Call Trace:
>>>> [25405.100008] [<ffffffffa02bc025>] btrfs_check_data_free_space+0x395/0x3a0 [btrfs]
>>>> [25405.100008] [<ffffffffa02c3bdc>] btrfs_write_dirty_block_groups+0x4ac/0x640 [btrfs]
>>>> [25405.100008] [<ffffffffa035755a>] commit_cowonly_roots+0x164/0x226 [btrfs]
>>>> [25405.100008] [<ffffffffa02d53cd>] btrfs_commit_transaction+0x4ed/0xab0 [btrfs]
>>>> [25405.100008] [<ffffffff8109c170>] ? bit_waitqueue+0xc0/0xc0
>>>> [25405.100008] [<ffffffffa02d6259>] start_transaction+0x459/0x620 [btrfs]
>>>> [25405.100008] [<ffffffffa02d67ab>] btrfs_start_transaction+0x1b/0x20 [btrfs]
>>>> [25405.100008] [<ffffffffa02e3407>] btrfs_create+0x47/0x210 [btrfs]
>>>> [25405.100008] [<ffffffffa02d74cc>] ? btrfs_permission+0x3c/0x80 [btrfs]
>>>> [25405.100008] [<ffffffff811bc63b>] vfs_create+0x9b/0x130
>>>> [25405.100008] [<ffffffff811bcf19>] do_last+0x849/0xe20
>>>> [25405.100008] [<ffffffff811b9409>] ? link_path_walk+0x79/0x820
>>>> [25405.100008] [<ffffffff811bd5b5>] path_openat+0xc5/0x690
>>>> [25405.100008] [<ffffffff810ab07d>] ? trace_hardirqs_on+0xd/0x10
>>>> [25405.100008] [<ffffffff811cdcd2>] ? __alloc_fd+0x32/0x1d0
>>>> [25405.100008] [<ffffffff811be2a3>] do_filp_open+0x43/0xa0
>>>> [25405.100008] [<ffffffff811cddf1>] ? __alloc_fd+0x151/0x1d0
>>>> [25405.100008] [<ffffffff811abcfc>] do_sys_open+0x13c/0x230
>>>> [25405.100008] [<ffffffff810aaea6>] ? trace_hardirqs_on_caller+0x16/0x1e0
>>>> [25405.100008] [<ffffffff811abe12>] SyS_open+0x22/0x30
>>>> [25405.100008] [<ffffffff81698452>] system_call_fastpath+0x16/0x1b
>>>> [25405.100008] Code: 0b 55 48 89 e5 0f 0b 55 48 89 e5 0f 0b 55 89 f1 48 c7 c2 51 13 36 a0 48 89 fe 31 c0 48 c7 c7 d0 43 36 a0 48 89 e5 e8 6d b5 32 e1 <0f> 0b 0f 1f 44 00 00 55 b9 11 00 00 00 48 89 e5 41 55 49 89 f5
>>>> [25405.100008] RIP [<ffffffffa03570ca>] assfail.constprop.88+0x1e/0x20 [btrfs]
>>>>
>>>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
>>>> ---
>>>>
>>>> V2: Removed test for current->journal_info == NULL. At this point it's
>>>> always expected to be NULL.
>>>
>>> Reviewed-by: Miao Xie <miaox@cn.fujitsu.com>
>>
>> Let me clarify my understanding since I'm not good at the transaction code.
>>
>> * What is the route cause?
>>
>> When start_transaction() is called with current->journal_transaction == NULL,
>> we hit BUG() in the following path.
>>
>> start_transaction
>> -> btrfs_commit_transaction
>> -> commit_cowonly_roots
>> -> update_cowonly_root
>> -> btrfs_write_dirty_block_groups
>> -> cache_save_setup
>> -> btrfs_check_data_free_space
>> ASSERT(current->journal_info) # it fails and hits BUG()!
>
> Yes, you are right.
>
>>
>> * Why NULL check is not necessary?
>>
>> fs/btrfs/transaction.c:
>> ===============================================================================
>> start_transaction(...) {
>> if (current->journal_info &&
>> current->journal_info != (void *)BTRFS_SEND_TRANS_STUB) {
>> ...
>> goto got_it;
>> ===============================================================================
>> To begin with, if current->journal_info == BTRFS_SEND_TRANS_STUB,
>> start_transaction() should not be called. Filipe's another patch
>> ([PATCH] Btrfs: assert send doesn't attempt to start) is to change
>> the second condition to ASSERT().
>>
>> If current->journal_transaction is a "real" transaction, the
>> above-mentioned code path is skipped because of "goto got_it".
>>
>> Is my understanding correct?
>
> Yes, you are right.
> And only JOIN/JOIN_NOLOCK can join the current transaction handle, the others is
> forbidden.
Thank your for your explanation!
Reviewed-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
Satoru
>
> Thanks
> Miao
>
>>
>> BTW, Filipe, although I've tried to reproduce this problem for two days
>> by running xfstests loop, it didn't happen yet with my environment ;-(
>> So all I can do seems to be code review.
>>
>> Thanks,
>> Satoru
>>
>>>>
>>>> fs/btrfs/transaction.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>>>> index ac984a3..614eac3 100644
>>>> --- a/fs/btrfs/transaction.c
>>>> +++ b/fs/btrfs/transaction.c
>>>> @@ -491,6 +491,7 @@ again:
>>>> smp_mb();
>>>> if (cur_trans->state >= TRANS_STATE_BLOCKED &&
>>>> may_wait_transaction(root, type)) {
>>>> + current->journal_info = h;
>>>> btrfs_commit_transaction(h, root);
>>>> goto again;
>>>> }
>>>>
>>>
>>> --
>>> 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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-07-03 23:14 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-23 11:58 [PATCH] Btrfs: fix crash when starting transaction Filipe David Borba Manana
2014-06-24 2:34 ` Satoru Takeuchi
2014-06-24 8:29 ` Filipe David Manana
2014-06-24 9:35 ` Satoru Takeuchi
2014-06-24 10:22 ` Miao Xie
2014-06-24 10:29 ` Filipe David Manana
2014-06-24 15:25 ` Josef Bacik
2014-06-24 16:46 ` [PATCH v2] " Filipe David Borba Manana
2014-07-03 8:30 ` Miao Xie
2014-07-03 10:32 ` Satoru Takeuchi
2014-07-03 11:07 ` Miao Xie
2014-07-03 23:13 ` Satoru Takeuchi
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).