| Message ID | 20241211111315.65007-1-sunjunchao2870@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | btrfs: fix an assertion failure related to squota feature. | expand |
Friendly ping... [+cc Boris] Julian Sun <sunjunchao2870@gmail.com> 于2024年12月11日周三 19:13写道： > > With the config CONFIG_BTRFS_ASSERT enabled, an assertion > failure occurs regarding the simple quota feature. > > [ 5.596534] assertion failed: btrfs_fs_incompat(fs_info, SIMPLE_QUOTA), in fs/btrfs/qgroup.c:365 > [ 5.597098] ------------[ cut here ]------------ > [ 5.597371] kernel BUG at fs/btrfs/qgroup.c:365! > [ 5.597946] CPU: 1 UID: 0 PID: 268 Comm: mount Not tainted 6.13.0-rc2-00031-gf92f4749861b #146 > [ 5.598450] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 > [ 5.599008] RIP: 0010:btrfs_read_qgroup_config+0x74d/0x7a0 > [ 5.604303] <TASK> > [ 5.605230] ? btrfs_read_qgroup_config+0x74d/0x7a0 > [ 5.605538] ? exc_invalid_op+0x56/0x70 > [ 5.605775] ? btrfs_read_qgroup_config+0x74d/0x7a0 > [ 5.606066] ? asm_exc_invalid_op+0x1f/0x30 > [ 5.606441] ? btrfs_read_qgroup_config+0x74d/0x7a0 > [ 5.606741] ? btrfs_read_qgroup_config+0x74d/0x7a0 > [ 5.607038] ? try_to_wake_up+0x317/0x760 > [ 5.607286] open_ctree+0xd9c/0x1710 > [ 5.607509] btrfs_get_tree+0x58a/0x7e0 > [ 5.608002] vfs_get_tree+0x2e/0x100 > [ 5.608224] fc_mount+0x16/0x60 > [ 5.608420] btrfs_get_tree+0x2f8/0x7e0 > [ 5.608897] vfs_get_tree+0x2e/0x100 > [ 5.609121] path_mount+0x4c8/0xbc0 > [ 5.609538] __x64_sys_mount+0x10d/0x150 > > The issue can be easily reproduced using the following reproduer: > root@q:linux# cat repro.sh > set -e > > mkfs.btrfs -f /dev/sdb > /dev/null > mount /dev/sdb /mnt/btrfs > btrfs quota enable -s /mnt/btrfs > umount /mnt/btrfs > mount /dev/sdb /mnt/btrfs > > The root cause of this issue is as follows: > When simple quota is enabled, the BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA > flag is set after btrfs_commit_transaction(), whereas the > BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE flag is set before btrfs_commit_transaction(), > which led to the first flag not being flushed to disk, and the second > flag is successfully flushed. Finally causes this assertion failure > after umount && mount again. > > To resolve this issue, the BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA flag > is set immediately after setting the BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE. > This ensures that both flags are flushed to disk within the same > transaction. > > Signed-off-by: Julian Sun <sunjunchao2870@gmail.com> > --- > fs/btrfs/qgroup.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c > index a6f92836c9b1..f9b214992212 100644 > --- a/fs/btrfs/qgroup.c > +++ b/fs/btrfs/qgroup.c > @@ -1121,6 +1121,7 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info, > fs_info->qgroup_flags = BTRFS_QGROUP_STATUS_FLAG_ON; > if (simple) { > fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE; > + btrfs_set_fs_incompat(fs_info, SIMPLE_QUOTA); > btrfs_set_qgroup_status_enable_gen(leaf, ptr, trans->transid); > } else { > fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT; > @@ -1254,8 +1255,6 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info, > spin_lock(&fs_info->qgroup_lock); > fs_info->quota_root = quota_root; > set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags); > - if (simple) > - btrfs_set_fs_incompat(fs_info, SIMPLE_QUOTA); > spin_unlock(&fs_info->qgroup_lock); > > /* Skip rescan for simple qgroups. */ > -- > 2.39.5 >
On Wed, Dec 11, 2024 at 11:13 AM Julian Sun <sunjunchao2870@gmail.com> wrote: > > With the config CONFIG_BTRFS_ASSERT enabled, an assertion > failure occurs regarding the simple quota feature. > > [ 5.596534] assertion failed: btrfs_fs_incompat(fs_info, SIMPLE_QUOTA), in fs/btrfs/qgroup.c:365 > [ 5.597098] ------------[ cut here ]------------ > [ 5.597371] kernel BUG at fs/btrfs/qgroup.c:365! > [ 5.597946] CPU: 1 UID: 0 PID: 268 Comm: mount Not tainted 6.13.0-rc2-00031-gf92f4749861b #146 > [ 5.598450] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 > [ 5.599008] RIP: 0010:btrfs_read_qgroup_config+0x74d/0x7a0 > [ 5.604303] <TASK> > [ 5.605230] ? btrfs_read_qgroup_config+0x74d/0x7a0 > [ 5.605538] ? exc_invalid_op+0x56/0x70 > [ 5.605775] ? btrfs_read_qgroup_config+0x74d/0x7a0 > [ 5.606066] ? asm_exc_invalid_op+0x1f/0x30 > [ 5.606441] ? btrfs_read_qgroup_config+0x74d/0x7a0 > [ 5.606741] ? btrfs_read_qgroup_config+0x74d/0x7a0 > [ 5.607038] ? try_to_wake_up+0x317/0x760 > [ 5.607286] open_ctree+0xd9c/0x1710 > [ 5.607509] btrfs_get_tree+0x58a/0x7e0 > [ 5.608002] vfs_get_tree+0x2e/0x100 > [ 5.608224] fc_mount+0x16/0x60 > [ 5.608420] btrfs_get_tree+0x2f8/0x7e0 > [ 5.608897] vfs_get_tree+0x2e/0x100 > [ 5.609121] path_mount+0x4c8/0xbc0 > [ 5.609538] __x64_sys_mount+0x10d/0x150 > > The issue can be easily reproduced using the following reproduer: > root@q:linux# cat repro.sh > set -e > > mkfs.btrfs -f /dev/sdb > /dev/null > mount /dev/sdb /mnt/btrfs > btrfs quota enable -s /mnt/btrfs > umount /mnt/btrfs > mount /dev/sdb /mnt/btrfs > > The root cause of this issue is as follows: > When simple quota is enabled, the BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA > flag is set after btrfs_commit_transaction(), whereas the > BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE flag is set before btrfs_commit_transaction(), > which led to the first flag not being flushed to disk, and the second > flag is successfully flushed. Finally causes this assertion failure > after umount && mount again. > > To resolve this issue, the BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA flag > is set immediately after setting the BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE. > This ensures that both flags are flushed to disk within the same > transaction. > > Signed-off-by: Julian Sun <sunjunchao2870@gmail.com> Fixes: 182940f4f4db ("btrfs: qgroup: add new quota mode for simple quotas") Reviewed-by: Filipe Manana <fdmanana@suse.com> Looks good, thanks. We should also turn the reproducer into a test case for fstests. > --- > fs/btrfs/qgroup.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c > index a6f92836c9b1..f9b214992212 100644 > --- a/fs/btrfs/qgroup.c > +++ b/fs/btrfs/qgroup.c > @@ -1121,6 +1121,7 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info, > fs_info->qgroup_flags = BTRFS_QGROUP_STATUS_FLAG_ON; > if (simple) { > fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE; > + btrfs_set_fs_incompat(fs_info, SIMPLE_QUOTA); > btrfs_set_qgroup_status_enable_gen(leaf, ptr, trans->transid); > } else { > fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT; > @@ -1254,8 +1255,6 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info, > spin_lock(&fs_info->qgroup_lock); > fs_info->quota_root = quota_root; > set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags); > - if (simple) > - btrfs_set_fs_incompat(fs_info, SIMPLE_QUOTA); > spin_unlock(&fs_info->qgroup_lock); > > /* Skip rescan for simple qgroups. */ > -- > 2.39.5 > >
On Mon, Dec 16, 2024 at 5:35 PM Filipe Manana <fdmanana@kernel.org> wrote: > > On Wed, Dec 11, 2024 at 11:13 AM Julian Sun <sunjunchao2870@gmail.com> wrote: > > > > With the config CONFIG_BTRFS_ASSERT enabled, an assertion > > failure occurs regarding the simple quota feature. > > > > [ 5.596534] assertion failed: btrfs_fs_incompat(fs_info, SIMPLE_QUOTA), in fs/btrfs/qgroup.c:365 > > [ 5.597098] ------------[ cut here ]------------ > > [ 5.597371] kernel BUG at fs/btrfs/qgroup.c:365! > > [ 5.597946] CPU: 1 UID: 0 PID: 268 Comm: mount Not tainted 6.13.0-rc2-00031-gf92f4749861b #146 > > [ 5.598450] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 > > [ 5.599008] RIP: 0010:btrfs_read_qgroup_config+0x74d/0x7a0 > > [ 5.604303] <TASK> > > [ 5.605230] ? btrfs_read_qgroup_config+0x74d/0x7a0 > > [ 5.605538] ? exc_invalid_op+0x56/0x70 > > [ 5.605775] ? btrfs_read_qgroup_config+0x74d/0x7a0 > > [ 5.606066] ? asm_exc_invalid_op+0x1f/0x30 > > [ 5.606441] ? btrfs_read_qgroup_config+0x74d/0x7a0 > > [ 5.606741] ? btrfs_read_qgroup_config+0x74d/0x7a0 > > [ 5.607038] ? try_to_wake_up+0x317/0x760 > > [ 5.607286] open_ctree+0xd9c/0x1710 > > [ 5.607509] btrfs_get_tree+0x58a/0x7e0 > > [ 5.608002] vfs_get_tree+0x2e/0x100 > > [ 5.608224] fc_mount+0x16/0x60 > > [ 5.608420] btrfs_get_tree+0x2f8/0x7e0 > > [ 5.608897] vfs_get_tree+0x2e/0x100 > > [ 5.609121] path_mount+0x4c8/0xbc0 > > [ 5.609538] __x64_sys_mount+0x10d/0x150 > > > > The issue can be easily reproduced using the following reproduer: > > root@q:linux# cat repro.sh > > set -e > > > > mkfs.btrfs -f /dev/sdb > /dev/null > > mount /dev/sdb /mnt/btrfs > > btrfs quota enable -s /mnt/btrfs > > umount /mnt/btrfs > > mount /dev/sdb /mnt/btrfs > > > > The root cause of this issue is as follows: > > When simple quota is enabled, the BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA > > flag is set after btrfs_commit_transaction(), whereas the > > BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE flag is set before btrfs_commit_transaction(), > > which led to the first flag not being flushed to disk, and the second > > flag is successfully flushed. Finally causes this assertion failure > > after umount && mount again. > > > > To resolve this issue, the BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA flag > > is set immediately after setting the BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE. > > This ensures that both flags are flushed to disk within the same > > transaction. > > > > Signed-off-by: Julian Sun <sunjunchao2870@gmail.com> > > Fixes: 182940f4f4db ("btrfs: qgroup: add new quota mode for simple quotas") > Reviewed-by: Filipe Manana <fdmanana@suse.com> > > Looks good, thanks. > We should also turn the reproducer into a test case for fstests. Pushed it to the for-next branch with a few changes to the changelog: 1) Renamed the subject to "btrfs: fix transaction atomicity bug when enabling simple quotas". Note that we don't add punctuation at the end of the subject line. 2) Changed the paragraphs that explain the bug below the reproducer to: "The issue is that when enabling quotas, at btrfs_quota_enable(), we set BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE at fs_info->qgroup_flags and persist it in the quota root in the item with the key BTRFS_QGROUP_STATUS_KEY, but we only set the incompat bit BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA after we commit the transaction used to enable simple quotas. This means that if after that transaction commit we unmount the filesystem without starting and committing any other transaction, or we have a power failure, the next time we mount the filesystem we will find the flag BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE set in the item with the key BTRFS_QGROUP_STATUS_KEY but we will not find the incompat bit BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA set in the superblock, triggering an assertion failure at: btrfs_read_qgroup_config() -> qgroup_read_enable_gen() To fix this issue, set the BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA flag immediately after setting the BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE. This ensures that both flags are flushed to disk within the same transaction." Thanks. > > > --- > > fs/btrfs/qgroup.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c > > index a6f92836c9b1..f9b214992212 100644 > > --- a/fs/btrfs/qgroup.c > > +++ b/fs/btrfs/qgroup.c > > @@ -1121,6 +1121,7 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info, > > fs_info->qgroup_flags = BTRFS_QGROUP_STATUS_FLAG_ON; > > if (simple) { > > fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE; > > + btrfs_set_fs_incompat(fs_info, SIMPLE_QUOTA); > > btrfs_set_qgroup_status_enable_gen(leaf, ptr, trans->transid); > > } else { > > fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT; > > @@ -1254,8 +1255,6 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info, > > spin_lock(&fs_info->qgroup_lock); > > fs_info->quota_root = quota_root; > > set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags); > > - if (simple) > > - btrfs_set_fs_incompat(fs_info, SIMPLE_QUOTA); > > spin_unlock(&fs_info->qgroup_lock); > > > > /* Skip rescan for simple qgroups. */ > > -- > > 2.39.5 > > > >
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c index a6f92836c9b1..f9b214992212 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -1121,6 +1121,7 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info, fs_info->qgroup_flags = BTRFS_QGROUP_STATUS_FLAG_ON; if (simple) { fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE; + btrfs_set_fs_incompat(fs_info, SIMPLE_QUOTA); btrfs_set_qgroup_status_enable_gen(leaf, ptr, trans->transid); } else { fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT; @@ -1254,8 +1255,6 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info, spin_lock(&fs_info->qgroup_lock); fs_info->quota_root = quota_root; set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags); - if (simple) - btrfs_set_fs_incompat(fs_info, SIMPLE_QUOTA); spin_unlock(&fs_info->qgroup_lock); /* Skip rescan for simple qgroups. */
With the config CONFIG_BTRFS_ASSERT enabled, an assertion failure occurs regarding the simple quota feature. [ 5.596534] assertion failed: btrfs_fs_incompat(fs_info, SIMPLE_QUOTA), in fs/btrfs/qgroup.c:365 [ 5.597098] ------------[ cut here ]------------ [ 5.597371] kernel BUG at fs/btrfs/qgroup.c:365! [ 5.597946] CPU: 1 UID: 0 PID: 268 Comm: mount Not tainted 6.13.0-rc2-00031-gf92f4749861b #146 [ 5.598450] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014 [ 5.599008] RIP: 0010:btrfs_read_qgroup_config+0x74d/0x7a0 [ 5.604303] <TASK> [ 5.605230] ? btrfs_read_qgroup_config+0x74d/0x7a0 [ 5.605538] ? exc_invalid_op+0x56/0x70 [ 5.605775] ? btrfs_read_qgroup_config+0x74d/0x7a0 [ 5.606066] ? asm_exc_invalid_op+0x1f/0x30 [ 5.606441] ? btrfs_read_qgroup_config+0x74d/0x7a0 [ 5.606741] ? btrfs_read_qgroup_config+0x74d/0x7a0 [ 5.607038] ? try_to_wake_up+0x317/0x760 [ 5.607286] open_ctree+0xd9c/0x1710 [ 5.607509] btrfs_get_tree+0x58a/0x7e0 [ 5.608002] vfs_get_tree+0x2e/0x100 [ 5.608224] fc_mount+0x16/0x60 [ 5.608420] btrfs_get_tree+0x2f8/0x7e0 [ 5.608897] vfs_get_tree+0x2e/0x100 [ 5.609121] path_mount+0x4c8/0xbc0 [ 5.609538] __x64_sys_mount+0x10d/0x150 The issue can be easily reproduced using the following reproduer: root@q:linux# cat repro.sh set -e mkfs.btrfs -f /dev/sdb > /dev/null mount /dev/sdb /mnt/btrfs btrfs quota enable -s /mnt/btrfs umount /mnt/btrfs mount /dev/sdb /mnt/btrfs The root cause of this issue is as follows: When simple quota is enabled, the BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA flag is set after btrfs_commit_transaction(), whereas the BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE flag is set before btrfs_commit_transaction(), which led to the first flag not being flushed to disk, and the second flag is successfully flushed. Finally causes this assertion failure after umount && mount again. To resolve this issue, the BTRFS_FEATURE_INCOMPAT_SIMPLE_QUOTA flag is set immediately after setting the BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE. This ensures that both flags are flushed to disk within the same transaction. Signed-off-by: Julian Sun <sunjunchao2870@gmail.com> --- fs/btrfs/qgroup.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)