Thread (117 messages) 117 messages, 8 authors, 2021-05-30

Re: [Patch v2 01/42] btrfs: scrub: fix subpage scrub repair error caused by hardcoded PAGE_SIZE

From: Qu Wenruo <hidden>
Date: 2021-05-13 23:33:08


On 2021/5/14 上午6:57, David Sterba wrote:
On Wed, Apr 28, 2021 at 07:03:08AM +0800, Qu Wenruo wrote:
quoted
[BUG]
For the following file layout, btrfs scrub will not be able to repair
all these two repairable error, but in fact make one corruption even
unrepairable:

	  inode offset 0      4k     8K
Mirror 1               |XXXXXX|      |
Mirror 2               |      |XXXXXX|

[CAUSE]
The root cause is the hard coded PAGE_SIZE, which makes scrub repair to
go crazy for subpage.

For above case, when reading the first sector, we use PAGE_SIZE other
than sectorsize to read, which makes us to read the full range [0, 64K).
In fact, after 8K there may be no data at all, we can just get some
garbage.

Then when doing the repair, we also writeback a full page from mirror 2,
this means, we will also writeback the corrupted data in mirror 2 back
to mirror 1, leaving the range [4K, 8K) unrepairable.

[FIX]
This patch will modify the following PAGE_SIZE use with sectorsize:
Let me take this as an example: the changelog is great and descriptive,
the only thing I often change is an extra newline between the
introductory paragraph ended by ":" and the item list. This is maybe a
personal preference but I find it easier to read.
Thanks for pointing out, in fact I'm not sure whether a new inline
should be added.

Now I have a solid answer and will not longer be stingy to use new lines.

Thanks,
Qu
quoted
- scrub_print_warning_inode()
   Remove the min() and replace PAGE_SIZE with sectorsize.
   The min() makes no sense, as csum is done for the full sector with
   padding.

   This fixes a bug that subpage report extra length like:
    checksum error at logical 298844160 on dev /dev/mapper/arm_nvme-test,
    physical 575668224, root 5, inode 257, offset 0, length 12288, links 1 (path: file)

   Where the error is only 1 sector.

- scrub_handle_errored_block()
   Comments with PAGE|page involved, all changed to sector.

- scrub_setup_recheck_block()
- scrub_repair_page_from_good_copy()
- scrub_add_page_to_wr_bio()
- scrub_wr_submit()
- scrub_add_page_to_rd_bio()
- scrub_block_complete()
   Replace PAGE_SIZE with sectorsize.
   This solves several problems where we read/write extra range for
   subpage case.
...
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help