btrfs: stripe_alloc: tunable park deadlines, and say why padding declined

The deadline constants are meant to be judged "from measurements rather
than taste", but they are compile-time, so every data point costs a kernel
build and a reboot.  Measuring a buffered fill -- dd, no fsync, so nothing
kicks the park and the backstop is the only exit -- found 1861 of 3984
parks expiring at the 100ms deadline, which is precisely the no-waiter case
the constant exists for and the one with no numbers behind it.

Expose both deadlines as writable sysfs files, clamped to 60s because a
parked rbio holds its stripe lock, with 0 disabling parking for that class
(useful as an experiment in itself):

  /sys/fs/btrfs/<uuid>/stripe_park_timeout_ms       (default 100)
  /sys/fs/btrfs/<uuid>/stripe_park_sync_timeout_ms  (default 3)

Add the counters the stats file cannot supply.  rmw_reads counts only
parked writes that still had to read, so it cannot answer "does this
workload read-modify-write at all"; data_rmw counts every data RMW, which
with allow_rmw empty on a covered block group is the number the
stripe-exclusive claim is about, and it should be zero.

And record why padding refused, because the reasons are different defects:
the rest of the stripe is allocated and its data has not arrived yet
(wait longer, or kick when the frontier advances); no open run covers the
stripe, so the run closed before its own write went down; or the frontier
never reached the stripe at all.  Attributing them turns "some RMW
remains" into a specific thing to fix.

No behaviour change at the defaults.

Assisted-by: Claude:claude-fable-5

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index ed09f006ccca4d2397a7cac3e57e3d87d12f0062..7580690b20e7392d7ca629bd38fdf45f3394937c 100644 (file)
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -1273,14 +1273,19 @@ bool btrfs_stripe_run_pad_start(struct btrfs_fs_info *fs_info,
 {
        struct btrfs_block_group *bg;
        struct btrfs_open_stripe_run *run;
+       atomic64_t *decline = NULL;
        unsigned long flags;
        bool ret = false;
 
-       if (list_empty_careful(&fs_info->open_stripe_bgs))
+       if (list_empty_careful(&fs_info->open_stripe_bgs)) {
+               atomic64_inc(&fs_info->stripe_park_stats.pad_decline_no_run);
                return false;
+       }
        bg = btrfs_lookup_block_group(fs_info, stripe_start);
-       if (!bg)
+       if (!bg) {
+               atomic64_inc(&fs_info->stripe_park_stats.pad_decline_no_run);
                return false;
+       }
 
        spin_lock_irqsave(&bg->stripe_run_lock, flags);
        list_for_each_entry(run, &bg->open_stripe_runs, list) {
@@ -1290,10 +1295,23 @@ bool btrfs_stripe_run_pad_start(struct btrfs_fs_info *fs_info,
                    run->offset < stripe_start + stripe_len) {
                        *pad_from = run->offset;
                        ret = true;
+               } else if (run->offset >= stripe_start + stripe_len) {
+                       /*
+                        * The frontier is past this stripe: every sector of
+                        * it is allocated, so there is nothing dead to pad
+                        * and the write is waiting on a neighbour's data.
+                        */
+                       decline = &fs_info->stripe_park_stats.pad_decline_live;
+               } else {
+                       /* The frontier has not reached the stripe at all. */
+                       decline = &fs_info->stripe_park_stats.pad_decline_unallocated;
                }
                break;
        }
        spin_unlock_irqrestore(&bg->stripe_run_lock, flags);
+       if (!ret)
+               atomic64_inc(decline ? decline :
+                            &fs_info->stripe_park_stats.pad_decline_no_run);
        btrfs_put_block_group(bg);
        return ret;
 }

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 39e3d3d196eb4cad55714e166dc0509e96f6f855..2097b938f43c94d610d5bfb589e9d72e33a37272 100644 (file)
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2782,6 +2782,8 @@ void btrfs_init_fs_info(struct btrfs_fs_info *fs_info)
        INIT_LIST_HEAD(&fs_info->delalloc_roots);
        INIT_LIST_HEAD(&fs_info->caching_block_groups);
        INIT_LIST_HEAD(&fs_info->open_stripe_bgs);
+       fs_info->stripe_park_timeout_ms = BTRFS_RBIO_PARK_TIMEOUT_MS;
+       fs_info->stripe_park_sync_timeout_ms = BTRFS_RBIO_PARK_SYNC_TIMEOUT_MS;
        spin_lock_init(&fs_info->open_stripe_lock);
        spin_lock_init(&fs_info->delalloc_root_lock);
        spin_lock_init(&fs_info->trans_lock);

diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
index c73b7990c99fd45e19d3ca1f39d4cbb6f9798b7c..dda2b6fe08b6920dbbe1c772a96f0d0d39df70f5 100644 (file)
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -513,6 +513,16 @@ struct btrfs_delayed_root {
 struct btrfs_free_space_ctl;
 struct btrfs_free_space;
 
+/*
+ * Park deadline defaults, ms.  Both are tunable at runtime through
+ * /sys/fs/btrfs/<uuid>/stripe_park_{,sync_}timeout_ms; the constants here
+ * are only the initial values.
+ */
+#define BTRFS_RBIO_PARK_TIMEOUT_MS     100
+#define BTRFS_RBIO_PARK_SYNC_TIMEOUT_MS        3
+/* Refuse absurd deadlines: a parked rbio holds its stripe lock. */
+#define BTRFS_RBIO_PARK_TIMEOUT_MS_MAX 60000
+
 struct btrfs_fs_info {
        u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
        unsigned long flags;
@@ -817,6 +827,26 @@ struct btrfs_fs_info {
                atomic64_t kicked;
                atomic64_t rmw_reads;
                atomic64_t padded;
+               /*
+                * Data read-modify-writes on a block group the allocator
+                * covers.  With allow_rmw empty this is the number the
+                * stripe-exclusive claim is about, and it should be zero;
+                * rmw_reads counts only the parked ones, so it cannot say
+                * whether a workload RMWs at all.
+                */
+               atomic64_t data_rmw;
+               /*
+                * Why padding refused, so "some RMW remains" names a defect
+                * instead of a symptom.  live: the rest of the stripe is
+                * allocated and its data has not arrived (wait longer, or
+                * kick when the frontier advances).  no_run: no open run
+                * covers the stripe, so the run closed before its own write
+                * went down (a lifetime problem).  unallocated: the run's
+                * frontier has not reached the stripe being written at all.
+                */
+               atomic64_t pad_decline_live;
+               atomic64_t pad_decline_no_run;
+               atomic64_t pad_decline_unallocated;
                atomic64_t meta_rmw;
                /*
                 * Breakdown of what a metadata read-modify-write found in the
@@ -831,6 +861,14 @@ struct btrfs_fs_info {
        /* BTRFS_STRIPE_RMW_* masks; see btrfs_stripe_allow_rmw(). */
        u32 stripe_rmw_opt;
        u32 stripe_rmw_prop;
+       /*
+        * Parking deadlines in ms, tunable through sysfs so the constants
+        * can be swept against a workload instead of rebuilt for each
+        * data point.  0 disables parking for that class.  See
+        * rbio_try_park().
+        */
+       u32 stripe_park_timeout_ms;
+       u32 stripe_park_sync_timeout_ms;
 
        /* Private scrub information */
        struct mutex scrub_lock;

diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
index db8b1d41981f4c84092f77bf1b14a5e3b327c02c..906f5651eb8a2c611ccf8df35fb5c6b0cd1bfb77 100644 (file)
--- a/fs/btrfs/raid56.c
+++ b/fs/btrfs/raid56.c
@@ -75,8 +75,6 @@
  * enough for the rest of one writeback pass over the same stripe to merge
  * in, short enough to stay invisible in fsync latency.
  */
-#define BTRFS_RBIO_PARK_TIMEOUT_MS     100
-#define BTRFS_RBIO_PARK_SYNC_TIMEOUT_MS        3
 /* Scan period of the park timer while anything is parked. */
 #define BTRFS_RBIO_PARK_SCAN_MS                3
 
@@ -792,7 +790,9 @@ static bool rbio_try_park(struct btrfs_raid_bio *rbio)
 {
        struct btrfs_fs_info *fs_info = rbio->bioc->fs_info;
        struct btrfs_stripe_hash_table *table = fs_info->stripe_hash_table;
-       unsigned int timeout_ms = BTRFS_RBIO_PARK_TIMEOUT_MS;
+       const unsigned int sync_timeout_ms =
+               READ_ONCE(fs_info->stripe_park_sync_timeout_ms);
+       unsigned int timeout_ms = READ_ONCE(fs_info->stripe_park_timeout_ms);
        enum btrfs_stripe_run_class class;
 
        if (!btrfs_stripe_open_run_class(fs_info,
@@ -802,9 +802,13 @@ static bool rbio_try_park(struct btrfs_raid_bio *rbio)
 
        spin_lock(&rbio->bio_list_lock);
        if (rbio_has_sync_bio(rbio))
-               timeout_ms = BTRFS_RBIO_PARK_SYNC_TIMEOUT_MS;
+               timeout_ms = sync_timeout_ms;
        spin_unlock(&rbio->bio_list_lock);
 
+       /* A zeroed deadline disables parking for that class outright. */
+       if (timeout_ms == 0)
+               return false;
+
        /*
         * A sync write into a nodatacow inode's private run has a waiter
         * behind it and nothing worth waiting to merge with: in-place
@@ -813,7 +817,7 @@ static bool rbio_try_park(struct btrfs_raid_bio *rbio)
         * async writeback there.
         */
        if (class == BTRFS_STRIPE_RUN_NOCOW &&
-           timeout_ms == BTRFS_RBIO_PARK_SYNC_TIMEOUT_MS)
+           timeout_ms == sync_timeout_ms)
                return false;
 
        /*
@@ -825,7 +829,7 @@ static bool rbio_try_park(struct btrfs_raid_bio *rbio)
         * path kicks it as soon as they have all been submitted.)
         */
        if (test_bit(RBIO_BATCH_DONE_BIT, &rbio->flags) &&
-           timeout_ms == BTRFS_RBIO_PARK_SYNC_TIMEOUT_MS)
+           timeout_ms == sync_timeout_ms)
                return false;
 
        spin_lock(&table->parked_lock);
@@ -1078,7 +1082,8 @@ static noinline int lock_stripe_add(struct btrfs_raid_bio *rbio)
                                        struct btrfs_stripe_hash_table *table =
                                                cur->bioc->fs_info->stripe_hash_table;
                                        unsigned long dl = jiffies +
-               msecs_to_jiffies(BTRFS_RBIO_PARK_SYNC_TIMEOUT_MS);
+               msecs_to_jiffies(READ_ONCE(cur->bioc->fs_info->
+                                          stripe_park_sync_timeout_ms));
 
                                        spin_lock(&table->parked_lock);
                                        if (test_bit(RBIO_PARKED_BIT,
@@ -2996,10 +3001,17 @@ static void rmw_rbio(struct btrfs_raid_bio *rbio)
         */
        if (!rbio_is_full(rbio) && !rmw_try_pad_full(rbio) &&
            need_read_stripe_sectors(rbio)) {
+               struct btrfs_fs_info *fs_info = rbio->bioc->fs_info;
+
+               /*
+                * Every data read-modify-write, parked or not.  With
+                * allow_rmw empty and a covered block group this is the
+                * count that should be zero.
+                */
+               atomic64_inc(&fs_info->stripe_park_stats.data_rmw);
                /* A parked-then-flushed write that still had to read. */
                if (test_bit(RBIO_WAS_PARKED_BIT, &rbio->flags))
-                       atomic64_inc(&rbio->bioc->fs_info->
-                                    stripe_park_stats.rmw_reads);
+                       atomic64_inc(&fs_info->stripe_park_stats.rmw_reads);
                /*
                 * Now we're doing sub-stripe write, also need all data stripes
                 * to do the full RMW.

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index a448aa5b2db3359a6af0a147a9d32772b0b01e9f..3bdb953f27b60837cfc2c27e14ed0ade1eea55c4 100644 (file)
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1239,6 +1239,10 @@ static ssize_t btrfs_stripe_park_stats_show(struct kobject *kobj,
                "expired %lld\n"
                "padded %lld\n"
                "rmw_reads %lld\n"
+               "data_rmw %lld\n"
+               "pad_decline_live %lld\n"
+               "pad_decline_no_run %lld\n"
+               "pad_decline_unallocated %lld\n"
                "meta_rmw %lld\n"
                "meta_rmw_cur %lld\n"
                "meta_rmw_old %lld\n"
@@ -1250,6 +1254,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.data_rmw),
+               atomic64_read(&fs_info->stripe_park_stats.pad_decline_live),
+               atomic64_read(&fs_info->stripe_park_stats.pad_decline_no_run),
+               atomic64_read(&fs_info->stripe_park_stats.pad_decline_unallocated),
                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),
@@ -1257,6 +1265,42 @@ static ssize_t btrfs_stripe_park_stats_show(struct kobject *kobj,
 }
 BTRFS_ATTR(, stripe_park_stats, btrfs_stripe_park_stats_show);
 
+/*
+ * Park deadlines, ms.  Writable so the constants can be swept against a
+ * workload rather than rebuilt per data point; 0 disables parking for that
+ * class.  See rbio_try_park().
+ */
+#define BTRFS_PARK_TIMEOUT_ATTR(_name, _field)                         \
+static ssize_t btrfs_##_name##_show(struct kobject *kobj,              \
+                                   struct kobj_attribute *a, char *buf)\
+{                                                                      \
+       struct btrfs_fs_info *fs_info = to_fs_info(kobj);               \
+                                                                       \
+       return sysfs_emit(buf, "%u\n", READ_ONCE(fs_info->_field));     \
+}                                                                      \
+static ssize_t btrfs_##_name##_store(struct kobject *kobj,             \
+                                    struct kobj_attribute *a,          \
+                                    const char *buf, size_t len)       \
+{                                                                      \
+       struct btrfs_fs_info *fs_info = to_fs_info(kobj);               \
+       unsigned int val;                                               \
+       int ret;                                                        \
+                                                                       \
+       ret = kstrtouint(buf, 10, &val);                                \
+       if (ret)                                                        \
+               return ret;                                             \
+       if (val > BTRFS_RBIO_PARK_TIMEOUT_MS_MAX)                       \
+               return -EINVAL;                                         \
+       WRITE_ONCE(fs_info->_field, val);                               \
+       return len;                                                     \
+}                                                                      \
+BTRFS_ATTR_RW(, _name, btrfs_##_name##_show, btrfs_##_name##_store)
+
+BTRFS_PARK_TIMEOUT_ATTR(stripe_park_timeout_ms, stripe_park_timeout_ms);
+BTRFS_PARK_TIMEOUT_ATTR(stripe_park_sync_timeout_ms, stripe_park_sync_timeout_ms);
+
+
+
 static ssize_t btrfs_clone_alignment_show(struct kobject *kobj,
                                struct kobj_attribute *a, char *buf)
 {
@@ -1627,6 +1671,8 @@ static const struct attribute *btrfs_attrs[] = {
        BTRFS_ATTR_PTR(, bg_reclaim_threshold),
        BTRFS_ATTR_PTR(, commit_stats),
        BTRFS_ATTR_PTR(, stripe_park_stats),
+       BTRFS_ATTR_PTR(, stripe_park_timeout_ms),
+       BTRFS_ATTR_PTR(, stripe_park_sync_timeout_ms),
        BTRFS_ATTR_PTR(, temp_fsid),
        NULL,
 };