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

System block groups are classified too, not just metadata ones.  A system
chunk carries no METADATA bit, so a flags test excludes it, but it holds
the chunk tree: the same tree block header, the same generation field, and
a worse consequence if a degraded crash tears it, since without the chunk
tree no logical address can be mapped at all.  meta_rmw already counted
system chunks; only the classification skipped them, so their read-modify-
writes were visible as a rate and never as a verdict.  Whether a raid56
system chunk ever rewrites parity over a committed tree block is therefore
an open question that this answers by measurement rather than by reasoning
about how the chunk tree is laid out.

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 2c212a02479bd0fd6bd1ce4eb25087c464a5a975..f503cd0953b8ef67c189888026aa38fce5745ff3 100644 (file)
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -819,6 +819,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 b4fc827224e4f5b6636ba1133f656a5a6cbae2ee..02e954200c73f63230f66e17afd3c5198f367d2e 100644 (file)
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -2803,6 +2803,132 @@ 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;
+
+       /*
+        * Tree blocks only, which means metadata block groups and system
+        * block groups both: a system chunk carries no METADATA bit, but it
+        * holds the chunk tree, whose blocks have the same header and whose
+        * loss is worse than any other tree's -- without it nothing maps.
+        * A mixed block group's sectors need not be tree blocks at all, so
+        * anything carrying DATA is excluded.
+        */
+       if (rbio->bioc->map_type & BTRFS_BLOCK_GROUP_DATA)
+               return;
+       if (!(rbio->bioc->map_type & (BTRFS_BLOCK_GROUP_METADATA |
+                                     BTRFS_BLOCK_GROUP_SYSTEM)))
+               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.
  *
@@ -2835,6 +2961,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 e4b70de5d414a099714c0d236c7bea28b650c8da..4fcd55bbb57532dab472d9d9e10339b02f886a28 100644 (file)
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1126,7 +1126,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),
@@ -1134,7 +1137,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);