Eliminate unwind stat error_catchall (#69)

* Eliminate unwind stat error_catchall - added 3 actual stats that composed it * Add tmate session for debugging * Remove --ignore-environment from Github workflow clippy step for now * Remove tmate shell session step * back to original CI workflow
javierhonduco · Sep 17, 2024 · f8fde6d · f8fde6d
1 parent e651efc
commit f8fde6d
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 12 deletions.
diff --git a/src/bpf/profiler.bpf.c b/src/bpf/profiler.bpf.c
@@ -470,7 +470,7 @@ int dwarf_unwind(struct bpf_perf_event_data *ctx) {
     // add it, it would be best to add it only during development.
     if (previous_rsp == 0) {
       LOG("[error] previous_rsp should not be zero.");
-      bump_unwind_error_catchall();
+      bump_unwind_error_previous_rsp_zero();
       return 1;
     }
 
@@ -491,7 +491,7 @@ int dwarf_unwind(struct bpf_perf_event_data *ctx) {
         LOG("[error] previous_rip should not be zero. This can mean that the "
             "read failed, ret=%d while reading @ %llx.",
             err, previous_rip_addr);
-        bump_unwind_error_catchall();
+        bump_unwind_error_previous_rip_zero();
       }
       return 1;
     }
@@ -510,7 +510,7 @@ int dwarf_unwind(struct bpf_perf_event_data *ctx) {
         LOG("[error] previous_rbp should not be zero. This can mean "
             "that the read has failed %d.",
             ret);
-        bump_unwind_error_catchall();
+        bump_unwind_error_previous_rbp_zero();
         return 1;
       }
     }

diff --git a/src/bpf/profiler.h b/src/bpf/profiler.h
@@ -76,7 +76,9 @@ struct unwinder_stats_t {
   u64 error_unsupported_expression;
   u64 error_unsupported_frame_pointer_action;
   u64 error_unsupported_cfa_register;
-  u64 error_catchall;
+  u64 error_previous_rsp_zero;
+  u64 error_previous_rip_zero;
+  u64 error_previous_rbp_zero;
   u64 error_should_never_happen;
   u64 error_bp_should_be_zero_for_bottom_frame;
   u64 error_mapping_not_found;
@@ -89,8 +91,8 @@ struct unwinder_stats_t {
   u64 jit_encountered;
 };
 
-const volatile struct lightswitch_config_t lightswitch_config = {.verbose_logging =
-                                                               true};
+const volatile struct lightswitch_config_t lightswitch_config = {
+    .verbose_logging = true};
 
 // A different stack produced the same hash.
 #define STACK_COLLISION(err) (err == -EEXIST)
@@ -99,7 +101,7 @@ const volatile struct lightswitch_config_t lightswitch_config = {.verbose_loggin
 
 #define LOG(fmt, ...)                                                          \
   ({                                                                           \
-    if (lightswitch_config.verbose_logging) {                                     \
+    if (lightswitch_config.verbose_logging) {                                  \
       bpf_printk(fmt, ##__VA_ARGS__);                                          \
     }                                                                          \
   })
@@ -226,4 +228,4 @@ unsigned long long hash_stack(native_stack_t *stack) {
   }
 
   return hash;
-}
+}
diff --git a/src/bpf/profiler_bindings.rs b/src/bpf/profiler_bindings.rs
@@ -49,7 +49,9 @@ impl Add for unwinder_stats_t {
                 + other.error_unsupported_frame_pointer_action,
             error_unsupported_cfa_register: self.error_unsupported_cfa_register
                 + other.error_unsupported_cfa_register,
-            error_catchall: self.error_catchall + other.error_catchall,
+            error_previous_rsp_zero: self.error_previous_rsp_zero + other.error_previous_rsp_zero,
+            error_previous_rip_zero: self.error_previous_rip_zero + other.error_previous_rip_zero,
+            error_previous_rbp_zero: self.error_previous_rbp_zero + other.error_previous_rbp_zero,
             error_should_never_happen: self.error_should_never_happen
                 + other.error_should_never_happen,
             error_bp_should_be_zero_for_bottom_frame: self.error_bp_should_be_zero_for_bottom_frame

diff --git a/src/bpf/shared_maps.h b/src/bpf/shared_maps.h
@@ -18,7 +18,6 @@ struct {
   __type(value, struct unwinder_stats_t);
 } percpu_stats SEC(".maps");
 
-
 #define DEFINE_COUNTER(__func__name)                                           \
   static void bump_unwind_##__func__name() {                                   \
     u32 zero = 0;                                                              \
@@ -35,7 +34,9 @@ DEFINE_COUNTER(error_truncated);
 DEFINE_COUNTER(error_unsupported_expression);
 DEFINE_COUNTER(error_unsupported_frame_pointer_action);
 DEFINE_COUNTER(error_unsupported_cfa_register);
-DEFINE_COUNTER(error_catchall);
+DEFINE_COUNTER(error_previous_rsp_zero);
+DEFINE_COUNTER(error_previous_rip_zero);
+DEFINE_COUNTER(error_previous_rbp_zero);
 DEFINE_COUNTER(error_should_never_happen);
 DEFINE_COUNTER(error_bp_should_be_zero_for_bottom_frame);
 DEFINE_COUNTER(error_mapping_not_found);
@@ -47,4 +48,4 @@ DEFINE_COUNTER(error_cfa_offset_did_not_fit);
 DEFINE_COUNTER(vdso_encountered);
 DEFINE_COUNTER(jit_encountered);
 
-#endif
+#endif