Re: [PATCH v2 3/3] btrfs: add tracepoint for COW amplification tracking

[Leo Martins <loemra.dev@gmail.com>]

On Mon, 16 Feb 2026 12:40:04 +0000 Filipe Manana <fdmanana@kernel.org> wrote:

> On Fri, Feb 13, 2026 at 8:37 PM Leo Martins <loemra.dev@gmail.com> wrote:
> >
> > Add a btrfs_search_slot_stats tracepoint to btrfs_search_slot() for
> > measuring COW amplification.
> >
> > The tracepoint fires when a search with at least one COW completes,
> > recording the root, total cow_count, restart_count, and return value.
> > cow_count and restart_count per search_slot call are useful metrics
> > for tracking COW amplification.
> >
> > Signed-off-by: Leo Martins <loemra.dev@gmail.com>
> > ---
> >  fs/btrfs/ctree.c             | 15 +++++++++++++--
> >  include/trace/events/btrfs.h | 26 ++++++++++++++++++++++++++
> >  2 files changed, 39 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> > index 55187ba59cc0..1971d7bb5f60 100644
> > --- a/fs/btrfs/ctree.c
> > +++ b/fs/btrfs/ctree.c
> > @@ -2069,6 +2069,8 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> >         u8 lowest_level = 0;
> >         int min_write_lock_level;
> >         int prev_cmp;
> > +       int cow_count = 0;
> > +       int restart_count = 0;
> >
> >         if (!root)
> >                 return -EINVAL;
> > @@ -2157,6 +2159,7 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> >                             p->nodes[level + 1])) {
> >                                 write_lock_level = level + 1;
> >                                 btrfs_release_path(p);
> > +                               restart_count++;
> >                                 goto again;
> >                         }
> >
> > @@ -2172,6 +2175,7 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> >                                 ret = ret2;
> >                                 goto done;
> >                         }
> > +                       cow_count++;
> >                 }
> >  cow_done:
> >                 p->nodes[level] = b;
> > @@ -2219,8 +2223,10 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> >                 p->slots[level] = slot;
> >                 ret2 = setup_nodes_for_search(trans, root, p, b, level, ins_len,
> >                                               &write_lock_level);
> > -               if (ret2 == -EAGAIN)
> > +               if (ret2 == -EAGAIN) {
> > +                       restart_count++;
> >                         goto again;
> > +               }
> >                 if (ret2) {
> >                         ret = ret2;
> >                         goto done;
> > @@ -2236,6 +2242,7 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> >                 if (slot == 0 && ins_len && write_lock_level < level + 1) {
> >                         write_lock_level = level + 1;
> >                         btrfs_release_path(p);
> > +                       restart_count++;
> >                         goto again;
> >                 }
> >
> > @@ -2249,8 +2256,10 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> >                 }
> >
> >                 ret2 = read_block_for_search(root, p, &b, slot, key);
> > -               if (ret2 == -EAGAIN && !p->nowait)
> > +               if (ret2 == -EAGAIN && !p->nowait) {
> > +                       restart_count++;
> >                         goto again;
> > +               }
> >                 if (ret2) {
> >                         ret = ret2;
> >                         goto done;
> > @@ -2281,6 +2290,8 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> >         }
> >         ret = 1;
> >  done:
> > +       if (cow_count > 0)
> > +               trace_btrfs_search_slot_stats(root, cow_count, restart_count, ret);
> 
> So I find this way too specific, plus even if trace points are
> disabled we have the overhead of the counters (and inside critical
> sections).
> 
> We already have a tracepoint for COW, trace_btrfs_cow_block(), and we
> could have one just for the retry thing, maybe naming it like
> trace_btrfs_search_slot_restart() or something.
> So we could use those two tracepoints to measure things (bpftrace
> scripts could easily report a count of each trace point and such),
> instead of this highly specialized tracepoint that adds some overhead
> when tracepoints are disabled.

Good point, added a per-restart-site trace_btrfs_search_slot_restart()
tracepoint in v3.

Thanks,
Leo

> 
> Thanks.
> 
> 
> >         if (ret < 0 && !p->skip_release_on_error)
> >                 btrfs_release_path(p);
> >
> > diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
> > index 125bdc166bfe..b8934938a087 100644
> > --- a/include/trace/events/btrfs.h
> > +++ b/include/trace/events/btrfs.h
> > @@ -1110,6 +1110,32 @@ TRACE_EVENT(btrfs_cow_block,
> >                   __entry->cow_level)
> >  );
> >
> > +TRACE_EVENT(btrfs_search_slot_stats,
> > +
> > +       TP_PROTO(const struct btrfs_root *root,
> > +                int cow_count, int restart_count, int ret),
> > +
> > +       TP_ARGS(root, cow_count, restart_count, ret),
> > +
> > +       TP_STRUCT__entry_btrfs(
> > +               __field(        u64,    root_objectid           )
> > +               __field(        int,    cow_count               )
> > +               __field(        int,    restart_count           )
> > +               __field(        int,    ret                     )
> > +       ),
> > +
> > +       TP_fast_assign_btrfs(root->fs_info,
> > +               __entry->root_objectid  = btrfs_root_id(root);
> > +               __entry->cow_count      = cow_count;
> > +               __entry->restart_count  = restart_count;
> > +               __entry->ret            = ret;
> > +       ),
> > +
> > +       TP_printk_btrfs("root=%llu(%s) cow_count=%d restarts=%d ret=%d",
> > +                 show_root_type(__entry->root_objectid),
> > +                 __entry->cow_count, __entry->restart_count, __entry->ret)
> > +);
> > +
> >  TRACE_EVENT(btrfs_space_reservation,
> >
> >         TP_PROTO(const struct btrfs_fs_info *fs_info, const char *type, u64 val,
> > --
> > 2.47.3
> >
> >