On Tue, Jun 5, 2018 at 12:36 AM, Qu Wenruo <w...@suse.com> wrote:
> [BUG]
> Btrfs can easily create compressed extent without checksum (even
> though it shouldn't), and if we then try to replace device containing
> such extent, the result device will contain all the uncompressed data
> instead of the compressed one.
>
> Test case already submitted to fstests:
> https://patchwork.kernel.org/patch/10442353/
>
> [CAUSE]
> When handling compressed extent without checksum, device replace will
> goes into copy_nocow_pages() function.
>
> In that function, btrfs will get all inodes referring to this data
> extents and then use find_or_create_page() to get pages direct from that
> inode.
>
> The problem here is, pages directly from inode are always uncompressed.
> And for compressed data extent, they mismatch with on-disk data.
> Thus this leads to corrupted compressed data extent written to replace
> device.
>
> [FIX]
> In this attempt, we could just remove the "optimization" branch, and let
> unified scrub_pages() to handle it.
>
> Although scrub_pages() won't bother reusing page cache, it will be a
> little slower, but it does the correct csum checking and won't cause
> such data corruption cause by "optimization".
>
> Reported-by: James Harvey <jamespharve...@gmail.com>
> Signed-off-by: Qu Wenruo <w...@suse.com>

Reviewed-by: James Harvey <jamespharve...@gmail.com>



As expected, this fixes the problem.

I originally ran into this running btrfs replace on devid 1, on a 3
device RAID1.

I temporarily restored dd images of devid 2 & 3, from before I ran the
original replace.  I was able to run "btrfs replace" with a btrfs
module loaded with this patch.

There's definitely no corruption.  I excessively checked, multiple ways.

I haven't tried with the previous RFC patch since it looks like this
one is getting used instead, but if requested to, I'd try that one.
--
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

Reply via email to