| Message ID | 5114A177.5020604@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 07, 2013 at 11:55:51PM -0700, Miao Xie wrote: > Here is the whole story: > Trans_Attach_Task Trans_Commit_Task > btrfs_commit_transaction() > |->wait writers to be 1 > btrfs_attach_transaction() | > btrfs_commit_transaction() | > | |->set trans_no_join to 1 > | | (close join transaction) > |->btrfs_run_ordered_operations | > (Those ordered operations | > are added when releasing | > file) | > |->btrfs_join_transaction() | > |->wait_commit() | > |->wait writers to be 1 > > Then these two tasks waited for each other. > > As we know, btrfs_attach_transaction() is used to catch the current > transaction, and commit it, so if someone has committed the transaction, > it is unnecessary to join it and commit it, wait is the best choice > for it. In this way, we can fix the above problem. > > Signed-off-by: Miao Xie <miaox@cn.fujitsu.com> This caused another problem [ 8050.503904] btrfs-transacti D 0000000000000000 0 5546 2 0x00000080 [ 8050.503913] ffff88037bfb9d18 0000000000000046 ffff88037bfb9cb8 ffffffff810c6d4d [ 8050.503924] ffff88037c4d8000 ffff88037bfb9fd8 ffff88037bfb9fd8 ffff88037bfb9fd8 [ 8050.503933] ffff88042f17a000 ffff88037c4d8000 ffff88042c33b000 ffff88037ba0bdb8 [ 8050.503943] Call Trace: [ 8050.503953] [<ffffffff810c6d4d>] ? trace_hardirqs_on+0xd/0x10 [ 8050.503962] [<ffffffff816507c9>] schedule+0x29/0x70 [ 8050.504002] [<ffffffffa084eb75>] wait_current_trans+0xb5/0x110 [btrfs] [ 8050.504011] [<ffffffff810891f0>] ? __init_waitqueue_head+0x60/0x60 [ 8050.504047] [<ffffffffa08503c0>] start_transaction+0x160/0x4e0 [btrfs] [ 8050.504082] [<ffffffffa0850757>] btrfs_attach_transaction+0x17/0x20 [btrfs] [ 8050.504114] [<ffffffffa084857a>] transaction_kthread+0x15a/0x240 [btrfs] [ 8050.504147] [<ffffffffa0848420>] ? btrfs_destroy_delayed_refs+0x330/0x330 [btrfs] [ 8050.504155] [<ffffffff8108883a>] kthread+0xea/0xf0 [ 8050.504166] [<ffffffff81088750>] ? flush_kthread_worker+0x150/0x150 [ 8050.504175] [<ffffffff8165a06c>] ret_from_fork+0x7c/0xb0 [ 8050.504183] [<ffffffff81088750>] ? flush_kthread_worker+0x150/0x150 [ 8050.504189] sync D 0000000000000000 0 5572 5342 0x00000080 [ 8050.504198] ffff88037c235dd8 0000000000000046 ffff88037c235d78 ffffffff810c6d4d [ 8050.504207] ffff88037ca8a000 ffff88037c235fd8 ffff88037c235fd8 ffff88037c235fd8 [ 8050.504217] ffff88042f184000 ffff88037ca8a000 ffff88042c33b000 ffff88037ba0bdb8 [ 8050.504227] Call Trace: [ 8050.504236] [<ffffffff810c6d4d>] ? trace_hardirqs_on+0xd/0x10 [ 8050.504245] [<ffffffff816507c9>] schedule+0x29/0x70 [ 8050.504278] [<ffffffffa084eb75>] wait_current_trans+0xb5/0x110 [btrfs] [ 8050.504287] [<ffffffff810891f0>] ? __init_waitqueue_head+0x60/0x60 [ 8050.504322] [<ffffffffa08503c0>] start_transaction+0x160/0x4e0 [btrfs] [ 8050.504360] [<ffffffffa0866d94>] ? btrfs_wait_ordered_extents+0x174/0x230 [btrfs] [ 8050.504395] [<ffffffffa0850757>] btrfs_attach_transaction+0x17/0x20 [btrfs] [ 8050.504420] [<ffffffffa0820133>] btrfs_sync_fs+0x53/0x130 [btrfs] [ 8050.504430] [<ffffffff811cac30>] ? __sync_filesystem+0x60/0x60 [ 8050.504438] [<ffffffff811cac30>] ? __sync_filesystem+0x60/0x60 [ 8050.504447] [<ffffffff811cac50>] sync_fs_one_sb+0x20/0x30 [ 8050.504455] [<ffffffff8119e0c1>] iterate_supers+0xf1/0x100 [ 8050.504463] [<ffffffff811cad25>] sys_sync+0x55/0x90 [ 8050.504472] [<ffffffff8165a119>] system_call_fastpath+0x16/0x1b So we're getting stuck in the if (may_wait_transaction()) wait_current_trans(); thing. If we set blocked in __btrfs_end_transaction we'll just sit there forever because nobody can actually commit the transaction. Probably need to change this to if (type == TRANS_ATTACH && trans->in_commit) or something like that. Me and kdave reproduced by running 274 in a loop, it happpened pretty quick. I'd fix it myself but I have to leave my house for people to come look at it. If you haven't fixed this by tomorrow I'll fix it up. Thanks, Josef -- 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
On Mon, Feb 11, 2013 at 03:35:37PM -0500, Josef Bacik wrote: > or something like that. Me and kdave reproduced by running 274 in a loop, it > happpened pretty quick. I'd fix it myself but I have to leave my house for > people to come look at it. If you haven't fixed this by tomorrow I'll fix it > up. Thanks, I found 224 stuck with this [ 3840.176147] INFO: task btrfs-transacti:3692 blocked for more than 480 seconds. [ 3840.176157] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 3840.176162] btrfs-transacti D 0000000000000000 0 3692 2 0x00000000 [ 3840.176172] ffff880103a3bd98 0000000000000046 0000000000000000 ffff880103a3a010 [ 3840.176183] ffff880103a3a000 ffff880103a3a010 ffff880103a3a000 ffff880103a3a000 [ 3840.176190] ffff880103a3a000 ffff880103a3bfd8 ffff880102814800 ffffffff81a14420 [ 3840.176200] Call Trace: [ 3840.176218] [<ffffffff8108a0c9>] ? enqueue_entity+0x229/0xa40 [ 3840.176230] [<ffffffff815efc14>] schedule+0x24/0x70 [ 3840.176296] [<ffffffffa00cd0f5>] wait_current_trans+0xc5/0x110 [btrfs] [ 3840.176306] [<ffffffff8106d890>] ? wake_up_bit+0x40/0x40 [ 3840.176361] [<ffffffffa00cfa48>] start_transaction+0x228/0x4c0 [btrfs] [ 3840.176400] [<ffffffffa00cfcf2>] btrfs_attach_transaction+0x12/0x20 [btrfs] [ 3840.176434] [<ffffffffa00c95c7>] transaction_kthread+0x167/0x220 [btrfs] [ 3840.176476] [<ffffffffa00c9460>] ? __btree_submit_bio_start+0x10/0x10 [btrfs] [ 3840.176494] [<ffffffff8106d086>] kthread+0xc6/0xd0 [ 3840.176506] [<ffffffff8106cfc0>] ? kthread_freezable_should_stop+0x70/0x70 [ 3840.176514] [<ffffffff815f82bc>] ret_from_fork+0x7c/0xb0 [ 3840.176522] [<ffffffff8106cfc0>] ? kthread_freezable_should_stop+0x70/0x70 [ 3840.176528] INFO: task rm:15763 blocked for more than 480 seconds. [ 3840.176531] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. [ 3840.176534] rm D 0000000000000002 0 15763 3517 0x00000000 [ 3840.176544] ffff8801071b7c88 0000000000000086 00000002000280da ffff8801071b6010 [ 3840.176551] ffff8801071b6000 ffff8801071b6010 ffff8801071b6000 ffff8801071b6000 [ 3840.176558] ffff8801071b6000 ffff8801071b7fd8 ffff8800be5762c0 ffff88012af063c0 [ 3840.176567] Call Trace: [ 3840.176578] [<ffffffff8111f942>] ? __alloc_pages_nodemask+0x162/0x250 [ 3840.176608] [<ffffffffa00be468>] ? reserve_metadata_bytes+0x378/0x490 [btrfs] [ 3840.176622] [<ffffffff815efc14>] schedule+0x24/0x70 [ 3840.176664] [<ffffffffa00cd0f5>] wait_current_trans+0xc5/0x110 [btrfs] [ 3840.176671] [<ffffffff8106d890>] ? wake_up_bit+0x40/0x40 [ 3840.176721] [<ffffffffa00cfa48>] start_transaction+0x228/0x4c0 [btrfs] [ 3840.176769] [<ffffffffa00cffd3>] btrfs_start_transaction+0x13/0x20 [btrfs] [ 3840.176812] [<ffffffffa00d192e>] __unlink_start_trans+0x7e/0x490 [btrfs] [ 3840.176832] [<ffffffff811891d7>] ? __d_lookup+0xa7/0x170 [ 3840.176839] [<ffffffff811432f1>] ? handle_pte_fault+0x91/0x200 [ 3840.176849] [<ffffffff81291fc9>] ? security_inode_permission+0x19/0x20 [ 3840.176893] [<ffffffffa00daca6>] btrfs_unlink+0x36/0xd0 [btrfs] [ 3840.176899] [<ffffffff8117f7f4>] vfs_unlink+0xb4/0x110 [ 3840.176905] [<ffffffff81183f83>] do_unlinkat+0x233/0x240 [ 3840.176912] [<ffffffff811841ad>] sys_unlinkat+0x1d/0x40 [ 3840.176918] [<ffffffff815f8369>] system_call_fastpath+0x16/0x1b mounted with noatime,space_cache david -- 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
On Thu, Feb 07, 2013 at 11:55:51PM -0700, Miao Xie wrote: > Here is the whole story: > Trans_Attach_Task Trans_Commit_Task > btrfs_commit_transaction() > |->wait writers to be 1 > btrfs_attach_transaction() | > btrfs_commit_transaction() | > | |->set trans_no_join to 1 > | | (close join transaction) > |->btrfs_run_ordered_operations | > (Those ordered operations | > are added when releasing | > file) | > |->btrfs_join_transaction() | > |->wait_commit() | > |->wait writers to be 1 I'm just dropping this patch, the way you describe this deadlock can't happen since the second btrfs_join_transaction() would see theres already a transaction in current->journal_info and use that and not do wait_commit(). If you observed a deadlock like this then look at it again, there is something else going wrong. Thanks, Josef -- 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
(Sorry for the late reply, I was on my vacation of the Spring Festival last week.) On Tue, 12 Feb 2013 13:56:32 +0100, David Sterba wrote: > On Mon, Feb 11, 2013 at 03:35:37PM -0500, Josef Bacik wrote: >> or something like that. Me and kdave reproduced by running 274 in a loop, it >> happpened pretty quick. I'd fix it myself but I have to leave my house for >> people to come look at it. If you haven't fixed this by tomorrow I'll fix it >> up. Thanks, > > I found 224 stuck with this > [SNIP] > > mounted with noatime,space_cache Thanks for your test. My test skipped the 274th case because it always fails, and all the other cases passed, so I didn't hit this problem. Anyways, very sorry for my stupid patch. (I have reviewed Josef's fix patch, and commented on it, please see the reply of that patch) Thanks Miao -- 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
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index f154946..7be9d5e 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c @@ -285,6 +285,14 @@ static int may_wait_transaction(struct btrfs_root *root, int type) if (type == TRANS_USERSPACE) return 1; + /* + * If we are ATTACH, it means we just want to catch the current + * transaction and commit it. So if someone is committing the + * current transaction now, it is very glad to wait it. + */ + if (type == TRANS_ATTACH) + return 1; + if (type == TRANS_START && !atomic_read(&root->fs_info->open_ioctl_trans)) return 1;
Here is the whole story: Trans_Attach_Task Trans_Commit_Task btrfs_commit_transaction() |->wait writers to be 1 btrfs_attach_transaction() | btrfs_commit_transaction() | | |->set trans_no_join to 1 | | (close join transaction) |->btrfs_run_ordered_operations | (Those ordered operations | are added when releasing | file) | |->btrfs_join_transaction() | |->wait_commit() | |->wait writers to be 1 Then these two tasks waited for each other. As we know, btrfs_attach_transaction() is used to catch the current transaction, and commit it, so if someone has committed the transaction, it is unnecessary to join it and commit it, wait is the best choice for it. In this way, we can fix the above problem. Signed-off-by: Miao Xie <miaox@cn.fujitsu.com> --- fs/btrfs/transaction.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)