Skip to content

Commit

Permalink
fix(bpf): cleanup initialising structs and nested ifs (sustainable-co…
Browse files Browse the repository at this point in the history
…mputing-io#1444)

This commit changes all declarae and then initialise code of form below

```
struct foobar x;
x.a = a;
x.b = b;
```

to
```
struct foobar x = { .a = a, .b = b, };
```
The difference in the code produced is as follows

```
;  new_pid_key.pid = cur_pid;                             | pid_time_t new_pid_key = {.pid = cur_pid };
  0: 63 6a fc ff 00 00 00 00	*(u32 *)(r10 - 0x4) = r6  |  0:	79 a1 68 ff 00 00 00 00	r1 = *(u64 *)(r10 - 0x98)
  1: bf a2 00 00 00 00 00 00	r2 = r10                  |  1:	63 1a fc ff 00 00 00 00	*(u32 *)(r10 - 0x4) = r1
  2: 07 02 00 00 fc ff ff ff	r2 += -0x4                |  2:	bf a2 00 00 00 00 00 00	r2 = r10
  3: bf a3 00 00 00 00 00 00	r3 = r10                  |  3:	07 02 00 00 f0 ff ff ff	r2 += -0x10
  4: 07 03 00 00 70 ff ff ff	r3 += -0x90

```

Modifies calc_delta to remove unnecessary pointer deferencing of second
arg - `val`

Signed-off-by: Sunil Thaha <[email protected]>
(cherry picked from commit ac43e26)
  • Loading branch information
sthaha committed May 18, 2024
1 parent f901313 commit 5f59172
Showing 1 changed file with 18 additions and 19 deletions.
37 changes: 18 additions & 19 deletions bpfassets/libbpf/src/kepler.bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,10 @@ int counter_sched_switch = 0;
static inline u64 get_on_cpu_time(u32 cur_pid, u32 prev_pid, u64 cur_ts)
{
u64 cpu_time = 0;
u64 *prev_ts;
pid_time_t prev_pid_key, new_pid_key = {};
pid_time_t prev_pid_key = { .pid = prev_pid };
pid_time_t new_pid_key = { .pid = cur_pid };

prev_pid_key.pid = prev_pid;
prev_ts = bpf_map_lookup_elem(&pid_time, &prev_pid_key);
u64 *prev_ts = bpf_map_lookup_elem(&pid_time, &prev_pid_key);
if (prev_ts) {
// Probably a clock issue where the recorded on-CPU event had a
// timestamp later than the recorded off-CPU event, or vice versa.
Expand All @@ -109,19 +108,18 @@ static inline u64 get_on_cpu_time(u32 cur_pid, u32 prev_pid, u64 cur_ts)
bpf_map_delete_elem(&pid_time, &prev_pid_key);
}
}
new_pid_key.pid = cur_pid;

bpf_map_update_elem(&pid_time, &new_pid_key, &cur_ts, BPF_NOEXIST);
return cpu_time;
}

static inline u64 calc_delta(u64 *prev_val, u64 *val)
static inline u64 calc_delta(u64 *prev_val, u64 val)
{
u64 delta = 0;

if (prev_val) {
if (*val > *prev_val)
delta = *val - *prev_val;
if (prev_val && val > *prev_val) {
delta = val - *prev_val;
}

return delta;
}

Expand All @@ -138,7 +136,7 @@ static inline u64 get_on_cpu_cycles(u32 *cpu_id)

val = c.counter;
prev_val = bpf_map_lookup_elem(&cpu_cycles, cpu_id);
delta = calc_delta(prev_val, &val);
delta = calc_delta(prev_val, val);
bpf_map_update_elem(&cpu_cycles, cpu_id, &val, BPF_ANY);

return delta;
Expand All @@ -152,12 +150,12 @@ static inline u64 get_on_cpu_instr(u32 *cpu_id)

error = bpf_perf_event_read_value(
&cpu_instructions_event_reader, *cpu_id, &c, sizeof(c));
if (error) {
if (error)
return 0;
}

val = c.counter;
prev_val = bpf_map_lookup_elem(&cpu_instructions, cpu_id);
delta = calc_delta(prev_val, &val);
delta = calc_delta(prev_val, val);
bpf_map_update_elem(&cpu_instructions, cpu_id, &val, BPF_ANY);

return delta;
Expand All @@ -176,7 +174,7 @@ static inline u64 get_on_cpu_cache_miss(u32 *cpu_id)
}
val = c.counter;
prev_val = bpf_map_lookup_elem(&cache_miss, cpu_id);
delta = calc_delta(prev_val, &val);
delta = calc_delta(prev_val, val);
bpf_map_update_elem(&cache_miss, cpu_id, &val, BPF_ANY);

return delta;
Expand Down Expand Up @@ -241,10 +239,11 @@ int kepler_sched_switch_trace(struct sched_switch_info *ctx)
// create new process metrics
cur_pid_metrics = bpf_map_lookup_elem(&processes, &cur_pid);
if (!cur_pid_metrics) {
process_metrics_t new_process = {};
new_process.pid = cur_pid;
new_process.tgid = tgid;
new_process.cgroup_id = cgroup_id;
process_metrics_t new_process = {
.pid = cur_pid,
.tgid = tgid,
.cgroup_id = cgroup_id,
};
bpf_get_current_comm(&new_process.comm, sizeof(new_process.comm));
bpf_map_update_elem(
&processes, &cur_pid, &new_process, BPF_NOEXIST);
Expand Down

0 comments on commit 5f59172

Please sign in to comment.