[PATCH] btrfs: qgroup: Fix root item corruption when multiple same source snapshiots are created with quota enabled
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
Signed-off-by: Qu Wenruo
---
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: qgroup: Fix root item corruption when multiple same source snapshiots are created with quota enabled
On Tue, Dec 19, 2017 at 7:44 AM, 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
> Signed-off-by: Qu Wenruo
Reviewed-by: Filipe Manana
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 majord...@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.”
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: qgroup: Fix root item corruption when multiple same source snapshiots are created with quota enabled
On Fri, Feb 02, 2018 at 11:45:46AM +, Filipe Manana wrote: > On Tue, Dec 19, 2017 at 7:44 AM, 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 > > Signed-off-by: Qu Wenruo > Reviewed-by: Filipe Manana I overlooked the patch, sorry. Added to next now, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: qgroup: Fix root item corruption when multiple same source snapshiots are created with quota enabled
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
> Signed-off-by: Qu Wenruo
> ---
> 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:bc7b04f1ba50 EFLAGS: 00010202
[ 7911.001345] RAX: 963ef63bcd08 RBX: 963e069239d8 RCX:963f0a89e7e8
[ 7911.008645] RDX: RSI: 963f0a89e7e8 RDI:963f0278b028
[ 7911.015940] RBP: bc7b04f1bb70 R08: 963e06923a10 R09:0001
[ 7911.023227] R10: 0020 R11: R12:963f2247c000
[ 7911.030523] R13: 963f0b3e0008 R14: 963f0f9608c8 R15:963f06561a60
[ 7911.037834] FS: 7fa037b1d8c0()
GS:963f26c0()knlGS:
[ 7911.047965] CS: 0010 DS: ES: CR0: 80050033
[ 7911.053871] CR2: 7ffeb64a8000 CR3: 000213d7f000 CR4:06e0
[ 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:7ffeb64ad058 EFLAGS: 0246
ORIG_RAX:0010
[ 7911.169028] RAX: ffda RBX: 7ffeb64c2370 RCX:7fa036bba417
[ 7911.176316] RDX: 7ffeb64ad0a0 RSI: 50009417 RDI:0003
[ 7911.183606] RBP: 00e63020 R08: R09:7fa036e73698
[ 7911.190883] R10: R11: 0246 R12:00e64260
[ 7911.198171] R13: 7ffeb64ae1d0 R14: 000e R15:7ffeb64ad0a0
[ 7911.224866] irq event stamp: 8428
[ 7911.228311] hardirqs last enabled at (8427):
[]cmpxchg_double_slab.isra.42+0x190/0x1b0
[ 7911.238215] hardirqs last disabled at (8428):
[]error_entry+0x6c/0xc0
[ 7911.246530] softirqs last enabled at (8418):
[]__do_softirq+0x452/0x7b4
[ 7911.255112] softirqs last disabled at (8393):
[]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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: qgroup: Fix root item corruption when multiple same source snapshiots are created with quota enabled
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
>> Signed-off-by: Qu Wenruo
>> ---
>> 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:bc7b04f1ba50 EFLAGS: 00010202
> [ 7911.001345] RAX: 963ef63bcd08 RBX: 963e069239d8
> RCX:963f0a89e7e8
> [ 7911.008645] RDX: RSI: 963f0a89e7e8
> RDI:963f0278b028
> [ 7911.015940] RBP: bc7b04f1bb70 R08: 963e06923a10
> R09:0001
> [ 7911.023227] R10: 0020 R11:
> R12:963f2247c000
> [ 7911.030523] R13: 963f0b3e0008 R14: 963f0f9608c8
> R15:963f06561a60
> [ 7911.037834] FS: 7fa037b1d8c0()
> GS:963f26c0()knlGS:
> [ 7911.047965] CS: 0010 DS: ES: CR0: 80050033
> [ 7911.053871] CR2: 7ffeb64a8000 CR3: 000213d7f000
> CR4:06e0
> [ 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:7ffeb64ad058 EFLAGS: 0246
> ORIG_RAX:0010
> [ 7911.169028] RAX: ffda RBX: 7ffeb64c2370
> RCX:7fa036bba417
> [ 7911.176316] RDX: 7ffeb64ad0a0 RSI: 50009417
> RDI:0003
> [ 7911.183606] RBP: 00e63020 R08:
> R09:7fa036e73698
> [ 7911.190883] R10: R11: 0246
> R12:00e64260
> [ 7911.198171] R13: 7ffeb64ae1d0 R14: 000e
> R15:7ffeb64ad0a0
> [ 7911.224866] irq event stamp: 8428
> [ 7911.228311] hardirqs last enabled at (8427):
> []cmpxchg_double_slab.isra.42+0x190/0x1b0
> [ 7911.238215] hardirqs last disabled at (8428):
> []error_entry+0x6c/0xc0
> [ 7911.246530] softirqs last enabled at (8418):
> []__do_softirq+0x452/0x7b4
> [ 7911.255112] softirqs last disabled at (8393):
> []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 m
