* [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).