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