btrfs: fallback to transaction csum tree on a commit root csum miss

We have been running with commit root csums enabled for some time and
have noticed a slight uptick in zero csum errors. Investigating those
revealed that they were same transaction reads of extents that were just
relocated, but the extent map generation was long ago.

It turns out that relocation intentionally does not update the extent
generation (replace_file_extents()), but must write a new csum since the
data has moved, so we must account for this with commit root csum reading.

Luckily this is a short lived condition: after the relocation transaction
the commit root will once again have the csum. So we can add a generic
fallback to the lookup to try again with the transaction csum root.

Fixes: f07b855c56b1 ("btrfs: try to search for data csums in commit root")
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Boris Burkov <boris@bur.io>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index 9f6454e9db818635eb5a2cc97dcde6bcdc804d00..cf50fd623f41a8013fc3a70c76fb93ee20a893a9 100644 (file)
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -358,6 +358,7 @@ int btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
        const unsigned int nblocks = orig_len >> fs_info->sectorsize_bits;
        int ret = 0;
        u32 bio_offset = 0;
+       bool using_commit_root = false;
 
        if ((inode->flags & BTRFS_INODE_NODATASUM) ||
            test_bit(BTRFS_FS_STATE_NO_DATA_CSUMS, &fs_info->fs_state))
@@ -431,6 +432,7 @@ int btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
         * from across transactions.
         */
        if (bbio->csum_search_commit_root) {
+               using_commit_root = true;
                path->search_commit_root = true;
                path->skip_locking = true;
                down_read(&fs_info->commit_root_sem);
@@ -463,6 +465,28 @@ int btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
                 * assume this is the case.
                 */
                if (count == 0) {
+                       /*
+                        * If an extent is relocated in the current transaction
+                        * then relocation writes a new csum without updating
+                        * the extent map generation. Until the next commit, we
+                        * will see a hole in that case, so we need to fallback
+                        * to searching the transaction csum root.
+                        *
+                        * Note that a commit root lookup of a referenced extent can
+                        * only miss, not return a stale csum. A freed extent's csum
+                        * is deleted in the same transaction and its bytenr is not
+                        * reusable until that transaction has committed and the
+                        * extent is unpinned.
+                        */
+                       if (using_commit_root) {
+                               up_read(&fs_info->commit_root_sem);
+                               using_commit_root = false;
+                               path->search_commit_root = false;
+                               path->skip_locking = false;
+                               btrfs_release_path(path);
+                               continue;
+                       }
+
                        memset(csum_dst, 0, csum_size);
                        count = 1;
 
@@ -481,7 +505,7 @@ int btrfs_lookup_bio_sums(struct btrfs_bio *bbio)
                bio_offset += count * sectorsize;
        }
 
-       if (bbio->csum_search_commit_root)
+       if (using_commit_root)
                up_read(&fs_info->commit_root_sem);
        return ret;
 }