btrfs: raid56: say whether a metadata read-modify-write is a write hole

meta_rmw counts sub-stripe metadata writes, which is a proxy for exposure
rather than a measurement of it.  A stripe modified in place is only a write
hole if it holds data some completed transaction is relying on; a stripe
that two writes of the *same* transaction happen to split costs an extra
read but risks nothing, because a tear loses that whole transaction anyway.
The counter cannot tell those apart, so it cannot answer the only question
that matters.

The rbio can.  A read-modify-write has already read every column the write
does not cover, so at the point of the report it is holding the stripe's
on-disk contents.  Walk the uncovered tree block positions and read their
headers: a block whose bytenr and fsid match belongs there, and its
generation says which transaction put it there.  Compare that against the
generation of the blocks this write is carrying, taken from the same rbio,
so a transaction committing concurrently cannot skew the verdict:

  meta_rmw_cur   the same transaction's own blocks -- a cost
  meta_rmw_old   an earlier transaction's -- a write hole
  meta_rmw_free  no tree block there at all

This trusts nothing the allocator says about itself.  The stripe runs, the
liveness map and the drain accounting are all bookkeeping that could be
wrong in the same way twice; the header in the sector is what a degraded
read would actually have to reconstruct.

On a 3-device raid5 filesystem with raid5 metadata, 6000 small files with
periodic syncs and then a third rewritten, plain stripe_alloc reports 249
sub-stripe metadata writes of which 174 rewrite parity over committed tree
blocks.  Adding stripe_meta leaves 37 sub-stripe writes and none of them.

Assisted-by: Claude:claude-fable-5

diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
index 59c9b7a3e3256dcbc457b3bbb7e1c5c8a9d1f3cb..c73b7990c99fd45e19d3ca1f39d4cbb6f9798b7c 100644 (file)
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -818,6 +818,15 @@ struct btrfs_fs_info {
                atomic64_t rmw_reads;
                atomic64_t padded;
                atomic64_t meta_rmw;
+               /*
+                * Breakdown of what a metadata read-modify-write found in the
+                * columns it did not cover: a tree block of the generation
+                * being written (a cost), an older one (a write hole), or no
+                * tree block at all.  See audit_uncovered_rmw().
+                */
+               atomic64_t meta_rmw_cur;
+               atomic64_t meta_rmw_old;
+               atomic64_t meta_rmw_free;
        } stripe_park_stats;
        /* BTRFS_STRIPE_RMW_* masks; see btrfs_stripe_allow_rmw(). */
        u32 stripe_rmw_opt;

diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index c3367e8ea0dec87600e451779d8819b156d7c612..db8b1d41981f4c84092f77bf1b14a5e3b327c02c 100644 (file)
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -2798,6 +2798,123 @@ static bool need_read_stripe_sectors(struct btrfs_raid_bio *rbio)
        return false;
 }
 
+/*
+ * Read the header of the tree block at data-stripe sector @i, if there is
+ * one, from whichever copy of the sector this rbio is holding.
+ *
+ * @from_bio picks the source: the caller's pages (what this write is putting
+ * there) or the stripe pages (what an RMW read found on the disk).  Returns
+ * false when the sector holds no tree block that belongs at this address --
+ * free space, or the remains of one from a previous life of the stripe.
+ */
+static bool rmw_peek_eb_generation(struct btrfs_raid_bio *rbio, u32 i,
+                                  bool from_bio, u64 *generation)
+{
+       struct btrfs_fs_info *fs_info = rbio->bioc->fs_info;
+       const int stripe_nr = i / rbio->stripe_nsectors;
+       const int sectornr = i % rbio->stripe_nsectors;
+       const u64 logical = rbio->bioc->full_stripe_logical +
+                           (u64)stripe_nr * BTRFS_STRIPE_LEN +
+                           (u64)sectornr * fs_info->sectorsize;
+       const phys_addr_t *paddrs = from_bio ? rbio->bio_paddrs
+                                            : rbio->stripe_paddrs;
+       struct btrfs_header *hdr;
+       phys_addr_t paddr;
+       u64 found_bytenr;
+       bool bad_fsid;
+       void *kaddr;
+
+       paddr = paddrs[i * rbio->sector_nsteps];
+       if (paddr == INVALID_PADDR)
+               return false;
+       if (!from_bio && !test_bit(i, rbio->stripe_uptodate_bitmap))
+               return false;
+
+       kaddr = kmap_local_paddr(paddr);
+       hdr = kaddr;
+       found_bytenr = btrfs_stack_header_bytenr(hdr);
+       *generation = btrfs_stack_header_generation(hdr);
+       bad_fsid = memcmp(hdr->fsid, fs_info->fs_devices->metadata_uuid,
+                         BTRFS_FSID_SIZE) != 0;
+       kunmap_local(kaddr);
+
+       return found_bytenr == logical && !bad_fsid;
+}
+
+/*
+ * Decide whether an uncovered read-modify-write is a write hole or only a
+ * cost, by looking at what is actually in the stripe.
+ *
+ * An RMW has read every column this write does not cover, so the rbio is
+ * holding the stripe's on-disk contents.  Any tree block found there is
+ * about to have its parity recomputed and rewritten underneath it while its
+ * own sectors stay put -- the classic torn-stripe window.  What decides
+ * whether that matters is the block's generation:
+ *
+ *  - equal to the generation of the blocks this write carries, and the block
+ *    belongs to the transaction now in flight.  A crash loses that whole
+ *    transaction anyway, so a torn stripe costs nothing that was ever
+ *    committed.  This is the residual stripe_meta leaves behind.
+ *
+ *  - older, and the stripe holds committed metadata.  That is a write hole,
+ *    and it means the allocator let a stripe be revisited after the
+ *    transaction that filled it completed.
+ *
+ * The comparison is against this rbio's own covered blocks rather than
+ * fs_info's committed generation so that a transaction committing
+ * concurrently cannot skew the verdict: both numbers come from the same
+ * stripe at the same instant.
+ */
+static void audit_uncovered_rmw(struct btrfs_raid_bio *rbio)
+{
+       struct btrfs_fs_info *fs_info = rbio->bioc->fs_info;
+       const u32 sectors_per_tree = fs_info->nodesize >> fs_info->sectorsize_bits;
+       const u32 nsectors = rbio->nr_data * rbio->stripe_nsectors;
+       u64 write_gen = 0;
+
+       /* A mixed block group's sectors need not be tree blocks at all. */
+       if (!(rbio->bioc->map_type & BTRFS_BLOCK_GROUP_METADATA) ||
+           (rbio->bioc->map_type & BTRFS_BLOCK_GROUP_DATA))
+               return;
+
+       /*
+        * Both loops read bio_paddrs to tell what this write covers, and the
+        * paths that reach here do not all populate it (a cached rbio that
+        * failed to pad never indexes).  Idempotent, so just do it.
+        */
+       index_rbio_pages(rbio);
+
+       /* The generation this write is putting into the stripe. */
+       for (u32 i = 0; i < nsectors; i += sectors_per_tree) {
+               u64 gen;
+
+               if (rmw_peek_eb_generation(rbio, i, true, &gen))
+                       write_gen = max(write_gen, gen);
+       }
+       if (!write_gen)
+               return;
+
+       for (u32 i = 0; i < nsectors; i += sectors_per_tree) {
+               u64 gen;
+
+               /* Covered by this write: it is what we are putting there. */
+               if (rbio->bio_paddrs[i * rbio->sector_nsteps] != INVALID_PADDR)
+                       continue;
+               if (!rmw_peek_eb_generation(rbio, i, false, &gen)) {
+                       atomic64_inc(&fs_info->stripe_park_stats.meta_rmw_free);
+                       continue;
+               }
+               if (gen >= write_gen) {
+                       atomic64_inc(&fs_info->stripe_park_stats.meta_rmw_cur);
+                       continue;
+               }
+               atomic64_inc(&fs_info->stripe_park_stats.meta_rmw_old);
+               btrfs_warn_rl(fs_info,
+"read-modify-write of stripe %llu rewrites parity over committed tree block at generation %llu while writing generation %llu: write hole",
+                             rbio->bioc->full_stripe_logical, gen, write_gen);
+       }
+}
+
 /*
  * Report a sub-stripe write that is going out as a read-modify-write.
  *
@@ -2830,6 +2947,7 @@ static void report_uncovered_rmw(struct btrfs_raid_bio *rbio)
                return;
 
        atomic64_inc(&fs_info->stripe_park_stats.meta_rmw);
+       audit_uncovered_rmw(rbio);
        btrfs_warn_rl(fs_info,
 "read-modify-write of full stripe %llu: this block group is not covered by stripe_alloc, the raid56 write hole applies to it",
                      rbio->bioc->full_stripe_logical);

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 9b2aade621459b5985fec4bdcd78d3fd6ac1f09c..a448aa5b2db3359a6af0a147a9d32772b0b01e9f 100644 (file)
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1239,7 +1239,10 @@ static ssize_t btrfs_stripe_park_stats_show(struct kobject *kobj,
                "expired %lld\n"
                "padded %lld\n"
                "rmw_reads %lld\n"
-               "meta_rmw %lld\n",
+               "meta_rmw %lld\n"
+               "meta_rmw_cur %lld\n"
+               "meta_rmw_old %lld\n"
+               "meta_rmw_free %lld\n",
                atomic64_read(&fs_info->stripe_park_stats.parked),
                atomic64_read(&fs_info->stripe_park_stats.merged_bytes),
                atomic64_read(&fs_info->stripe_park_stats.filled),
@@ -1247,7 +1250,10 @@ static ssize_t btrfs_stripe_park_stats_show(struct kobject *kobj,
                atomic64_read(&fs_info->stripe_park_stats.expired),
                atomic64_read(&fs_info->stripe_park_stats.padded),
                atomic64_read(&fs_info->stripe_park_stats.rmw_reads),
-               atomic64_read(&fs_info->stripe_park_stats.meta_rmw));
+               atomic64_read(&fs_info->stripe_park_stats.meta_rmw),
+               atomic64_read(&fs_info->stripe_park_stats.meta_rmw_cur),
+               atomic64_read(&fs_info->stripe_park_stats.meta_rmw_old),
+               atomic64_read(&fs_info->stripe_park_stats.meta_rmw_free));
 }
 BTRFS_ATTR(, stripe_park_stats, btrfs_stripe_park_stats_show);