Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

btrfs-progs: docs: extra notes about read-only scrub on read-write fs #935

Open
wants to merge 7 commits into
base: devel
Choose a base branch
from

Conversation

adam900710
Copy link
Collaborator

[BUG]
There is a bug report that read-only scrub on a read-write fs still causes writes into the fs, and that will be caught if there is a read-only block device among the storage stack.

This will cause a kernel warning on failed transaction commit:

 BTRFS info (device dm-3): first mount of filesystem e18f0c40-88de-413f-9d7e-dcc8136ad6dd
 BTRFS info (device dm-3): using crc32c (crc32c-intel) checksum algorithm
 BTRFS info (device dm-3): using free-space-tree
 BTRFS info (device dm-3): scrub: started on devid 1
 Trying to write to read-only block-device md127
 btrfs_dev_stat_inc_and_print: 362 callbacks suppressed
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 1, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 2, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 3, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 4, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 5, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 6, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 7, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 8, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 9, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 10, rd 0, flush 0, corrupt 0, gen 0
 BTRFS: error (device dm-3) in btrfs_commit_transaction:2523: errno=-5 IO failure (Error while writing out transaction)
 BTRFS info (device dm-3 state E): forced readonly
 BTRFS warning (device dm-3 state E): Skipping commit of aborted transaction.
 BTRFS error (device dm-3 state EA): Transaction aborted (error -5)
 BTRFS: error (device dm-3 state EA) in cleanup_transaction:2017: errno=-5 IO failure
 BTRFS warning (device dm-3 state EA): failed setting block group ro: -5
 BTRFS info (device dm-3 state EA): scrub: not finished on devid 1 with status: -5

[CAUSE]
The root cause is inside btrfs_inc_block_group_ro(), where we need to hold a transaction handle, to prevent the transaction to be committed, until we hold ro_block_group_mutex.

This will cause an empty transaction by itself, thus even if we can mark the block group read-only without any extra workload, we still need to commit the new and empty transaction.

Unfortunately this means RO scrub on RW filesystem will always cause the fs to be updated.

[FIX]
The best fix is to make btrfs to avoid empty commit transaction, but even with that done, read-only scrub on rw mount can still cause real metadata updates (e.g. allocate new chunks and update device error statistics).

It will be very complex to make read-only scrub to be fully read-only on a read-write btrfs.

Thankfully read-only scrub on read-write mount with read-only device in the storage stack is pretty rare, thus a documentation update should be enough.

Issue: #934

adam900710 and others added 7 commits December 13, 2024 15:16
The function btrfs_record_file_extent() has extra handling that's
specific to convert, like allowing the range to be split by block group
boundary and image file extent boundary.

All of these split can only lead to corruption for non-converted fs.
As the only caller out of btrfs-convert is rootdir, which expects the
file extent item insert to respect the reserved data extent, and never
to be split.

Thankfully this is not going to cause huge problem, as
btrfs_record_file_extent() has extra checks if the data extent overlaps
with any existing one, and if it doesn't the handling will be the same
as the kernel.

But to avoid abuse, change btrfs_record_file_extent() by:

- Rename it to btrfs_convert_file_extent()
  And add extra comments on that it is specific to btrfs-convert.

- Move it to convert/common.[ch]

- Introduce a helper insert_reserved_file_extent() for rootdir.c

Signed-off-by: Qu Wenruo <[email protected]>
…extent item

Just like insert_reserved_file_extent() from the kernel, we can make
btrfs_insert_file_extent() accept an on-stack file extent item
directly.

This makes btrfs_insert_file_extent() more flex, and it can now handle
the converted file extent where it has an non-zero offset.

And this makes it much easier to expand for future compressed file
extent generation.

Signed-off-by: Qu Wenruo <[email protected]>
There were two major problems with add_file_items(): it was
writing all files sector-by-sector, making compression impossible, and
it was assuming that pread would never do a short read.

Fix these problems, and create a new helper add_file_item_extent().

Signed-off-by: Mark Harmstone <[email protected]>
Add an option --compress to mkfs.btrfs, to allow creating files
using zlib when using --rootdir.

Signed-off-by: Mark Harmstone <[email protected]>
Allow --compress to work with zstd, when compiled in.

Signed-off-by: Mark Harmstone <[email protected]>
Allow --compress to work with lzo.

Signed-off-by: Mark Harmstone <[email protected]>
[ Add extra handling when LZO support is not compiled in ]
Signed-off-by: Qu Wenruo <[email protected]>
[BUG]
There is a bug report that read-only scrub on a read-write fs still
causes writes into the fs, and that will be caught if there is a
read-only block device among the storage stack.

This will cause a kernel warning on failed transaction commit:

 BTRFS info (device dm-3): first mount of filesystem e18f0c40-88de-413f-9d7e-dcc8136ad6dd
 BTRFS info (device dm-3): using crc32c (crc32c-intel) checksum algorithm
 BTRFS info (device dm-3): using free-space-tree
 BTRFS info (device dm-3): scrub: started on devid 1
 Trying to write to read-only block-device md127
 btrfs_dev_stat_inc_and_print: 362 callbacks suppressed
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 1, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 2, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 3, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 4, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 5, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 6, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 7, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 8, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 9, rd 0, flush 0, corrupt 0, gen 0
 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 10, rd 0, flush 0, corrupt 0, gen 0
 BTRFS: error (device dm-3) in btrfs_commit_transaction:2523: errno=-5 IO failure (Error while writing out transaction)
 BTRFS info (device dm-3 state E): forced readonly
 BTRFS warning (device dm-3 state E): Skipping commit of aborted transaction.
 BTRFS error (device dm-3 state EA): Transaction aborted (error -5)
 BTRFS: error (device dm-3 state EA) in cleanup_transaction:2017: errno=-5 IO failure
 BTRFS warning (device dm-3 state EA): failed setting block group ro: -5
 BTRFS info (device dm-3 state EA): scrub: not finished on devid 1 with status: -5

[CAUSE]
The root cause is inside btrfs_inc_block_group_ro(), where we need to
hold a transaction handle, to prevent the transaction to be committed,
until we hold ro_block_group_mutex.

This will cause an empty transaction by itself, thus even if we can mark
the block group read-only without any extra workload, we still need to
commit the new and empty transaction.

Unfortunately this means RO scrub on RW filesystem will always cause the
fs to be updated.

[FIX]
The best fix is to make btrfs to avoid empty commit transaction, but
even with that done, read-only scrub on rw mount can still cause real
metadata updates (e.g. allocate new chunks and update device error
statistics).

It will be very complex to make read-only scrub to be fully read-only
on a read-write btrfs.

Thankfully read-only scrub on read-write mount with read-only device in
the storage stack is pretty rare, thus a documentation update should be
enough.

Issue: kdave#934
Signed-off-by: Qu Wenruo <[email protected]>
Copy link
Owner

@kdave kdave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This got mixed with the compression patches so I've commented only the documentation change that's clear.

Note that a read-only scrub on a read-write filesystem can
still cause write into the filesystem due to some internal
limitations.
Only a read-only scrub on a read-only fs can avoid writes from
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fs

filesystem

@@ -46,6 +46,16 @@ read-write mounted filesystem.
used, with expert guidance, to rebuild certain corrupted filesystem structures
in the absence of any good replica.

.. note::
Read-only scrub on read-write filesystem will cause some write into the
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on read-write

on a read-write

some write

some writes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants