bcachefs: time_stats: split stats-with-quantiles into a separate structure
authorDarrick J. Wong <djwong@kernel.org>
Thu, 1 Feb 2024 20:41:42 +0000 (12:41 -0800)
committerKent Overstreet <kent.overstreet@linux.dev>
Thu, 14 Mar 2024 01:38:01 +0000 (21:38 -0400)
Currently, struct time_stats has the optional ability to quantize the
information that it collects.  This is /probably/ useful for callers who
want to see quantized information, but it more than doubles the size of
the structure from 224 bytes to 464.  For users who don't care about
that (e.g. upcoming xfs patches) and want to avoid wasting 240 bytes per
counter, split the two into separate pieces.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
fs/bcachefs/bcachefs.h
fs/bcachefs/io_write.c
fs/bcachefs/super.c
fs/bcachefs/sysfs.c
fs/bcachefs/time_stats.c
fs/bcachefs/time_stats.h
fs/bcachefs/util.c

index 17a5a71519019cfdd3f3feb2ed0b7036bd91ed0f..339dc3e1dcd39939b5f021db2665190ea07ceee9 100644 (file)
@@ -598,7 +598,7 @@ struct bch_dev {
 
        /* The rest of this all shows up in sysfs */
        atomic64_t              cur_latency[2];
-       struct bch2_time_stats  io_latency[2];
+       struct bch2_time_stats_quantiles io_latency[2];
 
 #define CONGESTED_MAX          1024
        atomic_t                congested;
index 150c272368bf76aa304b4c1f9c5040713792feba..f137252bccc575b42a012a7876f8c81ddee28a21 100644 (file)
@@ -88,7 +88,7 @@ void bch2_latency_acct(struct bch_dev *ca, u64 submit_time, int rw)
 
        bch2_congested_acct(ca, io_latency, now, rw);
 
-       __bch2_time_stats_update(&ca->io_latency[rw], submit_time, now);
+       __bch2_time_stats_update(&ca->io_latency[rw].stats, submit_time, now);
 }
 
 #endif
index 961b25860c3bef7239bf101a2bdb09261c63167d..233f864ed8b07ff321b64993a821d08137b70ecd 100644 (file)
@@ -1189,8 +1189,8 @@ static void bch2_dev_free(struct bch_dev *ca)
        bch2_dev_buckets_free(ca);
        free_page((unsigned long) ca->sb_read_scratch);
 
-       bch2_time_stats_exit(&ca->io_latency[WRITE]);
-       bch2_time_stats_exit(&ca->io_latency[READ]);
+       bch2_time_stats_quantiles_exit(&ca->io_latency[WRITE]);
+       bch2_time_stats_quantiles_exit(&ca->io_latency[READ]);
 
        percpu_ref_exit(&ca->io_ref);
        percpu_ref_exit(&ca->ref);
@@ -1281,8 +1281,8 @@ static struct bch_dev *__bch2_dev_alloc(struct bch_fs *c,
 
        INIT_WORK(&ca->io_error_work, bch2_io_error_work);
 
-       bch2_time_stats_init(&ca->io_latency[READ]);
-       bch2_time_stats_init(&ca->io_latency[WRITE]);
+       bch2_time_stats_quantiles_init(&ca->io_latency[READ]);
+       bch2_time_stats_quantiles_init(&ca->io_latency[WRITE]);
 
        ca->mi = bch2_mi_to_cpu(member);
 
index cee80c47feea2b27fa7d18fc55a39228db7f0b96..c86a93a8d8fc81bbe373efcbec74f3e2563e6da5 100644 (file)
@@ -930,10 +930,10 @@ SHOW(bch2_dev)
        sysfs_print(io_latency_write,           atomic64_read(&ca->cur_latency[WRITE]));
 
        if (attr == &sysfs_io_latency_stats_read)
-               bch2_time_stats_to_text(out, &ca->io_latency[READ]);
+               bch2_time_stats_to_text(out, &ca->io_latency[READ].stats);
 
        if (attr == &sysfs_io_latency_stats_write)
-               bch2_time_stats_to_text(out, &ca->io_latency[WRITE]);
+               bch2_time_stats_to_text(out, &ca->io_latency[WRITE].stats);
 
        sysfs_printf(congested,                 "%u%%",
                     clamp(atomic_read(&ca->congested), 0, CONGESTED_MAX)
index 4ac6ebfd264c768b6a343b107864242e1f409766..4508e9dcbee224b5a330a8941f8f9f279939b23d 100644 (file)
@@ -73,6 +73,8 @@ static inline void time_stats_update_one(struct bch2_time_stats *stats,
        bool initted = stats->last_event != 0;
 
        if (time_after64(end, start)) {
+               struct quantiles *quantiles = time_stats_to_quantiles(stats);
+
                duration = end - start;
                mean_and_variance_update(&stats->duration_stats, duration);
                mean_and_variance_weighted_update(&stats->duration_stats_weighted,
@@ -81,8 +83,8 @@ static inline void time_stats_update_one(struct bch2_time_stats *stats,
                stats->min_duration = min(stats->min_duration, duration);
                stats->total_duration += duration;
 
-               if (stats->quantiles_enabled)
-                       quantiles_update(&stats->quantiles, duration);
+               if (quantiles)
+                       quantiles_update(quantiles, duration);
        }
 
        if (stats->last_event && time_after64(end, stats->last_event)) {
index fd6e442443f94d34ef7fc4c06e016a8f0ad3c3e5..ed6c03c436c02e9e218c5ff1cf52244ee618cb02 100644 (file)
@@ -26,6 +26,7 @@
 
 #include <linux/sched/clock.h>
 #include <linux/spinlock_types.h>
+#include <linux/string.h>
 
 #include "mean_and_variance.h"
 
@@ -68,7 +69,7 @@ struct time_stat_buffer {
 
 struct bch2_time_stats {
        spinlock_t      lock;
-       bool            quantiles_enabled;
+       bool            have_quantiles;
        /* all fields are in nanoseconds */
        u64             min_duration;
        u64             max_duration;
@@ -77,7 +78,6 @@ struct bch2_time_stats {
        u64             min_freq;
        u64             last_event;
        u64             last_event_start;
-       struct quantiles quantiles;
 
        struct mean_and_variance          duration_stats;
        struct mean_and_variance          freq_stats;
@@ -90,6 +90,18 @@ struct bch2_time_stats {
        struct time_stat_buffer __percpu *buffer;
 };
 
+struct bch2_time_stats_quantiles {
+       struct bch2_time_stats  stats;
+       struct quantiles        quantiles;
+};
+
+static inline struct quantiles *time_stats_to_quantiles(struct bch2_time_stats *stats)
+{
+       return stats->have_quantiles
+               ? &container_of(stats, struct bch2_time_stats_quantiles, stats)->quantiles
+               : NULL;
+}
+
 void __bch2_time_stats_clear_buffer(struct bch2_time_stats *, struct time_stat_buffer *);
 void __bch2_time_stats_update(struct bch2_time_stats *stats, u64, u64);
 
@@ -133,4 +145,15 @@ static inline bool track_event_change(struct bch2_time_stats *stats, bool v)
 void bch2_time_stats_exit(struct bch2_time_stats *);
 void bch2_time_stats_init(struct bch2_time_stats *);
 
+static inline void bch2_time_stats_quantiles_exit(struct bch2_time_stats_quantiles *statq)
+{
+       bch2_time_stats_exit(&statq->stats);
+}
+static inline void bch2_time_stats_quantiles_init(struct bch2_time_stats_quantiles *statq)
+{
+       bch2_time_stats_init(&statq->stats);
+       statq->stats.have_quantiles = true;
+       memset(&statq->quantiles, 0, sizeof(statq->quantiles));
+}
+
 #endif /* _BCACHEFS_TIME_STATS_H */
index 0f11e0c4e46d34f9c4a5a3201a6378215f25425a..216fadf16928b9a73eb47da96a8a7b409657e8fe 100644 (file)
@@ -365,6 +365,7 @@ static inline void pr_name_and_units(struct printbuf *out, const char *name, u64
 
 void bch2_time_stats_to_text(struct printbuf *out, struct bch2_time_stats *stats)
 {
+       struct quantiles *quantiles = time_stats_to_quantiles(stats);
        s64 f_mean = 0, d_mean = 0;
        u64 f_stddev = 0, d_stddev = 0;
 
@@ -465,17 +466,17 @@ void bch2_time_stats_to_text(struct printbuf *out, struct bch2_time_stats *stats
 
        printbuf_tabstops_reset(out);
 
-       if (stats->quantiles_enabled) {
+       if (quantiles) {
                int i = eytzinger0_first(NR_QUANTILES);
                const struct time_unit *u =
-                       bch2_pick_time_units(stats->quantiles.entries[i].m);
+                       bch2_pick_time_units(quantiles->entries[i].m);
                u64 last_q = 0;
 
                prt_printf(out, "quantiles (%s):\t", u->name);
                eytzinger0_for_each(i, NR_QUANTILES) {
                        bool is_last = eytzinger0_next(i, NR_QUANTILES) == -1;
 
-                       u64 q = max(stats->quantiles.entries[i].m, last_q);
+                       u64 q = max(quantiles->entries[i].m, last_q);
                        prt_printf(out, "%llu ", div_u64(q, u->nsecs));
                        if (is_last)
                                prt_newline(out);