* [PATCH] btrfs: qgroup: Fix root item corruption when multiple same source snapshiots are created with quota enabled
@ 2017-12-19 7:44 Qu Wenruo
2018-02-02 11:45 ` Filipe Manana
2018-05-03 16:43 ` David Sterba
0 siblings, 2 replies; 5+ messages in thread
From: Qu Wenruo @ 2017-12-19 7:44 UTC (permalink / raw)
To: linux-btrfs
When multiple pending snapshots referring the same source subvolume are
executed, enabled quota will cause root item corruption, where root
items are using old bytenr (no backref in extent tree).
This can be triggered by fstests btrfs/152.
The cause is when source subvolume is still dirty, extra commit
(simplied transaction commit) of qgroup_account_snapshot() can skip
dirty roots not recorded in current transaction, making root item of
source subvolume not updated.
Fix it by forcing recording source subvolume in current transaction
before qgroup sub-transaction commit.
Reported-by: Justin Maggard <jmaggard@netgear.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/transaction.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index ddae813c01dd..f645e5de5fa5 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -319,7 +319,7 @@ static int record_root_in_trans(struct btrfs_trans_handle *trans,
if ((test_bit(BTRFS_ROOT_REF_COWS, &root->state) &&
root->last_trans < trans->transid) || force) {
WARN_ON(root == fs_info->extent_root);
- WARN_ON(root->commit_root != root->node);
+ WARN_ON(!force && root->commit_root != root->node);
/*
* see below for IN_TRANS_SETUP usage rules
@@ -1366,6 +1366,14 @@ static int qgroup_account_snapshot(struct btrfs_trans_handle *trans,
if (!test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags))
return 0;
+ /*
+ * Ensure dirty @src will be commited.
+ * Or after comming commit_fs_roots() and switch_commit_roots(),
+ * any dirty but not recorded root will never be updated again.
+ * Causing outdated root item.
+ */
+ record_root_in_trans(trans, src, 1);
+
/*
* We are going to commit transaction, see btrfs_commit_transaction()
* comment for reason locking tree_log_mutex
--
2.15.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: qgroup: Fix root item corruption when multiple same source snapshiots are created with quota enabled
2017-12-19 7:44 [PATCH] btrfs: qgroup: Fix root item corruption when multiple same source snapshiots are created with quota enabled Qu Wenruo
@ 2018-02-02 11:45 ` Filipe Manana
2018-03-07 16:22 ` David Sterba
2018-05-03 16:43 ` David Sterba
1 sibling, 1 reply; 5+ messages in thread
From: Filipe Manana @ 2018-02-02 11:45 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Tue, Dec 19, 2017 at 7:44 AM, Qu Wenruo <wqu@suse.com> wrote:
> When multiple pending snapshots referring the same source subvolume are
> executed, enabled quota will cause root item corruption, where root
> items are using old bytenr (no backref in extent tree).
>
> This can be triggered by fstests btrfs/152.
>
> The cause is when source subvolume is still dirty, extra commit
> (simplied transaction commit) of qgroup_account_snapshot() can skip
> dirty roots not recorded in current transaction, making root item of
> source subvolume not updated.
>
> Fix it by forcing recording source subvolume in current transaction
> before qgroup sub-transaction commit.
>
> Reported-by: Justin Maggard <jmaggard@netgear.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Looks good.
> ---
> fs/btrfs/transaction.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index ddae813c01dd..f645e5de5fa5 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -319,7 +319,7 @@ static int record_root_in_trans(struct btrfs_trans_handle *trans,
> if ((test_bit(BTRFS_ROOT_REF_COWS, &root->state) &&
> root->last_trans < trans->transid) || force) {
> WARN_ON(root == fs_info->extent_root);
> - WARN_ON(root->commit_root != root->node);
> + WARN_ON(!force && root->commit_root != root->node);
>
> /*
> * see below for IN_TRANS_SETUP usage rules
> @@ -1366,6 +1366,14 @@ static int qgroup_account_snapshot(struct btrfs_trans_handle *trans,
> if (!test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags))
> return 0;
>
> + /*
> + * Ensure dirty @src will be commited.
> + * Or after comming commit_fs_roots() and switch_commit_roots(),
> + * any dirty but not recorded root will never be updated again.
> + * Causing outdated root item.
> + */
> + record_root_in_trans(trans, src, 1);
> +
> /*
> * We are going to commit transaction, see btrfs_commit_transaction()
> * comment for reason locking tree_log_mutex
> --
> 2.15.1
>
> --
> 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
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: qgroup: Fix root item corruption when multiple same source snapshiots are created with quota enabled
2018-02-02 11:45 ` Filipe Manana
@ 2018-03-07 16:22 ` David Sterba
0 siblings, 0 replies; 5+ messages in thread
From: David Sterba @ 2018-03-07 16:22 UTC (permalink / raw)
To: Filipe Manana; +Cc: Qu Wenruo, linux-btrfs
On Fri, Feb 02, 2018 at 11:45:46AM +0000, Filipe Manana wrote:
> On Tue, Dec 19, 2017 at 7:44 AM, Qu Wenruo <wqu@suse.com> wrote:
> > When multiple pending snapshots referring the same source subvolume are
> > executed, enabled quota will cause root item corruption, where root
> > items are using old bytenr (no backref in extent tree).
> >
> > This can be triggered by fstests btrfs/152.
> >
> > The cause is when source subvolume is still dirty, extra commit
> > (simplied transaction commit) of qgroup_account_snapshot() can skip
> > dirty roots not recorded in current transaction, making root item of
> > source subvolume not updated.
> >
> > Fix it by forcing recording source subvolume in current transaction
> > before qgroup sub-transaction commit.
> >
> > Reported-by: Justin Maggard <jmaggard@netgear.com>
> > Signed-off-by: Qu Wenruo <wqu@suse.com>
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
I overlooked the patch, sorry. Added to next now, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: qgroup: Fix root item corruption when multiple same source snapshiots are created with quota enabled
2017-12-19 7:44 [PATCH] btrfs: qgroup: Fix root item corruption when multiple same source snapshiots are created with quota enabled Qu Wenruo
2018-02-02 11:45 ` Filipe Manana
@ 2018-05-03 16:43 ` David Sterba
2018-05-03 23:32 ` Qu Wenruo
1 sibling, 1 reply; 5+ messages in thread
From: David Sterba @ 2018-05-03 16:43 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, fdmanana
On Tue, Dec 19, 2017 at 03:44:54PM +0800, Qu Wenruo wrote:
> When multiple pending snapshots referring the same source subvolume are
> executed, enabled quota will cause root item corruption, where root
> items are using old bytenr (no backref in extent tree).
>
> This can be triggered by fstests btrfs/152.
>
> The cause is when source subvolume is still dirty, extra commit
> (simplied transaction commit) of qgroup_account_snapshot() can skip
> dirty roots not recorded in current transaction, making root item of
> source subvolume not updated.
>
> Fix it by forcing recording source subvolume in current transaction
> before qgroup sub-transaction commit.
>
> Reported-by: Justin Maggard <jmaggard@netgear.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/transaction.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index ddae813c01dd..f645e5de5fa5 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -319,7 +319,7 @@ static int record_root_in_trans(struct btrfs_trans_handle *trans,
> if ((test_bit(BTRFS_ROOT_REF_COWS, &root->state) &&
> root->last_trans < trans->transid) || force) {
> WARN_ON(root == fs_info->extent_root);
> - WARN_ON(root->commit_root != root->node);
> + WARN_ON(!force && root->commit_root != root->node);
I see this warning in current misc-next, test btrfs/152. The warning
does not appear with reference master or 'to be sent as next pull
rquest' testing runs.
All the warnings seem to be the same, so I'm pasting the first one:
[ 7910.900575] WARNING: CPU: 3 PID: 12891 at fs/btrfs/transaction.c:303record_root_in_trans+0x40/0xe0 [btrfs]
[ 7910.973625] CPU: 3 PID: 12891 Comm: btrfs Tainted: G W4.17.0-rc3-1.ge195904-vanilla+ #231
[ 7910.983421] Hardware name: empty empty/S3993, BIOS PAQEX0-302/24/2008
[ 7910.990140] RIP: 0010:record_root_in_trans+0x40/0xe0 [btrfs]
[ 7910.995967] RSP: 0018:ffffbc7b04f1ba50 EFLAGS: 00010202
[ 7911.001345] RAX: ffff963ef63bcd08 RBX: ffff963e069239d8 RCX:ffff963f0a89e7e8
[ 7911.008645] RDX: 0000000000000000 RSI: ffff963f0a89e7e8 RDI:ffff963f0278b028
[ 7911.015940] RBP: ffffbc7b04f1bb70 R08: ffff963e06923a10 R09:0000000000000001
[ 7911.023227] R10: 0000000000200000 R11: 0000000000000000 R12:ffff963f2247c000
[ 7911.030523] R13: ffff963f0b3e0008 R14: ffff963f0f9608c8 R15:ffff963f06561a60
[ 7911.037834] FS: 00007fa037b1d8c0(0000) GS:ffff963f26c00000(0000)knlGS:0000000000000000
[ 7911.047965] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 7911.053871] CR2: 00007ffeb64a8000 CR3: 0000000213d7f000 CR4:00000000000006e0
[ 7911.061161] Call Trace:
[ 7911.063780] create_pending_snapshot+0x1ff/0x1130 [btrfs]
[ 7911.069417] ? create_pending_snapshots+0xab/0xd0 [btrfs]
[ 7911.075000] create_pending_snapshots+0xab/0xd0 [btrfs]
[ 7911.080420] btrfs_commit_transaction+0x2ac/0xa60 [btrfs]
[ 7911.086033] btrfs_mksubvol+0x5e8/0x650 [btrfs]
[ 7911.090734] ? mnt_want_write_file+0x3b/0xc0
[ 7911.095210] btrfs_ioctl_snap_create_transid+0x16f/0x1a0 [btrfs]
[ 7911.101429] btrfs_ioctl_snap_create_v2+0x102/0x150 [btrfs]
[ 7911.107209] btrfs_ioctl+0x39b/0x29a0 [btrfs]
[ 7911.111730] ? __slab_free+0xf9/0x2a0
[ 7911.115562] ? trace_hardirqs_on_caller+0x105/0x1c0
[ 7911.120623] ? do_vfs_ioctl+0x91/0x6b0
[ 7911.124511] do_vfs_ioctl+0x91/0x6b0
[ 7911.128244] ? kfree+0x259/0x310
[ 7911.131645] ? syscall_trace_enter+0x1ce/0x3c0
[ 7911.136253] ksys_ioctl+0x70/0x80
[ 7911.139721] ? trace_hardirqs_off_thunk+0x1a/0x1c
[ 7911.144592] __x64_sys_ioctl+0x16/0x20
[ 7911.148491] do_syscall_64+0x62/0x1c0
[ 7911.152300] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 7911.157503] RIP: 0033:0x7fa036bba417
[ 7911.161220] RSP: 002b:00007ffeb64ad058 EFLAGS: 00000246 ORIG_RAX:0000000000000010
[ 7911.169028] RAX: ffffffffffffffda RBX: 00007ffeb64c2370 RCX:00007fa036bba417
[ 7911.176316] RDX: 00007ffeb64ad0a0 RSI: 0000000050009417 RDI:0000000000000003
[ 7911.183606] RBP: 0000000000e63020 R08: 000000000000ffff R09:00007fa036e73698
[ 7911.190883] R10: 0000000000000000 R11: 0000000000000246 R12:0000000000e64260
[ 7911.198171] R13: 00007ffeb64ae1d0 R14: 000000000000000e R15:00007ffeb64ad0a0
[ 7911.224866] irq event stamp: 8428
[ 7911.228311] hardirqs last enabled at (8427): [<ffffffffbd3017a0>]cmpxchg_double_slab.isra.42+0x190/0x1b0
[ 7911.238215] hardirqs last disabled at (8428): [<ffffffffbda011ac>]error_entry+0x6c/0xc0
[ 7911.246530] softirqs last enabled at (8418): [<ffffffffbdc00452>]__do_softirq+0x452/0x7b4
[ 7911.255112] softirqs last disabled at (8393): [<ffffffffbd0b4043>]irq_exit+0xc3/0xf0
[ 7911.263167] ---[ end trace 0d6ecffc816873c4 ]---
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs: qgroup: Fix root item corruption when multiple same source snapshiots are created with quota enabled
2018-05-03 16:43 ` David Sterba
@ 2018-05-03 23:32 ` Qu Wenruo
0 siblings, 0 replies; 5+ messages in thread
From: Qu Wenruo @ 2018-05-03 23:32 UTC (permalink / raw)
To: dsterba, Qu Wenruo, linux-btrfs, fdmanana
[-- Attachment #1.1: Type: text/plain, Size: 5225 bytes --]
On 2018年05月04日 00:43, David Sterba wrote:
> On Tue, Dec 19, 2017 at 03:44:54PM +0800, Qu Wenruo wrote:
>> When multiple pending snapshots referring the same source subvolume are
>> executed, enabled quota will cause root item corruption, where root
>> items are using old bytenr (no backref in extent tree).
>>
>> This can be triggered by fstests btrfs/152.
>>
>> The cause is when source subvolume is still dirty, extra commit
>> (simplied transaction commit) of qgroup_account_snapshot() can skip
>> dirty roots not recorded in current transaction, making root item of
>> source subvolume not updated.
>>
>> Fix it by forcing recording source subvolume in current transaction
>> before qgroup sub-transaction commit.
>>
>> Reported-by: Justin Maggard <jmaggard@netgear.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> fs/btrfs/transaction.c | 10 +++++++++-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>> index ddae813c01dd..f645e5de5fa5 100644
>> --- a/fs/btrfs/transaction.c
>> +++ b/fs/btrfs/transaction.c
>> @@ -319,7 +319,7 @@ static int record_root_in_trans(struct btrfs_trans_handle *trans,
>> if ((test_bit(BTRFS_ROOT_REF_COWS, &root->state) &&
>> root->last_trans < trans->transid) || force) {
>> WARN_ON(root == fs_info->extent_root);
>> - WARN_ON(root->commit_root != root->node);
>> + WARN_ON(!force && root->commit_root != root->node);
>
> I see this warning in current misc-next, test btrfs/152. The warning
> does not appear with reference master or 'to be sent as next pull
> rquest' testing runs.
I'll try to pin down the reason,
Thanks,
Qu
>
> All the warnings seem to be the same, so I'm pasting the first one:
>
> [ 7910.900575] WARNING: CPU: 3 PID: 12891 at fs/btrfs/transaction.c:303record_root_in_trans+0x40/0xe0 [btrfs]
> [ 7910.973625] CPU: 3 PID: 12891 Comm: btrfs Tainted: G W4.17.0-rc3-1.ge195904-vanilla+ #231
> [ 7910.983421] Hardware name: empty empty/S3993, BIOS PAQEX0-302/24/2008
> [ 7910.990140] RIP: 0010:record_root_in_trans+0x40/0xe0 [btrfs]
> [ 7910.995967] RSP: 0018:ffffbc7b04f1ba50 EFLAGS: 00010202
> [ 7911.001345] RAX: ffff963ef63bcd08 RBX: ffff963e069239d8 RCX:ffff963f0a89e7e8
> [ 7911.008645] RDX: 0000000000000000 RSI: ffff963f0a89e7e8 RDI:ffff963f0278b028
> [ 7911.015940] RBP: ffffbc7b04f1bb70 R08: ffff963e06923a10 R09:0000000000000001
> [ 7911.023227] R10: 0000000000200000 R11: 0000000000000000 R12:ffff963f2247c000
> [ 7911.030523] R13: ffff963f0b3e0008 R14: ffff963f0f9608c8 R15:ffff963f06561a60
> [ 7911.037834] FS: 00007fa037b1d8c0(0000) GS:ffff963f26c00000(0000)knlGS:0000000000000000
> [ 7911.047965] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 7911.053871] CR2: 00007ffeb64a8000 CR3: 0000000213d7f000 CR4:00000000000006e0
> [ 7911.061161] Call Trace:
> [ 7911.063780] create_pending_snapshot+0x1ff/0x1130 [btrfs]
> [ 7911.069417] ? create_pending_snapshots+0xab/0xd0 [btrfs]
> [ 7911.075000] create_pending_snapshots+0xab/0xd0 [btrfs]
> [ 7911.080420] btrfs_commit_transaction+0x2ac/0xa60 [btrfs]
> [ 7911.086033] btrfs_mksubvol+0x5e8/0x650 [btrfs]
> [ 7911.090734] ? mnt_want_write_file+0x3b/0xc0
> [ 7911.095210] btrfs_ioctl_snap_create_transid+0x16f/0x1a0 [btrfs]
> [ 7911.101429] btrfs_ioctl_snap_create_v2+0x102/0x150 [btrfs]
> [ 7911.107209] btrfs_ioctl+0x39b/0x29a0 [btrfs]
> [ 7911.111730] ? __slab_free+0xf9/0x2a0
> [ 7911.115562] ? trace_hardirqs_on_caller+0x105/0x1c0
> [ 7911.120623] ? do_vfs_ioctl+0x91/0x6b0
> [ 7911.124511] do_vfs_ioctl+0x91/0x6b0
> [ 7911.128244] ? kfree+0x259/0x310
> [ 7911.131645] ? syscall_trace_enter+0x1ce/0x3c0
> [ 7911.136253] ksys_ioctl+0x70/0x80
> [ 7911.139721] ? trace_hardirqs_off_thunk+0x1a/0x1c
> [ 7911.144592] __x64_sys_ioctl+0x16/0x20
> [ 7911.148491] do_syscall_64+0x62/0x1c0
> [ 7911.152300] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [ 7911.157503] RIP: 0033:0x7fa036bba417
> [ 7911.161220] RSP: 002b:00007ffeb64ad058 EFLAGS: 00000246 ORIG_RAX:0000000000000010
> [ 7911.169028] RAX: ffffffffffffffda RBX: 00007ffeb64c2370 RCX:00007fa036bba417
> [ 7911.176316] RDX: 00007ffeb64ad0a0 RSI: 0000000050009417 RDI:0000000000000003
> [ 7911.183606] RBP: 0000000000e63020 R08: 000000000000ffff R09:00007fa036e73698
> [ 7911.190883] R10: 0000000000000000 R11: 0000000000000246 R12:0000000000e64260
> [ 7911.198171] R13: 00007ffeb64ae1d0 R14: 000000000000000e R15:00007ffeb64ad0a0
> [ 7911.224866] irq event stamp: 8428
> [ 7911.228311] hardirqs last enabled at (8427): [<ffffffffbd3017a0>]cmpxchg_double_slab.isra.42+0x190/0x1b0
> [ 7911.238215] hardirqs last disabled at (8428): [<ffffffffbda011ac>]error_entry+0x6c/0xc0
> [ 7911.246530] softirqs last enabled at (8418): [<ffffffffbdc00452>]__do_softirq+0x452/0x7b4
> [ 7911.255112] softirqs last disabled at (8393): [<ffffffffbd0b4043>]irq_exit+0xc3/0xf0
> [ 7911.263167] ---[ end trace 0d6ecffc816873c4 ]---
> --
> 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
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-03 23:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-19 7:44 [PATCH] btrfs: qgroup: Fix root item corruption when multiple same source snapshiots are created with quota enabled Qu Wenruo
2018-02-02 11:45 ` Filipe Manana
2018-03-07 16:22 ` David Sterba
2018-05-03 16:43 ` David Sterba
2018-05-03 23:32 ` Qu Wenruo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.