From 8c208b15e66efdbedb6cf4910f9df691b3d66207 Mon Sep 17 00:00:00 2001 From: Sanghyeok Park Date: Sun, 13 Sep 2026 14:43:30 +0900 Subject: [PATCH 1/2] fix(osdtrace): don't leak ops map entries when the subop length read fails uprobe_log_subop_stats and uprobe_repop_commit returned early when reading the write length failed after the ops lookup had already succeeded, skipping the bpf_map_delete_elem(&ops, &key) that follows the submit. The sub-op has completed by then, so the entry is dead either way and nothing will ever delete it. Delete the entry on that failure path too, as #188 did for the ring buffer reserve failure path. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RawRtYaKZCaZZQMvsDyWV7 --- src/osdtrace.bpf.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/osdtrace.bpf.c b/src/osdtrace.bpf.c index a497e52..3f9a938 100644 --- a/src/osdtrace.bpf.c +++ b/src/osdtrace.bpf.c @@ -744,8 +744,10 @@ int uprobe_log_subop_stats(struct pt_regs *ctx) if (NULL == vp) return 0; __u64 len = 0; - if (read_hprobe_varfield(ctx, varid++, &len, sizeof(len)) != 0) + if (read_hprobe_varfield(ctx, varid++, &len, sizeof(len)) != 0) { + bpf_map_delete_elem(&ops, &key); return 0; + } vp->wb = len; vp->reply_stamp = bpf_ktime_get_boot_ns(); @@ -837,8 +839,10 @@ int uprobe_repop_commit(struct pt_regs *ctx) } __u64 len = 0; - if (read_hprobe_varfield(ctx, varid++, &len, sizeof(len)) != 0) + if (read_hprobe_varfield(ctx, varid++, &len, sizeof(len)) != 0) { + bpf_map_delete_elem(&ops, &key); return 0; + } vp->wb = len; vp->reply_stamp = bpf_ktime_get_boot_ns(); From 6fb8d71793b4db35656e1935b5adc8a94fec5caf Mon Sep 17 00:00:00 2001 From: Sanghyeok Park Date: Sun, 13 Sep 2026 14:43:30 +0900 Subject: [PATCH 2/2] fix(osdtrace): make the ops map an LRU hash so leaks can't stop tracing uprobe_enqueue_op inserts every client op and replica sub-op into the ops map, and only the completion probes (log_op_stats, log_subop_stats, repop_commit, ec_submit_transaction) delete them. Several ordinary OSD paths finish an op without reaching any of those probes: - reads that fail: PrimaryLogPG::complete_read_ctx only calls log_op_stats for result >= 0, so every -ENOENT read leaks - writes that fail (record_write_error, then return) - the reply_op_error paths in do_op and friends - dup/resent ops answered from the pg log (already_complete) - ops dropped by can_discard_request Client tids are monotonic, so the orphan handling in uprobe_enqueue_op never sees these keys again. With the plain 8192-entry hash the leaked entries accumulate until bpf_map_update_elem fails with -E2BIG for every new op; from then on no op is tracked, every completion lookup misses, and osdtrace silently stops emitting events while still looking healthy. A workload with a steady stream of stats/reads on missing objects gets there in seconds. Switch the map to BPF_MAP_TYPE_LRU_HASH so a full map evicts instead of rejecting inserts. Lookups set the LRU reference bit, so ops that are still moving through the probes are preferred over leaked entries that are never touched again. The trade-off is that an op left untouched long enough while the map is under pressure (e.g. still waiting in the op queue) can be evicted and go missing from the output, and eviction can begin before max_entries is reached because of the per-CPU free lists. Plugging the leaking paths themselves is still worthwhile; this change makes sure the ones we have not found cannot take the tracer down. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RawRtYaKZCaZZQMvsDyWV7 --- src/osdtrace.bpf.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/osdtrace.bpf.c b/src/osdtrace.bpf.c index 3f9a938..d8ab9eb 100644 --- a/src/osdtrace.bpf.c +++ b/src/osdtrace.bpf.c @@ -14,8 +14,30 @@ char LICENSE[] SEC("license") = "Dual BSD/GPL"; +/* LRU rather than plain HASH on purpose. + * + * An entry is inserted by uprobe_enqueue_op and deleted by a completion + * probe (log_op_stats, log_subop_stats, repop_commit, + * ec_submit_transaction). Ops that finish without reaching any of them + * leave their entry behind: failed reads (log_op_stats is only called for + * result >= 0, so every -ENOENT read leaks), failed writes, the + * reply_op_error paths, dup/resent ops answered from the pg log, and ops + * dropped by can_discard_request. Client tids are monotonic, so the + * orphan handling in uprobe_enqueue_op never sees these keys again. + * + * With a plain HASH the leaked entries eventually fill the map, every later + * bpf_map_update_elem fails with -E2BIG, and osdtrace silently stops + * emitting events until it is restarted. + * + * An LRU hash evicts instead of rejecting. Lookups set the reference bit, + * so ops still moving through the probes are kept over leaked entries that + * are never touched again. An op that sits untouched long enough while the + * map is under pressure (e.g. waiting in the op queue) can still be evicted + * and go missing from the output, and eviction can start before max_entries + * is reached because of the per-CPU free lists. + */ struct { - __uint(type, BPF_MAP_TYPE_HASH); + __uint(type, BPF_MAP_TYPE_LRU_HASH); __type(key, struct op_k); __type(value, struct op_v); __uint(max_entries, 8192);